Align safe-outputs bundle pre-check with post-apply file detection

.github/mcp.json

@@ -3,18 +3,8 @@
     "github-agentic-workflows": {
       "type": "local",
       "command": "gh",
-      "args": [
-        "aw",
-        "mcp-server"
-      ],
-      "tools": [
-        "compile",
-        "audit",
-        "logs",
-        "inspect",
-        "status",
-        "audit-diff"
-      ]
+      "args": ["aw", "mcp-server"],
+      "tools": ["compile", "audit", "logs", "inspect", "status", "audit-diff"]
     }
   }
-}
+}

actions/setup/js/push_to_pull_request_branch.cjs

@@ -102,6 +102,23 @@ function parsePositiveInteger(value) {
   return Number.isInteger(parsed) && parsed > 0 ? parsed : null;
 }
 
+/**
+ * Uses git as the source of truth for the files modified by a fetched bundle ref.
+ *
+ * @param {{ getExecOutput: (command: string, args?: string[], options?: any) => Promise<{ stdout: string }> }} exec
+ * @param {Record<string, unknown>} gitOptions
+ * @param {string} rangeBaseRef
+ * @param {string} bundleRef
+ * @returns {Promise<string[]>}
+ */
+async function getBundlePreApplyFiles(exec, gitOptions, rangeBaseRef, bundleRef) {
+  const bundleDiffResult = await exec.getExecOutput("git", ["diff", "--name-only", "--no-renames", `${rangeBaseRef}..${bundleRef}`], gitOptions);
+  return bundleDiffResult.stdout
+    .split("\n")
+    .map(f => f.trim())
+    .filter(Boolean);
+}
+
 /**
  * Main handler factory for push_to_pull_request_branch
  * Returns a message handler function that processes individual push_to_pull_request_branch messages
@@ -208,9 +225,9 @@ async function main(config = {}) {
       core.warning(`Bundle file path was provided but file is not present on disk: ${bundleFilePath}; falling back to patch transport`);
     }
 
-    // Always require a patch file for policy enforcement. Bundle is used for apply-time
-    // transport, but allowed-files/protected-files checks must run on patch content
-    // (see validation block below that calls checkFileProtection on patchContent).
+    // Always require a patch file. The patch remains the preview/debug artifact and
+    // the first-pass validation input; bundle transport adds an authoritative
+    // pre-apply git diff check later after the bundle ref has been fetched.
     if (!hasPatchFile) {
       const msg = "No patch file found - cannot push without changes";
 
@@ -789,6 +806,32 @@ async function main(config = {}) {
           }
           core.info(`Fetched bundle to ${bundleRef}`);
 
+          // SECURITY: Use git's own diff against the fetched bundle ref as the
+          // authoritative pre-apply file set for bundle transport. This keeps
+          // bundle pre-check and post-apply verification aligned even when the
+          // patch artifact under-detects files (for example, merge-resolution
+          // content preserved only by the bundle transport).
+          {
+            const bundleFiles = await getBundlePreApplyFiles(exec, baseGitOpts, rangeBaseRef, bundleRef);
+            if (bundleFiles.length > 0) {
+              core.info(`Pre-apply bundle verification: ${bundleFiles.length} file(s) detected from bundle transport`);
+              const bundleProtection = checkFileProtectionPostApply(bundleFiles, config);
+              if (bundleProtection.action === "deny") {
+                const filesStr = bundleProtection.files.join(", ");
+                const msg =
+                  bundleProtection.source === "post-apply"
+                    ? `Cannot push to pull request branch: bundle modifies files outside the allowed-files list (${filesStr}). Add the files to the allowed-files configuration field or remove them from the bundle.`
+                    : `Cannot push to pull request branch: bundle 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 (bundleProtection.action === "fallback") {
+                core.warning(`Protected file protection triggered (fallback-to-issue): ${bundleProtection.files.join(", ")}. Will create review issue instead of pushing.`);
+                return await createProtectedFilesFallbackIssue(bundleProtection.files);
+              }
+            }
+          }
+
           // Point the checked-out branch at the bundle tip directly. In shallow
           // checkouts, merge --ff-only can fail to discover the ancestry even
           // when the bundle tip is based on the current branch tip and the
@@ -1300,4 +1343,4 @@ async function main(config = {}) {
   };
 }
 
-module.exports = { main, HANDLER_TYPE };
+module.exports = { main, HANDLER_TYPE, getBundlePreApplyFiles };

actions/setup/js/push_to_pull_request_branch.integration.test.cjs

@@ -0,0 +1,142 @@
+import { describe, it, expect, afterEach, vi } from "vitest";
+import { createRequire } from "module";
+import fs from "fs";
+import os from "os";
+import path from "path";
+import { spawnSync } from "child_process";
+
+const require = createRequire(import.meta.url);
+const { getBundlePreApplyFiles } = require("./push_to_pull_request_branch.cjs");
+
+global.core = {
+  debug: vi.fn(),
+  error: vi.fn(),
+  info: vi.fn(),
+  warning: vi.fn(),
+};
+
+function execGit(args, options = {}) {
+  const result = spawnSync("git", args, {
+    encoding: "utf8",
+    ...options,
+  });
+  if (result.error) {
+    throw result.error;
+  }
+  if (result.status !== 0 && !options.allowFailure) {
+    throw new Error(`git ${args.join(" ")} failed: ${result.stderr}`);
+  }
+  return result;
+}
+
+function createRepo(prefix) {
+  const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
+  execGit(["init"], { cwd: repoDir });
+  execGit(["config", "user.name", "Test User"], { cwd: repoDir });
+  execGit(["config", "user.email", "test@example.com"], { cwd: repoDir });
+  return repoDir;
+}
+
+function writeRepoFile(repoDir, relativePath, content) {
+  const fullPath = path.join(repoDir, relativePath);
+  fs.mkdirSync(path.dirname(fullPath), { recursive: true });
+  fs.writeFileSync(fullPath, content);
+}
+
+function createExecApi(cwd) {
+  return {
+    async getExecOutput(command, args = [], options = {}) {
+      if (command !== "git") {
+        throw new Error(`unexpected command: ${command}`);
+      }
+      const result = execGit(args, { cwd, allowFailure: true });
+      if (result.status !== 0 && !options.ignoreReturnCode) {
+        throw new Error(result.stderr || result.stdout);
+      }
+      return { exitCode: result.status, stdout: result.stdout, stderr: result.stderr };
+    },
+  };
+}
+
+function fetchBaseCommit(targetRepo, sourceRepo, baseSha, branchName) {
+  execGit(["remote", "add", "origin", sourceRepo], { cwd: targetRepo });
+  execGit(["fetch", "origin", baseSha], { cwd: targetRepo });
+  execGit(["checkout", "-b", branchName, "FETCH_HEAD"], { cwd: targetRepo });
+}
+
+describe("push_to_pull_request_branch bundle integration", () => {
+  const tempDirs = [];
+
+  afterEach(() => {
+    for (const tempDir of tempDirs.splice(0)) {
+      fs.rmSync(tempDir, { recursive: true, force: true });
+    }
+    vi.clearAllMocks();
+  });
+
+  it("lists files from a fetched bundle before applying it", async () => {
+    const branchName = "autoloop/simple-bundle";
+    const sourceRepo = createRepo("push-pr-bundle-source-");
+    const targetRepo = createRepo("push-pr-bundle-target-");
+    tempDirs.push(sourceRepo, targetRepo);
+
+    writeRepoFile(sourceRepo, "README.md", "base\n");
+    execGit(["add", "README.md"], { cwd: sourceRepo });
+    execGit(["commit", "-m", "base"], { cwd: sourceRepo });
+    execGit(["branch", "-M", "main"], { cwd: sourceRepo });
+    const baseSha = execGit(["rev-parse", "HEAD"], { cwd: sourceRepo }).stdout.trim();
+
+    execGit(["checkout", "-b", branchName], { cwd: sourceRepo });
+    writeRepoFile(sourceRepo, ".changeset/fix.md", "patch\n");
+    writeRepoFile(sourceRepo, "docs/guide.md", "guide\n");
+    execGit(["add", ".changeset/fix.md", "docs/guide.md"], { cwd: sourceRepo });
+    execGit(["commit", "-m", "bundle change"], { cwd: sourceRepo });
+
+    const bundlePath = path.join(sourceRepo, "bundle.bundle");
+    execGit(["bundle", "create", bundlePath, `refs/heads/${branchName}`], { cwd: sourceRepo });
+
+    fetchBaseCommit(targetRepo, sourceRepo, baseSha, branchName);
+    const bundleRef = "refs/bundles/test-simple-bundle";
+    execGit(["fetch", bundlePath, `refs/heads/${branchName}:${bundleRef}`], { cwd: targetRepo });
+
+    const actualFiles = await getBundlePreApplyFiles(createExecApi(targetRepo), {}, baseSha, bundleRef);
+
+    expect(actualFiles.sort()).toEqual([".changeset/fix.md", "docs/guide.md"]);
+  });
+
+  it("includes files introduced through merge-commit bundle history", async () => {
+    const branchName = "autoloop/merge-bundle";
+    const sourceRepo = createRepo("push-pr-merge-source-");
+    const targetRepo = createRepo("push-pr-merge-target-");
+    tempDirs.push(sourceRepo, targetRepo);
+
+    writeRepoFile(sourceRepo, "README.md", "base\n");
+    execGit(["add", "README.md"], { cwd: sourceRepo });
+    execGit(["commit", "-m", "base"], { cwd: sourceRepo });
+    execGit(["branch", "-M", "main"], { cwd: sourceRepo });
+    const baseSha = execGit(["rev-parse", "HEAD"], { cwd: sourceRepo }).stdout.trim();
+
+    execGit(["checkout", "-b", "feature"], { cwd: sourceRepo });
+    writeRepoFile(sourceRepo, "feature.txt", "feature branch change\n");
+    execGit(["add", "feature.txt"], { cwd: sourceRepo });
+    execGit(["commit", "-m", "feature commit"], { cwd: sourceRepo });
+
+    execGit(["checkout", "main"], { cwd: sourceRepo });
+    writeRepoFile(sourceRepo, "main.txt", "main branch change\n");
+    execGit(["add", "main.txt"], { cwd: sourceRepo });
+    execGit(["commit", "-m", "main commit"], { cwd: sourceRepo });
+    execGit(["merge", "--no-ff", "feature", "-m", "merge feature"], { cwd: sourceRepo });
+    execGit(["checkout", "-b", branchName], { cwd: sourceRepo });
+
+    const bundlePath = path.join(sourceRepo, "merge.bundle");
+    execGit(["bundle", "create", bundlePath, `refs/heads/${branchName}`], { cwd: sourceRepo });
+
+    fetchBaseCommit(targetRepo, sourceRepo, baseSha, branchName);
+    const bundleRef = "refs/bundles/test-merge-bundle";
+    execGit(["fetch", bundlePath, `refs/heads/${branchName}:${bundleRef}`], { cwd: targetRepo });
+
+    const actualFiles = await getBundlePreApplyFiles(createExecApi(targetRepo), {}, baseSha, bundleRef);
+
+    expect(actualFiles.sort()).toEqual(["feature.txt", "main.txt"]);
+  });
+});

actions/setup/js/push_to_pull_request_branch.test.cjs

@@ -1532,6 +1532,70 @@ index 0000000..abc1234
       }
     });
 
+    it("should use authoritative bundle file detection before apply and match post-apply verification", async () => {
+      const bundlePath = canonicalBundlePath("feature-branch");
+      const patchPath = createPatchFile(
+        "feature-branch",
+        `From abc123 Mon Sep 17 00:00:00 2001
+From: Test Author <test@example.com>
+Date: Mon, 1 Jan 2024 00:00:00 +0000
+Subject: [PATCH] Test commit
+
+diff --git a/.changeset/patch-fix.md b/.changeset/patch-fix.md
+new file mode 100644
+index 0000000..abc1234
+--- /dev/null
++++ b/.changeset/patch-fix.md
+@@ -0,0 +1 @@
++content
+--
+2.34.1
+`
+      );
+      fs.writeFileSync(bundlePath, "bundle content");
+
+      const pushSignedCommitsModule = require("./push_signed_commits.cjs");
+      const pushSignedSpy = vi.spyOn(pushSignedCommitsModule, "pushSignedCommits").mockResolvedValue("bundle-tip");
+
+      try {
+        const actualFiles = [".changeset/patch-fix.md", "pkg/workflow/pi_byok_env_passthrough_integration_test.go", "pkg/workflow/pi_engine.go", "pkg/workflow/pi_engine_test.go"];
+        mockExec.getExecOutput.mockImplementation((cmd, args, options) => {
+          if (cmd === "git" && args[0] === "ls-remote") {
+            return Promise.resolve({ exitCode: 0, stdout: "remote-head\trefs/heads/feature-branch\n", stderr: "" });
+          }
+          if (cmd === "git" && args[0] === "rev-parse" && args[1] === "HEAD") {
+            return Promise.resolve({ exitCode: 0, stdout: "remote-head\n", stderr: "" });
+          }
+          if (cmd === "git" && args[0] === "rev-parse" && args[1] === "--is-shallow-repository") {
+            return Promise.resolve({ exitCode: 0, stdout: "false\n", stderr: "" });
+          }
+          if (cmd === "git" && args[0] === "fetch" && args[1] === bundlePath && options && options.ignoreReturnCode) {
+            return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
+          }
+          if (cmd === "git" && args[0] === "diff" && args[1] === "--name-only" && args[2] === "--no-renames") {
+            return Promise.resolve({ exitCode: 0, stdout: `${actualFiles.join("\n")}\n`, stderr: "" });
+          }
+          if (cmd === "git" && args[0] === "rev-list") {
+            return Promise.resolve({ exitCode: 0, stdout: "2\n", stderr: "" });
+          }
+          return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
+        });
+
+        const module = await loadModule();
+        const handler = await module.main({ allowed_files: [".changeset/**", "pkg/workflow/**"] });
+        const result = await handler({ branch: "feature-branch", diff_size: 5 * 1024 }, {});
+
+        expect(result.success).toBe(true);
+        expect(mockCore.info).toHaveBeenCalledWith("Pre-apply bundle verification: 4 file(s) detected from bundle transport");
+
+        const diffCalls = mockExec.getExecOutput.mock.calls.filter(([, args]) => Array.isArray(args) && args[0] === "diff" && args[1] === "--name-only" && args[2] === "--no-renames");
+        expect(diffCalls.map(([, args]) => args[3])).toContain("remote-head..refs/bundles/push-feature-branch");
+        expect(diffCalls.map(([, args]) => args[3])).toContain("remote-head..HEAD");
+      } finally {
+        pushSignedSpy.mockRestore();
+      }
+    });
+
     it("should use sanitized branch name (not agent-supplied message.branch) in bundle fetch refspec", async () => {
       // The agent may supply a message.branch value; the bundle fetch must use the
       // sanitized branchName from the GitHub API — never the raw agent input.