Add allowed-files strict allowlist for protected-file protection on PR safe outputs#20051
Add allowed-files strict allowlist for protected-file protection on PR safe outputs#20051
allowed-files strict allowlist for protected-file protection on PR safe outputs#20051Conversation
…-request-branch and create-pull-request Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
allowed-files glob exemptions for protected-file protection on PR safe outputs
|
@copilot configure allowed files in the instruction janitor agentic workflow and in the changeset generator |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 3b1c57f.
|
|
@copilot once specified, allowed-files are an allow list (anything outside of allowed files is refused), and protected-files are overriden. |
…ns, bypass protected-files Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in 303143b.
|
allowed-files glob exemptions for protected-file protection on PR safe outputsallowed-files strict allowlist for protected-file protection on PR safe outputs
|
@copilot add tests, add allowed files to instruction janitor to let it write instruction files. |
…thub/aw/** files Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 0538d6f. Tests added (
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:
|
|
@copilot refactor added code for clarity. Use functions, avoid too much code/branches. |
…ching in handlers Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Refactored in 4882a6c. Added Both handlers now replace the 40-line nested if/else with a flat 15-line block that just switches on |
|
@lpcox message for you above. |
Contribution Check Report — PR #20051Hey ✅ Checklist
🟢 Verdict: AlignedEverything looks on track. The PR correctly follows the agentic development process and covers the full implementation surface:
The PR is currently in draft status — once any remaining in-progress items are resolved, it looks ready for maintainer review. 🚀
|
…pass, require explicit configuration Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 753ab9a. |
There was a problem hiding this comment.
Pull request overview
This PR adds a new allowed-files field to both push-to-pull-request-branch and create-pull-request safe output configurations. It acts as a strict allowlist of glob patterns: when set, every file in a patch must match at least one pattern. The checks are orthogonal with the existing protected-files policy — both must pass independently.
Changes:
- Adds
AllowedFilesto Go structs and wires it through the compiler to JS handlers; addscheckAllowedFilesandcheckFileProtectionhelpers to consolidate file protection logic - Refactors both handler files to use the new
checkFileProtectionfunction, replacing ~40 lines of nested logic with a flat ~15-line block - Adds unit tests for
checkAllowedFiles, integration tests for both handlers, JSON schema entries, documentation, and workflow configuration updates
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/push_to_pull_request_branch.go |
Adds AllowedFiles field and its parsing |
pkg/workflow/create_pull_request.go |
Adds AllowedFiles field (parsed via YAML unmarshal) |
pkg/workflow/compiler_safe_outputs_config.go |
Wires allowed_files into handler config for both handlers |
pkg/parser/schemas/main_workflow_schema.json |
Adds allowed-files schema for both safe outputs |
docs/src/content/docs/reference/safe-outputs-pull-requests.md |
Documents allowed-files behavior |
actions/setup/js/manifest_file_helpers.cjs |
New checkAllowedFiles and checkFileProtection helpers |
actions/setup/js/manifest_file_helpers.test.cjs |
Unit tests for checkAllowedFiles |
actions/setup/js/push_to_pull_request_branch.cjs |
Refactored to use checkFileProtection |
actions/setup/js/push_to_pull_request_branch.test.cjs |
Integration tests for allowed-files |
actions/setup/js/create_pull_request.cjs |
Refactored to use checkFileProtection |
actions/setup/js/create_pull_request.test.cjs |
Integration tests for allowed-files |
.github/workflows/instructions-janitor.md |
Configured allowed-files and protected-files: allowed |
.github/workflows/instructions-janitor.lock.yml |
Updated lock file with new config |
.github/workflows/changeset.md |
Configured allowed-files: [.changeset/**] |
.github/workflows/changeset.lock.yml |
Updated lock file with new config |
Comments suppressed due to low confidence (1)
pkg/parser/schemas/main_workflow_schema.json:6331
- Same issue as the
create-pull-requestschema: the description says "Takes priority over the protected-files policy" but the implementation treats them as orthogonal checks that both must pass independently.
"allowed-files": {
"type": "array",
"items": {
"type": "string"
},
"description": "List of glob patterns for files that are exempt from protected-file protection. Takes priority over the protected-files policy. Use to allow specific files (e.g. go.mod) without disabling all protected-file checks. Supports * (any characters except /) and ** (any characters including /)."
💡 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.
| const result = checkAllowedFiles(patch, [".changeset/**"]); | ||
| expect(result.hasDisallowedFiles).toBe(false); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The new checkFileProtection function (added in this PR in manifest_file_helpers.cjs) is tested indirectly via the integration tests in push_to_pull_request_branch.test.cjs and create_pull_request.test.cjs, but has no direct unit tests here. Given that checkAllowedFiles has its own unit test block here, checkFileProtection should have one as well — covering the various combinations of allowed_files, protected_files, protected_path_prefixes, and protected_files_policy.
… fix stale bypass/priority comments and add checkFileProtection unit tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in e428693. The TypeScript errors were caused by 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:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
pkg/parser/schemas/main_workflow_schema.json:6331
- Same issue as the
create-pull-requestschema: the description starts with "List of glob patterns for files that are exempt from protected-file protection" but the actual behavior is a strict allowlist where both checks must pass independently. The word "exempt" is misleading. Consider rephrasing to match the actual semantics, e.g., "Strict allowlist of glob patterns for files eligible for modification."
"description": "List of glob patterns for files that are exempt from protected-file protection. Acts as a strict allowlist checked independently of the protected-files policy; both checks must pass for a file to be allowed. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)."
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ence Document the protected-files and allowed-files fields for both create-pull-request and push-to-pull-request-branch in the comprehensive frontmatter reference. These fields were introduced in #20051 (allowed-files strict allowlist) but were missing from frontmatter-full.md even though they were already described in safe-outputs-pull-requests.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add three new terms from recent changes: - Guard Policy: experimental GitHub MCP server access control (repos + min-integrity fields, PR #20096 terminology update) - Protected Files: security mechanism for PR safe outputs (blocks sensitive file changes, PR #20051) - Allowed Files: strict scope filter for PR safe outputs (allowlist of permitted file patterns, PR #20051) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AllowedFiles []stringto Go structs and wire through compilercheckAllowedFilesinmanifest_file_helpers.cjscheckAllowedFiles, integration tests in push/create handlersinstructions-janitor.md—allowed-files: [.github/aw/**]+protected-files: allowedchangeset.md—allowed-files: [.changeset/**]checkFileProtection(patchContent, config)— encapsulates branching into a{ action, source?, files }resultallowed-filesandprotected-filesrun independently; both must passHandlerConfiginterface; updatedcheckFileProtectionJSDoc param type toHandlerConfigcheckAllowedFiles(removed "protected-files bypassed" statement)checkFileProtectioninmanifest_file_helpers.test.cjsOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.