fix: propagate worker job permissions to call-workflow caller jobs (#21061)#21080
fix: propagate worker job permissions to call-workflow caller jobs (#21061)#21080
call-workflow caller jobs (#21061)#21080Conversation
When buildCallWorkflowJobs() generates conditional `uses:` fan-out jobs
for call-workflow safe outputs, it now computes and attaches a
permissions block that is the union of all nested job permissions in the
target worker workflow.
GitHub validates reusable workflow calls against the caller job's
declared permission envelope. Without a permissions block on the call-*
job, nested worker jobs that request any non-none permissions are
rejected before execution starts.
- Add call_workflow_permissions.go with three helpers:
* extractJobPermissionsFromParsedWorkflow: unions all job-level
permissions from a parsed workflow map
* extractCallWorkflowPermissions: resolves the worker file
(.lock.yml > .yml > .md) and extracts permissions
* extractPermissionsFromYAMLFile / extractPermissionsFromMDFile:
file-type-specific readers
- Modify buildCallWorkflowJobs to accept markdownPath and call
extractCallWorkflowPermissions for each generated call-* job;
sets callJob.Permissions when a non-empty permission set is found
- Add regression tests in call_workflow_permissions_test.go covering
.lock.yml, .yml, .md, and missing-file cases, plus YAML output checks
Fixes #21061
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Clarify nosec comments in call_workflow_permissions.go to explicitly reference the isPathWithinDir() pre-validation performed by findWorkflowFile() - Improve warning message in buildCallWorkflowJobs to explain impact and suggest remediation when permissions cannot be extracted - Add require.NotEqual guards before index comparison in YAML ordering test to prevent false passes when strings are absent Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot add tests |
There was a problem hiding this comment.
Pull request overview
Propagates required job-level permissions into generated call-* reusable-workflow caller jobs so nested worker jobs aren’t rejected when the top-level workflow sets permissions: {}.
Changes:
- Adds permission extraction utilities to union worker job permissions from
.lock.yml/.yml(or.mdfrontmatter as a proxy). - Updates
buildCallWorkflowJobsto compute and attach the permission superset to each generatedcall-*job. - Adds/updates unit tests to cover permission extraction and YAML rendering behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/workflow/call_workflow_permissions.go | New helpers to resolve worker workflow files and extract/merge permissions. |
| pkg/workflow/compiler_safe_output_jobs.go | Threads markdownPath into buildCallWorkflowJobs and sets callJob.Permissions based on extracted permissions. |
| pkg/workflow/safe_outputs_call_workflow_test.go | Updates existing tests to match the new buildCallWorkflowJobs signature. |
| pkg/workflow/call_workflow_permissions_test.go | New regression tests for permission extraction and call-job YAML output ordering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // Returns nil when no workflow file is found or no permissions are declared. | ||
| // The caller should omit the permissions block on the call-* job in that case. |
| if err != nil || result == nil { | ||
| callWorkflowPermissionsLog.Printf("Failed to extract frontmatter from %s: %v", mdPath, err) |
| rendered := perms.RenderToYAML() | ||
| if rendered != "" { | ||
| callJob.Permissions = rendered | ||
| compilerSafeOutputJobsLog.Printf("Set permissions on call-workflow job '%s': %s", jobName, rendered) |
- TestExtractCallWorkflowPermissions_LockYMLPriorityOverYML: verifies .lock.yml takes priority over .yml when both exist for a worker - TestCallWorkflowPermissions_EndToEnd: full CompileWorkflow run with two workers that have different job-level permissions; asserts each generated call-* job in the lock file contains the correct superset - TestCallWorkflowPermissions_EndToEnd_YMLWorker: same end-to-end check for workers delivered as plain .yml files (no .lock.yml counterpart) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added 3 more tests in 8587422:
|
The integration test TestAddWorkflowWithDispatchWorkflowFromSharedImport fetches smoke-copilot.md from main which uses close-older-key in both create-issue and create-discussion safe outputs. The schema was missing this property, causing schema validation to fail and the compiled lock file to not be generated. - Add close-older-key string property to create-issue schema - Add close-older-key string property to create-discussion schema - Sync smoke-copilot.md with main (add close-older-key fields) - Rebuild binary and recompile smoke-copilot.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in ee26f48. The |
|
@copilot merge main and recompile |
- Resolve schema conflict: use main's close-older-key description with pattern constraint (\\S) for both create-issue and create-discussion - Rebuild binary with updated embedded schema - Recompile all 173 workflow files Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 73152ae. Merged main, resolved the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Summary
call-workflowgenerated caller jobs omit requiredpermissions:for reusable workflows.Problem
buildCallWorkflowJobs()generatedcall-*reusable-workflow caller jobs with nopermissions:block. Becausegh-awwritespermissions: {}at the top level, the caller job inheritednonefor all scopes, and GitHub rejected any nested worker job that declared permissions before execution started.Representative failures:
Solution
At compile time, resolve the target worker file (
.lock.yml > .yml > .md), union all job-levelpermissionsblocks found in that file, and attach the result as apermissions:block on the generatedcall-*job.Changes
New file:
pkg/workflow/call_workflow_permissions.goThree focused helpers:
extractJobPermissionsFromParsedWorkflowmap[string]anyworkflow, merges every job'spermissions:into a single supersetextractCallWorkflowPermissionsfindWorkflowFile(), dispatches to the right readerextractPermissionsFromYAMLFile/extractPermissionsFromMDFile.lock.yml/.ymlor.mdfrontmatter respectivelyFor same-batch
.md-only workers (not yet compiled), the frontmatter-levelpermissions:is used as a proxy — the compiler will turn it into per-job permissions when the worker is eventually compiled.Modified:
compiler_safe_output_jobs.gobuildCallWorkflowJobsnow acceptsmarkdownPath stringcallJob, callsextractCallWorkflowPermissions; if a non-empty permission set is found, setscallJob.Permissions = perms.RenderToYAML()Modified:
safe_outputs_call_workflow_test.goUpdated four existing call sites to pass
""asmarkdownPath(maintains original behaviour — no permission extraction attempted).New tests:
call_workflow_permissions_test.go15 tests covering:
extractJobPermissionsFromParsedWorkflow: no jobs, single job, multiple jobs with write-wins-read merging, jobs with no permissionsextractCallWorkflowPermissions: from.lock.yml, from.yml, from.mdfrontmatter, file-not-found (returns nil),.lock.ymlpriority over.ymlwhen both existbuildCallWorkflowJobs: permissions set from.lock.yml, permissions set from.md, no permissions when worker has nonepermissions:block present and rendered beforeuses:CompileWorkflowwith multiple workers and different permission scopes (.lock.ymlworkers)CompileWorkflowwith plain.ymlworkerBefore / After
Before:
After:
Security Summary
No new security vulnerabilities introduced. The two
#nosec G304suppressions foros.ReadFileare safe because all file paths originate fromfindWorkflowFile(), which validates every path viaisPathWithinDir()before returning them, preventing directory traversal.📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.