From 3091bbd0fffda4dd2fcaf0bb173593a80655e70d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:46:48 +0000 Subject: [PATCH 1/5] Initial plan From ef3224596cf5ea78a176bb5329d4a24be36498b8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:04:32 +0000 Subject: [PATCH 2/5] Resolve trusted PR bundle baseline from base branch tip Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../setup/js/push_to_pull_request_branch.cjs | 25 ++++-- .../js/push_to_pull_request_branch.test.cjs | 80 ++++++++++++++++++- .../reference/safe-outputs-pull-requests.md | 2 +- 3 files changed, 97 insertions(+), 10 deletions(-) diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index b1df498000a..2db6fc1b68a 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -1100,11 +1100,12 @@ async function main(config = {}) { // Resolve from GitHub metadata and fetch from the target checkout's // origin, never from agent-supplied refs or patch headers. let trustedBase = ""; - const baseSha = pullRequest.base?.sha; - if (!signedCommits && typeof baseSha === "string" && /^[0-9a-f]{40}$/.test(baseSha)) { + const baseRef = pullRequest.base?.ref; + if (!signedCommits && typeof baseRef === "string" && baseRef) { try { - await exec.exec("git", ["fetch", "origin", baseSha], { env: { ...process.env, ...gitAuthEnv }, ...baseGitOpts }); - trustedBase = baseSha; + await exec.exec("git", ["fetch", "origin", `refs/heads/${baseRef}`], { env: { ...process.env, ...gitAuthEnv }, ...baseGitOpts }); + const { stdout } = await exec.getExecOutput("git", ["rev-parse", "--verify", "FETCH_HEAD^{commit}"], baseGitOpts); + if (/^[0-9a-f]{40}$/.test(stdout.trim())) trustedBase = stdout.trim(); } catch (baseFetchError) { core.warning(`Unable to fetch trusted PR base; using PR-head validation: ${getErrorMessage(baseFetchError)}`); } @@ -1185,7 +1186,21 @@ async function main(config = {}) { } } core.info(`Fetched bundle to ${bundleRef}`); - const validationBase = await getBundleValidationBase(exec, baseGitOpts, rangeBaseRef, bundleRef, trustedBase); + let validationBase = await getBundleValidationBase(exec, baseGitOpts, rangeBaseRef, bundleRef, trustedBase); + const workflowSha = process.env.GITHUB_SHA; + if (trustedBase && validationBase.ref === rangeBaseRef && typeof workflowSha === "string" && /^[0-9a-f]{40}$/.test(workflowSha) && workflowSha !== trustedBase) { + try { + // A run SHA is trusted only if it belongs to the fetched base branch, + // not an agent-controlled PR head or a pull_request merge ref. + const runOnBase = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", workflowSha, trustedBase], { ...baseGitOpts, ignoreReturnCode: true }); + if (runOnBase.exitCode === 0) { + validationBase = await getBundleValidationBase(exec, baseGitOpts, rangeBaseRef, bundleRef, workflowSha); + if (validationBase.ref !== rangeBaseRef) trustedBase = workflowSha; + } + } catch { + core.warning("Unable to verify workflow SHA on the trusted base branch; using PR-head validation"); + } + } validationBaseRef = validationBase.ref; validationConflictFiles = validationBase.conflictedFiles; const trustedBaseInValidationBase = validationBaseRef !== rangeBaseRef; 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 926834013e4..c14039e962b 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import * as fs from "fs"; import * as path from "path"; import * as os from "os"; +import { spawnSync } from "child_process"; const { getPatchPathForBranch, getPatchPathForBranchInRepo } = require("./git_patch_utils.cjs"); const { getBundlePathForBranch, getBundlePathForBranchInRepo } = require("./generate_git_bundle.cjs"); @@ -2344,9 +2345,12 @@ index 0000000..abc1234 const changedPath = path.join(process.cwd(), "merge-agent.txt"); fs.writeFileSync(changedPath, Buffer.alloc(contentSize)); const pr = (await mockGithub.rest.pulls.get()).data; - pr.base.sha = trustedBase; + pr.base.ref = "main"; + pr.base.sha = "d".repeat(40); mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[0] === "rev-parse" && args[2] === "FETCH_HEAD^{commit}") return { exitCode: 0, stdout: trustedBase + "\n", stderr: "" }; if (args[0] === "merge-base") { + if (args[2] === pr.base.sha) return { exitCode: 0, stdout: "", stderr: "" }; return { exitCode: args[3] === "remote-head" ? 1 : 0, stdout: "", stderr: "" }; } if (args[0] === "merge-tree") { @@ -2375,7 +2379,8 @@ index 0000000..abc1234 expect(pushSpy).toHaveBeenCalledWith(expect.objectContaining({ baseRef: "remote-head", signedCommits: false })); expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["diff", "--name-only", "--no-renames", "-z", `${validationTree}..HEAD`], expect.any(Object)); } - expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", trustedBase], expect.any(Object)); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "refs/heads/main"], expect.any(Object)); + expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["fetch", "origin", pr.base.sha], expect.any(Object)); if (expectedError === "bundle modifies protected files") { const historyExclusions = mergeTreeFailure ? ["remote-head"] : ["remote-head", trustedBase]; expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", "refs/bundles/push-feature-branch", "--not", ...historyExclusions], expect.any(Object)); @@ -2393,6 +2398,73 @@ index 0000000..abc1234 } }); + it.each([ + ["current base tip", false, false], + ["workflow-start base commit after the branch advances", true, false], + ["untrusted workflow SHA", true, true], + ])("resolves a real git baseline with merge-base PR metadata (%s)", async (_name, baseAdvanced, untrustedRun) => { + const git = (args, allowFailure = false) => { + const result = spawnSync("git", args, { cwd: tempDir, encoding: "utf8" }); + if (result.error) throw result.error; + if (!allowFailure && result.status !== 0) throw new Error(result.stderr); + return { exitCode: result.status, stdout: result.stdout, stderr: result.stderr }; + }; + git(["init", "-b", "main"]); + git(["config", "user.name", "Test User"]); + git(["config", "user.email", "test@example.com"]); + git(["commit", "--allow-empty", "-m", "common base"]); + const mergeBase = git(["rev-parse", "HEAD"]).stdout.trim(); + git(["checkout", "-b", "remote-head"]); + git(["commit", "--allow-empty", "-m", "old PR head"]); + git(["checkout", "main"]); + fs.writeFileSync(path.join(tempDir, "package.json"), "{}\n"); + git(["add", "."]); + git(["commit", "-m", "trusted base changes"]); + const mergedBase = git(["rev-parse", "HEAD"]).stdout.trim(); + git(["checkout", "-b", "candidate", "remote-head"]); + git(["merge", "--no-ff", "main", "-m", "merge base"]); + const candidate = git(["rev-parse", "HEAD"]).stdout.trim(); + git(["update-ref", "refs/bundles/push-feature-branch", candidate]); + if (baseAdvanced) { + git(["checkout", "main"]); + git(["commit", "--allow-empty", "-m", "base advances after agent merge"]); + git(["checkout", "candidate"]); + } + git(["remote", "add", "origin", tempDir]); + process.env.GITHUB_SHA = untrustedRun ? candidate : mergedBase; + const pr = (await mockGithub.rest.pulls.get()).data; + pr.base.ref = "main"; + pr.base.sha = mergeBase; + expect(git(["merge-base", "remote-head", "main"]).stdout.trim()).toBe(pr.base.sha); + mockExec.exec.mockImplementation(async (_cmd, args) => { + if (args[0] === "fetch" && args[2] === "refs/heads/main") return git(args).exitCode; + return 0; + }); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[0] === "merge-base" || args[0] === "merge-tree" || (args[0] === "rev-parse" && args[2] === "FETCH_HEAD^{commit}")) return git(args, true); + if (args[0] === "rev-parse" && args[1] === "HEAD") return { exitCode: 0, stdout: "remote-head\n", stderr: "" }; + return { exitCode: 0, stdout: "", stderr: "" }; + }); + const branch = "real-merge-base-metadata"; + createPatchFile(branch, "small patch\n"); + fs.writeFileSync(canonicalBundlePath(branch), "bundle content"); + const pushModule = require("./push_signed_commits.cjs"); + const pushSpy = vi.spyOn(pushModule, "pushSignedCommits").mockResolvedValue(candidate); + try { + const module = await loadModule(); + const handler = await module.main({ signed_commits: false }); + expect((await handler({ branch }, {})).success).toBe(true); + const validationTree = git(["merge-tree", "--write-tree", "remote-head", mergedBase]).stdout.trim(); + const baseline = untrustedRun ? "remote-head" : validationTree; + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["diff", "--name-only", "--no-renames", "-z", `${baseline}..refs/bundles/push-feature-branch`], expect.any(Object)); + const historyExclusions = untrustedRun ? ["remote-head"] : ["remote-head", mergedBase]; + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", "refs/bundles/push-feature-branch", "--not", ...historyExclusions], expect.any(Object)); + expect(mockExec.getExecOutput.mock.calls.some(([, args]) => args[0] === "merge-base" && args[2] === pr.base.sha)).toBe(false); + } finally { + pushSpy.mockRestore(); + } + }); + it.each([128, "throw"])("retains the PR-head baseline when merge-tree fails (%s)", async failure => { mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { if (args[0] === "merge-base") return { exitCode: args[3] === "old-head" ? 1 : 0, stdout: "", stderr: "" }; @@ -2423,9 +2495,9 @@ index 0000000..abc1234 createPatchFile(branch, "x".repeat(4096)); fs.writeFileSync(canonicalBundlePath(branch), "bundle content"); const pr = (await mockGithub.rest.pulls.get()).data; - pr.base.sha = "b".repeat(40); + pr.base.ref = "main"; mockExec.exec.mockImplementation(async (_cmd, args) => { - if (args[0] === "fetch" && args[2] === pr.base.sha) throw new Error("base unavailable"); + if (args[0] === "fetch" && args[2] === "refs/heads/main") throw new Error("base unavailable"); return 0; }); mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { 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 e64e63cf1d2..ec8825f0121 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -435,7 +435,7 @@ branch name (see [issue #37835](https://github.com/github/gh-aw/issues/37835)). By default, pushes are replayed through GitHub's signed commit API because `signed-commits: true` means signed commits are required. Set `signed-commits: false` only for repositories that do not require signed commits; this uses direct `git push` and can preserve merge commits that the signed commit API cannot represent. This field is supported by both `create-pull-request` and `push-to-pull-request-branch`. -For unsigned bundle pushes that newly incorporate the PR base commit fetched by the handler, patch-size, changed-content-size, and protected-file checks compare against a validation-only merge of the old PR head and that trusted base. Unmodified base-branch changes are excluded; agent changes and conflict resolutions remain subject to the checks. Agent-created octopus merges (three or more parents) are rejected in this mode because their resolution changes cannot be validated with Git's remerge diffs. If the trusted baseline cannot be established, checks use the old PR head. Staged mode retains offline patch-preview checks. The push remains append-only: merge an already-fetched base ref rather than rebasing existing PR commits. +For unsigned bundle pushes that newly incorporate the PR base branch tip fetched by the handler, patch-size, changed-content-size, and protected-file checks compare against a validation-only merge of the old PR head and that trusted base. The handler resolves the tip from the base branch ref, not the PR API's `base.sha`. If the branch advances beyond the agent's merge, the handler also tries the workflow run's `GITHUB_SHA`, provided it is an ancestor of the fetched base tip and newly incorporated in the bundle. Unmodified base-branch changes are excluded; agent changes and conflict resolutions remain subject to the checks. Agent-created octopus merges (three or more parents) are rejected in this mode because their resolution changes cannot be validated with Git's remerge diffs. If the trusted baseline cannot be established, checks use the old PR head. Staged mode retains offline patch-preview checks. The push remains append-only: merge an already-fetched base ref rather than rebasing existing PR commits. ### Cross-repo usage From 8ffb3828f3cf8c563304a4342e61f2f86b7c6423 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 14 Oct 2026 16:20:28 +0000 Subject: [PATCH 3/5] Trace trusted-base validation decisions without private Git details Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../setup/js/push_to_pull_request_branch.cjs | 60 +++++++++++++++---- .../js/push_to_pull_request_branch.test.cjs | 40 ++++++++++++- 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 2db6fc1b68a..6a6b76c649e 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -139,18 +139,33 @@ async function getBundlePreApplyFiles(exec, gitOptions, rangeBaseRef, bundleRef, */ async function getBundleValidationBase(exec, gitOptions, oldHead, candidate, trustedBase) { const fallback = { ref: oldHead, conflictedFiles: [] }; - if (!trustedBase) return fallback; + if (!trustedBase) { + core.debug("Bundle validation: no trusted base available; using PR-head validation"); + return fallback; + } try { - const opts = { ...gitOptions, ignoreReturnCode: true }; + const opts = { ...gitOptions, ignoreReturnCode: true, silent: true }; const oldAncestor = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", oldHead, candidate], opts); const baseAncestor = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", trustedBase, candidate], opts); const alreadyMerged = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", trustedBase, oldHead], opts); - if (oldAncestor.exitCode !== 0 || baseAncestor.exitCode !== 0 || alreadyMerged.exitCode !== 1) return fallback; + if (oldAncestor.exitCode !== 0) { + core.debug("Bundle validation: candidate does not descend from PR head or ancestry is unavailable; using PR-head validation"); + return fallback; + } + if (baseAncestor.exitCode !== 0) { + core.debug("Bundle validation: candidate does not include trusted base or ancestry is unavailable; using PR-head validation"); + return fallback; + } + if (alreadyMerged.exitCode !== 1) { + core.debug("Bundle validation: trusted base is already in PR head or ancestry is unavailable; using PR-head validation"); + return fallback; + } const merge = await exec.getExecOutput("git", ["merge-tree", "--write-tree", "--name-only", "-z", oldHead, trustedBase], opts); const [tree, ...fields] = merge.stdout.split("\0"); if ((merge.exitCode === 0 || merge.exitCode === 1) && /^[0-9a-f]{40}$/.test(tree)) { const separatorIndex = fields.indexOf(""); + core.debug(merge.exitCode === 1 ? "Bundle validation: trusted-base merge tree selected with conflicts" : "Bundle validation: trusted-base merge tree selected without conflicts"); return { ref: tree, conflictedFiles: merge.exitCode === 1 ? fields.slice(0, separatorIndex === -1 ? fields.length : separatorIndex) : [], @@ -1103,12 +1118,19 @@ async function main(config = {}) { const baseRef = pullRequest.base?.ref; if (!signedCommits && typeof baseRef === "string" && baseRef) { try { - await exec.exec("git", ["fetch", "origin", `refs/heads/${baseRef}`], { env: { ...process.env, ...gitAuthEnv }, ...baseGitOpts }); - const { stdout } = await exec.getExecOutput("git", ["rev-parse", "--verify", "FETCH_HEAD^{commit}"], baseGitOpts); - if (/^[0-9a-f]{40}$/.test(stdout.trim())) trustedBase = stdout.trim(); - } catch (baseFetchError) { - core.warning(`Unable to fetch trusted PR base; using PR-head validation: ${getErrorMessage(baseFetchError)}`); + await exec.exec("git", ["fetch", "origin", `refs/heads/${baseRef}`], { env: { ...process.env, ...gitAuthEnv }, ...baseGitOpts, silent: true }); + const { stdout } = await exec.getExecOutput("git", ["rev-parse", "--verify", "FETCH_HEAD^{commit}"], { ...baseGitOpts, silent: true }); + if (/^[0-9a-f]{40}$/.test(stdout.trim())) { + trustedBase = stdout.trim(); + core.debug("Bundle validation: fetched trusted base branch tip"); + } else { + core.debug("Bundle validation: fetched base tip is invalid; using PR-head validation"); + } + } catch { + core.warning("Unable to fetch trusted PR base; using PR-head validation"); } + } else { + core.debug(signedCommits ? "Bundle validation: trusted-base resolution skipped for signed commits" : "Bundle validation: trusted-base resolution skipped because base branch metadata is unavailable"); } await ensureFullHistoryForBundle( exec, @@ -1189,23 +1211,39 @@ async function main(config = {}) { let validationBase = await getBundleValidationBase(exec, baseGitOpts, rangeBaseRef, bundleRef, trustedBase); const workflowSha = process.env.GITHUB_SHA; if (trustedBase && validationBase.ref === rangeBaseRef && typeof workflowSha === "string" && /^[0-9a-f]{40}$/.test(workflowSha) && workflowSha !== trustedBase) { + core.debug("Bundle validation: checking workflow-start base fallback"); try { // A run SHA is trusted only if it belongs to the fetched base branch, // not an agent-controlled PR head or a pull_request merge ref. - const runOnBase = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", workflowSha, trustedBase], { ...baseGitOpts, ignoreReturnCode: true }); + const runOnBase = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", workflowSha, trustedBase], { ...baseGitOpts, ignoreReturnCode: true, silent: true }); if (runOnBase.exitCode === 0) { validationBase = await getBundleValidationBase(exec, baseGitOpts, rangeBaseRef, bundleRef, workflowSha); - if (validationBase.ref !== rangeBaseRef) trustedBase = workflowSha; + if (validationBase.ref !== rangeBaseRef) { + trustedBase = workflowSha; + core.debug("Bundle validation: workflow-start base fallback selected"); + } else { + core.debug("Bundle validation: workflow-start base fallback did not establish a baseline"); + } + } else { + core.debug("Bundle validation: workflow-start base fallback rejected because base-branch ancestry is unverified"); } } catch { core.warning("Unable to verify workflow SHA on the trusted base branch; using PR-head validation"); } + } else { + core.debug( + validationBase.ref !== rangeBaseRef + ? "Bundle validation: workflow-start base fallback skipped because base tip established a baseline" + : "Bundle validation: workflow-start base fallback skipped because no distinct valid workflow base is available" + ); } validationBaseRef = validationBase.ref; validationConflictFiles = validationBase.conflictedFiles; const trustedBaseInValidationBase = validationBaseRef !== rangeBaseRef; if (trustedBaseInValidationBase) { - core.info(`Bundle validation baseline includes trusted base ${trustedBase}: ${validationBaseRef}`); + core.info("Bundle validation baseline includes trusted base changes"); + } else { + core.info("Bundle validation baseline uses PR head"); } const agentHistoryExclusions = [rangeBaseRef]; if (trustedBaseInValidationBase) agentHistoryExclusions.push(trustedBase); 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 c14039e962b..66916860f9a 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -2460,11 +2460,45 @@ index 0000000..abc1234 const historyExclusions = untrustedRun ? ["remote-head"] : ["remote-head", mergedBase]; expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", "refs/bundles/push-feature-branch", "--not", ...historyExclusions], expect.any(Object)); expect(mockExec.getExecOutput.mock.calls.some(([, args]) => args[0] === "merge-base" && args[2] === pr.base.sha)).toBe(false); + expect(mockCore.debug).toHaveBeenCalledWith("Bundle validation: fetched trusted base branch tip"); + expect(mockCore.info).toHaveBeenCalledWith(untrustedRun ? "Bundle validation baseline uses PR head" : "Bundle validation baseline includes trusted base changes"); + expect(mockCore.debug).toHaveBeenCalledWith( + untrustedRun + ? "Bundle validation: workflow-start base fallback rejected because base-branch ancestry is unverified" + : baseAdvanced + ? "Bundle validation: workflow-start base fallback selected" + : "Bundle validation: workflow-start base fallback skipped because base tip established a baseline" + ); + const baselineLogs = [...mockCore.debug.mock.calls, ...mockCore.info.mock.calls, ...mockCore.warning.mock.calls].map(([message]) => message).filter(message => message.startsWith("Bundle validation")); + for (const message of baselineLogs) { + expect(message).not.toMatch(/[0-9a-f]{40}/); + expect(message).not.toContain("remote-head"); + expect(message).not.toContain("refs/"); + } + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "refs/heads/main"], expect.objectContaining({ silent: true })); + for (const [, args, options] of mockExec.getExecOutput.mock.calls) { + if (args[0] === "merge-base" || args[0] === "merge-tree" || args[2] === "FETCH_HEAD^{commit}") expect(options.silent).toBe(true); + } } finally { pushSpy.mockRestore(); } }); + it.each([ + ["", 0, 0, 1, "no trusted base available"], + ["trusted-base", 1, 0, 1, "candidate does not descend from PR head"], + ["trusted-base", 0, 1, 1, "candidate does not include trusted base"], + ["trusted-base", 0, 0, 0, "trusted base is already in PR head"], + ["trusted-base", 0, 0, 128, "trusted base is already in PR head"], + ])("logs the conservative baseline decision without refs (%s, %s, %s, %s)", async (trustedBase, oldAncestor, baseAncestor, alreadyMerged, reason) => { + mockExec.getExecOutput.mockResolvedValueOnce({ exitCode: oldAncestor }).mockResolvedValueOnce({ exitCode: baseAncestor }).mockResolvedValueOnce({ exitCode: alreadyMerged }); + const module = await loadModule(); + expect(await module.getBundleValidationBase(mockExec, {}, "private-old-head", "private-candidate", trustedBase)).toEqual({ ref: "private-old-head", conflictedFiles: [] }); + expect(mockCore.debug).toHaveBeenCalledWith(expect.stringContaining(reason)); + expect(mockCore.debug).toHaveBeenCalledWith(expect.stringContaining("using PR-head validation")); + expect(mockCore.debug.mock.calls.flat().join(" ")).not.toMatch(/private-|trusted-base/); + }); + it.each([128, "throw"])("retains the PR-head baseline when merge-tree fails (%s)", async failure => { mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { if (args[0] === "merge-base") return { exitCode: args[3] === "old-head" ? 1 : 0, stdout: "", stderr: "" }; @@ -2488,6 +2522,8 @@ index 0000000..abc1234 ref: validationTree, conflictedFiles: ["only-conflict.txt"], }); + expect(mockCore.debug).toHaveBeenCalledWith("Bundle validation: trusted-base merge tree selected with conflicts"); + expect(mockCore.debug.mock.calls.flat().join(" ")).not.toContain("only-conflict.txt"); }); it("retains the preview size limit when the trusted base fetch fails", async () => { @@ -2497,7 +2533,7 @@ index 0000000..abc1234 const pr = (await mockGithub.rest.pulls.get()).data; pr.base.ref = "main"; mockExec.exec.mockImplementation(async (_cmd, args) => { - if (args[0] === "fetch" && args[2] === "refs/heads/main") throw new Error("base unavailable"); + if (args[0] === "fetch" && args[2] === "refs/heads/main") throw new Error("private remote details"); return 0; }); mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { @@ -2509,6 +2545,8 @@ index 0000000..abc1234 const result = await handler({ branch, diff_size: 0 }, {}); expect(result.success).toBe(false); expect(result.error).toContain("Patch size"); + expect(mockCore.warning).toHaveBeenCalledWith("Unable to fetch trusted PR base; using PR-head validation"); + expect(mockCore.warning.mock.calls.flat().join(" ")).not.toContain("private remote details"); expect(mockExec.exec.mock.calls.some(([, args]) => args[0] === "update-ref" && args[1] === "refs/heads/feature-branch")).toBe(false); }); From c96edcf1c5ce3417a053dbfa4733f77e59f76e8f Mon Sep 17 00:00:00 2001 From: pelikhan Date: Wed, 14 Oct 2026 09:41:02 -0700 Subject: [PATCH 4/5] Test PR-head fallback when trusted base ref is missing Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../js/push_to_pull_request_branch.test.cjs | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) 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 66916860f9a..fcfccad3bd0 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -2526,6 +2526,53 @@ index 0000000..abc1234 expect(mockCore.debug.mock.calls.flat().join(" ")).not.toContain("only-conflict.txt"); }); + it.each([ + ["missing", undefined, false], + ["empty", "", false], + ["missing", undefined, true], + ["empty", "", true], + ])("uses PR-head validation for a %s base ref (%s; oversized: %s)", async (_name, baseRef, oversized) => { + const branch = "missing-base-ref"; + const agentPatch = oversized ? "x".repeat(4096) : "small patch\n"; + createPatchFile(branch, agentPatch); + fs.writeFileSync(canonicalBundlePath(branch), "bundle content"); + const pr = (await mockGithub.rest.pulls.get()).data; + pr.base.ref = baseRef; + process.env.GITHUB_SHA = "b".repeat(40); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[0] === "rev-parse" && args[1] === "HEAD") return { exitCode: 0, stdout: "remote-head\n", stderr: "" }; + if (args[0] === "log" && args.includes("--patch")) return { exitCode: 0, stdout: agentPatch, stderr: "" }; + return { exitCode: 0, stdout: "", stderr: "" }; + }); + const pushModule = require("./push_signed_commits.cjs"); + const pushSpy = vi.spyOn(pushModule, "pushSignedCommits").mockResolvedValue("candidate-head"); + try { + const module = await loadModule(); + const handler = await module.main({ max_patch_size: 1, signed_commits: false }); + const result = await handler({ branch, diff_size: 0 }, {}); + if (oversized) { + expect(result.success).toBe(false); + expect(result.error).toContain("Patch size"); + expect(pushSpy).not.toHaveBeenCalled(); + } else { + expect(result.success).toBe(true); + expect(pushSpy).toHaveBeenCalled(); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["diff", "--name-only", "--no-renames", "-z", "remote-head..refs/bundles/push-feature-branch"], expect.any(Object)); + } + expect(mockExec.getExecOutput).toHaveBeenCalledWith( + "git", + ["log", "--format=", "--patch", "--binary", "--no-ext-diff", "--no-textconv", "--diff-merges=remerge", "refs/bundles/push-feature-branch", "--not", "remote-head"], + expect.any(Object) + ); + expect(mockExec.exec.mock.calls.some(([, args]) => args[0] === "fetch" && args.some(arg => arg.startsWith("refs/heads/")))).toBe(false); + expect(mockExec.getExecOutput.mock.calls.some(([, args]) => args[0] === "merge-base" || args[0] === "merge-tree" || args.includes("FETCH_HEAD^{commit}"))).toBe(false); + expect(mockCore.debug).toHaveBeenCalledWith("Bundle validation: trusted-base resolution skipped because base branch metadata is unavailable"); + expect(mockCore.info).toHaveBeenCalledWith("Bundle validation baseline uses PR head"); + } finally { + pushSpy.mockRestore(); + } + }); + it("retains the preview size limit when the trusted base fetch fails", async () => { const branch = "merge-base-fetch-fails"; createPatchFile(branch, "x".repeat(4096)); From 300405e1855212bce80e95048eacc84e5436c29f Mon Sep 17 00:00:00 2001 From: pelikhan Date: Wed, 14 Oct 2026 11:16:15 -0700 Subject: [PATCH 5/5] Cover trusted-base clone and credential compatibility Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../js/push_to_pull_request_branch.test.cjs | 137 ++++++++++++++++-- .../reference/safe-outputs-pull-requests.md | 2 + 2 files changed, 126 insertions(+), 13 deletions(-) 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 fcfccad3bd0..746896fd5a0 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -2402,9 +2402,15 @@ index 0000000..abc1234 ["current base tip", false, false], ["workflow-start base commit after the branch advances", true, false], ["untrusted workflow SHA", true, true], - ])("resolves a real git baseline with merge-base PR metadata (%s)", async (_name, baseAdvanced, untrustedRun) => { + ["older merged base distinct from workflow SHA", true, false, "full", true], + ["shallow clone with sufficient ancestry", false, false, "shallow"], + ["shallow clone with missing ancestry", false, false, "missing-ancestry"], + ["partial clone with lazy blob fetching", false, false, "partial"], + ["partial clone after the branch advances", true, false, "partial"], + ])("resolves a real git baseline with merge-base PR metadata (%s)", async (_name, baseAdvanced, untrustedRun, cloneMode = "full", olderMergedBase = false) => { + let repoDir = tempDir; const git = (args, allowFailure = false) => { - const result = spawnSync("git", args, { cwd: tempDir, encoding: "utf8" }); + const result = spawnSync("git", args, { cwd: repoDir, encoding: "utf8" }); if (result.error) throw result.error; if (!allowFailure && result.status !== 0) throw new Error(result.stderr); return { exitCode: result.status, stdout: result.stdout, stderr: result.stderr }; @@ -2412,10 +2418,13 @@ index 0000000..abc1234 git(["init", "-b", "main"]); git(["config", "user.name", "Test User"]); git(["config", "user.email", "test@example.com"]); + for (let index = 0; index < 5; index++) git(["commit", "--allow-empty", "-m", `earlier history ${index}`]); git(["commit", "--allow-empty", "-m", "common base"]); const mergeBase = git(["rev-parse", "HEAD"]).stdout.trim(); + const earlierWorkflowBase = git(["rev-parse", "HEAD^"]).stdout.trim(); git(["checkout", "-b", "remote-head"]); git(["commit", "--allow-empty", "-m", "old PR head"]); + const oldHead = git(["rev-parse", "HEAD"]).stdout.trim(); git(["checkout", "main"]); fs.writeFileSync(path.join(tempDir, "package.json"), "{}\n"); git(["add", "."]); @@ -2424,18 +2433,39 @@ index 0000000..abc1234 git(["checkout", "-b", "candidate", "remote-head"]); git(["merge", "--no-ff", "main", "-m", "merge base"]); const candidate = git(["rev-parse", "HEAD"]).stdout.trim(); - git(["update-ref", "refs/bundles/push-feature-branch", candidate]); + const validationTree = git(["merge-tree", "--write-tree", "remote-head", mergedBase]).stdout.trim(); if (baseAdvanced) { git(["checkout", "main"]); git(["commit", "--allow-empty", "-m", "base advances after agent merge"]); git(["checkout", "candidate"]); } - git(["remote", "add", "origin", tempDir]); - process.env.GITHUB_SHA = untrustedRun ? candidate : mergedBase; + if (cloneMode === "full") { + git(["remote", "add", "origin", tempDir]); + } else { + git(["config", "uploadpack.allowFilter", "true"]); + const checkout = path.join(tempDir, "checkout"); + const cloneOptions = cloneMode === "partial" ? ["--filter=blob:none", "--no-checkout"] : [`--depth=${cloneMode === "shallow" ? 3 : 1}`]; + git(["clone", "--branch", "candidate", ...cloneOptions, `file://${tempDir}`, checkout]); + repoDir = checkout; + git(["config", "user.name", "Test User"]); + git(["config", "user.email", "test@example.com"]); + if (cloneMode === "missing-ancestry") git(["fetch", "--depth=1", "origin", "remote-head:refs/heads/remote-head"]); + else git(["update-ref", "refs/heads/remote-head", oldHead]); + if (cloneMode === "partial") { + expect(git(["config", "--get", "remote.origin.promisor"]).stdout.trim()).toBe("true"); + expect(git(["config", "--get", "remote.origin.partialclonefilter"]).stdout.trim()).toBe("blob:none"); + expect(git(["rev-list", "--objects", "--all", "--missing=print"]).stdout).toMatch(/^\?/m); + expect(git(["cat-file", "-e", `${mergedBase}:package.json`]).exitCode).toBe(0); + } else { + expect(git(["rev-parse", "--is-shallow-repository"]).stdout.trim()).toBe("true"); + } + } + git(["update-ref", "refs/bundles/push-feature-branch", candidate]); + process.env.GITHUB_SHA = untrustedRun ? candidate : olderMergedBase ? earlierWorkflowBase : mergedBase; const pr = (await mockGithub.rest.pulls.get()).data; pr.base.ref = "main"; pr.base.sha = mergeBase; - expect(git(["merge-base", "remote-head", "main"]).stdout.trim()).toBe(pr.base.sha); + if (cloneMode === "full") expect(git(["merge-base", "remote-head", "main"]).stdout.trim()).toBe(pr.base.sha); mockExec.exec.mockImplementation(async (_cmd, args) => { if (args[0] === "fetch" && args[2] === "refs/heads/main") return git(args).exitCode; return 0; @@ -2443,6 +2473,7 @@ index 0000000..abc1234 mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { if (args[0] === "merge-base" || args[0] === "merge-tree" || (args[0] === "rev-parse" && args[2] === "FETCH_HEAD^{commit}")) return git(args, true); if (args[0] === "rev-parse" && args[1] === "HEAD") return { exitCode: 0, stdout: "remote-head\n", stderr: "" }; + if (args[0] === "rev-parse" && args[1] === "--is-shallow-repository") return git(args); return { exitCode: 0, stdout: "", stderr: "" }; }); const branch = "real-merge-base-metadata"; @@ -2454,20 +2485,24 @@ index 0000000..abc1234 const module = await loadModule(); const handler = await module.main({ signed_commits: false }); expect((await handler({ branch }, {})).success).toBe(true); - const validationTree = git(["merge-tree", "--write-tree", "remote-head", mergedBase]).stdout.trim(); - const baseline = untrustedRun ? "remote-head" : validationTree; + const usesPrHead = untrustedRun || olderMergedBase || cloneMode === "missing-ancestry"; + const baseline = usesPrHead ? "remote-head" : validationTree; expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["diff", "--name-only", "--no-renames", "-z", `${baseline}..refs/bundles/push-feature-branch`], expect.any(Object)); - const historyExclusions = untrustedRun ? ["remote-head"] : ["remote-head", mergedBase]; + const historyExclusions = usesPrHead ? ["remote-head"] : ["remote-head", mergedBase]; expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", "refs/bundles/push-feature-branch", "--not", ...historyExclusions], expect.any(Object)); expect(mockExec.getExecOutput.mock.calls.some(([, args]) => args[0] === "merge-base" && args[2] === pr.base.sha)).toBe(false); expect(mockCore.debug).toHaveBeenCalledWith("Bundle validation: fetched trusted base branch tip"); - expect(mockCore.info).toHaveBeenCalledWith(untrustedRun ? "Bundle validation baseline uses PR head" : "Bundle validation baseline includes trusted base changes"); + expect(mockCore.info).toHaveBeenCalledWith(usesPrHead ? "Bundle validation baseline uses PR head" : "Bundle validation baseline includes trusted base changes"); expect(mockCore.debug).toHaveBeenCalledWith( untrustedRun ? "Bundle validation: workflow-start base fallback rejected because base-branch ancestry is unverified" - : baseAdvanced - ? "Bundle validation: workflow-start base fallback selected" - : "Bundle validation: workflow-start base fallback skipped because base tip established a baseline" + : olderMergedBase + ? "Bundle validation: workflow-start base fallback did not establish a baseline" + : cloneMode === "missing-ancestry" + ? "Bundle validation: workflow-start base fallback skipped because no distinct valid workflow base is available" + : baseAdvanced + ? "Bundle validation: workflow-start base fallback selected" + : "Bundle validation: workflow-start base fallback skipped because base tip established a baseline" ); const baselineLogs = [...mockCore.debug.mock.calls, ...mockCore.info.mock.calls, ...mockCore.warning.mock.calls].map(([message]) => message).filter(message => message.startsWith("Bundle validation")); for (const message of baselineLogs) { @@ -2573,6 +2608,82 @@ index 0000000..abc1234 } }); + it.each([ + [false, false], + [false, true], + [true, false], + [true, true], + ])("uses target-checkout credentials for base fetch (fork: %s, unavailable: %s)", async (fork, unavailable) => { + const branch = "cross-repo-base-credentials"; + const checkout = path.join(tempDir, "target-checkout"); + fs.mkdirSync(checkout); + process.env.GITHUB_WORKSPACE = tempDir; + const targetRepo = "other-owner/other-repo"; + const headRepo = fork ? "automation/other-repo" : targetRepo; + const baseHeader = "Authorization: basic synthetic-target-checkout-credential"; + const headToken = "synthetic-fork-head-token"; + const headHeader = `Authorization: basic ${Buffer.from(`x-access-token:${headToken}`).toString("base64")}`; + let activeHeader = baseHeader; + const pr = (await mockGithub.rest.pulls.get()).data; + pr.head.repo = { full_name: headRepo, fork, owner: { login: fork ? "automation" : "other-owner" } }; + pr.base = { ref: "main", repo: { full_name: targetRepo, owner: { login: "other-owner" } } }; + createPatchFile(branch, "small patch\n"); + fs.writeFileSync(canonicalBundlePath(branch), "bundle content"); + mockExec.exec.mockImplementation(async (_cmd, args, options) => { + if (args[0] === "config" && args.includes("--replace-all")) { + expect(options).toEqual(expect.objectContaining({ cwd: checkout, silent: true })); + activeHeader = args.at(-1); + } + if (args[0] === "fetch") { + expect(options.cwd).toBe(checkout); + if (args[2] === "refs/heads/main") { + expect(args[1]).toBe("origin"); + expect(activeHeader).toBe(baseHeader); + expect(options.silent).toBe(true); + if (unavailable) throw new Error(`authentication unavailable: ${baseHeader}`); + } else if (fork && args[1].startsWith("https://github.com/automation/")) { + expect(activeHeader).toBe(headHeader); + } + } + return 0; + }); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[0] === "config" && args.includes("--get-all")) return { exitCode: 0, stdout: activeHeader + "\n", stderr: "" }; + if (args[0] === "config" && args.includes("--unset-all")) activeHeader = ""; + if (args[0] === "rev-parse" && args[1] === "HEAD") return { exitCode: 0, stdout: "remote-head\n", stderr: "" }; + if (args[0] === "rev-parse" && args[2] === "FETCH_HEAD^{commit}") return { exitCode: 0, stdout: "b".repeat(40) + "\n", stderr: "" }; + if (args[0] === "merge-base") return { exitCode: 1, stdout: "", stderr: "" }; + return { exitCode: 0, stdout: "", stderr: "" }; + }); + const pushModule = require("./push_signed_commits.cjs"); + const pushSpy = vi.spyOn(pushModule, "pushSignedCommits").mockResolvedValue("candidate-head"); + try { + const module = await loadModule(); + const handler = await module.main({ + signed_commits: false, + "target-repo": targetRepo, + allowed_repos: [targetRepo, headRepo], + checkout_mapping: { [targetRepo]: "target-checkout" }, + ...(fork ? { "head-repo": headRepo, "head-github-token": headToken } : {}), + }); + const result = await handler({ branch, pull_request_number: 42 }, {}); + expect(result.error).toBeUndefined(); + expect(result.success).toBe(true); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "refs/heads/main"], expect.objectContaining({ cwd: checkout, silent: true })); + expect(activeHeader).toBe(baseHeader); + if (unavailable) { + expect(mockCore.warning).toHaveBeenCalledWith("Unable to fetch trusted PR base; using PR-head validation"); + } else { + expect(mockCore.debug).toHaveBeenCalledWith("Bundle validation: fetched trusted base branch tip"); + expect(mockCore.warning).not.toHaveBeenCalledWith("Unable to fetch trusted PR base; using PR-head validation"); + } + const logs = [...mockCore.info.mock.calls, ...mockCore.debug.mock.calls, ...mockCore.warning.mock.calls, ...mockCore.error.mock.calls].flat().join("\n"); + for (const secret of [baseHeader, headToken, headHeader]) expect(logs).not.toContain(secret); + } finally { + pushSpy.mockRestore(); + } + }); + it("retains the preview size limit when the trusted base fetch fails", async () => { const branch = "merge-base-fetch-fails"; createPatchFile(branch, "x".repeat(4096)); 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 ec8825f0121..9e35a4bbddf 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -437,6 +437,8 @@ By default, pushes are replayed through GitHub's signed commit API because `sign For unsigned bundle pushes that newly incorporate the PR base branch tip fetched by the handler, patch-size, changed-content-size, and protected-file checks compare against a validation-only merge of the old PR head and that trusted base. The handler resolves the tip from the base branch ref, not the PR API's `base.sha`. If the branch advances beyond the agent's merge, the handler also tries the workflow run's `GITHUB_SHA`, provided it is an ancestor of the fetched base tip and newly incorporated in the bundle. Unmodified base-branch changes are excluded; agent changes and conflict resolutions remain subject to the checks. Agent-created octopus merges (three or more parents) are rejected in this mode because their resolution changes cannot be validated with Git's remerge diffs. If the trusted baseline cannot be established, checks use the old PR head. Staged mode retains offline patch-preview checks. The push remains append-only: merge an already-fetched base ref rather than rebasing existing PR commits. +If the bundle merged an older base commit that matches neither the fetched tip nor an eligible `GITHUB_SHA`, or a shallow checkout lacks the ancestry needed to verify it, validation remains based on the old PR head. Upstream changes can then count toward size and protected-file limits. Fetching the base tip uses the target checkout's existing credentials and can download additional objects; it does not add REST API requests or impose a new fetch-depth limit. + ### Cross-repo usage `push-to-pull-request-branch` supports pushing to pull requests in a different repository via `target-repo` (and optionally `allowed-repos`). For fork-backed pull requests, set `head-repo` to the exact repository that owns the PR head branch; follow-up pushes are permitted only when the PR head repository matches that value exactly. When `target-repo` is set, **the target repository must be checked out into the workflow workspace** using either the `checkout:` frontmatter field with a `path:` specified or a custom `steps:` entry that clones it under `$GITHUB_WORKSPACE`. Use `target-repo: "*"` to let the agent choose the target repository at runtime (the safe_outputs job will check out all `checkout:` repositories into subdirectories automatically).