diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 8623f22e6cb..ed854bbebd6 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -183,15 +183,16 @@ async function tryRecoverGitAmAddAddConflict(execApi) { * @param {string} branchName - Target branch name * @param {string} originalAgentBranch - Original source branch name from the agent, if different * @param {{ exec: Function, getExecOutput: Function }} execApi - GitHub Actions exec API + * @param {string} [baseBranch] - Base branch name (used for iterative shallow-clone deepening) * @returns {Promise} */ -async function applyBundleToBranch(bundleFilePath, branchName, originalAgentBranch, execApi) { +async function applyBundleToBranch(bundleFilePath, branchName, originalAgentBranch, execApi, baseBranch) { let bundleBranchRef = `refs/heads/${originalAgentBranch || branchName}`; const bundleTargetRef = `refs/heads/${branchName}`; const bundleTempRef = createBundleTempRef(branchName); try { - await ensureFullHistoryForBundle(execApi); + await ensureFullHistoryForBundle(execApi, {}, { baseRef: baseBranch, bundleFilePath }); core.info(`Applying bundle ${bundleFilePath} to ${bundleTargetRef} using temp ref ${bundleTempRef} from ${bundleBranchRef}`); // Fetch from bundle into a temporary ref, then update the target branch. @@ -1482,7 +1483,7 @@ async function main(config = {}) { // unlike git format-patch which flattens history and drops merge resolution content. core.info(`Applying changes from bundle: ${bundleFilePath}`); try { - await applyBundleToBranch(bundleFilePath, branchName, originalAgentBranch, exec); + await applyBundleToBranch(bundleFilePath, branchName, originalAgentBranch, exec, baseBranch); } catch (bundleError) { core.error(`Failed to apply bundle: ${bundleError instanceof Error ? bundleError.message : String(bundleError)}`); return { success: false, error: "Failed to apply bundle" }; @@ -1506,6 +1507,7 @@ async function main(config = {}) { signedCommits, resolvedTemporaryIds, currentRepo: itemRepo, + validationConfig: config, }); core.info("Changes pushed to branch (from bundle)"); @@ -1538,6 +1540,7 @@ async function main(config = {}) { signedCommits, resolvedTemporaryIds, currentRepo: itemRepo, + validationConfig: config, }); core.info("Changes pushed to branch after bundle rewrite retry"); @@ -1869,6 +1872,7 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo signedCommits, resolvedTemporaryIds, currentRepo: itemRepo, + validationConfig: config, }); core.info("Changes pushed to branch"); @@ -2017,6 +2021,7 @@ ${patchPreview}`; signedCommits, resolvedTemporaryIds, currentRepo: itemRepo, + validationConfig: config, }); core.info("Empty branch pushed successfully"); diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index c5be67fe4bd..b00493457ca 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -218,6 +218,17 @@ describe("create_pull_request - bundle transport shallow checkout", () => { if (cmd === "git" && args[0] === "rev-parse" && args[1] === "--is-shallow-repository") { return Promise.resolve({ exitCode: 0, stdout: "true\n", stderr: "" }); } + if (cmd === "git" && args[0] === "bundle" && args[1] === "verify") { + // Declare a fake prerequisite so ensureFullHistoryForBundle proceeds to deepen. + return Promise.resolve({ exitCode: 1, stdout: "", stderr: `The bundle requires this ref:\n${"a".repeat(40)}\n` }); + } + if (cmd === "git" && args[0] === "merge-base" && args[1] === "--is-ancestor") { + // Report prereq missing initially → iterative deepen kicks in; after the + // first deepen fetch we still report missing so the fallback --unshallow + // path is exercised. The default mock for exec() resolves successfully, + // so all 7 deepen steps complete instantly before the fallback fires. + return Promise.resolve({ exitCode: 1, stdout: "", stderr: "" }); + } if (cmd === "git" && args[0] === "rev-list") { return Promise.resolve({ exitCode: 0, stdout: "1\n", stderr: "" }); } @@ -256,7 +267,7 @@ describe("create_pull_request - bundle transport shallow checkout", () => { vi.clearAllMocks(); }); - it("should fetch bundle without forcing an unshallow fetch", async () => { + it("should deepen origin/ before fetching bundle in shallow repositories", async () => { const patchPath = canonicalPatchPath("feature/test"); fs.writeFileSync( patchPath, @@ -295,8 +306,9 @@ index 0000000..abc1234 expect(global.exec.exec).toHaveBeenCalledWith("git", ["update-ref", "refs/heads/feature/test", bundleTempRef]); expect(global.exec.exec).toHaveBeenCalledWith("git", ["reset", "--hard"]); const bundleFetchCallIndex = global.exec.getExecOutput.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); - const unshallowCallIndex = global.exec.exec.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === "--unshallow"); - expect(unshallowCallIndex).toBe(-1); + // Iterative deepen replaces a single --unshallow: assert the first --deepen step ran. + const deepenCallIndex = global.exec.exec.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && typeof args[1] === "string" && args[1].startsWith("--deepen=")); + expect(deepenCallIndex).toBeGreaterThanOrEqual(0); expect(bundleFetchCallIndex).toBeGreaterThanOrEqual(0); }); diff --git a/actions/setup/js/git_helpers.cjs b/actions/setup/js/git_helpers.cjs index 0aaba63f22f..f81f2a9221f 100644 --- a/actions/setup/js/git_helpers.cjs +++ b/actions/setup/js/git_helpers.cjs @@ -233,21 +233,108 @@ function hasMergeCommitsInRange(baseRef, headRef, options = {}) { } /** - * Probe shallow-repository status before fetching a git bundle. + * Deepen sequence (per call to `git fetch --deepen=N`). Each value adds N + * commits to the existing shallow history. Total reachable depth after the + * final step is the sum of these values (~7850 commits). + */ +const BUNDLE_DEEPEN_STEPS = [50, 100, 200, 500, 1000, 2000, 4000]; + +/** + * Extract prerequisite commit SHAs declared in a git bundle file. + * + * Runs `git bundle verify ` (with `ignoreReturnCode`) and parses the + * "The bundle requires this ref:" section as well as the + * "Repository lacks these prerequisite commits:" error block. Both formats + * list the prerequisite commit SHAs. + * + * @param {{ getExecOutput: Function }} execApi + * @param {string} bundleFilePath + * @param {Object} [options] + * @returns {Promise} Deduplicated lowercase 40-char SHAs, or [] on failure. + */ +async function getBundlePrerequisites(execApi, bundleFilePath, options = {}) { + try { + const { stdout, stderr } = await execApi.getExecOutput("git", ["bundle", "verify", bundleFilePath], { ...options, ignoreReturnCode: true, silent: true }); + const combined = `${stdout || ""}\n${stderr || ""}`; + const prereqs = new Set(); + const lines = combined.split(/\r?\n/); + let inRequires = false; + for (const line of lines) { + if (/the bundle (requires|records) (this|these)/i.test(line)) { + inRequires = true; + continue; + } + if (/the bundle contains/i.test(line)) { + inRequires = false; + continue; + } + if (inRequires) { + const match = line.match(/\b([0-9a-f]{40})\b/i); + if (match) { + prereqs.add(match[1].toLowerCase()); + continue; + } + if (line.trim() === "") { + inRequires = false; + } + } + } + // Also pick up "Repository lacks these prerequisite commits:" block. + for (const sha of extractBundlePrerequisiteCommits(combined)) { + prereqs.add(sha); + } + return [...prereqs]; + } catch (error) { + core.debug(`getBundlePrerequisites failed: ${getErrorMessage(error)}`); + return []; + } +} + +/** + * Check which of the given SHAs are NOT yet ancestors of `targetRef`. + * + * @param {{ getExecOutput: Function }} execApi + * @param {string[]} shas + * @param {string} targetRef + * @param {Object} [options] + * @returns {Promise} SHAs still missing (not ancestors / not present). + */ +async function findMissingAncestors(execApi, shas, targetRef, options = {}) { + const missing = []; + for (const sha of shas) { + const { exitCode } = await execApi.getExecOutput("git", ["merge-base", "--is-ancestor", sha, targetRef], { ...options, ignoreReturnCode: true, silent: true }); + if (exitCode !== 0) { + missing.push(sha); + } + } + return missing; +} + +/** + * Probe shallow-repository status before fetching a git bundle, and deepen + * the local clone as needed so the bundle's prerequisite commits become + * reachable from `origin/`. * * Bundles generated from a commit range can declare prerequisite commits. A - * depth-1 checkout may not contain those prerequisites, and `git fetch ` - * can reject the bundle before the caller can update refs. + * shallow checkout (e.g. `fetch-depth: 20`) may not contain those prerequisites, + * and `git fetch ` will reject the bundle before the caller can update + * refs. On a high-churn monorepo, `git fetch --unshallow` is catastrophic — it + * downloads the entire history. Instead we iterate `git fetch origin + * --deepen=` with progressively larger N until every declared prerequisite + * satisfies `git merge-base --is-ancestor origin/`. * - * IMPORTANT: Do not unshallow here. Full-history fetches are prohibitively - * expensive for large monorepos. Callers recover from prerequisite failures by - * fetching only the missing commit objects from origin and retrying. + * When `deepenOptions.baseRef` or `deepenOptions.bundleFilePath` is missing + * (legacy callers), the function falls back to the previous behavior of a + * single `git fetch --unshallow origin`. * * @param {{ getExecOutput: Function, exec: Function }} execApi - Exec API to run git commands. * @param {Object} [options] - Options passed through to exec calls. + * @param {Object} [deepenOptions] + * @param {string} [deepenOptions.baseRef] - Remote branch name to deepen (no `origin/` prefix). + * @param {string} [deepenOptions.bundleFilePath] - Path to the bundle file whose prerequisites must become reachable. * @returns {Promise} */ -async function ensureFullHistoryForBundle(execApi, options = {}) { +async function ensureFullHistoryForBundle(execApi, options = {}, deepenOptions = {}) { let stdout; try { ({ stdout } = await execApi.getExecOutput("git", ["rev-parse", "--is-shallow-repository"], options)); @@ -256,8 +343,56 @@ async function ensureFullHistoryForBundle(execApi, options = {}) { core.warning(`Could not determine shallow repository status; skipping full-history fetch probe: ${message}`); return; } - if (stdout.trim() === "true") { - core.info("Repository is shallow; skipping full-history fetch and relying on prerequisite recovery"); + if (stdout.trim() !== "true") { + return; + } + + const { baseRef, bundleFilePath } = deepenOptions || {}; + + // Legacy path: no base ref / bundle info known — fall back to a single + // unshallow. Callers in monorepos should always supply baseRef + bundleFilePath + // to get incremental deepening instead. + if (!baseRef || !bundleFilePath) { + core.info("Repository is shallow; fetching full history before bundle processing (no baseRef/bundle info; using --unshallow)"); + await execApi.exec("git", ["fetch", "--unshallow", "origin"], options); + return; + } + + const prereqs = await getBundlePrerequisites(execApi, bundleFilePath, options); + if (prereqs.length === 0) { + core.info("Bundle declares no prerequisites; no deepen required"); + return; + } + + const targetRef = `origin/${baseRef}`; + const alreadyMissing = await findMissingAncestors(execApi, prereqs, targetRef, options); + if (alreadyMissing.length === 0) { + core.info(`Bundle prerequisites already reachable from ${targetRef}; no deepen required`); + return; + } + + core.info(`Repository is shallow; iteratively deepening ${targetRef} to satisfy ${alreadyMissing.length} bundle prerequisite commit(s)`); + let missing = alreadyMissing; + for (const depth of BUNDLE_DEEPEN_STEPS) { + core.info(`Fetching origin ${baseRef} with --deepen=${depth} (${missing.length} prerequisite(s) still missing)`); + try { + await execApi.exec("git", ["fetch", `--deepen=${depth}`, "origin", baseRef], options); + } catch (fetchError) { + core.warning(`git fetch --deepen=${depth} origin ${baseRef} failed: ${getErrorMessage(fetchError)}; aborting iterative deepen`); + break; + } + missing = await findMissingAncestors(execApi, prereqs, targetRef, options); + if (missing.length === 0) { + core.info(`Bundle prerequisites reachable after --deepen=${depth}`); + return; + } + } + + core.warning(`Bundle prerequisites still not reachable after iterative deepen (${missing.length} remaining); attempting --unshallow as a last resort`); + try { + await execApi.exec("git", ["fetch", "--unshallow", "origin", baseRef], options); + } catch (unshallowError) { + core.warning(`Fallback --unshallow fetch failed: ${getErrorMessage(unshallowError)}; bundle apply may still fail`); } } diff --git a/actions/setup/js/git_helpers.test.cjs b/actions/setup/js/git_helpers.test.cjs index 9ac6ca4a618..b338c2ad48b 100644 --- a/actions/setup/js/git_helpers.test.cjs +++ b/actions/setup/js/git_helpers.test.cjs @@ -283,7 +283,7 @@ describe("git_helpers.cjs", () => { }); describe("ensureFullHistoryForBundle", () => { - it("should not fetch full history when the repository is shallow", async () => { + it("should unshallow the repository when the repository is shallow", async () => { const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs"); const execApi = { getExecOutput: vi.fn().mockResolvedValue({ stdout: "true\n" }), @@ -294,7 +294,7 @@ describe("git_helpers.cjs", () => { await ensureFullHistoryForBundle(execApi, options); expect(execApi.getExecOutput).toHaveBeenCalledWith("git", ["rev-parse", "--is-shallow-repository"], options); - expect(execApi.exec).not.toHaveBeenCalled(); + expect(execApi.exec).toHaveBeenCalledWith("git", ["fetch", "--unshallow", "origin"], options); }); it("should not fetch full history when the repository is not shallow", async () => { @@ -338,6 +338,86 @@ describe("git_helpers.cjs", () => { expect(warning).toHaveBeenCalledTimes(1); expect(warning).toHaveBeenCalledWith("Could not determine shallow repository status; skipping full-history fetch probe: unknown failure"); }); + + it("should iteratively deepen origin/ when bundle prereqs are known and shallow", async () => { + const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs"); + const prereq = "a".repeat(40); + let deepenCalls = 0; + const execApi = { + getExecOutput: vi.fn().mockImplementation((cmd, args) => { + if (args[0] === "rev-parse" && args[1] === "--is-shallow-repository") { + return Promise.resolve({ stdout: "true\n" }); + } + if (args[0] === "bundle" && args[1] === "verify") { + return Promise.resolve({ + stdout: "", + stderr: `The bundle requires this ref:\n${prereq}\n`, + exitCode: 1, + }); + } + if (args[0] === "merge-base" && args[1] === "--is-ancestor") { + // Become reachable only after the second deepen fetch. + return Promise.resolve({ exitCode: deepenCalls >= 2 ? 0 : 1, stdout: "", stderr: "" }); + } + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + }), + exec: vi.fn().mockImplementation((cmd, args) => { + if (args && args[0] === "fetch" && args[1] && args[1].startsWith("--deepen=")) { + deepenCalls++; + } + return Promise.resolve(0); + }), + }; + + await ensureFullHistoryForBundle(execApi, {}, { baseRef: "main", bundleFilePath: "/tmp/test.bundle" }); + + // Two deepen fetches before ancestry succeeds; no --unshallow. + const fetchCalls = execApi.exec.mock.calls.filter(c => c[1] && c[1][0] === "fetch"); + expect(fetchCalls.length).toBe(2); + expect(fetchCalls[0][1]).toEqual(["fetch", "--deepen=50", "origin", "main"]); + expect(fetchCalls[1][1]).toEqual(["fetch", "--deepen=100", "origin", "main"]); + expect(execApi.exec).not.toHaveBeenCalledWith("git", ["fetch", "--unshallow", "origin"], expect.anything()); + }); + + it("should skip deepening when bundle declares no prerequisites", async () => { + const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs"); + const execApi = { + getExecOutput: vi.fn().mockImplementation((cmd, args) => { + if (args[0] === "rev-parse") return Promise.resolve({ stdout: "true\n" }); + if (args[0] === "bundle" && args[1] === "verify") { + return Promise.resolve({ stdout: "The bundle contains this ref:\ndeadbeef refs/heads/x\n", stderr: "", exitCode: 0 }); + } + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + }), + exec: vi.fn().mockResolvedValue(0), + }; + + await ensureFullHistoryForBundle(execApi, {}, { baseRef: "main", bundleFilePath: "/tmp/test.bundle" }); + + expect(execApi.exec).not.toHaveBeenCalled(); + }); + + it("should skip deepening when prereqs are already reachable from origin/", async () => { + const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs"); + const prereq = "b".repeat(40); + const execApi = { + getExecOutput: vi.fn().mockImplementation((cmd, args) => { + if (args[0] === "rev-parse") return Promise.resolve({ stdout: "true\n" }); + if (args[0] === "bundle" && args[1] === "verify") { + return Promise.resolve({ stdout: `The bundle requires this ref:\n${prereq}\n`, stderr: "", exitCode: 0 }); + } + if (args[0] === "merge-base" && args[1] === "--is-ancestor") { + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + } + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + }), + exec: vi.fn().mockResolvedValue(0), + }; + + await ensureFullHistoryForBundle(execApi, {}, { baseRef: "main", bundleFilePath: "/tmp/test.bundle" }); + + expect(execApi.exec).not.toHaveBeenCalled(); + }); }); describe("isShallowOrSparseCheckout", () => { diff --git a/actions/setup/js/push_signed_commits.cjs b/actions/setup/js/push_signed_commits.cjs index dbf3e0f8e8a..845c3ce5665 100644 --- a/actions/setup/js/push_signed_commits.cjs +++ b/actions/setup/js/push_signed_commits.cjs @@ -9,6 +9,7 @@ const { ERR_API } = require("./error_codes.cjs"); const { loadTemporaryIdMapFromResolved, replaceTemporaryIdReferencesInPatch, TEMPORARY_ID_CANDIDATE_REFERENCE_PATTERN } = require("./temporary_id.cjs"); +const { checkFileProtectionPostApply } = require("./manifest_file_helpers.cjs"); const OID_PATTERN = /^[0-9a-f]{40}$/i; /** Sentinel error class used to signal that the commit range contains a shape @@ -27,6 +28,15 @@ class PushSignedCommitsUnsupportedShape extends Error { } } +/** Sentinel error class for synthesized-payload policy violations. */ +class PushSignedCommitsPolicyViolation extends Error { + /** @param {string} message */ + constructor(message) { + super(message); + this.name = "PushSignedCommitsPolicyViolation"; + } +} + /** * Unescape a C-quoted path returned by `git diff-tree --raw`. * @@ -183,6 +193,43 @@ async function pushBranchAndResolveHead({ branch, cwd, gitAuthEnv }) { return resolveLocalHeadSha(cwd); } +/** + * Enforce limits and file-protection policy on the synthesized GraphQL payload. + * + * @param {Array<{path: string, contents: string}>} additions + * @param {Array<{path: string}>} deletions + * @param {Record | undefined} validationConfig + */ +function validateSynthesizedFileChanges(additions, deletions, validationConfig) { + if (!validationConfig) { + return; + } + + const uniquePaths = Array.from(new Set([...deletions.map(entry => entry.path), ...additions.map(entry => entry.path)].map(path => String(path || "").trim()).filter(Boolean))); + + const maxFilesRaw = Number.parseInt(String(validationConfig.max_patch_files ?? ""), 10); + if (Number.isFinite(maxFilesRaw) && maxFilesRaw > 0 && uniquePaths.length > maxFilesRaw) { + throw new PushSignedCommitsPolicyViolation(`E003: Signed-commit payload exceeds max-patch-files (${maxFilesRaw}). ` + `Synthesized payload touches ${uniquePaths.length} file(s): ${uniquePaths.join(", ")}`); + } + + const maxSizeKbRaw = Number.parseInt(String(validationConfig.max_patch_size ?? ""), 10); + if (Number.isFinite(maxSizeKbRaw) && maxSizeKbRaw > 0) { + const additionsBytes = additions.reduce((total, entry) => total + Buffer.from(entry.contents, "base64").length, 0); + const additionsKb = Math.ceil(additionsBytes / 1024); + if (additionsKb > maxSizeKbRaw) { + throw new PushSignedCommitsPolicyViolation(`E003: Signed-commit payload exceeds max-patch-size (${maxSizeKbRaw} KB). ` + `Synthesized payload additions total ${additionsKb} KB`); + } + } + + const protection = checkFileProtectionPostApply(uniquePaths, { + ...validationConfig, + protected_files_policy: validationConfig.protected_files_policy ?? "request_review", + }); + if (protection.action !== "allow") { + throw new PushSignedCommitsPolicyViolation(`Signed-commit payload violates file-protection policy (${protection.action}): ${protection.files.join(", ")}`); + } +} + /** * Resolve the local HEAD SHA. * @@ -211,9 +258,10 @@ async function resolveLocalHeadSha(cwd) { * @param {boolean} [opts.allowGitPushFallback=true] - When false, refuse any fallback path that would use direct git push * @param {Record} [opts.resolvedTemporaryIds] - Resolved temporary IDs map * @param {string} [opts.currentRepo] - Repository slug used for same-repo temporary ID resolution + * @param {Record} [opts.validationConfig] - Optional safe-output policy config applied to synthesized GraphQL fileChanges * @returns {Promise} SHA of the commit that landed on the target branch */ -async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, cwd, gitAuthEnv, signedCommits = true, allowGitPushFallback = true, resolvedTemporaryIds, currentRepo }) { +async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, cwd, gitAuthEnv, signedCommits = true, allowGitPushFallback = true, resolvedTemporaryIds, currentRepo, validationConfig }) { const effectiveCurrentRepo = currentRepo || `${owner}/${repo}`; const temporaryIdMap = loadTemporaryIdMapFromResolved(resolvedTemporaryIds, { defaultRepo: effectiveCurrentRepo, @@ -292,18 +340,71 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c const fields = line.split(" "); return { line, fields, sha: fields[0] }; }); - const revListEntries = baseRefOid !== undefined ? revListEntriesRaw.filter(entry => entry.sha !== baseRefOid) : revListEntriesRaw; + let revListEntries = baseRefOid !== undefined ? revListEntriesRaw.filter(entry => entry.sha !== baseRefOid) : revListEntriesRaw; const droppedBoundaryCount = revListEntriesRaw.length - revListEntries.length; if (baseRefOid !== undefined && droppedBoundaryCount > 0) { core.info(`pushSignedCommits: dropped ${droppedBoundaryCount} baseRef boundary commit(s) from replay set`); } - const shas = revListEntries.map(entry => entry.sha); + let shas = revListEntries.map(entry => entry.sha); if (shas.length === 0) { core.info("pushSignedCommits: no new commits to push via GraphQL"); return undefined; } + let remoteHeadOid; + try { + const { stdout: oidOut } = await exec.getExecOutput("git", ["ls-remote", "origin", `refs/heads/${branch}`], { cwd, env: { ...process.env, ...(gitAuthEnv || {}) } }); + const resolved = oidOut.trim().split(/\s+/)[0]; + if (OID_PATTERN.test(resolved)) { + remoteHeadOid = resolved; + } + } catch { + // Non-fatal. The existing branch-creation logic handles missing remote refs. + } + const firstReplayParentOid = revListEntries[0] && revListEntries[0].fields.length > 1 ? revListEntries[0].fields[1] : undefined; + const firstGraphqlParentOid = remoteHeadOid || baseRefOid; + let graphqlParentIsAncestorOfHead = true; + if (firstGraphqlParentOid) { + try { + const ancestryCheck = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", firstGraphqlParentOid, "HEAD"], { cwd, ignoreReturnCode: true }); + graphqlParentIsAncestorOfHead = ancestryCheck.exitCode === 0; + } catch { + // If ancestry probing fails, keep the default (true) and avoid rewrite. + } + } + if (firstReplayParentOid && firstGraphqlParentOid && firstReplayParentOid !== firstGraphqlParentOid && !graphqlParentIsAncestorOfHead) { + core.warning(`pushSignedCommits: replay parent ${firstReplayParentOid} does not match GraphQL parent ${firstGraphqlParentOid}; ` + `rebasing commit range before signed replay to avoid stale-base file synthesis`); + try { + await exec.exec("git", ["rebase", "--onto", firstGraphqlParentOid, firstReplayParentOid, "HEAD"], { cwd }); + } catch (rebaseError) { + try { + await exec.exec("git", ["rebase", "--abort"], { cwd }); + } catch { + // Ignore cleanup failures. + } + throw new Error( + `pushSignedCommits: failed to rebase commit range onto current GraphQL parent (${firstGraphqlParentOid}). ` + + `Resolve conflicts by rebasing/cherry-picking locally and retry. Root cause: ${rebaseError instanceof Error ? rebaseError.message : String(rebaseError)}`, + { cause: rebaseError } + ); + } + const { stdout: rebasedRevListOut } = await exec.getExecOutput("git", ["rev-list", "--parents", "--topo-order", "--reverse", `${firstGraphqlParentOid}..HEAD`], { cwd }); + revListEntries = rebasedRevListOut + .trim() + .split("\n") + .filter(Boolean) + .map(line => { + const fields = line.split(" "); + return { line, fields, sha: fields[0] }; + }); + shas = revListEntries.map(entry => entry.sha); + if (shas.length === 0) { + core.info("pushSignedCommits: no new commits to replay after rebase"); + return undefined; + } + } + core.info(`pushSignedCommits: replaying ${shas.length} commit(s) via GraphQL createCommitOnBranch (branch: ${branch}, repo: ${owner}/${repo})`); try { @@ -513,6 +614,7 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c const additions = additionsMap.get(sha) || []; const deletions = deletionsMap.get(sha) || []; + validateSynthesizedFileChanges(additions, deletions, validationConfig); core.info(`pushSignedCommits: file changes: ${additions.length} addition(s), ${deletions.length} deletion(s)`); /** @type {any} */ @@ -550,6 +652,9 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c { cause: err } ); } + if (err instanceof PushSignedCommitsPolicyViolation) { + throw new Error(`pushSignedCommits: refusing unsigned push for branch '${branch}': ${err.message}`, { cause: err }); + } if (allowGitPushFallback === false) { throw new Error(`pushSignedCommits: signed commit push failed for branch '${branch}' and git push fallback is disabled: ${err instanceof Error ? err.message : String(err)}`, { cause: err }); } diff --git a/actions/setup/js/push_signed_commits.test.cjs b/actions/setup/js/push_signed_commits.test.cjs index fc92767aff9..070f9a9291e 100644 --- a/actions/setup/js/push_signed_commits.test.cjs +++ b/actions/setup/js/push_signed_commits.test.cjs @@ -847,14 +847,13 @@ describe("push_signed_commits integration tests", () => { cwd: workDir, }); - // createRef was attempted but threw 422 – should continue, not fall back - expect(githubClient.rest.git.createRef).toHaveBeenCalledTimes(1); + // Signed replay should proceed even if branch creation races; depending on + // ls-remote timing, createRef may be attempted once or skipped. + expect(githubClient.rest.git.createRef.mock.calls.length).toBeLessThanOrEqual(1); expect(githubClient.graphql).toHaveBeenCalledTimes(1); const callArg = githubClient.graphql.mock.calls[0][1].input; expect(callArg.expectedHeadOid).toBe(expectedParentOid); expect(callArg.message.headline).toBe("Race commit"); - // Should log the concurrent-creation info message - expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("created concurrently")); }); }); @@ -1179,7 +1178,7 @@ describe("push_signed_commits integration tests", () => { } const networkGitCalls = getExecOutput.mock.calls.filter(call => call[1][0] === "ls-remote"); - expect(networkGitCalls).toHaveLength(1); + expect(networkGitCalls.length).toBeGreaterThanOrEqual(1); for (const call of networkGitCalls) { expect(call[2]).toEqual( expect.objectContaining({ @@ -1921,4 +1920,99 @@ describe("push_signed_commits integration tests", () => { expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("submodule change detected in mysubmodule")); }); }); + + describe("stale-base and synthesized payload safety", () => { + it("should fail signed replay when rebasing stale commits onto current base conflicts", async () => { + // Base branch starts with shared file. + fs.writeFileSync(path.join(workDir, "shared.txt"), "base\n"); + execGit(["add", "shared.txt"], { cwd: workDir }); + execGit(["commit", "-m", "Add shared file"], { cwd: workDir }); + execGit(["push", "origin", "main"], { cwd: workDir }); + + // Agent branch diverges from old main and edits shared.txt. + execGit(["checkout", "-b", "stale-conflict-branch"], { cwd: workDir }); + fs.writeFileSync(path.join(workDir, "shared.txt"), "agent change\n"); + execGit(["add", "shared.txt"], { cwd: workDir }); + execGit(["commit", "-m", "Agent edit shared"], { cwd: workDir }); + + // Base branch advances with conflicting edit. + execGit(["checkout", "main"], { cwd: workDir }); + fs.writeFileSync(path.join(workDir, "shared.txt"), "upstream change\n"); + execGit(["add", "shared.txt"], { cwd: workDir }); + execGit(["commit", "-m", "Upstream edit shared"], { cwd: workDir }); + execGit(["push", "origin", "main"], { cwd: workDir }); + + execGit(["checkout", "stale-conflict-branch"], { cwd: workDir }); + + global.exec = makeRealExec(workDir); + const githubClient = makeMockGithubClient(); + + await expect( + pushSignedCommits({ + githubClient, + owner: "test-owner", + repo: "test-repo", + branch: "stale-conflict-branch", + baseRef: "origin/main", + cwd: workDir, + }) + ).rejects.toThrow("failed to rebase commit range onto current GraphQL parent"); + + expect(githubClient.graphql).not.toHaveBeenCalled(); + }); + + it("should enforce protected-files policy against synthesized GraphQL payload", async () => { + execGit(["checkout", "-b", "protected-payload-branch"], { cwd: workDir }); + fs.writeFileSync(path.join(workDir, "CODEOWNERS"), "* @octocat\n"); + execGit(["add", "CODEOWNERS"], { cwd: workDir }); + execGit(["commit", "-m", "Touch CODEOWNERS"], { cwd: workDir }); + + global.exec = makeRealExec(workDir); + const githubClient = makeMockGithubClient(); + + await expect( + pushSignedCommits({ + githubClient, + owner: "test-owner", + repo: "test-repo", + branch: "protected-payload-branch", + baseRef: "origin/main", + cwd: workDir, + validationConfig: { + protected_files: ["CODEOWNERS"], + protected_files_policy: "blocked", + }, + }) + ).rejects.toThrow("Signed-commit payload violates file-protection policy"); + + expect(githubClient.graphql).not.toHaveBeenCalled(); + }); + + it("should enforce max-patch-files against synthesized GraphQL payload", async () => { + execGit(["checkout", "-b", "max-files-payload-branch"], { cwd: workDir }); + fs.writeFileSync(path.join(workDir, "alpha.txt"), "alpha\n"); + fs.writeFileSync(path.join(workDir, "beta.txt"), "beta\n"); + execGit(["add", "alpha.txt", "beta.txt"], { cwd: workDir }); + execGit(["commit", "-m", "Touch two files"], { cwd: workDir }); + + global.exec = makeRealExec(workDir); + const githubClient = makeMockGithubClient(); + + await expect( + pushSignedCommits({ + githubClient, + owner: "test-owner", + repo: "test-repo", + branch: "max-files-payload-branch", + baseRef: "origin/main", + cwd: workDir, + validationConfig: { + max_patch_files: 1, + }, + }) + ).rejects.toThrow("exceeds max-patch-files"); + + expect(githubClient.graphql).not.toHaveBeenCalled(); + }); + }); }); diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 1ac61054a65..22e056ccb1f 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -741,10 +741,14 @@ async function main(config = {}) { core.info(`Applying changes from bundle: ${bundleFilePath}`); const bundleRef = `refs/bundles/push-${branchName.replace(/[^a-zA-Z0-9-]/g, "-")}`; try { - await ensureFullHistoryForBundle(exec, { - env: { ...process.env, ...gitAuthEnv }, - ...baseGitOpts, - }); + await ensureFullHistoryForBundle( + exec, + { + env: { ...process.env, ...gitAuthEnv }, + ...baseGitOpts, + }, + { baseRef: branchName, bundleFilePath } + ); // Fetch from bundle into a temporary ref. // Use getExecOutput with ignoreReturnCode so we can read the actual stderr from git — @@ -1076,6 +1080,7 @@ async function main(config = {}) { signedCommits, resolvedTemporaryIds, currentRepo: itemRepo, + validationConfig: config, }); if (pushedSha) { pushedCommitSha = pushedSha; 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 829a041114e..ab1733caae8 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -1477,6 +1477,12 @@ index 0000000..abc1234 if (cmd === "git" && args[0] === "rev-parse" && args[1] === "--is-shallow-repository") { return Promise.resolve({ exitCode: 0, stdout: "true\n", stderr: "" }); } + if (cmd === "git" && args[0] === "bundle" && args[1] === "verify") { + return Promise.resolve({ exitCode: 1, stdout: "", stderr: `The bundle requires this ref:\n${"a".repeat(40)}\n` }); + } + if (cmd === "git" && args[0] === "merge-base" && args[1] === "--is-ancestor") { + return Promise.resolve({ exitCode: 1, stdout: "", stderr: "" }); + } if (cmd === "git" && args[0] === "rev-list") { return Promise.resolve({ exitCode: 0, stdout: "2\n", stderr: "" }); } @@ -1495,8 +1501,9 @@ index 0000000..abc1234 expect(mockExec.exec).toHaveBeenCalledWith("git", ["update-ref", "refs/heads/feature-branch", "refs/bundles/push-feature-branch", "remote-head"], expect.any(Object)); expect(mockExec.exec).toHaveBeenCalledWith("git", ["reset", "--hard"], expect.any(Object)); expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["merge", "--ff-only", "refs/bundles/push-feature-branch"], expect.any(Object)); - const unshallowCallIndex = mockExec.exec.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === "--unshallow"); - expect(unshallowCallIndex).toBe(-1); + // Iterative deepen replaces a single --unshallow: assert the first --deepen step ran. + const deepenCallIndex = mockExec.exec.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && typeof args[1] === "string" && args[1].startsWith("--deepen=")); + expect(deepenCallIndex).toBeGreaterThanOrEqual(0); } finally { pushSignedSpy.mockRestore(); }