fix: Safe-outputs file-protection bypass via patch-parser differential

.github/skills/agentic-workflows/SKILL.md

@@ -27,6 +27,7 @@ Load these files from `github/gh-aw` (they are not available locally).
 - `.github/aw/github-agentic-workflows.md`
 - `.github/aw/github-mcp-server.md`
 - `.github/aw/llms.md`
+- `.github/aw/mcp-clis.md`
 - `.github/aw/memory.md`
 - `.github/aw/messages.md`
 - `.github/aw/network.md`

actions/setup/js/create_pull_request.cjs

@@ -28,7 +28,7 @@ const { findRepoCheckout } = require("./find_repo_checkout.cjs");
 const { getBaseBranch } = require("./get_base_branch.cjs");
 const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs");
 const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs");
-const { checkFileProtection } = require("./manifest_file_helpers.cjs");
+const { checkFileProtection, checkFileProtectionPostApply } = require("./manifest_file_helpers.cjs");
 const { renderTemplateFromFile, renderFilesList, buildProtectedFileList, getPromptPath } = require("./messages_core.cjs");
 const { COPILOT_REVIEWER_BOT, FAQ_CREATE_PR_PERMISSIONS_URL } = require("./constants.cjs");
 const { isStagedMode } = require("./safe_output_helpers.cjs");
