From e21ad3379a28b9cbbff3bff90e3b9b9fb6c931af Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 12:08:54 +0000 Subject: [PATCH 01/10] Initial plan From 7f101ea8f6ed852b9c6682d9ece36b0ad99d8835 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 12:33:28 +0000 Subject: [PATCH 02/10] Add allowed-files field for protected-file exemptions in push-to-pull-request-branch and create-pull-request Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pull_request.cjs | 14 ++++---- actions/setup/js/manifest_file_helpers.cjs | 26 +++++++++++++- .../setup/js/push_to_pull_request_branch.cjs | 8 +++-- .../reference/safe-outputs-pull-requests.md | 35 +++++++++++++++++++ pkg/parser/schemas/main_workflow_schema.json | 14 ++++++++ pkg/workflow/compiler_safe_outputs_config.go | 4 ++- pkg/workflow/create_pull_request.go | 1 + pkg/workflow/push_to_pull_request_branch.go | 4 +++ 8 files changed, 95 insertions(+), 11 deletions(-) diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 44dbae7de8c..763047117e1 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -23,7 +23,7 @@ const { createCheckoutManager } = require("./dynamic_checkout.cjs"); const { getBaseBranch } = require("./get_base_branch.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); -const { checkForManifestFiles, checkForProtectedPaths } = require("./manifest_file_helpers.cjs"); +const { checkForManifestFiles, checkForProtectedPaths, filterAllowedFiles } = require("./manifest_file_helpers.cjs"); const { renderTemplate } = require("./messages_core.cjs"); /** @@ -424,11 +424,12 @@ async function main(config = {}) { // Set protected-files: fallback-to-issue to push the branch but create a review issue // instead of a pull request, so a human can carefully review the changes first. // Set protected-files: allowed only when the workflow is explicitly designed to manage these files. - /** @type {{ manifestFilesFound: string[], protectedPathsFound: string[] } | null} */ + /** @type {{ allFound: string[] } | null} */ let manifestProtectionFallback = null; if (!isEmpty) { const manifestFiles = Array.isArray(config.protected_files) ? config.protected_files : []; const protectedPathPrefixes = Array.isArray(config.protected_path_prefixes) ? config.protected_path_prefixes : []; + const allowedFilePatterns = Array.isArray(config.allowed_files) ? config.allowed_files : []; // protected_files_policy is a string enum: "allowed" = allow, "fallback-to-issue" = fallback, "blocked" (default) = deny. const policy = config.protected_files_policy; const isAllowed = policy === "allowed"; @@ -436,14 +437,15 @@ async function main(config = {}) { if (!isAllowed) { const { hasManifestFiles, manifestFilesFound } = checkForManifestFiles(patchContent, manifestFiles); const { hasProtectedPaths, protectedPathsFound } = checkForProtectedPaths(patchContent, protectedPathPrefixes); - const allFound = [...manifestFilesFound, ...protectedPathsFound]; + // Filter out files explicitly allowed via allowed-files globs (higher priority than protected-files) + const allFound = filterAllowedFiles([...manifestFilesFound, ...protectedPathsFound], allowedFilePatterns); if (allFound.length > 0) { if (isFallback) { // Record for fallback-to-issue handling below; let patch application proceed - manifestProtectionFallback = { manifestFilesFound, protectedPathsFound }; + manifestProtectionFallback = { allFound }; core.warning(`Protected file protection triggered (fallback-to-issue): ${allFound.join(", ")}. Will create review issue instead of pull request.`); } else { - const message = `Cannot create pull request: patch modifies protected files (${allFound.join(", ")}). Set protected-files: fallback-to-issue to create a review issue instead.`; + const message = `Cannot create pull request: patch modifies protected files (${allFound.join(", ")}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; core.error(message); return { success: false, error: message }; } @@ -931,7 +933,7 @@ ${patchPreview}`; // was not created and provides a PR intent URL so the reviewer can create it // after manually inspecting the protected file changes. if (manifestProtectionFallback) { - const allFound = [...manifestProtectionFallback.manifestFilesFound, ...manifestProtectionFallback.protectedPathsFound]; + const allFound = manifestProtectionFallback.allFound; const githubServer = process.env.GITHUB_SERVER_URL || "https://github.com"; const encodedBase = baseBranch.split("/").map(encodeURIComponent).join("/"); const encodedHead = branchName.split("/").map(encodeURIComponent).join("/"); diff --git a/actions/setup/js/manifest_file_helpers.cjs b/actions/setup/js/manifest_file_helpers.cjs index 7bb0338c9ab..f9169187151 100644 --- a/actions/setup/js/manifest_file_helpers.cjs +++ b/actions/setup/js/manifest_file_helpers.cjs @@ -96,4 +96,28 @@ function checkForProtectedPaths(patchContent, pathPrefixes) { return { hasProtectedPaths: found.length > 0, protectedPathsFound: found }; } -module.exports = { extractFilenamesFromPatch, extractPathsFromPatch, checkForManifestFiles, checkForProtectedPaths }; +/** + * Filters a list of protected file entries by removing entries that match at least one + * of the given allowed-files glob patterns. Entries may be either basenames (from manifest + * file checks) or full paths (from path-prefix checks). Glob matching supports `*` (matches + * any characters except `/`) and `**` (matches any characters including `/`). + * + * When `allowedFilePatterns` is empty or absent, the original list is returned unchanged. + * + * @param {string[]} protectedEntries - Mixed list of basenames and full paths that triggered protection + * @param {string[]} allowedFilePatterns - Glob patterns for files exempt from protection (e.g. ["go.mod", ".github/**"]) + * @returns {string[]} Entries that are still protected after applying the allowed-files filter + */ +function filterAllowedFiles(protectedEntries, allowedFilePatterns) { + if (!allowedFilePatterns || allowedFilePatterns.length === 0) { + return protectedEntries; + } + if (!protectedEntries || protectedEntries.length === 0) { + return []; + } + const { globPatternToRegex } = require("./glob_pattern_helpers.cjs"); + const compiledPatterns = allowedFilePatterns.map(p => globPatternToRegex(p)); + return protectedEntries.filter(entry => !compiledPatterns.some(re => re.test(entry))); +} + +module.exports = { extractFilenamesFromPatch, extractPathsFromPatch, checkForManifestFiles, checkForProtectedPaths, filterAllowedFiles }; diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 0d3ff1eb126..04956c7cea9 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -11,7 +11,7 @@ const { pushExtraEmptyCommit } = require("./extra_empty_commit.cjs"); const { detectForkPR } = require("./pr_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); -const { checkForManifestFiles, checkForProtectedPaths } = require("./manifest_file_helpers.cjs"); +const { checkForManifestFiles, checkForProtectedPaths, filterAllowedFiles } = require("./manifest_file_helpers.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { renderTemplate } = require("./messages_core.cjs"); @@ -149,6 +149,7 @@ async function main(config = {}) { if (!isEmpty) { const manifestFiles = Array.isArray(config.protected_files) ? config.protected_files : []; const protectedPathPrefixes = Array.isArray(config.protected_path_prefixes) ? config.protected_path_prefixes : []; + const allowedFilePatterns = Array.isArray(config.allowed_files) ? config.allowed_files : []; // protected_files_policy is a string enum: "allowed" = allow, "fallback-to-issue" = fallback, "blocked" (default) = deny. const policy = config.protected_files_policy; const isAllowed = policy === "allowed"; @@ -156,14 +157,15 @@ async function main(config = {}) { if (!isAllowed) { const { manifestFilesFound } = checkForManifestFiles(patchContent, manifestFiles); const { protectedPathsFound } = checkForProtectedPaths(patchContent, protectedPathPrefixes); - const allFound = [...manifestFilesFound, ...protectedPathsFound]; + // Filter out files explicitly allowed via allowed-files globs (higher priority than protected-files) + const allFound = filterAllowedFiles([...manifestFilesFound, ...protectedPathsFound], allowedFilePatterns); if (allFound.length > 0) { if (isFallback) { // Store for deferred issue creation (needs PR metadata resolved first) protectedFilesForFallback = allFound; core.warning(`Protected file protection triggered (fallback-to-issue): ${allFound.join(", ")}. Will create review issue instead of pushing.`); } else { - const msg = `Cannot push to pull request branch: patch modifies protected files (${allFound.join(", ")}). Set protected-files: fallback-to-issue to create a review issue instead.`; + const msg = `Cannot push to pull request branch: patch modifies protected files (${allFound.join(", ")}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; core.error(msg); return { success: false, error: msg }; } diff --git a/docs/src/content/docs/reference/safe-outputs-pull-requests.md b/docs/src/content/docs/reference/safe-outputs-pull-requests.md index 3971355d422..b9efbf8b348 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -113,6 +113,41 @@ safe-outputs: When protected file protection triggers and is set to `blocked`, the 🛡️ **Protected Files** section appears in the agent failure issue or comment generated by the conclusion job. It includes the blocked operation, the specific files found, and a YAML remediation snippet showing how to configure `protected-files: fallback-to-issue`. +### Exempting Specific Files with `allowed-files` + +When a workflow intentionally manages certain protected files, use `allowed-files` to exempt them without disabling all protection. Files matching any pattern in `allowed-files` bypass the `protected-files` policy entirely. + +```yaml wrap +safe-outputs: + push-to-pull-request-branch: + allowed-files: + - go.mod # allow dependency updates to go.mod + - go.sum # and the accompanying checksum file + - .github/dependabot.yml # allow Dependabot configuration updates + + create-pull-request: + allowed-files: + - package.json # allow package.json changes + - "*.lock" # allow any lockfile changes +``` + +Patterns support `*` (any characters except `/`) and `**` (any characters including `/`): + +| Pattern | Matches | +|---------|---------| +| `go.mod` | Exactly `go.mod` anywhere (basename match) | +| `*.json` | Any JSON file in the root (e.g. `package.json`) | +| `go.*` | `go.mod`, `go.sum`, etc. | +| `.github/**` | All files under `.github/` at any depth | +| `.github/workflows/*.yml` | Only YAML files directly in `.github/workflows/` | +| `**/package.json` | `package.json` at any path depth | + +> [!NOTE] +> `allowed-files` takes priority over `protected-files`. A file matching an `allowed-files` pattern is always permitted, regardless of the `protected-files` policy. + +> [!WARNING] +> `allowed-files` should list only the files the workflow legitimately manages. Listing broad patterns (e.g., `.github/**`) defeats the supply-chain protections for those paths. + ### Protected Files Protection covers three categories: diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 6323a2a4521..fe5647a9730 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -5281,6 +5281,13 @@ "enum": ["blocked", "allowed", "fallback-to-issue"], "description": "Controls protected-file protection. blocked (default): hard-block any patch that modifies package manifests (e.g. package.json, go.mod), engine instruction files (e.g. AGENTS.md, CLAUDE.md) or .github/ files. allowed: allow all changes. fallback-to-issue: push the branch but create a review issue instead of a PR, so a human can review the manifest changes before merging.", "default": "blocked" + }, + "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 /)." } }, "additionalProperties": false, @@ -6315,6 +6322,13 @@ "enum": ["blocked", "allowed", "fallback-to-issue"], "description": "Controls protected-file protection. blocked (default): hard-block any patch that modifies package manifests (e.g. package.json, go.mod), engine instruction files (e.g. AGENTS.md, CLAUDE.md) or .github/ files. allowed: allow all changes. fallback-to-issue: create a review issue instead of pushing to the PR branch, so a human can review the changes before applying.", "default": "blocked" + }, + "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 /)." } }, "additionalProperties": false diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index fe38f54b26c..c601e7e966f 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -483,7 +483,8 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("base_branch", c.BaseBranch). AddStringPtr("protected_files_policy", c.ManifestFilesPolicy). AddStringSlice("protected_files", getAllManifestFiles()). - AddStringSlice("protected_path_prefixes", getProtectedPathPrefixes()) + AddStringSlice("protected_path_prefixes", getProtectedPathPrefixes()). + AddStringSlice("allowed_files", c.AllowedFiles) return builder.Build() }, "push_to_pull_request_branch": func(cfg *SafeOutputsConfig) map[string]any { @@ -510,6 +511,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringPtr("protected_files_policy", c.ManifestFilesPolicy). AddStringSlice("protected_files", getAllManifestFiles()). AddStringSlice("protected_path_prefixes", getProtectedPathPrefixes()). + AddStringSlice("allowed_files", c.AllowedFiles). Build() }, "update_pull_request": func(cfg *SafeOutputsConfig) map[string]any { diff --git a/pkg/workflow/create_pull_request.go b/pkg/workflow/create_pull_request.go index 1134887d3db..e32684b4f2d 100644 --- a/pkg/workflow/create_pull_request.go +++ b/pkg/workflow/create_pull_request.go @@ -33,6 +33,7 @@ type CreatePullRequestsConfig struct { FallbackAsIssue *bool `yaml:"fallback-as-issue,omitempty"` // When true (default), creates an issue if PR creation fails. When false, no fallback occurs and issues: write permission is not requested. GithubTokenForExtraEmptyCommit string `yaml:"github-token-for-extra-empty-commit,omitempty"` // Token used to push an empty commit to trigger CI events. Use a PAT or "app" for GitHub App auth. ManifestFilesPolicy *string `yaml:"protected-files,omitempty"` // Controls protected-file protection: "blocked" (default) hard-blocks, "allowed" permits all changes, "fallback-to-issue" pushes the branch but creates a review issue. + AllowedFiles []string `yaml:"allowed-files,omitempty"` // List of glob patterns for files that are exempt from protected-file protection. Takes priority over protected-files. } // parsePullRequestsConfig handles only create-pull-request (singular) configuration diff --git a/pkg/workflow/push_to_pull_request_branch.go b/pkg/workflow/push_to_pull_request_branch.go index 5e159b8d8d4..02ae7b7bd1d 100644 --- a/pkg/workflow/push_to_pull_request_branch.go +++ b/pkg/workflow/push_to_pull_request_branch.go @@ -21,6 +21,7 @@ type PushToPullRequestBranchConfig struct { TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository push to pull request branch AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories in format "owner/repo" that push to pull request branch can target ManifestFilesPolicy *string `yaml:"protected-files,omitempty"` // Controls protected-file protection: "blocked" (default) hard-blocks, "allowed" permits all changes, "fallback-to-issue" creates a review issue instead of pushing. + AllowedFiles []string `yaml:"allowed-files,omitempty"` // List of glob patterns for files that are exempt from protected-file protection. Takes priority over protected-files. } // buildCheckoutRepository generates a checkout step with optional target repository and custom token @@ -142,6 +143,9 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any) pushToBranchConfig.ManifestFilesPolicy = &strVal } + // Parse allowed-files: list of glob patterns for files exempt from protection + pushToBranchConfig.AllowedFiles = ParseStringArrayFromConfig(configMap, "allowed-files", pushToPullRequestBranchLog) + // Parse common base fields with default max of 0 (no limit) c.parseBaseSafeOutputConfig(configMap, &pushToBranchConfig.BaseSafeOutputConfig, 0) } From 3b1c57f28d5eb5cda01335fd89c423b8a48d569f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 12:57:01 +0000 Subject: [PATCH 03/10] Configure allowed-files in instructions-janitor and changeset workflows Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/changeset.lock.yml | 4 ++-- .github/workflows/changeset.md | 2 ++ .github/workflows/instructions-janitor.lock.yml | 4 ++-- .github/workflows/instructions-janitor.md | 2 ++ 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/changeset.lock.yml b/.github/workflows/changeset.lock.yml index e300c9a9252..e5a82037aaa 100644 --- a/.github/workflows/changeset.lock.yml +++ b/.github/workflows/changeset.lock.yml @@ -29,7 +29,7 @@ # - shared/jqschema.md # - shared/safe-output-app.md # -# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"30ae52017856bedddf21f6ea82fde8c336122be8d203aea1514f1cb2b3ce4268","strict":true} +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"a68f2edc9e5d35050adaf025a4b924895ab5ec5219264fd0237515b01c0fbb76","strict":true} name: "Changeset Generator" "on": @@ -1277,7 +1277,7 @@ jobs: GH_AW_ALLOWED_DOMAINS: "*.jsr.io,172.30.0.1,api.npms.io,api.openai.com,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,bun.sh,cdn.jsdelivr.net,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,deb.nodesource.com,deno.land,esm.sh,get.pnpm.io,go.dev,golang.org,googleapis.deno.dev,googlechromelabs.github.io,goproxy.io,host.docker.internal,json-schema.org,json.schemastore.org,jsr.io,keyserver.ubuntu.com,nodejs.org,npm.pkg.github.com,npmjs.com,npmjs.org,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,openai.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,pkg.go.dev,ppa.launchpad.net,proxy.golang.org,registry.bower.io,registry.npmjs.com,registry.npmjs.org,registry.yarnpkg.com,repo.yarnpkg.com,s.symcb.com,s.symcd.com,security.ubuntu.com,skimdb.npmjs.com,storage.googleapis.com,sum.golang.org,telemetry.vercel.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com,www.npmjs.com,www.npmjs.org,yarnpkg.com" GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"missing_data\":{},\"missing_tool\":{},\"push_to_pull_request_branch\":{\"commit_title_suffix\":\" [skip-ci]\",\"if_no_changes\":\"warn\",\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"AGENTS.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".codex/\"]},\"update_pull_request\":{\"allow_body\":true,\"allow_title\":false,\"default_operation\":\"append\",\"max\":1}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"missing_data\":{},\"missing_tool\":{},\"push_to_pull_request_branch\":{\"allowed_files\":[\".changeset/**\"],\"commit_title_suffix\":\" [skip-ci]\",\"if_no_changes\":\"warn\",\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"AGENTS.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".codex/\"]},\"update_pull_request\":{\"allow_body\":true,\"allow_title\":false,\"default_operation\":\"append\",\"max\":1}}" GH_AW_CI_TRIGGER_TOKEN: ${{ secrets.GH_AW_CI_TRIGGER_TOKEN }} with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/changeset.md b/.github/workflows/changeset.md index 948d04b78bd..4b47c588d93 100644 --- a/.github/workflows/changeset.md +++ b/.github/workflows/changeset.md @@ -19,6 +19,8 @@ strict: true safe-outputs: push-to-pull-request-branch: commit-title-suffix: " [skip-ci]" + allowed-files: + - .changeset/** update-pull-request: title: false operation: append diff --git a/.github/workflows/instructions-janitor.lock.yml b/.github/workflows/instructions-janitor.lock.yml index 6a5cda6fe82..86f6e730b91 100644 --- a/.github/workflows/instructions-janitor.lock.yml +++ b/.github/workflows/instructions-janitor.lock.yml @@ -23,7 +23,7 @@ # # Reviews and cleans up instruction files to ensure clarity, consistency, and adherence to best practices # -# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"2b58ac826f62d19d5c8c1a4e00a7fcb7716118e1f6a7035bb9a05f66507246d3","strict":true} +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"9d2aafb5bf68f13f57b7e8826c71a618e00f1a11779f42818787e9337c64fc5b","strict":true} name: "Instructions Janitor" "on": @@ -1271,7 +1271,7 @@ jobs: GH_AW_ALLOWED_DOMAINS: "*.githubusercontent.com,anthropic.com,api.anthropic.com,api.github.com,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,cdn.playwright.dev,codeload.github.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,files.pythonhosted.org,ghcr.io,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.com,github.githubassets.com,host.docker.internal,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,lfs.github.com,objects.githubusercontent.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,playwright.download.prss.microsoft.com,ppa.launchpad.net,pypi.org,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,sentry.io,statsig.anthropic.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com" GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"create_pull_request\":{\"draft\":false,\"expires\":48,\"labels\":[\"documentation\",\"automation\",\"instructions\"],\"max\":1,\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"CLAUDE.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".claude/\"],\"title_prefix\":\"[instructions] \"},\"missing_data\":{},\"missing_tool\":{}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"create_pull_request\":{\"allowed_files\":[\".github/aw/github-agentic-workflows.md\"],\"draft\":false,\"expires\":48,\"labels\":[\"documentation\",\"automation\",\"instructions\"],\"max\":1,\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"CLAUDE.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".claude/\"],\"title_prefix\":\"[instructions] \"},\"missing_data\":{},\"missing_tool\":{}}" GH_AW_CI_TRIGGER_TOKEN: ${{ secrets.GH_AW_CI_TRIGGER_TOKEN }} with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/instructions-janitor.md b/.github/workflows/instructions-janitor.md index 7ce75d2c789..6acfb27b5ff 100644 --- a/.github/workflows/instructions-janitor.md +++ b/.github/workflows/instructions-janitor.md @@ -24,6 +24,8 @@ safe-outputs: title-prefix: "[instructions] " labels: [documentation, automation, instructions] draft: false + allowed-files: + - .github/aw/github-agentic-workflows.md tools: cache-memory: true From 303143b4e4c1a46fbc5325c010c7cfea18f75088 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 16:17:50 +0000 Subject: [PATCH 04/10] Change allowed-files to strict allowlist: refuse files outside patterns, bypass protected-files Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pull_request.cjs | 48 +++++++++------ actions/setup/js/manifest_file_helpers.cjs | 33 ++++++----- .../setup/js/manifest_file_helpers.test.cjs | 58 ++++++++++++++++++- .../setup/js/push_to_pull_request_branch.cjs | 48 +++++++++------ .../reference/safe-outputs-pull-requests.md | 19 +++--- 5 files changed, 142 insertions(+), 64 deletions(-) diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 763047117e1..2aa81406d6f 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -23,7 +23,7 @@ const { createCheckoutManager } = require("./dynamic_checkout.cjs"); const { getBaseBranch } = require("./get_base_branch.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); -const { checkForManifestFiles, checkForProtectedPaths, filterAllowedFiles } = require("./manifest_file_helpers.cjs"); +const { checkForManifestFiles, checkForProtectedPaths, checkAllowedFiles } = require("./manifest_file_helpers.cjs"); const { renderTemplate } = require("./messages_core.cjs"); /** @@ -430,24 +430,34 @@ async function main(config = {}) { const manifestFiles = Array.isArray(config.protected_files) ? config.protected_files : []; const protectedPathPrefixes = Array.isArray(config.protected_path_prefixes) ? config.protected_path_prefixes : []; const allowedFilePatterns = Array.isArray(config.allowed_files) ? config.allowed_files : []; - // protected_files_policy is a string enum: "allowed" = allow, "fallback-to-issue" = fallback, "blocked" (default) = deny. - const policy = config.protected_files_policy; - const isAllowed = policy === "allowed"; - const isFallback = policy === "fallback-to-issue"; - if (!isAllowed) { - const { hasManifestFiles, manifestFilesFound } = checkForManifestFiles(patchContent, manifestFiles); - const { hasProtectedPaths, protectedPathsFound } = checkForProtectedPaths(patchContent, protectedPathPrefixes); - // Filter out files explicitly allowed via allowed-files globs (higher priority than protected-files) - const allFound = filterAllowedFiles([...manifestFilesFound, ...protectedPathsFound], allowedFilePatterns); - if (allFound.length > 0) { - if (isFallback) { - // Record for fallback-to-issue handling below; let patch application proceed - manifestProtectionFallback = { allFound }; - core.warning(`Protected file protection triggered (fallback-to-issue): ${allFound.join(", ")}. Will create review issue instead of pull request.`); - } else { - const message = `Cannot create pull request: patch modifies protected files (${allFound.join(", ")}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; - core.error(message); - return { success: false, error: message }; + if (allowedFilePatterns.length > 0) { + // Strict allowlist mode: only files matching allowed-files patterns are permitted. + // protected-files checks are completely bypassed. + const { hasDisallowedFiles, disallowedFiles } = checkAllowedFiles(patchContent, allowedFilePatterns); + if (hasDisallowedFiles) { + const message = `Cannot create pull request: patch modifies files outside the allowed-files list (${disallowedFiles.join(", ")}). Add the files to the allowed-files configuration field or remove them from the patch.`; + core.error(message); + return { success: false, error: message }; + } + } else { + // protected_files_policy is a string enum: "allowed" = allow, "fallback-to-issue" = fallback, "blocked" (default) = deny. + const policy = config.protected_files_policy; + const isAllowed = policy === "allowed"; + const isFallback = policy === "fallback-to-issue"; + if (!isAllowed) { + const { hasManifestFiles, manifestFilesFound } = checkForManifestFiles(patchContent, manifestFiles); + const { hasProtectedPaths, protectedPathsFound } = checkForProtectedPaths(patchContent, protectedPathPrefixes); + const allFound = [...manifestFilesFound, ...protectedPathsFound]; + if (allFound.length > 0) { + if (isFallback) { + // Record for fallback-to-issue handling below; let patch application proceed + manifestProtectionFallback = { allFound }; + core.warning(`Protected file protection triggered (fallback-to-issue): ${allFound.join(", ")}. Will create review issue instead of pull request.`); + } else { + const message = `Cannot create pull request: patch modifies protected files (${allFound.join(", ")}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; + core.error(message); + return { success: false, error: message }; + } } } } diff --git a/actions/setup/js/manifest_file_helpers.cjs b/actions/setup/js/manifest_file_helpers.cjs index f9169187151..f17162ba834 100644 --- a/actions/setup/js/manifest_file_helpers.cjs +++ b/actions/setup/js/manifest_file_helpers.cjs @@ -97,27 +97,32 @@ function checkForProtectedPaths(patchContent, pathPrefixes) { } /** - * Filters a list of protected file entries by removing entries that match at least one - * of the given allowed-files glob patterns. Entries may be either basenames (from manifest - * file checks) or full paths (from path-prefix checks). Glob matching supports `*` (matches - * any characters except `/`) and `**` (matches any characters including `/`). + * Checks all files in a patch against an allowlist of glob patterns. + * When `allowed-files` is configured, it acts as a strict allowlist: every file + * touched by the patch must match at least one pattern; files that do not match + * are returned as disallowed. Protected-files checks are completely bypassed + * when this function is used. * - * When `allowedFilePatterns` is empty or absent, the original list is returned unchanged. + * Glob matching supports `*` (matches any characters except `/`) and `**` (matches + * any characters including `/`). Each changed file is tested as its full path + * (e.g. `.github/workflows/ci.yml`) against the provided patterns. * - * @param {string[]} protectedEntries - Mixed list of basenames and full paths that triggered protection - * @param {string[]} allowedFilePatterns - Glob patterns for files exempt from protection (e.g. ["go.mod", ".github/**"]) - * @returns {string[]} Entries that are still protected after applying the allowed-files filter + * @param {string} patchContent - The git patch content + * @param {string[]} allowedFilePatterns - Glob patterns for files permitted by the allowlist + * @returns {{ hasDisallowedFiles: boolean, disallowedFiles: string[] }} */ -function filterAllowedFiles(protectedEntries, allowedFilePatterns) { +function checkAllowedFiles(patchContent, allowedFilePatterns) { if (!allowedFilePatterns || allowedFilePatterns.length === 0) { - return protectedEntries; + return { hasDisallowedFiles: false, disallowedFiles: [] }; } - if (!protectedEntries || protectedEntries.length === 0) { - return []; + const allPaths = extractPathsFromPatch(patchContent); + if (allPaths.length === 0) { + return { hasDisallowedFiles: false, disallowedFiles: [] }; } const { globPatternToRegex } = require("./glob_pattern_helpers.cjs"); const compiledPatterns = allowedFilePatterns.map(p => globPatternToRegex(p)); - return protectedEntries.filter(entry => !compiledPatterns.some(re => re.test(entry))); + const disallowedFiles = allPaths.filter(p => !compiledPatterns.some(re => re.test(p))); + return { hasDisallowedFiles: disallowedFiles.length > 0, disallowedFiles }; } -module.exports = { extractFilenamesFromPatch, extractPathsFromPatch, checkForManifestFiles, checkForProtectedPaths, filterAllowedFiles }; +module.exports = { extractFilenamesFromPatch, extractPathsFromPatch, checkForManifestFiles, checkForProtectedPaths, checkAllowedFiles }; diff --git a/actions/setup/js/manifest_file_helpers.test.cjs b/actions/setup/js/manifest_file_helpers.test.cjs index 573d2c5f031..5fbdc675e48 100644 --- a/actions/setup/js/manifest_file_helpers.test.cjs +++ b/actions/setup/js/manifest_file_helpers.test.cjs @@ -3,7 +3,7 @@ import { describe, it, expect } from "vitest"; import { createRequire } from "module"; const require = createRequire(import.meta.url); -const { extractFilenamesFromPatch, checkForManifestFiles } = require("./manifest_file_helpers.cjs"); +const { extractFilenamesFromPatch, checkForManifestFiles, checkAllowedFiles } = require("./manifest_file_helpers.cjs"); describe("manifest_file_helpers", () => { describe("extractFilenamesFromPatch", () => { @@ -279,4 +279,60 @@ index abc..def 100644 expect(basenameResult.hasManifestFiles).toBe(true); }); }); + + describe("checkAllowedFiles", () => { + it("should return no disallowed files when patterns is empty", () => { + const patch = `diff --git a/src/index.js b/src/index.js\n`; + const result = checkAllowedFiles(patch, []); + expect(result.hasDisallowedFiles).toBe(false); + expect(result.disallowedFiles).toEqual([]); + }); + + it("should return no disallowed files for empty patch", () => { + const result = checkAllowedFiles("", [".changeset/**"]); + expect(result.hasDisallowedFiles).toBe(false); + expect(result.disallowedFiles).toEqual([]); + }); + + it("should allow all files when all match the allowlist", () => { + const patch = `diff --git a/.changeset/patch-fix.md b/.changeset/patch-fix.md\nindex abc..def 100644\n`; + const result = checkAllowedFiles(patch, [".changeset/**"]); + expect(result.hasDisallowedFiles).toBe(false); + expect(result.disallowedFiles).toEqual([]); + }); + + it("should flag files not matching any allowed pattern", () => { + const patch = `diff --git a/src/index.js b/src/index.js\nindex abc..def 100644\n`; + const result = checkAllowedFiles(patch, [".changeset/**"]); + expect(result.hasDisallowedFiles).toBe(true); + expect(result.disallowedFiles).toContain("src/index.js"); + }); + + it("should flag only the file outside the allowlist when mixed", () => { + const patch = [`diff --git a/.changeset/patch-fix.md b/.changeset/patch-fix.md`, `index abc..def 100644`, `diff --git a/src/index.js b/src/index.js`, `index abc..def 100644`].join("\n"); + const result = checkAllowedFiles(patch, [".changeset/**"]); + expect(result.hasDisallowedFiles).toBe(true); + expect(result.disallowedFiles).toContain("src/index.js"); + expect(result.disallowedFiles).not.toContain(".changeset/patch-fix.md"); + }); + + it("should allow protected files when they are in the allowlist (protected-files bypassed)", () => { + const patch = `diff --git a/.github/aw/instructions.md b/.github/aw/instructions.md\nindex abc..def 100644\n`; + const result = checkAllowedFiles(patch, [".github/aw/instructions.md"]); + expect(result.hasDisallowedFiles).toBe(false); + }); + + it("should flag protected files not in the allowlist", () => { + const patch = `diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml\nindex abc..def 100644\n`; + const result = checkAllowedFiles(patch, [".github/aw/instructions.md"]); + expect(result.hasDisallowedFiles).toBe(true); + expect(result.disallowedFiles).toContain(".github/workflows/ci.yml"); + }); + + it("should support ** glob for deep path matching", () => { + const patch = `diff --git a/.changeset/deep/nested/entry.md b/.changeset/deep/nested/entry.md\nindex abc..def 100644\n`; + const result = checkAllowedFiles(patch, [".changeset/**"]); + expect(result.hasDisallowedFiles).toBe(false); + }); + }); }); diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 04956c7cea9..7b34d3e5c15 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -11,7 +11,7 @@ const { pushExtraEmptyCommit } = require("./extra_empty_commit.cjs"); const { detectForkPR } = require("./pr_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); -const { checkForManifestFiles, checkForProtectedPaths, filterAllowedFiles } = require("./manifest_file_helpers.cjs"); +const { checkForManifestFiles, checkForProtectedPaths, checkAllowedFiles } = require("./manifest_file_helpers.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { renderTemplate } = require("./messages_core.cjs"); @@ -150,24 +150,34 @@ async function main(config = {}) { const manifestFiles = Array.isArray(config.protected_files) ? config.protected_files : []; const protectedPathPrefixes = Array.isArray(config.protected_path_prefixes) ? config.protected_path_prefixes : []; const allowedFilePatterns = Array.isArray(config.allowed_files) ? config.allowed_files : []; - // protected_files_policy is a string enum: "allowed" = allow, "fallback-to-issue" = fallback, "blocked" (default) = deny. - const policy = config.protected_files_policy; - const isAllowed = policy === "allowed"; - const isFallback = policy === "fallback-to-issue"; - if (!isAllowed) { - const { manifestFilesFound } = checkForManifestFiles(patchContent, manifestFiles); - const { protectedPathsFound } = checkForProtectedPaths(patchContent, protectedPathPrefixes); - // Filter out files explicitly allowed via allowed-files globs (higher priority than protected-files) - const allFound = filterAllowedFiles([...manifestFilesFound, ...protectedPathsFound], allowedFilePatterns); - if (allFound.length > 0) { - if (isFallback) { - // Store for deferred issue creation (needs PR metadata resolved first) - protectedFilesForFallback = allFound; - core.warning(`Protected file protection triggered (fallback-to-issue): ${allFound.join(", ")}. Will create review issue instead of pushing.`); - } else { - const msg = `Cannot push to pull request branch: patch modifies protected files (${allFound.join(", ")}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; - core.error(msg); - return { success: false, error: msg }; + if (allowedFilePatterns.length > 0) { + // Strict allowlist mode: only files matching allowed-files patterns are permitted. + // protected-files checks are completely bypassed. + const { hasDisallowedFiles, disallowedFiles } = checkAllowedFiles(patchContent, allowedFilePatterns); + if (hasDisallowedFiles) { + const msg = `Cannot push to pull request branch: patch modifies files outside the allowed-files list (${disallowedFiles.join(", ")}). Add the files to the allowed-files configuration field or remove them from the patch.`; + core.error(msg); + return { success: false, error: msg }; + } + } else { + // protected_files_policy is a string enum: "allowed" = allow, "fallback-to-issue" = fallback, "blocked" (default) = deny. + const policy = config.protected_files_policy; + const isAllowed = policy === "allowed"; + const isFallback = policy === "fallback-to-issue"; + if (!isAllowed) { + const { manifestFilesFound } = checkForManifestFiles(patchContent, manifestFiles); + const { protectedPathsFound } = checkForProtectedPaths(patchContent, protectedPathPrefixes); + const allFound = [...manifestFilesFound, ...protectedPathsFound]; + if (allFound.length > 0) { + if (isFallback) { + // Store for deferred issue creation (needs PR metadata resolved first) + protectedFilesForFallback = allFound; + core.warning(`Protected file protection triggered (fallback-to-issue): ${allFound.join(", ")}. Will create review issue instead of pushing.`); + } else { + const msg = `Cannot push to pull request branch: patch modifies protected files (${allFound.join(", ")}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; + core.error(msg); + return { success: false, error: msg }; + } } } } diff --git a/docs/src/content/docs/reference/safe-outputs-pull-requests.md b/docs/src/content/docs/reference/safe-outputs-pull-requests.md index b9efbf8b348..548f86e54ca 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -115,38 +115,35 @@ When protected file protection triggers and is set to `blocked`, the 🛡️ **P ### Exempting Specific Files with `allowed-files` -When a workflow intentionally manages certain protected files, use `allowed-files` to exempt them without disabling all protection. Files matching any pattern in `allowed-files` bypass the `protected-files` policy entirely. +When a workflow is designed to modify only specific files, use `allowed-files` to define a strict allowlist. When set, every file touched by the patch must match at least one pattern — any file outside the list is refused. The `protected-files` policy is completely bypassed when `allowed-files` is configured. ```yaml wrap safe-outputs: push-to-pull-request-branch: allowed-files: - - go.mod # allow dependency updates to go.mod - - go.sum # and the accompanying checksum file - - .github/dependabot.yml # allow Dependabot configuration updates + - .changeset/** # only changeset files may be pushed create-pull-request: allowed-files: - - package.json # allow package.json changes - - "*.lock" # allow any lockfile changes + - .github/aw/instructions.md # only this one file may be modified ``` Patterns support `*` (any characters except `/`) and `**` (any characters including `/`): | Pattern | Matches | |---------|---------| -| `go.mod` | Exactly `go.mod` anywhere (basename match) | -| `*.json` | Any JSON file in the root (e.g. `package.json`) | -| `go.*` | `go.mod`, `go.sum`, etc. | +| `go.mod` | Exactly `go.mod` at any path (full path comparison) | +| `*.json` | Any JSON file at the root (e.g. `package.json`) | +| `go.*` | `go.mod`, `go.sum`, etc. at the root | | `.github/**` | All files under `.github/` at any depth | | `.github/workflows/*.yml` | Only YAML files directly in `.github/workflows/` | | `**/package.json` | `package.json` at any path depth | > [!NOTE] -> `allowed-files` takes priority over `protected-files`. A file matching an `allowed-files` pattern is always permitted, regardless of the `protected-files` policy. +> When `allowed-files` is set, it acts as a strict allowlist: only the listed files may be modified. Any file outside the list is refused and `protected-files` is ignored. When `allowed-files` is not set, the `protected-files` policy applies as usual. > [!WARNING] -> `allowed-files` should list only the files the workflow legitimately manages. Listing broad patterns (e.g., `.github/**`) defeats the supply-chain protections for those paths. +> `allowed-files` should enumerate exactly the files the workflow legitimately manages. Overly broad patterns (e.g., `**`) disable all protection. ### Protected Files From 0538d6f44b33217fc58927887ac68cbd5ab80ba3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 16:30:52 +0000 Subject: [PATCH 05/10] Add allowed-files tests; expand instructions-janitor to allow all .github/aw/** files Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../workflows/instructions-janitor.lock.yml | 4 +- .github/workflows/instructions-janitor.md | 2 +- actions/setup/js/create_pull_request.test.cjs | 168 +++++++++++++++++- .../js/push_to_pull_request_branch.test.cjs | 102 +++++++++++ 4 files changed, 272 insertions(+), 4 deletions(-) diff --git a/.github/workflows/instructions-janitor.lock.yml b/.github/workflows/instructions-janitor.lock.yml index 86f6e730b91..8f628f55f95 100644 --- a/.github/workflows/instructions-janitor.lock.yml +++ b/.github/workflows/instructions-janitor.lock.yml @@ -23,7 +23,7 @@ # # Reviews and cleans up instruction files to ensure clarity, consistency, and adherence to best practices # -# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"9d2aafb5bf68f13f57b7e8826c71a618e00f1a11779f42818787e9337c64fc5b","strict":true} +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"9c6a532a9148c023dd6cb7c1e750e515005ddd97d1e9a2a54397bb1701f88848","strict":true} name: "Instructions Janitor" "on": @@ -1271,7 +1271,7 @@ jobs: GH_AW_ALLOWED_DOMAINS: "*.githubusercontent.com,anthropic.com,api.anthropic.com,api.github.com,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,cdn.playwright.dev,codeload.github.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,files.pythonhosted.org,ghcr.io,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.com,github.githubassets.com,host.docker.internal,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,lfs.github.com,objects.githubusercontent.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,playwright.download.prss.microsoft.com,ppa.launchpad.net,pypi.org,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,sentry.io,statsig.anthropic.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com" GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"create_pull_request\":{\"allowed_files\":[\".github/aw/github-agentic-workflows.md\"],\"draft\":false,\"expires\":48,\"labels\":[\"documentation\",\"automation\",\"instructions\"],\"max\":1,\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"CLAUDE.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".claude/\"],\"title_prefix\":\"[instructions] \"},\"missing_data\":{},\"missing_tool\":{}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"create_pull_request\":{\"allowed_files\":[\".github/aw/**\"],\"draft\":false,\"expires\":48,\"labels\":[\"documentation\",\"automation\",\"instructions\"],\"max\":1,\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"CLAUDE.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".claude/\"],\"title_prefix\":\"[instructions] \"},\"missing_data\":{},\"missing_tool\":{}}" GH_AW_CI_TRIGGER_TOKEN: ${{ secrets.GH_AW_CI_TRIGGER_TOKEN }} with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/instructions-janitor.md b/.github/workflows/instructions-janitor.md index 6acfb27b5ff..77b57d94acb 100644 --- a/.github/workflows/instructions-janitor.md +++ b/.github/workflows/instructions-janitor.md @@ -25,7 +25,7 @@ safe-outputs: labels: [documentation, automation, instructions] draft: false allowed-files: - - .github/aw/github-agentic-workflows.md + - .github/aw/** tools: cache-memory: true diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index a36aefb7499..226515aced6 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -1,6 +1,9 @@ // @ts-check -import { describe, it, expect, beforeEach, vi } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { createRequire } from "module"; +import * as fs from "fs"; +import * as path from "path"; +import * as os from "os"; const require = createRequire(import.meta.url); @@ -235,3 +238,166 @@ describe("create_pull_request - security: branch name sanitization", () => { expect(normalizeBranchName("UPPERCASE")).toBe("uppercase"); }); }); + +// ────────────────────────────────────────────────────── +// allowed-files strict allowlist +// ────────────────────────────────────────────────────── + +describe("create_pull_request - allowed-files strict allowlist", () => { + let tempDir; + let originalEnv; + + beforeEach(() => { + originalEnv = { ...process.env }; + process.env.GH_AW_WORKFLOW_ID = "test-workflow"; + process.env.GITHUB_REPOSITORY = "test-owner/test-repo"; + process.env.GITHUB_BASE_REF = "main"; + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-allowed-test-")); + + global.core = { + info: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + setFailed: vi.fn(), + setOutput: vi.fn(), + startGroup: vi.fn(), + endGroup: vi.fn(), + summary: { + addRaw: vi.fn().mockReturnThis(), + write: vi.fn().mockResolvedValue(undefined), + }, + }; + global.github = { + rest: { + pulls: { + create: vi.fn().mockResolvedValue({ data: { number: 1, html_url: "https://github.com/test" } }), + }, + repos: { + get: vi.fn().mockResolvedValue({ data: { default_branch: "main" } }), + }, + }, + graphql: vi.fn(), + }; + global.context = { + eventName: "workflow_dispatch", + repo: { owner: "test-owner", repo: "test-repo" }, + payload: {}, + }; + global.exec = { + exec: vi.fn().mockResolvedValue(0), + getExecOutput: vi.fn().mockResolvedValue({ exitCode: 0, stdout: "abc123\n", stderr: "" }), + }; + + // Clear module cache so globals are picked up fresh + delete require.cache[require.resolve("./create_pull_request.cjs")]; + }); + + afterEach(() => { + for (const key of Object.keys(process.env)) { + if (!(key in originalEnv)) { + delete process.env[key]; + } + } + Object.assign(process.env, originalEnv); + + if (tempDir && fs.existsSync(tempDir)) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + + delete global.core; + delete global.github; + delete global.context; + delete global.exec; + vi.clearAllMocks(); + }); + + /** + * Creates a minimal git patch touching the given file paths. + */ + function createPatchWithFiles(...filePaths) { + const diffs = filePaths + .map( + p => `diff --git a/${p} b/${p} +new file mode 100644 +index 0000000..abc1234 +--- /dev/null ++++ b/${p} +@@ -0,0 +1 @@ ++content +` + ) + .join("\n"); + return `From abc123 Mon Sep 17 00:00:00 2001 +From: Test Author +Date: Mon, 1 Jan 2024 00:00:00 +0000 +Subject: [PATCH] Test commit + +${diffs} +-- +2.34.1 +`; + } + + function writePatch(content) { + const p = path.join(tempDir, "test.patch"); + fs.writeFileSync(p, content); + return p; + } + + it("should reject files outside the allowed-files allowlist", async () => { + const patchPath = writePatch(createPatchWithFiles("src/index.js")); + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({ allowed_files: [".github/aw/**"] }); + const result = await handler({ patch_path: patchPath, title: "Test PR", body: "" }, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("outside the allowed-files list"); + expect(result.error).toContain("src/index.js"); + }); + + it("should reject a mixed patch where some files are outside the allowlist", async () => { + const patchPath = writePatch(createPatchWithFiles(".github/aw/github-agentic-workflows.md", "src/index.js")); + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({ allowed_files: [".github/aw/**"] }); + const result = await handler({ patch_path: patchPath, title: "Test PR", body: "" }, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("outside the allowed-files list"); + expect(result.error).toContain("src/index.js"); + expect(result.error).not.toContain(".github/aw/github-agentic-workflows.md"); + }); + + it("should bypass protected-files when allowed-files is configured and file matches", async () => { + // .github/ is a protected path prefix, but when allowed_files is set and the file + // matches, protected-files is bypassed entirely + const patchPath = writePatch(createPatchWithFiles(".github/aw/instructions.md")); + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({ + allowed_files: [".github/aw/**"], + protected_path_prefixes: [".github/"], + protected_files_policy: "blocked", + }); + const result = await handler({ patch_path: patchPath, title: "Test PR", body: "" }, {}); + + // Should not be blocked by protected-files — either succeeds or fails for other reasons + expect(result.error || "").not.toContain("protected files"); + }); + + it("should still enforce protected-files when allowed-files is not set", async () => { + const patchPath = writePatch(createPatchWithFiles(".github/aw/instructions.md")); + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({ + protected_path_prefixes: [".github/"], + protected_files_policy: "blocked", + }); + const result = await handler({ patch_path: patchPath, title: "Test PR", body: "" }, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("protected files"); + }); +}); diff --git a/actions/setup/js/push_to_pull_request_branch.test.cjs b/actions/setup/js/push_to_pull_request_branch.test.cjs index dad9804787f..3cb1b87d761 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -1046,6 +1046,108 @@ index 0000000..abc1234 expect(typeof module.main).toBe("function"); }); }); + + // ────────────────────────────────────────────────────── + // allowed-files strict allowlist + // ────────────────────────────────────────────────────── + + describe("allowed-files strict allowlist", () => { + /** + * Helper to create a patch that touches only the given file path(s). + * Produces minimal but valid `diff --git` headers so extractPathsFromPatch works. + */ + function createPatchWithFiles(...filePaths) { + const diffs = filePaths + .map( + p => `diff --git a/${p} b/${p} +new file mode 100644 +index 0000000..abc1234 +--- /dev/null ++++ b/${p} +@@ -0,0 +1 @@ ++content +` + ) + .join("\n"); + return `From abc123 Mon Sep 17 00:00:00 2001 +From: Test Author +Date: Mon, 1 Jan 2024 00:00:00 +0000 +Subject: [PATCH] Test commit + +${diffs} +-- +2.34.1 +`; + } + + it("should reject files outside the allowed-files allowlist", async () => { + const patchPath = createPatchFile(createPatchWithFiles("src/index.js")); + + const module = await loadModule(); + const handler = await module.main({ allowed_files: [".changeset/**"] }); + const result = await handler({ patch_path: patchPath }, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("outside the allowed-files list"); + expect(result.error).toContain("src/index.js"); + }); + + it("should accept files that match the allowed-files pattern", async () => { + const patchPath = createPatchFile(createPatchWithFiles(".changeset/my-feature-fix.md")); + mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: "abc123\n", stderr: "" }); + + const module = await loadModule(); + const handler = await module.main({ allowed_files: [".changeset/**"] }); + const result = await handler({ patch_path: patchPath }, {}); + + expect(result.success).toBe(true); + }); + + it("should bypass protected-files check when file is in the allowlist", async () => { + // package.json would normally be blocked by protected-files, but + // when allowed_files is set and it matches, protected-files is bypassed entirely + const patchPath = createPatchFile(createPatchWithFiles("package.json")); + mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: "abc123\n", stderr: "" }); + + const module = await loadModule(); + const handler = await module.main({ + allowed_files: ["package.json"], + protected_files: ["package.json"], + protected_files_policy: "blocked", + }); + const result = await handler({ patch_path: patchPath }, {}); + + expect(result.success).toBe(true); + }); + + it("should block a protected file when no allowed-files list is configured", async () => { + const patchPath = createPatchFile(createPatchWithFiles("package.json")); + + const module = await loadModule(); + const handler = await module.main({ + protected_files: ["package.json"], + protected_files_policy: "blocked", + }); + const result = await handler({ patch_path: patchPath }, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("protected files"); + expect(result.error).toContain("package.json"); + }); + + it("should reject a mixed patch where at least one file is outside the allowlist", async () => { + const patchPath = createPatchFile(createPatchWithFiles(".changeset/my-fix.md", "src/index.js")); + + const module = await loadModule(); + const handler = await module.main({ allowed_files: [".changeset/**"] }); + const result = await handler({ patch_path: patchPath }, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("outside the allowed-files list"); + expect(result.error).toContain("src/index.js"); + expect(result.error).not.toContain(".changeset/my-fix.md"); + }); + }); }); // ────────────────────────────────────────────────────── From 4882a6c46ebf437504c2b47cf1891fb4693ca719 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 16:40:05 +0000 Subject: [PATCH 06/10] Refactor: extract checkFileProtection helper, flatten protection branching in handlers Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pull_request.cjs | 58 ++++++------------- actions/setup/js/manifest_file_helpers.cjs | 42 +++++++++++++- .../setup/js/push_to_pull_request_branch.cjs | 56 +++++------------- 3 files changed, 74 insertions(+), 82 deletions(-) diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 2aa81406d6f..5ef77fbb3d3 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -23,7 +23,7 @@ const { createCheckoutManager } = require("./dynamic_checkout.cjs"); const { getBaseBranch } = require("./get_base_branch.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); -const { checkForManifestFiles, checkForProtectedPaths, checkAllowedFiles } = require("./manifest_file_helpers.cjs"); +const { checkFileProtection } = require("./manifest_file_helpers.cjs"); const { renderTemplate } = require("./messages_core.cjs"); /** @@ -419,47 +419,23 @@ async function main(config = {}) { core.info("Patch size validation passed"); } - // Check for protected file modifications (e.g., package.json, go.mod, .github/ files, AGENTS.md, CLAUDE.md) - // By default, protected file modifications are refused to prevent supply chain attacks. - // Set protected-files: fallback-to-issue to push the branch but create a review issue - // instead of a pull request, so a human can carefully review the changes first. - // Set protected-files: allowed only when the workflow is explicitly designed to manage these files. - /** @type {{ allFound: string[] } | null} */ + // Check file protection: allowlist (strict) or protected-files policy. + /** @type {string[] | null} Protected files that trigger fallback-to-issue handling */ let manifestProtectionFallback = null; if (!isEmpty) { - const manifestFiles = Array.isArray(config.protected_files) ? config.protected_files : []; - const protectedPathPrefixes = Array.isArray(config.protected_path_prefixes) ? config.protected_path_prefixes : []; - const allowedFilePatterns = Array.isArray(config.allowed_files) ? config.allowed_files : []; - if (allowedFilePatterns.length > 0) { - // Strict allowlist mode: only files matching allowed-files patterns are permitted. - // protected-files checks are completely bypassed. - const { hasDisallowedFiles, disallowedFiles } = checkAllowedFiles(patchContent, allowedFilePatterns); - if (hasDisallowedFiles) { - const message = `Cannot create pull request: patch modifies files outside the allowed-files list (${disallowedFiles.join(", ")}). Add the files to the allowed-files configuration field or remove them from the patch.`; - core.error(message); - return { success: false, error: message }; - } - } else { - // protected_files_policy is a string enum: "allowed" = allow, "fallback-to-issue" = fallback, "blocked" (default) = deny. - const policy = config.protected_files_policy; - const isAllowed = policy === "allowed"; - const isFallback = policy === "fallback-to-issue"; - if (!isAllowed) { - const { hasManifestFiles, manifestFilesFound } = checkForManifestFiles(patchContent, manifestFiles); - const { hasProtectedPaths, protectedPathsFound } = checkForProtectedPaths(patchContent, protectedPathPrefixes); - const allFound = [...manifestFilesFound, ...protectedPathsFound]; - if (allFound.length > 0) { - if (isFallback) { - // Record for fallback-to-issue handling below; let patch application proceed - manifestProtectionFallback = { allFound }; - core.warning(`Protected file protection triggered (fallback-to-issue): ${allFound.join(", ")}. Will create review issue instead of pull request.`); - } else { - const message = `Cannot create pull request: patch modifies protected files (${allFound.join(", ")}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; - core.error(message); - return { success: false, error: message }; - } - } - } + const protection = checkFileProtection(patchContent, config); + if (protection.action === "deny") { + const filesStr = protection.files.join(", "); + const message = + protection.source === "allowlist" + ? `Cannot create pull request: patch modifies files outside the allowed-files list (${filesStr}). Add the files to the allowed-files configuration field or remove them from the patch.` + : `Cannot create pull request: patch modifies protected files (${filesStr}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; + core.error(message); + return { success: false, error: message }; + } + if (protection.action === "fallback") { + manifestProtectionFallback = protection.files; + core.warning(`Protected file protection triggered (fallback-to-issue): ${protection.files.join(", ")}. Will create review issue instead of pull request.`); } } @@ -943,7 +919,7 @@ ${patchPreview}`; // was not created and provides a PR intent URL so the reviewer can create it // after manually inspecting the protected file changes. if (manifestProtectionFallback) { - const allFound = manifestProtectionFallback.allFound; + const allFound = manifestProtectionFallback; const githubServer = process.env.GITHUB_SERVER_URL || "https://github.com"; const encodedBase = baseBranch.split("/").map(encodeURIComponent).join("/"); const encodedHead = branchName.split("/").map(encodeURIComponent).join("/"); diff --git a/actions/setup/js/manifest_file_helpers.cjs b/actions/setup/js/manifest_file_helpers.cjs index f17162ba834..9d15b585fd0 100644 --- a/actions/setup/js/manifest_file_helpers.cjs +++ b/actions/setup/js/manifest_file_helpers.cjs @@ -125,4 +125,44 @@ function checkAllowedFiles(patchContent, allowedFilePatterns) { return { hasDisallowedFiles: disallowedFiles.length > 0, disallowedFiles }; } -module.exports = { extractFilenamesFromPatch, extractPathsFromPatch, checkForManifestFiles, checkForProtectedPaths, checkAllowedFiles }; +/** + * Evaluates a patch against the configured file-protection policy and returns a + * single structured result, eliminating nested branching in callers. + * + * Resolution order: + * 1. If `allowed_files` is set → strict allowlist check; `protected-files` is bypassed. + * 2. If `protected_files_policy === "allowed"` → no restriction. + * 3. Otherwise → manifest + path-prefix check, honouring `fallback-to-issue`. + * + * @param {string} patchContent - The git patch content + * @param {{ allowed_files?: string[], protected_files?: string[], protected_path_prefixes?: string[], protected_files_policy?: string }} config + * @returns {{ action: 'allow' } | { action: 'deny', source: 'allowlist'|'protected', files: string[] } | { action: 'fallback', files: string[] }} + */ +function checkFileProtection(patchContent, config) { + const allowedFilePatterns = Array.isArray(config.allowed_files) ? config.allowed_files : []; + if (allowedFilePatterns.length > 0) { + const { disallowedFiles } = checkAllowedFiles(patchContent, allowedFilePatterns); + if (disallowedFiles.length > 0) { + return { action: "deny", source: "allowlist", files: disallowedFiles }; + } + return { action: "allow" }; + } + + if (config.protected_files_policy === "allowed") { + return { action: "allow" }; + } + + const manifestFiles = Array.isArray(config.protected_files) ? config.protected_files : []; + const prefixes = Array.isArray(config.protected_path_prefixes) ? config.protected_path_prefixes : []; + const { manifestFilesFound } = checkForManifestFiles(patchContent, manifestFiles); + const { protectedPathsFound } = checkForProtectedPaths(patchContent, prefixes); + const allFound = [...manifestFilesFound, ...protectedPathsFound]; + + if (allFound.length === 0) { + return { action: "allow" }; + } + + return config.protected_files_policy === "fallback-to-issue" ? { action: "fallback", files: allFound } : { action: "deny", source: "protected", files: allFound }; +} + +module.exports = { extractFilenamesFromPatch, extractPathsFromPatch, checkForManifestFiles, checkForProtectedPaths, checkAllowedFiles, checkFileProtection }; diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 7b34d3e5c15..412a7614793 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -11,7 +11,7 @@ const { pushExtraEmptyCommit } = require("./extra_empty_commit.cjs"); const { detectForkPR } = require("./pr_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); -const { checkForManifestFiles, checkForProtectedPaths, checkAllowedFiles } = require("./manifest_file_helpers.cjs"); +const { checkFileProtection } = require("./manifest_file_helpers.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { renderTemplate } = require("./messages_core.cjs"); @@ -138,48 +138,24 @@ async function main(config = {}) { core.info("Patch size validation passed"); } - // Check for protected file modifications (e.g., package.json, go.mod, .github/ files, AGENTS.md, CLAUDE.md) - // By default, protected file modifications are refused to prevent supply chain attacks. - // Set protected-files: fallback-to-issue to create a review issue instead of pushing. - // Set protected-files: allowed only when the workflow is explicitly designed to manage these files. - // NOTE: fallback-to-issue detection is done here but issue creation is deferred until after - // the PR metadata (repoParts, prTitle, pullNumber) has been resolved below. + // Check file protection: allowlist (strict) or protected-files policy. + // Fallback-to-issue detection is deferred until after PR metadata is resolved below. /** @type {string[] | null} Protected files found in the patch (manifest basenames + path-prefix matches) */ let protectedFilesForFallback = null; if (!isEmpty) { - const manifestFiles = Array.isArray(config.protected_files) ? config.protected_files : []; - const protectedPathPrefixes = Array.isArray(config.protected_path_prefixes) ? config.protected_path_prefixes : []; - const allowedFilePatterns = Array.isArray(config.allowed_files) ? config.allowed_files : []; - if (allowedFilePatterns.length > 0) { - // Strict allowlist mode: only files matching allowed-files patterns are permitted. - // protected-files checks are completely bypassed. - const { hasDisallowedFiles, disallowedFiles } = checkAllowedFiles(patchContent, allowedFilePatterns); - if (hasDisallowedFiles) { - const msg = `Cannot push to pull request branch: patch modifies files outside the allowed-files list (${disallowedFiles.join(", ")}). Add the files to the allowed-files configuration field or remove them from the patch.`; - core.error(msg); - return { success: false, error: msg }; - } - } else { - // protected_files_policy is a string enum: "allowed" = allow, "fallback-to-issue" = fallback, "blocked" (default) = deny. - const policy = config.protected_files_policy; - const isAllowed = policy === "allowed"; - const isFallback = policy === "fallback-to-issue"; - if (!isAllowed) { - const { manifestFilesFound } = checkForManifestFiles(patchContent, manifestFiles); - const { protectedPathsFound } = checkForProtectedPaths(patchContent, protectedPathPrefixes); - const allFound = [...manifestFilesFound, ...protectedPathsFound]; - if (allFound.length > 0) { - if (isFallback) { - // Store for deferred issue creation (needs PR metadata resolved first) - protectedFilesForFallback = allFound; - core.warning(`Protected file protection triggered (fallback-to-issue): ${allFound.join(", ")}. Will create review issue instead of pushing.`); - } else { - const msg = `Cannot push to pull request branch: patch modifies protected files (${allFound.join(", ")}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; - core.error(msg); - return { success: false, error: msg }; - } - } - } + const protection = checkFileProtection(patchContent, config); + if (protection.action === "deny") { + const filesStr = protection.files.join(", "); + const msg = + protection.source === "allowlist" + ? `Cannot push to pull request branch: patch modifies files outside the allowed-files list (${filesStr}). Add the files to the allowed-files configuration field or remove them from the patch.` + : `Cannot push to pull request branch: patch modifies protected files (${filesStr}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; + core.error(msg); + return { success: false, error: msg }; + } + if (protection.action === "fallback") { + protectedFilesForFallback = protection.files; + core.warning(`Protected file protection triggered (fallback-to-issue): ${protection.files.join(", ")}. Will create review issue instead of pushing.`); } } From 753ab9ad20a2a91cd596c65bd47e36c5e71450c7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 17:12:29 +0000 Subject: [PATCH 07/10] Make allowed-files and protected-files orthogonal: remove implicit bypass, require explicit configuration Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../workflows/instructions-janitor.lock.yml | 4 +-- .github/workflows/instructions-janitor.md | 1 + actions/setup/js/create_pull_request.test.cjs | 25 ++++++++++++++++--- actions/setup/js/manifest_file_helpers.cjs | 14 +++++++---- .../setup/js/manifest_file_helpers.test.cjs | 2 +- .../js/push_to_pull_request_branch.test.cjs | 25 ++++++++++++++++--- 6 files changed, 55 insertions(+), 16 deletions(-) diff --git a/.github/workflows/instructions-janitor.lock.yml b/.github/workflows/instructions-janitor.lock.yml index 8f628f55f95..1cb06ce0d77 100644 --- a/.github/workflows/instructions-janitor.lock.yml +++ b/.github/workflows/instructions-janitor.lock.yml @@ -23,7 +23,7 @@ # # Reviews and cleans up instruction files to ensure clarity, consistency, and adherence to best practices # -# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"9c6a532a9148c023dd6cb7c1e750e515005ddd97d1e9a2a54397bb1701f88848","strict":true} +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"48d94b6ad7a9d9001f6fec2efbf84050be5385bdf3e892f411ee41f445c5f8af","strict":true} name: "Instructions Janitor" "on": @@ -1271,7 +1271,7 @@ jobs: GH_AW_ALLOWED_DOMAINS: "*.githubusercontent.com,anthropic.com,api.anthropic.com,api.github.com,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,cdn.playwright.dev,codeload.github.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,files.pythonhosted.org,ghcr.io,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.com,github.githubassets.com,host.docker.internal,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,lfs.github.com,objects.githubusercontent.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,playwright.download.prss.microsoft.com,ppa.launchpad.net,pypi.org,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,sentry.io,statsig.anthropic.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com" GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"create_pull_request\":{\"allowed_files\":[\".github/aw/**\"],\"draft\":false,\"expires\":48,\"labels\":[\"documentation\",\"automation\",\"instructions\"],\"max\":1,\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"CLAUDE.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".claude/\"],\"title_prefix\":\"[instructions] \"},\"missing_data\":{},\"missing_tool\":{}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"create_pull_request\":{\"allowed_files\":[\".github/aw/**\"],\"draft\":false,\"expires\":48,\"labels\":[\"documentation\",\"automation\",\"instructions\"],\"max\":1,\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"CLAUDE.md\"],\"protected_files_policy\":\"allowed\",\"protected_path_prefixes\":[\".github/\",\".agents/\",\".claude/\"],\"title_prefix\":\"[instructions] \"},\"missing_data\":{},\"missing_tool\":{}}" GH_AW_CI_TRIGGER_TOKEN: ${{ secrets.GH_AW_CI_TRIGGER_TOKEN }} with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/instructions-janitor.md b/.github/workflows/instructions-janitor.md index 77b57d94acb..6dfcc7c8c5f 100644 --- a/.github/workflows/instructions-janitor.md +++ b/.github/workflows/instructions-janitor.md @@ -26,6 +26,7 @@ safe-outputs: draft: false allowed-files: - .github/aw/** + protected-files: allowed tools: cache-memory: true diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index 226515aced6..bba2c9df350 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -370,9 +370,9 @@ ${diffs} expect(result.error).not.toContain(".github/aw/github-agentic-workflows.md"); }); - it("should bypass protected-files when allowed-files is configured and file matches", async () => { - // .github/ is a protected path prefix, but when allowed_files is set and the file - // matches, protected-files is bypassed entirely + it("should still enforce protected-files when allowed-files matches (orthogonal checks)", async () => { + // allowed-files and protected-files are orthogonal: both checks must pass. + // Matching the allowlist does NOT bypass the protected-files policy. const patchPath = writePatch(createPatchWithFiles(".github/aw/instructions.md")); const { main } = require("./create_pull_request.cjs"); @@ -383,8 +383,25 @@ ${diffs} }); const result = await handler({ patch_path: patchPath, title: "Test PR", body: "" }, {}); - // Should not be blocked by protected-files — either succeeds or fails for other reasons + expect(result.success).toBe(false); + expect(result.error).toContain("protected files"); + }); + + it("should allow a protected file when both allowed-files matches and protected-files: allowed is set", async () => { + // Both checks are satisfied explicitly: allowlist scope + protected-files permission. + const patchPath = writePatch(createPatchWithFiles(".github/aw/instructions.md")); + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({ + allowed_files: [".github/aw/**"], + protected_path_prefixes: [".github/"], + protected_files_policy: "allowed", + }); + const result = await handler({ patch_path: patchPath, title: "Test PR", body: "" }, {}); + + // Should not be blocked by either check expect(result.error || "").not.toContain("protected files"); + expect(result.error || "").not.toContain("outside the allowed-files list"); }); it("should still enforce protected-files when allowed-files is not set", async () => { diff --git a/actions/setup/js/manifest_file_helpers.cjs b/actions/setup/js/manifest_file_helpers.cjs index 9d15b585fd0..819b40a89ef 100644 --- a/actions/setup/js/manifest_file_helpers.cjs +++ b/actions/setup/js/manifest_file_helpers.cjs @@ -129,25 +129,29 @@ function checkAllowedFiles(patchContent, allowedFilePatterns) { * Evaluates a patch against the configured file-protection policy and returns a * single structured result, eliminating nested branching in callers. * - * Resolution order: - * 1. If `allowed_files` is set → strict allowlist check; `protected-files` is bypassed. - * 2. If `protected_files_policy === "allowed"` → no restriction. - * 3. Otherwise → manifest + path-prefix check, honouring `fallback-to-issue`. + * The two checks are orthogonal and both must pass: + * 1. If `allowed_files` is set → every file must match at least one pattern (deny if not). + * 2. `protected-files` policy applies independently: "allowed" = skip, "fallback-to-issue" + * = create review issue, default ("blocked") = deny. + * + * To allow an agent to write protected files, set both `allowed-files` (strict scope) and + * `protected-files: allowed` (explicit permission) — neither overrides the other implicitly. * * @param {string} patchContent - The git patch content * @param {{ allowed_files?: string[], protected_files?: string[], protected_path_prefixes?: string[], protected_files_policy?: string }} config * @returns {{ action: 'allow' } | { action: 'deny', source: 'allowlist'|'protected', files: string[] } | { action: 'fallback', files: string[] }} */ function checkFileProtection(patchContent, config) { + // Step 1: allowlist check (if configured) const allowedFilePatterns = Array.isArray(config.allowed_files) ? config.allowed_files : []; if (allowedFilePatterns.length > 0) { const { disallowedFiles } = checkAllowedFiles(patchContent, allowedFilePatterns); if (disallowedFiles.length > 0) { return { action: "deny", source: "allowlist", files: disallowedFiles }; } - return { action: "allow" }; } + // Step 2: protected-files check (independent of allowlist) if (config.protected_files_policy === "allowed") { return { action: "allow" }; } diff --git a/actions/setup/js/manifest_file_helpers.test.cjs b/actions/setup/js/manifest_file_helpers.test.cjs index 5fbdc675e48..97c4776659c 100644 --- a/actions/setup/js/manifest_file_helpers.test.cjs +++ b/actions/setup/js/manifest_file_helpers.test.cjs @@ -316,7 +316,7 @@ index abc..def 100644 expect(result.disallowedFiles).not.toContain(".changeset/patch-fix.md"); }); - it("should allow protected files when they are in the allowlist (protected-files bypassed)", () => { + it("should not flag a protected file that is in the allowlist", () => { const patch = `diff --git a/.github/aw/instructions.md b/.github/aw/instructions.md\nindex abc..def 100644\n`; const result = checkAllowedFiles(patch, [".github/aw/instructions.md"]); expect(result.hasDisallowedFiles).toBe(false); diff --git a/actions/setup/js/push_to_pull_request_branch.test.cjs b/actions/setup/js/push_to_pull_request_branch.test.cjs index 3cb1b87d761..0a2dc34f42d 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -1103,11 +1103,10 @@ ${diffs} expect(result.success).toBe(true); }); - it("should bypass protected-files check when file is in the allowlist", async () => { - // package.json would normally be blocked by protected-files, but - // when allowed_files is set and it matches, protected-files is bypassed entirely + it("should still block a protected file when it is in the allowlist but protected-files: allowed is not set", async () => { + // allowed-files and protected-files are orthogonal: both checks must pass. + // Matching the allowlist does NOT bypass the protected-files policy. const patchPath = createPatchFile(createPatchWithFiles("package.json")); - mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: "abc123\n", stderr: "" }); const module = await loadModule(); const handler = await module.main({ @@ -1117,6 +1116,24 @@ ${diffs} }); const result = await handler({ patch_path: patchPath }, {}); + expect(result.success).toBe(false); + expect(result.error).toContain("protected files"); + expect(result.error).toContain("package.json"); + }); + + it("should allow a protected file when both allowed-files matches and protected-files: allowed is set", async () => { + // Both checks are satisfied explicitly: allowlist scope + protected-files permission. + const patchPath = createPatchFile(createPatchWithFiles("package.json")); + mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: "abc123\n", stderr: "" }); + + const module = await loadModule(); + const handler = await module.main({ + allowed_files: ["package.json"], + protected_files: ["package.json"], + protected_files_policy: "allowed", + }); + const result = await handler({ patch_path: patchPath }, {}); + expect(result.success).toBe(true); }); From e42869359a6bc0605bd93ad6ae40896f7407baf8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 18:06:03 +0000 Subject: [PATCH 08/10] Fix TS typecheck failure: add file-protection props to HandlerConfig; fix stale bypass/priority comments and add checkFileProtection unit tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/manifest_file_helpers.cjs | 7 +- .../setup/js/manifest_file_helpers.test.cjs | 78 ++++++++++++++++++- actions/setup/js/types/handler-factory.d.ts | 8 ++ .../reference/safe-outputs-pull-requests.md | 4 +- pkg/parser/schemas/main_workflow_schema.json | 4 +- pkg/workflow/create_pull_request.go | 2 +- pkg/workflow/push_to_pull_request_branch.go | 2 +- 7 files changed, 95 insertions(+), 10 deletions(-) diff --git a/actions/setup/js/manifest_file_helpers.cjs b/actions/setup/js/manifest_file_helpers.cjs index 819b40a89ef..5ed98e04d58 100644 --- a/actions/setup/js/manifest_file_helpers.cjs +++ b/actions/setup/js/manifest_file_helpers.cjs @@ -1,5 +1,7 @@ // @ts-check +/** @typedef {import('./types/handler-factory').HandlerConfig} HandlerConfig */ + /** * Extracts the unique set of file basenames (filename without directory path) changed in a git patch. * Parses "diff --git a/ b/" headers to determine which files were modified. @@ -100,8 +102,7 @@ function checkForProtectedPaths(patchContent, pathPrefixes) { * Checks all files in a patch against an allowlist of glob patterns. * When `allowed-files` is configured, it acts as a strict allowlist: every file * touched by the patch must match at least one pattern; files that do not match - * are returned as disallowed. Protected-files checks are completely bypassed - * when this function is used. + * are returned as disallowed. * * Glob matching supports `*` (matches any characters except `/`) and `**` (matches * any characters including `/`). Each changed file is tested as its full path @@ -138,7 +139,7 @@ function checkAllowedFiles(patchContent, allowedFilePatterns) { * `protected-files: allowed` (explicit permission) — neither overrides the other implicitly. * * @param {string} patchContent - The git patch content - * @param {{ allowed_files?: string[], protected_files?: string[], protected_path_prefixes?: string[], protected_files_policy?: string }} config + * @param {HandlerConfig} config * @returns {{ action: 'allow' } | { action: 'deny', source: 'allowlist'|'protected', files: string[] } | { action: 'fallback', files: string[] }} */ function checkFileProtection(patchContent, config) { diff --git a/actions/setup/js/manifest_file_helpers.test.cjs b/actions/setup/js/manifest_file_helpers.test.cjs index 97c4776659c..1c832bcf87d 100644 --- a/actions/setup/js/manifest_file_helpers.test.cjs +++ b/actions/setup/js/manifest_file_helpers.test.cjs @@ -3,7 +3,7 @@ import { describe, it, expect } from "vitest"; import { createRequire } from "module"; const require = createRequire(import.meta.url); -const { extractFilenamesFromPatch, checkForManifestFiles, checkAllowedFiles } = require("./manifest_file_helpers.cjs"); +const { extractFilenamesFromPatch, checkForManifestFiles, checkAllowedFiles, checkFileProtection } = require("./manifest_file_helpers.cjs"); describe("manifest_file_helpers", () => { describe("extractFilenamesFromPatch", () => { @@ -335,4 +335,80 @@ index abc..def 100644 expect(result.hasDisallowedFiles).toBe(false); }); }); + + describe("checkFileProtection", () => { + const makePatch = (...filePaths) => filePaths.map(p => `diff --git a/${p} b/${p}\nindex abc..def 100644\n`).join("\n"); + + it("should allow when patch is empty", () => { + const result = checkFileProtection("", {}); + expect(result.action).toBe("allow"); + }); + + it("should allow when no protected files or allowlist configured", () => { + const result = checkFileProtection(makePatch("src/index.js"), {}); + expect(result.action).toBe("allow"); + }); + + it("should deny when file is outside the allowlist", () => { + const result = checkFileProtection(makePatch("src/index.js"), { allowed_files: [".changeset/**"] }); + expect(result.action).toBe("deny"); + expect(result.source).toBe("allowlist"); + expect(result.files).toContain("src/index.js"); + }); + + it("should allow when all files match the allowlist and no protected-files configured", () => { + const result = checkFileProtection(makePatch(".changeset/fix.md"), { allowed_files: [".changeset/**"] }); + expect(result.action).toBe("allow"); + }); + + it("should deny protected file even when it matches the allowlist (orthogonal checks)", () => { + const result = checkFileProtection(makePatch("package.json"), { + allowed_files: ["package.json"], + protected_files: ["package.json"], + protected_files_policy: "blocked", + }); + expect(result.action).toBe("deny"); + expect(result.source).toBe("protected"); + expect(result.files).toContain("package.json"); + }); + + it("should allow protected file when allowlist matches and protected-files: allowed", () => { + const result = checkFileProtection(makePatch("package.json"), { + allowed_files: ["package.json"], + protected_files: ["package.json"], + protected_files_policy: "allowed", + }); + expect(result.action).toBe("allow"); + }); + + it("should return fallback when protected file found and policy is fallback-to-issue", () => { + const result = checkFileProtection(makePatch("package.json"), { + protected_files: ["package.json"], + protected_files_policy: "fallback-to-issue", + }); + expect(result.action).toBe("fallback"); + expect(result.files).toContain("package.json"); + }); + + it("should deny on protected path prefix when no allowlist", () => { + const result = checkFileProtection(makePatch(".github/workflows/ci.yml"), { + protected_path_prefixes: [".github/"], + protected_files_policy: "blocked", + }); + expect(result.action).toBe("deny"); + expect(result.source).toBe("protected"); + expect(result.files).toContain(".github/workflows/ci.yml"); + }); + + it("should deny allowlist violation before checking protected-files (deny on first failure)", () => { + // file is outside allowlist AND would be protected — allowlist check fires first + const result = checkFileProtection(makePatch("src/outside.js"), { + allowed_files: [".changeset/**"], + protected_files: ["src/outside.js"], + protected_files_policy: "blocked", + }); + expect(result.action).toBe("deny"); + expect(result.source).toBe("allowlist"); + }); + }); }); diff --git a/actions/setup/js/types/handler-factory.d.ts b/actions/setup/js/types/handler-factory.d.ts index d64106ef972..9360b9ab506 100644 --- a/actions/setup/js/types/handler-factory.d.ts +++ b/actions/setup/js/types/handler-factory.d.ts @@ -8,6 +8,14 @@ interface HandlerConfig { /** Maximum number of items this handler should process */ max?: number; + /** Strict allowlist of glob patterns for files eligible for push/create. Checked independently of protected-files; both checks must pass. */ + allowed_files?: string[]; + /** List of filenames (basenames) whose presence in a patch triggers protected-file handling */ + protected_files?: string[]; + /** List of path prefixes that trigger protected-file handling when any changed file matches */ + protected_path_prefixes?: string[]; + /** Policy for how protected file matches are handled: "blocked" (default), "fallback-to-issue", or "allowed" */ + protected_files_policy?: string; /** Additional handler-specific configuration properties */ [key: string]: any; } diff --git a/docs/src/content/docs/reference/safe-outputs-pull-requests.md b/docs/src/content/docs/reference/safe-outputs-pull-requests.md index 548f86e54ca..501f22c6aa6 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -115,7 +115,7 @@ When protected file protection triggers and is set to `blocked`, the 🛡️ **P ### Exempting Specific Files with `allowed-files` -When a workflow is designed to modify only specific files, use `allowed-files` to define a strict allowlist. When set, every file touched by the patch must match at least one pattern — any file outside the list is refused. The `protected-files` policy is completely bypassed when `allowed-files` is configured. +When a workflow is designed to modify only specific files, use `allowed-files` to define a strict allowlist. When set, every file touched by the patch must match at least one pattern — any file outside the list is refused. The `allowed-files` and `protected-files` checks are **orthogonal**: both run independently and both must pass. To modify a protected file, it must both match `allowed-files` **and** `protected-files` must be set to `allowed`. ```yaml wrap safe-outputs: @@ -140,7 +140,7 @@ Patterns support `*` (any characters except `/`) and `**` (any characters includ | `**/package.json` | `package.json` at any path depth | > [!NOTE] -> When `allowed-files` is set, it acts as a strict allowlist: only the listed files may be modified. Any file outside the list is refused and `protected-files` is ignored. When `allowed-files` is not set, the `protected-files` policy applies as usual. +> When `allowed-files` is set, it acts as a strict scope filter: only files matching the patterns may be modified, and any file outside the list is always refused. Files that *do* match are still subject to the `protected-files` policy, which runs independently. To modify a protected file, it must both match `allowed-files` **and** `protected-files` must be set to `allowed`. When `allowed-files` is not set, only the `protected-files` policy applies. > [!WARNING] > `allowed-files` should enumerate exactly the files the workflow legitimately manages. Overly broad patterns (e.g., `**`) disable all protection. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index fe5647a9730..698bba6eb73 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -5287,7 +5287,7 @@ "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 /)." + "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 /)." } }, "additionalProperties": false, @@ -6328,7 +6328,7 @@ "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 /)." + "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 /)." } }, "additionalProperties": false diff --git a/pkg/workflow/create_pull_request.go b/pkg/workflow/create_pull_request.go index e32684b4f2d..4ec802bf59c 100644 --- a/pkg/workflow/create_pull_request.go +++ b/pkg/workflow/create_pull_request.go @@ -33,7 +33,7 @@ type CreatePullRequestsConfig struct { FallbackAsIssue *bool `yaml:"fallback-as-issue,omitempty"` // When true (default), creates an issue if PR creation fails. When false, no fallback occurs and issues: write permission is not requested. GithubTokenForExtraEmptyCommit string `yaml:"github-token-for-extra-empty-commit,omitempty"` // Token used to push an empty commit to trigger CI events. Use a PAT or "app" for GitHub App auth. ManifestFilesPolicy *string `yaml:"protected-files,omitempty"` // Controls protected-file protection: "blocked" (default) hard-blocks, "allowed" permits all changes, "fallback-to-issue" pushes the branch but creates a review issue. - AllowedFiles []string `yaml:"allowed-files,omitempty"` // List of glob patterns for files that are exempt from protected-file protection. Takes priority over protected-files. + AllowedFiles []string `yaml:"allowed-files,omitempty"` // Strict allowlist of glob patterns for files eligible for create. Checked independently of protected-files; both checks must pass. } // parsePullRequestsConfig handles only create-pull-request (singular) configuration diff --git a/pkg/workflow/push_to_pull_request_branch.go b/pkg/workflow/push_to_pull_request_branch.go index 02ae7b7bd1d..59da8e7707a 100644 --- a/pkg/workflow/push_to_pull_request_branch.go +++ b/pkg/workflow/push_to_pull_request_branch.go @@ -21,7 +21,7 @@ type PushToPullRequestBranchConfig struct { TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository push to pull request branch AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories in format "owner/repo" that push to pull request branch can target ManifestFilesPolicy *string `yaml:"protected-files,omitempty"` // Controls protected-file protection: "blocked" (default) hard-blocks, "allowed" permits all changes, "fallback-to-issue" creates a review issue instead of pushing. - AllowedFiles []string `yaml:"allowed-files,omitempty"` // List of glob patterns for files that are exempt from protected-file protection. Takes priority over protected-files. + AllowedFiles []string `yaml:"allowed-files,omitempty"` // Strict allowlist of glob patterns for files eligible for push. Checked independently of protected-files; both checks must pass. } // buildCheckoutRepository generates a checkout step with optional target repository and custom token From 36291e644017ec63cb63c71cc258e1b2eaa3c4d2 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Sun, 8 Mar 2026 18:45:10 +0000 Subject: [PATCH 09/10] Update pkg/workflow/push_to_pull_request_branch.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/workflow/push_to_pull_request_branch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/workflow/push_to_pull_request_branch.go b/pkg/workflow/push_to_pull_request_branch.go index 59da8e7707a..a319d4d1326 100644 --- a/pkg/workflow/push_to_pull_request_branch.go +++ b/pkg/workflow/push_to_pull_request_branch.go @@ -143,7 +143,7 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any) pushToBranchConfig.ManifestFilesPolicy = &strVal } - // Parse allowed-files: list of glob patterns for files exempt from protection + // Parse allowed-files: list of glob patterns forming a strict allowlist of eligible files pushToBranchConfig.AllowedFiles = ParseStringArrayFromConfig(configMap, "allowed-files", pushToPullRequestBranchLog) // Parse common base fields with default max of 0 (no limit) From aa4db01fc9f909ad639af3215425bd06012a7b61 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Sun, 8 Mar 2026 18:45:20 +0000 Subject: [PATCH 10/10] Update docs/src/content/docs/reference/safe-outputs-pull-requests.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- docs/src/content/docs/reference/safe-outputs-pull-requests.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/content/docs/reference/safe-outputs-pull-requests.md b/docs/src/content/docs/reference/safe-outputs-pull-requests.md index 501f22c6aa6..606d6351117 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -132,7 +132,7 @@ Patterns support `*` (any characters except `/`) and `**` (any characters includ | Pattern | Matches | |---------|---------| -| `go.mod` | Exactly `go.mod` at any path (full path comparison) | +| `go.mod` | Exactly `go.mod` at the repository root (full path comparison) | | `*.json` | Any JSON file at the root (e.g. `package.json`) | | `go.*` | `go.mod`, `go.sum`, etc. at the root | | `.github/**` | All files under `.github/` at any depth |