fix: Safe-outputs file-protection bypass via patch-parser differential#36752
Conversation
Addresses a security vulnerability where the JS patch parser and git am can disagree on which files a patch contains, allowing an attacker to bypass allowed-files/protected-files policies. Three fixes: 1. Post-apply enforcement (primary): After git am applies a patch and BEFORE push, run `git diff --name-only` against the pre-apply commit to get the actual files written. Re-check file protection against this ground-truth list. This eliminates all parser-differential attacks regardless of variant. 2. Fail-closed on unparseable headers: extractPathsFromPatch() now throws on any diff --git header where parseable === false, instead of silently skipping it. This prevents crafted headers from evading policy checks. 3. Regression tests: Added test cases for unparseable headers, post-apply detection of files outside allowed-files, protected paths, and top-level dot folders. Fixes github/agentic-workflows#539 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ smoke-ci: safeoutputs CLI comment + comment-memory run (26919435461)
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
There was a problem hiding this comment.
Pull request overview
This PR hardens safe-output file-protection enforcement against patch-parser differentials by verifying the actual files written after git am (via git diff --name-only --no-renames <base>..HEAD) and by failing closed when encountering unparseable diff --git headers.
Changes:
- Add post-apply file-protection verification to
push_to_pull_request_branchandcreate_pull_requestflows. - Update patch path extraction to throw on unparseable
diff --githeaders (fail-closed). - Add regression/unit tests covering unparseable headers and post-apply protection outcomes.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/push_to_pull_request_branch.cjs | Adds post-apply git diff-based enforcement before pushing to an existing PR branch. |
| actions/setup/js/create_pull_request.cjs | Adds post-apply enforcement after patch apply and before pushing/creating PR. |
| actions/setup/js/manifest_file_helpers.cjs | Makes unparseable diff --git headers fail-closed; adds checkFileProtectionPostApply(actualFiles, config). |
| actions/setup/js/manifest_file_helpers.test.cjs | Adds/updates regression tests for fail-closed parsing and post-apply policy checks. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 7
| if (file.startsWith(".") && file.includes("/")) { | ||
| const topFolder = file.split("/")[0]; | ||
| if (!dotFolderExcludes.includes(topFolder) && !allProtected.includes(file)) { | ||
| allProtected.push(file); | ||
| } |
| it("should respect dot folder excludes", () => { | ||
| const result = checkFileProtectionPostApply( | ||
| [".changeset/fix.md"], | ||
| { protect_top_level_dot_folders: true, protected_dot_folder_excludes: [".changeset"], protected_files_policy: "deny" } | ||
| ); |
| throw new Error( | ||
| `Patch contains unparseable diff --git header (fail-closed): "${entry.headerLine}". ` + | ||
| `Cannot verify file-protection policy. Rejecting patch.` |
| if (postApplyProtection.action === "fallback") { | ||
| protectedFilesForFallback = postApplyProtection.files; | ||
| core.warning(`Post-apply: Protected file protection triggered (fallback-to-issue): ${postApplyProtection.files.join(", ")}`); | ||
| } |
| const diffResult = await exec.getExecOutput("git", ["diff", "--name-only", "--no-renames", `origin/${baseBranch}..HEAD`]); | ||
| const actualFiles = diffResult.stdout.split("\n").map(f => f.trim()).filter(Boolean); | ||
| if (actualFiles.length > 0) { |
| const diffResult = await exec.getExecOutput("git", ["diff", "--name-only", "--no-renames", `${rangeBaseRef}..HEAD`], baseGitOpts); | ||
| const actualFiles = diffResult.stdout.split("\n").map(f => f.trim()).filter(Boolean); | ||
| if (actualFiles.length > 0) { | ||
| core.info(`Post-apply verification: ${actualFiles.length} file(s) actually modified`); | ||
| const postApplyProtection = checkFileProtectionPostApply(actualFiles, config); |
| const diffResult = await exec.getExecOutput("git", ["diff", "--name-only", "--no-renames", `origin/${baseBranch}..HEAD`]); | ||
| const actualFiles = diffResult.stdout.split("\n").map(f => f.trim()).filter(Boolean); | ||
| if (actualFiles.length > 0) { | ||
| core.info(`Post-apply verification: ${actualFiles.length} file(s) actually modified`); | ||
| const postApplyProtection = checkFileProtectionPostApply(actualFiles, { |
|
@copilot address review feedback |
Addressed in 401b8ca. The update normalizes dot-folder excludes post-apply, sanitizes unparseable header errors, uses the actual pre-apply HEAD for create-pull-request post-apply diffs, enforces fallback-to-issue after post-apply detection in push-to-PR, and adds focused regression coverage for both handlers. |
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
``
|
|
@copilot apply copilot-review skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Applied the copilot-review workflow: I re-collected the in-scope review feedback, verified the previously requested fixes are present in |
|
|
|
🚀 Smoke Antigravity MISSION COMPLETE! Antigravity has spoken. ✨ |
|
🚀 Smoke Gemini MISSION COMPLETE! Gemini has spoken. ✨ Caution agentic threat detected |
|
🚀 Smoke Pi MISSION COMPLETE! Pi delivered. 🥧 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ All tools validated successfully! Agent Container Smoke Test confirms agent container is ready. |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
|
Caution agentic threat detected Smoke Test Results
Overall Status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
🤖 Smoke Test: Claude — Run 26927612883Core #1-13: ✅ (note: #2 Overall: PARTIAL (run) Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
💥 [THE END] — Illustrated by Smoke Claude · opus48 5.1M
| /** @typedef {import('./types/handler-factory').HandlerConfig} HandlerConfig */ | ||
| const { extractDiffGitHeaderEntries } = require("./patch_path_helpers.cjs"); | ||
|
|
||
| function normalizeDotFolderExcludes(excludes) { |
There was a problem hiding this comment.
Nice helper — normalizeDotFolderExcludes cleanly trims trailing slashes and filters empties. Consider a brief JSDoc for clarity. (smoke test 26927612883)
| return new Set((Array.isArray(excludes) ? excludes : []).map(exclude => String(exclude || "").replace(/\/+$/, "")).filter(Boolean)); | ||
| } | ||
|
|
||
| function formatHeaderLineForError(headerLine) { |
There was a problem hiding this comment.
formatHeaderLineForError truncation at 200 chars is a sensible guard for log readability. (smoke test 26927612883)
|
Commit pushed:
|
|
Smoke bot done. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
fix: Safe-outputs file-protection bypass via patch-parser differential
Summary
Fixes a security vulnerability (CVE-class) where a crafted patch could bypass
safe-outputs file protection checks by exploiting parser differences between the
JavaScript patch parser and
git am. The JS parser andgit amcan disagree onwhich files a patch touches, so an adversarial patch could pass JS-side
protection checks but write to protected files when applied by
git am.The fix introduces post-apply enforcement: the actual files written to disk
by
git amare diffed against the file-protection policy using agit diffof the pre-apply HEAD ref. Violations cause an immediate HEAD reset and either a
hard failure or a fallback issue, depending on context.
Problem
File protection in safe-outputs was enforced pre-apply by parsing
diff --githeaders with a JS parser. A specially crafted patch could include headers that
the JS parser accepted as non-protected, but that
git amwould apply toprotected paths. This created a parser-differential bypass window.
Additionally, unparseable
diff --githeaders were previously silently ignored(fail-open), meaning malformed headers could also be used to evade the check.
Changes
Security fix — post-apply file-protection enforcement
actions/setup/js/manifest_file_helpers.cjscheckFileProtectionPostApply()— verifies protection policy against files actually written bygit am(viagit diff <pre-apply-HEAD>). Fail-closed on unparseablediff --githeaders. Normalized dot-folder excludes handling.actions/setup/js/create_pull_request.cjsgit am, then invokes post-apply protection check against the real written files.actions/setup/js/push_to_pull_request_branch.cjscheckFileProtectionPostApplyand runs it aftergit am; resets HEAD and creates a fallback issue on violations.Tests
actions/setup/js/manifest_file_helpers.test.cjsextractPathsFromPatchsecurity edge cases andcheckFileProtectionPostApplyregression variants.actions/setup/js/create_pull_request.test.cjsactions/setup/js/push_to_pull_request_branch.test.cjsHousekeeping
.changeset/patch-fix-patch-parser-file-protection-bypass.md.github/skills/agentic-workflows/SKILL.md.github/aw/mcp-clis.mdto the remote-file load list.Security posture
Breaking changes
None. All changes are internal enforcement logic with no public API surface changes.
Testing