@@ -1666,6 +1666,15 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo
 
         // Apply the patch using git CLI (skip if empty)
         if (!isEmpty) {
+          let postApplyBaseRef = null;
+          const capturePostApplyBaseRef = async () => {
+            const headResult = await exec.getExecOutput("git", ["rev-parse", "HEAD"]);
+            const resolvedRef = headResult.stdout.trim();
+            if (resolvedRef) {
+              postApplyBaseRef = resolvedRef;
+            }
+          };
+
           // Resolve temporary ID references in patch content before applying
           // This handles references like #aw_XXX in committed source code
           if (resolvedTemporaryIds && Object.keys(resolvedTemporaryIds).length > 0) {
@@ -1691,6 +1700,7 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo
           // This allows git to resolve create-vs-modify mismatches when a file exists in target but not source
           let patchApplied = false;
           try {
+            await capturePostApplyBaseRef();
             await exec.exec("git", ["am", "--3way", patchFilePath]);
             core.info("Patch applied successfully");
             patchApplied = true;
@@ -1771,6 +1781,7 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo
                   // Try --3way first to maximize repair opportunities even on fallback branches.
                   // If that still fails with add/add conflicts, recover and continue git am.
                   try {
+                    await capturePostApplyBaseRef();
                     await exec.exec("git", ["am", "--3way", patchFilePath]);
                   } catch (fallbackPatchError) {
                     core.warning(`Fallback git am --3way failed: ${getErrorMessage(fallbackPatchError)}`);
@@ -1802,58 +1813,92 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo
             }
           }
 
-          // Push the applied commits to the branch (with fallback to issue creation on failure)
-          // Note: when manifestProtectionFallback is set we still push the branch so the
-          // fallback issue can include a compare URL.  Genuine push failures are handled in
-          // the catch block below.
+          // POST-APPLY FILE PROTECTION: Verify actual files written match policy.
+          // This is the primary defense against parser-differential attacks where the JS
+          // patch parser and git am disagree on which files a patch contains.
+          // (see github/agentic-workflows#539)
           {
-            try {
-              branchName = await handleRemoteBranchCollision(branchName, preserveBranchName, { recreateRef, githubClient, owner: repoParts.owner, repo: repoParts.repo });
-
-              await pushSignedCommits({
-                githubClient,
-                owner: repoParts.owner,
-                repo: repoParts.repo,
-                branch: branchName,
-                baseRef: `origin/${baseBranch}`,
-                cwd: process.cwd(),
-                signedCommits,
-                resolvedTemporaryIds,
-                currentRepo: itemRepo,
+            const diffBaseRef = postApplyBaseRef || `origin/${baseBranch}`;
+            const diffResult = await exec.getExecOutput("git", ["diff", "--name-only", "--no-renames", `${diffBaseRef}..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, {
+                ...config,
+                protected_files_policy: config.protected_files_policy ?? "request_review",
               });
-              core.info("Changes pushed to branch");
-
-              // Count new commits on PR branch relative to base, used to restrict
-              // the extra empty CI-trigger commit to exactly 1 new commit.
-              try {
-                const { stdout: countStr } = await exec.getExecOutput("git", ["rev-list", "--count", `origin/${baseBranch}..HEAD`]);
-                newCommitCount = parseInt(countStr.trim(), 10);
-                core.info(`${newCommitCount} new commit(s) on branch relative to origin/${baseBranch}`);
-              } catch {
-                // Non-fatal - newCommitCount stays 0, extra empty commit will be skipped
-                core.info("Could not count new commits - extra empty commit will be skipped");
+              if (postApplyProtection.action === "deny") {
+                const filesStr = postApplyProtection.files.join(", ");
+                const msg = `SECURITY: Post-apply file-protection check failed. ` + `The patch applied files that were not detected by the pre-apply parser: ${filesStr}. ` + `This may indicate a patch-parser bypass attempt. Aborting.`;
+                core.error(msg);
+                return { success: false, error: msg };
               }
-            } catch (pushError) {
-              // Push failed - create fallback issue instead of PR (if fallback is enabled)
-              core.error(`Git push failed: ${pushError instanceof Error ? pushError.message : String(pushError)}`);
+              if (postApplyProtection.action === "fallback") {
+                manifestProtectionFallback = postApplyProtection.files;
+                core.warning(`Post-apply: Protected file protection triggered (fallback-to-issue): ${postApplyProtection.files.join(", ")}`);
+              }
+              if (postApplyProtection.action === "request_review") {
+                manifestProtectionRequestReview = postApplyProtection.files;
+                core.warning(`Post-apply: Protected file protection triggered (request_review): ${postApplyProtection.files.join(", ")}`);
+              }
+            }
+          }
 
-              if (manifestProtectionFallback) {
-                // Push failed specifically for a protected-file modification. Don't create
-                // a generic push-failed issue — fall through to the manifestProtectionFallback
-                // block below, which will create the proper protected-file review issue with
-                // patch artifact download instructions (since the branch was not pushed).
-                core.warning("Git push failed for protected-file modification - deferring to protected-file review issue");
-                manifestProtectionPushFailedError = pushError;
-              } else if (!fallbackAsIssue) {
-                // Fallback is disabled - return error without creating issue
-                core.error("fallback-as-issue is disabled - not creating fallback issue");
-                const error = `Failed to push changes: ${pushError instanceof Error ? pushError.message : String(pushError)}`;
-                return {
-                  success: false,
-                  error,
-                  error_type: "push_failed",
-                };
-              } else {
+        // Push the applied commits to the branch (with fallback to issue creation on failure)
+        // Note: when manifestProtectionFallback is set we still push the branch so the
+        // fallback issue can include a compare URL.  Genuine push failures are handled in
+        // the catch block below.
+        {
+          try {
+            branchName = await handleRemoteBranchCollision(branchName, preserveBranchName, { recreateRef, githubClient, owner: repoParts.owner, repo: repoParts.repo });
+
+            await pushSignedCommits({
+              githubClient,
+              owner: repoParts.owner,
+              repo: repoParts.repo,
+              branch: branchName,
+              baseRef: `origin/${baseBranch}`,
+              cwd: process.cwd(),
+              signedCommits,
+              resolvedTemporaryIds,
+              currentRepo: itemRepo,
+            });
+            core.info("Changes pushed to branch");
+
+            // Count new commits on PR branch relative to base, used to restrict
+            // the extra empty CI-trigger commit to exactly 1 new commit.
+            try {
+              const { stdout: countStr } = await exec.getExecOutput("git", ["rev-list", "--count", `origin/${baseBranch}..HEAD`]);
+              newCommitCount = parseInt(countStr.trim(), 10);
+              core.info(`${newCommitCount} new commit(s) on branch relative to origin/${baseBranch}`);
+            } catch {
+              // Non-fatal - newCommitCount stays 0, extra empty commit will be skipped
+              core.info("Could not count new commits - extra empty commit will be skipped");
+            }
+          } catch (pushError) {
+            // Push failed - create fallback issue instead of PR (if fallback is enabled)
+            core.error(`Git push failed: ${pushError instanceof Error ? pushError.message : String(pushError)}`);
+
+            if (manifestProtectionFallback) {
+              // Push failed specifically for a protected-file modification. Don't create
+              // a generic push-failed issue — fall through to the manifestProtectionFallback
+              // block below, which will create the proper protected-file review issue with
+              // patch artifact download instructions (since the branch was not pushed).
+              core.warning("Git push failed for protected-file modification - deferring to protected-file review issue");
+              manifestProtectionPushFailedError = pushError;
+            } else if (!fallbackAsIssue) {
+              // Fallback is disabled - return error without creating issue
+              core.error("fallback-as-issue is disabled - not creating fallback issue");
+              const error = `Failed to push changes: ${pushError instanceof Error ? pushError.message : String(pushError)}`;
+              return {
+                success: false,
+                error,
+                error_type: "push_failed",
+              };
+            } else {
                 core.warning("Git push operation failed - creating fallback issue instead of pull request");
 
                 const runUrl = buildWorkflowRunUrl(context, context.repo);

actions/setup/js/create_pull_request.test.cjs

@@ -2510,6 +2510,65 @@ describe("create_pull_request - patch apply fallback to original base commit", (
     expect(global.core.warning).toHaveBeenCalledWith(expect.stringContaining("merge conflicts"));
   });
 
+  it("should diff against the fallback pre-apply HEAD for post-apply protection checks", async () => {
+    let primaryAmAttempted = false;
+    const promptsDir = path.join(tempDir, "prompts");
+    fs.mkdirSync(promptsDir, { recursive: true });
+    fs.writeFileSync(path.join(promptsDir, "manifest_protection_request_review.md"), "Protected files: {{files}}", "utf8");
+    fs.writeFileSync(path.join(promptsDir, "manifest_protection_request_changes_review.md"), "Protected files: {{files}}", "utf8");
+    process.env.GH_AW_PROMPTS_DIR = promptsDir;
+    global.exec = {
+      exec: vi.fn().mockImplementation((cmd, args) => {
+        if (isGitAm3Way(cmd, args) && !primaryAmAttempted) {
+          primaryAmAttempted = true;
+          throw new Error("CONFLICT (content): Merge conflict in file.txt");
+        }
+        return Promise.resolve(0);
+      }),
+      getExecOutput: vi.fn().mockImplementation((cmd, args) => {
+        if (cmd === "git" && Array.isArray(args) && args[0] === "merge-base") {
+          return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
+        }
+        if (cmd === "git" && Array.isArray(args) && args[0] === "rev-parse" && args[1] === "HEAD") {
+          return Promise.resolve({ exitCode: 0, stdout: primaryAmAttempted ? `${MOCK_BASE_COMMIT_SHA}\n` : "origin-main-head\n", stderr: "" });
+        }
+        if (cmd === "git" && Array.isArray(args) && args[0] === "diff" && args[1] === "--name-only" && args[2] === "--no-renames") {
+          expect(args[3]).toBe(`${MOCK_BASE_COMMIT_SHA}..HEAD`);
+          return Promise.resolve({ exitCode: 0, stdout: ".github/CODEOWNERS\n", stderr: "" });
+        }
+        if (cmd === "git" && Array.isArray(args) && args[0] === "diff" && args.includes("--diff-filter=U")) {
+          return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
+        }
+        if (cmd === "git" && Array.isArray(args) && args[0] === "status") {
+          return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
+        }
+        if (cmd === "git" && Array.isArray(args) && args[0] === "am" && args[1] === "--show-current-patch=diff") {
+          return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
+        }
+        return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
+      }),
+    };
+
+    const { main } = require("./create_pull_request.cjs");
+    const handler = await main({
+      protected_path_prefixes: [".github/"],
+    });
+    const result = await handler(
+      {
+        title: "Test PR",
+        body: "Test body",
+        patch_path: patchFilePath,
+        branch: "test-branch",
+        base_commit: MOCK_BASE_COMMIT_SHA,
+      },
+      {}
+    );
+
+    expect(result.success).toBe(true);
+    expect(global.github.rest.pulls.createReview).toHaveBeenCalled();
+    expect(global.exec.getExecOutput).toHaveBeenCalledWith("git", ["diff", "--name-only", "--no-renames", `${MOCK_BASE_COMMIT_SHA}..HEAD`]);
+  });
+
   it("should return error when both git am --3way and the fallback git am fail", async () => {
     global.exec = {
       exec: vi.fn().mockImplementation((cmd, args) => {