fix: derive call-workflow job permissions from caller, not worker (#40169)#40175
fix: derive call-workflow job permissions from caller, not worker (#40169)#40175dsyme wants to merge 7 commits into
Conversation
…0169) buildCallWorkflowJobs previously reverse-engineered the call-<worker> job's permissions block from the union of the worker's job-level permissions. When a worker job declared read-all, Merge() expanded it across GetAllPermissionScopes(), materialising vulnerability-alerts: read into the caller's lockfile. GitHub Actions rejects that scope on GITHUB_TOKEN, causing startup_failure on every run. The call-<worker> job now derives its permissions block from the caller's own declared permissions. extractCallWorkflowPermissions becomes a validation-only step: findUncoveredWorkerPermissions compares the caller's declared permissions against the worker's requirements and emits a compiler warning when the caller is insufficient, without ever modifying the compiled permissions. - pkg/workflow/compiler_safe_output_jobs.go: use caller's permissions, validate worker coverage, warn on gaps - pkg/workflow/call_workflow_permissions.go: add permissionLevelRank and findUncoveredWorkerPermissions helpers; document validation-only role - pkg/cli/workflows/test-copilot-call-workflow.md: widen caller perms to cover its worker under the new model - regenerated affected lock files
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
…workflow-caller-permissions-40169
There was a problem hiding this comment.
⚠️ Not ready to approve
The new worker-coverage validator misses the valid copilot-requests permission scope, which can prevent warnings for real permission gaps and lead to runtime failures.
Pull request overview
This PR fixes a safe-outputs call-workflow compilation design flaw by ensuring the generated call-<worker> job permissions are derived from the caller workflow’s declared permissions, rather than being inflated from the worker’s job permissions. It also adds validation that compares caller vs worker requirements and emits warnings when the caller’s permissions may be insufficient—without mutating the compiled permissions.
Changes:
- Derive
call-<worker>jobpermissions:from the caller’s declared permissions, and validate worker coverage via warnings rather than propagating worker permissions. - Add permission-level comparison helpers and a new unit test for uncovered worker permissions.
- Update the test workflow’s declared permissions and regenerate affected lock files to reflect the caller-derived model.
File summaries
| File | Description |
|---|---|
| pkg/workflow/compiler_safe_output_jobs.go | Switches call-workflow job permissions to be derived from caller permissions and adds warning-only validation against worker permissions. |
| pkg/workflow/call_workflow_permissions.go | Adds permission ranking and uncovered-scope detection helper for validation-only comparison. |
| pkg/workflow/call_workflow_permissions_test.go | Updates existing tests to assert caller-derived permissions and adds coverage for uncovered-scope detection. |
| pkg/cli/workflows/test-copilot-call-workflow.md | Expands declared permissions so the caller covers its worker under the new model. |
| .github/workflows/smoke-call-workflow.lock.yml | Regenerated to reflect caller-derived call-job permissions. |
| .github/workflows/dependabot-campaign.lock.yml | Regenerated to reflect caller-derived call-job permissions. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 1
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The stored frontmatter_hash/body_hash in the lock file no longer matched the values computed by the current hash logic, causing TestHashConsistencyAcrossLockFiles to fail. Recompiling regenerates the hash metadata; the generated workflow body is byte-identical (only the gh-aw-metadata comment changes).
|
|
``
|
|
@copilot review all comments and address unresolved review feedback.
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@copilot review all comments and address unresolved review feedback. Please land the remaining fix and summarize any blockers.
|
|
Please fix the copilot-requests warning and rerun checks.
|
|
Please fix the copilot-requests warning and rerun checks.
|
|
``
|
|
``
|
|
@copilot fix the CI errors |
|
@copilot review all comments and address unresolved review feedback.
|
|
@copilot summarize the remaining CI failures and propose the smallest fix.
|
…ions Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
Fixed in 3a7cebc. The CI failure was a build error in |
|
@copilot review all comments and address unresolved review feedback.
|
|
Please re-review now that the CI fix is pushed and summarize any remaining blockers.
|
|
@copilot review all comments and address unresolved review feedback.
|
|
@copilot please request a fresh review so maintainers can re-evaluate the branch.
|
…github.com/github/gh-aw into fix/call-workflow-caller-permissions-40169
Fixes #40169
Problem
test-copilot-call-workflowhad astartup_failureon every run. The immediate trigger wasvulnerability-alerts: readappearing in the generatedcall-<worker>job'spermissions:blockThe root cause was a design issue in
buildCallWorkflowJobs: it reverse-engineered thecall-<worker>job'spermissions:block from the union of the worker's job-level permissions. When a worker job declaredread-all,Merge()expanded it acrossGetAllPermissionScopes()— materializingvulnerability-alerts: readinto the caller's lockfile.Fix
The compiler no longer inflates the caller's permissions to match the worker. Callers control their own permission surface; the compiler validates (rather than rewrites) it.
call-<worker>job'spermissions:block is now derived from the caller's own declared permissions (data.CachedPermissions/data.Permissions).extractCallWorkflowPermissionsbecomes a validation-only step. New helperfindUncoveredWorkerPermissionscompares the caller's declared permissions against the worker's requirements and emits a compilerwarningwhen the caller is insufficient — without ever modifying the compiled permissions.Changes
pkg/workflow/compiler_safe_output_jobs.go— use the caller's permissions for the call job; validate worker coverage and warn on gaps.pkg/workflow/call_workflow_permissions.go— addpermissionLevelRankandfindUncoveredWorkerPermissions; document the validation-only role of the extraction helpers.pkg/cli/workflows/test-copilot-call-workflow.md— widen the caller's declared permissions (issues: read,pull-requests: read) so it covers its workertest-copilot-noopunder the new model.smoke-call-workflow.lock.yml,dependabot-campaign.lock.yml): the call jobs now carry the caller's permissions instead of the worker's.Verification
TestFindUncoveredWorkerPermissionscovers coverage, gaps, lower-level grants, nil caller/worker, andnone-level scopes.vulnerability-alertsoccurrences, with the call job carrying the caller's declared permissions.