Skip to content

Commit 32ae300

Browse files
Copilotdsyme
andauthored
Prevent stale-base signed replay from synthesizing unrelated changes; enforce policy on GraphQL payload (#36937)
* Initial plan * Plan fix for signed-commit stale-base replay Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com> * Fix signed commit stale-base replay and payload safeguards Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com> * Replace --unshallow with iterative --deepen against base ref Single 'git fetch --unshallow origin' is disastrous on high-churn monorepos. Instead iterate 'git fetch origin <base> --deepen=N' with progressively larger N (50, 100, 200, 500, 1000, 2000, 4000) until every bundle-declared prerequisite satisfies 'git merge-base --is-ancestor <prereq> origin/<base>'. Falls back to --unshallow only if iterative deepen exhausts, or if a legacy caller omits the deepen options. * Apply formatter to push_signed_commits.cjs * Update shallow-checkout tests to assert --deepen instead of --unshallow The two tests that asserted 'git fetch --unshallow' was called on a shallow checkout were written against the old single-unshallow path. With iterative --deepen they need to: declare a bundle prerequisite (via mocked 'git bundle verify'), report it as not-an-ancestor of origin/<base>, and then assert the first --deepen=<N> step was issued. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com> Co-authored-by: Don Syme <dsyme@github.com> Co-authored-by: Don Syme <dsyme@users.noreply.github.com>
1 parent 978d201 commit 32ae300

8 files changed

Lines changed: 474 additions & 31 deletions

actions/setup/js/create_pull_request.cjs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,15 +183,16 @@ async function tryRecoverGitAmAddAddConflict(execApi) {
183183
* @param {string} branchName - Target branch name
184184
* @param {string} originalAgentBranch - Original source branch name from the agent, if different
185185
* @param {{ exec: Function, getExecOutput: Function }} execApi - GitHub Actions exec API
186+
* @param {string} [baseBranch] - Base branch name (used for iterative shallow-clone deepening)
186187
* @returns {Promise<void>}
187188
*/
188-
async function applyBundleToBranch(bundleFilePath, branchName, originalAgentBranch, execApi) {
189+
async function applyBundleToBranch(bundleFilePath, branchName, originalAgentBranch, execApi, baseBranch) {
189190
let bundleBranchRef = `refs/heads/${originalAgentBranch || branchName}`;
190191
const bundleTargetRef = `refs/heads/${branchName}`;
191192
const bundleTempRef = createBundleTempRef(branchName);
192193

193194
try {
194-
await ensureFullHistoryForBundle(execApi);
195+
await ensureFullHistoryForBundle(execApi, {}, { baseRef: baseBranch, bundleFilePath });
195196
core.info(`Applying bundle ${bundleFilePath} to ${bundleTargetRef} using temp ref ${bundleTempRef} from ${bundleBranchRef}`);
196197

197198
// Fetch from bundle into a temporary ref, then update the target branch.
@@ -1482,7 +1483,7 @@ async function main(config = {}) {
14821483
// unlike git format-patch which flattens history and drops merge resolution content.
14831484
core.info(`Applying changes from bundle: ${bundleFilePath}`);
14841485
try {
1485-
await applyBundleToBranch(bundleFilePath, branchName, originalAgentBranch, exec);
1486+
await applyBundleToBranch(bundleFilePath, branchName, originalAgentBranch, exec, baseBranch);
14861487
} catch (bundleError) {
14871488
core.error(`Failed to apply bundle: ${bundleError instanceof Error ? bundleError.message : String(bundleError)}`);
14881489
return { success: false, error: "Failed to apply bundle" };
@@ -1506,6 +1507,7 @@ async function main(config = {}) {
15061507
signedCommits,
15071508
resolvedTemporaryIds,
15081509
currentRepo: itemRepo,
1510+
validationConfig: config,
15091511
});
15101512
core.info("Changes pushed to branch (from bundle)");
15111513

@@ -1538,6 +1540,7 @@ async function main(config = {}) {
15381540
signedCommits,
15391541
resolvedTemporaryIds,
15401542
currentRepo: itemRepo,
1543+
validationConfig: config,
15411544
});
15421545
core.info("Changes pushed to branch after bundle rewrite retry");
15431546

@@ -1869,6 +1872,7 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo
18691872
signedCommits,
18701873
resolvedTemporaryIds,
18711874
currentRepo: itemRepo,
1875+
validationConfig: config,
18721876
});
18731877
core.info("Changes pushed to branch");
18741878

@@ -2017,6 +2021,7 @@ ${patchPreview}`;
20172021
signedCommits,
20182022
resolvedTemporaryIds,
20192023
currentRepo: itemRepo,
2024+
validationConfig: config,
20202025
});
20212026
core.info("Empty branch pushed successfully");
20222027

actions/setup/js/create_pull_request.test.cjs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,17 @@ describe("create_pull_request - bundle transport shallow checkout", () => {
218218
if (cmd === "git" && args[0] === "rev-parse" && args[1] === "--is-shallow-repository") {
219219
return Promise.resolve({ exitCode: 0, stdout: "true\n", stderr: "" });
220220
}
221+
if (cmd === "git" && args[0] === "bundle" && args[1] === "verify") {
222+
// Declare a fake prerequisite so ensureFullHistoryForBundle proceeds to deepen.
223+
return Promise.resolve({ exitCode: 1, stdout: "", stderr: `The bundle requires this ref:\n${"a".repeat(40)}\n` });
224+
}
225+
if (cmd === "git" && args[0] === "merge-base" && args[1] === "--is-ancestor") {
226+
// Report prereq missing initially → iterative deepen kicks in; after the
227+
// first deepen fetch we still report missing so the fallback --unshallow
228+
// path is exercised. The default mock for exec() resolves successfully,
229+
// so all 7 deepen steps complete instantly before the fallback fires.
230+
return Promise.resolve({ exitCode: 1, stdout: "", stderr: "" });
231+
}
221232
if (cmd === "git" && args[0] === "rev-list") {
222233
return Promise.resolve({ exitCode: 0, stdout: "1\n", stderr: "" });
223234
}
@@ -256,7 +267,7 @@ describe("create_pull_request - bundle transport shallow checkout", () => {
256267
vi.clearAllMocks();
257268
});
258269

259-
it("should fetch bundle without forcing an unshallow fetch", async () => {
270+
it("should deepen origin/<base> before fetching bundle in shallow repositories", async () => {
260271
const patchPath = canonicalPatchPath("feature/test");
261272
fs.writeFileSync(
262273
patchPath,
@@ -295,8 +306,9 @@ index 0000000..abc1234
295306
expect(global.exec.exec).toHaveBeenCalledWith("git", ["update-ref", "refs/heads/feature/test", bundleTempRef]);
296307
expect(global.exec.exec).toHaveBeenCalledWith("git", ["reset", "--hard"]);
297308
const bundleFetchCallIndex = global.exec.getExecOutput.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath);
298-
const unshallowCallIndex = global.exec.exec.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === "--unshallow");
299-
expect(unshallowCallIndex).toBe(-1);
309+
// Iterative deepen replaces a single --unshallow: assert the first --deepen step ran.
310+
const deepenCallIndex = global.exec.exec.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && typeof args[1] === "string" && args[1].startsWith("--deepen="));
311+
expect(deepenCallIndex).toBeGreaterThanOrEqual(0);
300312
expect(bundleFetchCallIndex).toBeGreaterThanOrEqual(0);
301313
});
302314

actions/setup/js/git_helpers.cjs

Lines changed: 144 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -233,21 +233,108 @@ function hasMergeCommitsInRange(baseRef, headRef, options = {}) {
233233
}
234234

235235
/**
236-
* Probe shallow-repository status before fetching a git bundle.
236+
* Deepen sequence (per call to `git fetch --deepen=N`). Each value adds N
237+
* commits to the existing shallow history. Total reachable depth after the
238+
* final step is the sum of these values (~7850 commits).
239+
*/
240+
const BUNDLE_DEEPEN_STEPS = [50, 100, 200, 500, 1000, 2000, 4000];
241+
242+
/**
243+
* Extract prerequisite commit SHAs declared in a git bundle file.
244+
*
245+
* Runs `git bundle verify <file>` (with `ignoreReturnCode`) and parses the
246+
* "The bundle requires this ref:" section as well as the
247+
* "Repository lacks these prerequisite commits:" error block. Both formats
248+
* list the prerequisite commit SHAs.
249+
*
250+
* @param {{ getExecOutput: Function }} execApi
251+
* @param {string} bundleFilePath
252+
* @param {Object} [options]
253+
* @returns {Promise<string[]>} Deduplicated lowercase 40-char SHAs, or [] on failure.
254+
*/
255+
async function getBundlePrerequisites(execApi, bundleFilePath, options = {}) {
256+
try {
257+
const { stdout, stderr } = await execApi.getExecOutput("git", ["bundle", "verify", bundleFilePath], { ...options, ignoreReturnCode: true, silent: true });
258+
const combined = `${stdout || ""}\n${stderr || ""}`;
259+
const prereqs = new Set();
260+
const lines = combined.split(/\r?\n/);
261+
let inRequires = false;
262+
for (const line of lines) {
263+
if (/the bundle (requires|records) (this|these)/i.test(line)) {
264+
inRequires = true;
265+
continue;
266+
}
267+
if (/the bundle contains/i.test(line)) {
268+
inRequires = false;
269+
continue;
270+
}
271+
if (inRequires) {
272+
const match = line.match(/\b([0-9a-f]{40})\b/i);
273+
if (match) {
274+
prereqs.add(match[1].toLowerCase());
275+
continue;
276+
}
277+
if (line.trim() === "") {
278+
inRequires = false;
279+
}
280+
}
281+
}
282+
// Also pick up "Repository lacks these prerequisite commits:" block.
283+
for (const sha of extractBundlePrerequisiteCommits(combined)) {
284+
prereqs.add(sha);
285+
}
286+
return [...prereqs];
287+
} catch (error) {
288+
core.debug(`getBundlePrerequisites failed: ${getErrorMessage(error)}`);
289+
return [];
290+
}
291+
}
292+
293+
/**
294+
* Check which of the given SHAs are NOT yet ancestors of `targetRef`.
295+
*
296+
* @param {{ getExecOutput: Function }} execApi
297+
* @param {string[]} shas
298+
* @param {string} targetRef
299+
* @param {Object} [options]
300+
* @returns {Promise<string[]>} SHAs still missing (not ancestors / not present).
301+
*/
302+
async function findMissingAncestors(execApi, shas, targetRef, options = {}) {
303+
const missing = [];
304+
for (const sha of shas) {
305+
const { exitCode } = await execApi.getExecOutput("git", ["merge-base", "--is-ancestor", sha, targetRef], { ...options, ignoreReturnCode: true, silent: true });
306+
if (exitCode !== 0) {
307+
missing.push(sha);
308+
}
309+
}
310+
return missing;
311+
}
312+
313+
/**
314+
* Probe shallow-repository status before fetching a git bundle, and deepen
315+
* the local clone as needed so the bundle's prerequisite commits become
316+
* reachable from `origin/<baseRef>`.
237317
*
238318
* Bundles generated from a commit range can declare prerequisite commits. A
239-
* depth-1 checkout may not contain those prerequisites, and `git fetch <bundle>`
240-
* can reject the bundle before the caller can update refs.
319+
* shallow checkout (e.g. `fetch-depth: 20`) may not contain those prerequisites,
320+
* and `git fetch <bundle>` will reject the bundle before the caller can update
321+
* refs. On a high-churn monorepo, `git fetch --unshallow` is catastrophic — it
322+
* downloads the entire history. Instead we iterate `git fetch origin <baseRef>
323+
* --deepen=<N>` with progressively larger N until every declared prerequisite
324+
* satisfies `git merge-base --is-ancestor <prereq> origin/<baseRef>`.
241325
*
242-
* IMPORTANT: Do not unshallow here. Full-history fetches are prohibitively
243-
* expensive for large monorepos. Callers recover from prerequisite failures by
244-
* fetching only the missing commit objects from origin and retrying.
326+
* When `deepenOptions.baseRef` or `deepenOptions.bundleFilePath` is missing
327+
* (legacy callers), the function falls back to the previous behavior of a
328+
* single `git fetch --unshallow origin`.
245329
*
246330
* @param {{ getExecOutput: Function, exec: Function }} execApi - Exec API to run git commands.
247331
* @param {Object} [options] - Options passed through to exec calls.
332+
* @param {Object} [deepenOptions]
333+
* @param {string} [deepenOptions.baseRef] - Remote branch name to deepen (no `origin/` prefix).
334+
* @param {string} [deepenOptions.bundleFilePath] - Path to the bundle file whose prerequisites must become reachable.
248335
* @returns {Promise<void>}
249336
*/
250-
async function ensureFullHistoryForBundle(execApi, options = {}) {
337+
async function ensureFullHistoryForBundle(execApi, options = {}, deepenOptions = {}) {
251338
let stdout;
252339
try {
253340
({ stdout } = await execApi.getExecOutput("git", ["rev-parse", "--is-shallow-repository"], options));
@@ -256,8 +343,56 @@ async function ensureFullHistoryForBundle(execApi, options = {}) {
256343
core.warning(`Could not determine shallow repository status; skipping full-history fetch probe: ${message}`);
257344
return;
258345
}
259-
if (stdout.trim() === "true") {
260-
core.info("Repository is shallow; skipping full-history fetch and relying on prerequisite recovery");
346+
if (stdout.trim() !== "true") {
347+
return;
348+
}
349+
350+
const { baseRef, bundleFilePath } = deepenOptions || {};
351+
352+
// Legacy path: no base ref / bundle info known — fall back to a single
353+
// unshallow. Callers in monorepos should always supply baseRef + bundleFilePath
354+
// to get incremental deepening instead.
355+
if (!baseRef || !bundleFilePath) {
356+
core.info("Repository is shallow; fetching full history before bundle processing (no baseRef/bundle info; using --unshallow)");
357+
await execApi.exec("git", ["fetch", "--unshallow", "origin"], options);
358+
return;
359+
}
360+
361+
const prereqs = await getBundlePrerequisites(execApi, bundleFilePath, options);
362+
if (prereqs.length === 0) {
363+
core.info("Bundle declares no prerequisites; no deepen required");
364+
return;
365+
}
366+
367+
const targetRef = `origin/${baseRef}`;
368+
const alreadyMissing = await findMissingAncestors(execApi, prereqs, targetRef, options);
369+
if (alreadyMissing.length === 0) {
370+
core.info(`Bundle prerequisites already reachable from ${targetRef}; no deepen required`);
371+
return;
372+
}
373+
374+
core.info(`Repository is shallow; iteratively deepening ${targetRef} to satisfy ${alreadyMissing.length} bundle prerequisite commit(s)`);
375+
let missing = alreadyMissing;
376+
for (const depth of BUNDLE_DEEPEN_STEPS) {
377+
core.info(`Fetching origin ${baseRef} with --deepen=${depth} (${missing.length} prerequisite(s) still missing)`);
378+
try {
379+
await execApi.exec("git", ["fetch", `--deepen=${depth}`, "origin", baseRef], options);
380+
} catch (fetchError) {
381+
core.warning(`git fetch --deepen=${depth} origin ${baseRef} failed: ${getErrorMessage(fetchError)}; aborting iterative deepen`);
382+
break;
383+
}
384+
missing = await findMissingAncestors(execApi, prereqs, targetRef, options);
385+
if (missing.length === 0) {
386+
core.info(`Bundle prerequisites reachable after --deepen=${depth}`);
387+
return;
388+
}
389+
}
390+
391+
core.warning(`Bundle prerequisites still not reachable after iterative deepen (${missing.length} remaining); attempting --unshallow as a last resort`);
392+
try {
393+
await execApi.exec("git", ["fetch", "--unshallow", "origin", baseRef], options);
394+
} catch (unshallowError) {
395+
core.warning(`Fallback --unshallow fetch failed: ${getErrorMessage(unshallowError)}; bundle apply may still fail`);
261396
}
262397
}
263398

actions/setup/js/git_helpers.test.cjs

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ describe("git_helpers.cjs", () => {
283283
});
284284

285285
describe("ensureFullHistoryForBundle", () => {
286-
it("should not fetch full history when the repository is shallow", async () => {
286+
it("should unshallow the repository when the repository is shallow", async () => {
287287
const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs");
288288
const execApi = {
289289
getExecOutput: vi.fn().mockResolvedValue({ stdout: "true\n" }),
@@ -294,7 +294,7 @@ describe("git_helpers.cjs", () => {
294294
await ensureFullHistoryForBundle(execApi, options);
295295

296296
expect(execApi.getExecOutput).toHaveBeenCalledWith("git", ["rev-parse", "--is-shallow-repository"], options);
297-
expect(execApi.exec).not.toHaveBeenCalled();
297+
expect(execApi.exec).toHaveBeenCalledWith("git", ["fetch", "--unshallow", "origin"], options);
298298
});
299299

300300
it("should not fetch full history when the repository is not shallow", async () => {
@@ -338,6 +338,86 @@ describe("git_helpers.cjs", () => {
338338
expect(warning).toHaveBeenCalledTimes(1);
339339
expect(warning).toHaveBeenCalledWith("Could not determine shallow repository status; skipping full-history fetch probe: unknown failure");
340340
});
341+
342+
it("should iteratively deepen origin/<base> when bundle prereqs are known and shallow", async () => {
343+
const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs");
344+
const prereq = "a".repeat(40);
345+
let deepenCalls = 0;
346+
const execApi = {
347+
getExecOutput: vi.fn().mockImplementation((cmd, args) => {
348+
if (args[0] === "rev-parse" && args[1] === "--is-shallow-repository") {
349+
return Promise.resolve({ stdout: "true\n" });
350+
}
351+
if (args[0] === "bundle" && args[1] === "verify") {
352+
return Promise.resolve({
353+
stdout: "",
354+
stderr: `The bundle requires this ref:\n${prereq}\n`,
355+
exitCode: 1,
356+
});
357+
}
358+
if (args[0] === "merge-base" && args[1] === "--is-ancestor") {
359+
// Become reachable only after the second deepen fetch.
360+
return Promise.resolve({ exitCode: deepenCalls >= 2 ? 0 : 1, stdout: "", stderr: "" });
361+
}
362+
return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
363+
}),
364+
exec: vi.fn().mockImplementation((cmd, args) => {
365+
if (args && args[0] === "fetch" && args[1] && args[1].startsWith("--deepen=")) {
366+
deepenCalls++;
367+
}
368+
return Promise.resolve(0);
369+
}),
370+
};
371+
372+
await ensureFullHistoryForBundle(execApi, {}, { baseRef: "main", bundleFilePath: "/tmp/test.bundle" });
373+
374+
// Two deepen fetches before ancestry succeeds; no --unshallow.
375+
const fetchCalls = execApi.exec.mock.calls.filter(c => c[1] && c[1][0] === "fetch");
376+
expect(fetchCalls.length).toBe(2);
377+
expect(fetchCalls[0][1]).toEqual(["fetch", "--deepen=50", "origin", "main"]);
378+
expect(fetchCalls[1][1]).toEqual(["fetch", "--deepen=100", "origin", "main"]);
379+
expect(execApi.exec).not.toHaveBeenCalledWith("git", ["fetch", "--unshallow", "origin"], expect.anything());
380+
});
381+
382+
it("should skip deepening when bundle declares no prerequisites", async () => {
383+
const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs");
384+
const execApi = {
385+
getExecOutput: vi.fn().mockImplementation((cmd, args) => {
386+
if (args[0] === "rev-parse") return Promise.resolve({ stdout: "true\n" });
387+
if (args[0] === "bundle" && args[1] === "verify") {
388+
return Promise.resolve({ stdout: "The bundle contains this ref:\ndeadbeef refs/heads/x\n", stderr: "", exitCode: 0 });
389+
}
390+
return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
391+
}),
392+
exec: vi.fn().mockResolvedValue(0),
393+
};
394+
395+
await ensureFullHistoryForBundle(execApi, {}, { baseRef: "main", bundleFilePath: "/tmp/test.bundle" });
396+
397+
expect(execApi.exec).not.toHaveBeenCalled();
398+
});
399+
400+
it("should skip deepening when prereqs are already reachable from origin/<base>", async () => {
401+
const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs");
402+
const prereq = "b".repeat(40);
403+
const execApi = {
404+
getExecOutput: vi.fn().mockImplementation((cmd, args) => {
405+
if (args[0] === "rev-parse") return Promise.resolve({ stdout: "true\n" });
406+
if (args[0] === "bundle" && args[1] === "verify") {
407+
return Promise.resolve({ stdout: `The bundle requires this ref:\n${prereq}\n`, stderr: "", exitCode: 0 });
408+
}
409+
if (args[0] === "merge-base" && args[1] === "--is-ancestor") {
410+
return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
411+
}
412+
return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
413+
}),
414+
exec: vi.fn().mockResolvedValue(0),
415+
};
416+
417+
await ensureFullHistoryForBundle(execApi, {}, { baseRef: "main", bundleFilePath: "/tmp/test.bundle" });
418+
419+
expect(execApi.exec).not.toHaveBeenCalled();
420+
});
341421
});
342422

343423
describe("isShallowOrSparseCheckout", () => {

0 commit comments

Comments
 (0)