Conversation
|
@copilot update dev-hawk to restrict workflow_run branches to copilot/* |
…n strict mode) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds validation for workflow_run triggers to ensure they include branch restrictions, which is a security and performance best practice. Without branch restrictions, workflows run on all branches, potentially causing unexpected behavior and security issues.
Key changes:
- Adds a new validation function
validateWorkflowRunBranchesthat checks for branch restrictions inworkflow_runtriggers - Validation emits warnings in normal mode and errors in strict mode
- Includes comprehensive test coverage for various
workflow_runconfigurations - Updates workflow documentation and applies the branch restriction fix to the dev-hawk workflow
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/validation.go | Adds validateWorkflowRunBranches function to validate branch restrictions in workflow_run triggers |
| pkg/workflow/compiler.go | Integrates the new validation function into the compilation pipeline |
| pkg/workflow/workflow_run_validation_test.go | Comprehensive test suite covering various workflow_run configurations and edge cases |
| .github/workflows/dev-hawk.md | Fixes branch pattern from copilot/** to copilot/* (applies the validation fix) |
| .github/workflows/dev-hawk.lock.yml | Generated lock file reflecting the branch pattern fix |
| docs/src/content/docs/status.mdx | Updates workflow table entries and removes extraneous blank lines |
| docs/src/content/docs/reference/frontmatter-full.md | Documentation cleanup removing blank lines and undocumented min fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _, hasBranches := workflowRunMap["branches"] | ||
| if hasBranches { | ||
| // Has branch restrictions, validation passed | ||
| if c.verbose { | ||
| fmt.Fprintln(os.Stderr, console.FormatInfoMessage("✓ workflow_run trigger has branch restrictions")) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
The validation incorrectly passes when the branches field exists but is empty. According to test case "workflow_run with empty branches array", an empty branches array (branches: []) should NOT trigger a warning, but the current logic treats it as valid branch restrictions. The check should verify that branches not only exists but also contains at least one branch pattern, or explicitly handle empty arrays as a special case if they're intentionally allowed.
| _, hasBranches := workflowRunMap["branches"] | |
| if hasBranches { | |
| // Has branch restrictions, validation passed | |
| if c.verbose { | |
| fmt.Fprintln(os.Stderr, console.FormatInfoMessage("✓ workflow_run trigger has branch restrictions")) | |
| } | |
| return nil | |
| branchesVal, hasBranches := workflowRunMap["branches"] | |
| if hasBranches { | |
| // Check that branches is a non-empty array | |
| if branchesSlice, ok := branchesVal.([]any); ok && len(branchesSlice) > 0 { | |
| // Has branch restrictions, validation passed | |
| if c.verbose { | |
| fmt.Fprintln(os.Stderr, console.FormatInfoMessage("✓ workflow_run trigger has branch restrictions")) | |
| } | |
| return nil | |
| } | |
| // branches exists but is empty or not an array, treat as missing restrictions |
Add warning/error for workflow_run without branch restrictions ✅
Summary
Implemented validation to ensure
workflow_runtriggers include branch restrictions for security and performance. The validation emits warnings in normal mode and errors in strict mode.Implementation Details
Files Changed:
validateWorkflowRunBranches()functionValidation Logic:
on:YAML field to detectworkflow_runtriggersbranchesfield is present in theworkflow_runconfigurationTesting
✅ All tests pass:
make test-unit- PASSmake lint- PASSmake fmt- PASSmake agent-finish- PASS✅ Test Coverage:
✅ Repository Impact:
Example Output
Normal mode (warning):
Strict mode (error):
Security & Performance Benefits
This validation helps prevent:
Recent Updates
copilot/**tocopilot/*per feedbackChecklist
make agent-finish- ALL PASS ✅Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.