diff --git a/actions/setup/js/checkout_pr_branch.cjs b/actions/setup/js/checkout_pr_branch.cjs index e6d517c7a4c..16c6cfe7d10 100644 --- a/actions/setup/js/checkout_pr_branch.cjs +++ b/actions/setup/js/checkout_pr_branch.cjs @@ -7,8 +7,8 @@ * This script handles checkout for different GitHub event types: * * 1. pull_request: Runs in merge commit context (PR head + base merged) - * - Can use direct git commands since we're already in PR context - * - Branch exists in current checkout + * - Uses refs/pull/N/head even for same-repository PRs + * - The retained PR ref survives head branch deletion after merge * * 2. pull_request_target: Runs in BASE repository context (not PR head) * - CRITICAL: For fork PRs, the head branch doesn't exist in base repo @@ -467,24 +467,22 @@ async function main() { const { isFork } = logPRContext(eventName, pullRequest); if (eventName === "pull_request" && isFork === false && !isClosed) { - // For non-fork pull_request events, we run in the merge commit context. - // The PR branch is in the same repo as origin, so we can use direct git commands. - // Fork PRs cannot use git fetch because their head branch only exists in the fork - // (not in origin/base repo), so they must use gh pr checkout in the else branch below. + // Use the retained PR head ref, not the branch name: the PR may have + // merged and its branch disappeared since the webhook was captured. const branchName = pullRequest.head.ref; // commits is in the payload for pull_request events; +1 to include the merge base const commitCount = pullRequest.commits || 1; const fetchDepth = commitCount + 1; - logCheckoutStrategy(eventName, "git fetch + checkout", "pull_request event runs in merge commit context with PR branch available"); + logCheckoutStrategy(eventName, "git fetch refs/pull + checkout", "same-repository pull_request event; retained PR head ref survives branch deletion"); - core.info(`Fetching branch: ${branchName} from origin (depth: ${fetchDepth} for ${commitCount} PR commit(s))`); + core.info(`Fetching PR #${pullRequest.number} head via refs/pull/${pullRequest.number}/head (depth: ${fetchDepth} for ${commitCount} PR commit(s))`); const fetchArgs = await depthArgs(fetchDepth); core.info(fetchArgs.length > 0 ? `Fetching with ${fetchArgs.join(" ")}` : "Fetching without --depth (full history preserved)"); - await fetchPRHead([branchName, ...fetchArgs]); + await fetchPRHead([`+refs/pull/${pullRequest.number}/head:${PR_HEAD_BASE_REF}`, ...fetchArgs]); core.info(`Checking out branch: ${branchName}`); - await exec.exec("git", ["checkout", branchName]); + await exec.exec("git", ["checkout", "-B", branchName, PR_HEAD_BASE_REF]); await exportPRHeadBaseline({ branchName, @@ -533,7 +531,7 @@ async function main() { const branchName = headRef || `pr-${prNumber}`; core.info(`Checking out branch: ${branchName}`); - await exec.exec("git", ["checkout", "-B", branchName, "origin/pr-head"]); + await exec.exec("git", ["checkout", "-B", branchName, PR_HEAD_BASE_REF]); await exportPRHeadBaseline({ branchName, diff --git a/actions/setup/js/checkout_pr_branch.test.cjs b/actions/setup/js/checkout_pr_branch.test.cjs index 52e849fd441..bc782675573 100644 --- a/actions/setup/js/checkout_pr_branch.test.cjs +++ b/actions/setup/js/checkout_pr_branch.test.cjs @@ -1,4 +1,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { execFileSync, spawnSync } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; const { ERR_API } = require("./error_codes.cjs"); describe("checkout_pr_branch.cjs", () => { let mockCore; @@ -246,15 +251,15 @@ If the pull request is still open, verify that: // Verify strategy logging expect(mockCore.startGroup).toHaveBeenCalledWith("๐Ÿ”„ Checkout Strategy"); - expect(mockCore.info).toHaveBeenCalledWith("Strategy: git fetch + checkout"); + expect(mockCore.info).toHaveBeenCalledWith("Strategy: git fetch refs/pull + checkout"); // Verify actual checkout commands // commits is undefined in mock payload, so defaults to 1; depth = 1+1 = 2 - expect(mockCore.info).toHaveBeenCalledWith("Fetching branch: feature-branch from origin (depth: 2 for 1 PR commit(s))"); - expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"], { silent: true, ignoreReturnCode: true }); + expect(mockCore.info).toHaveBeenCalledWith("Fetching PR #123 head via refs/pull/123/head (depth: 2 for 1 PR commit(s))"); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); expect(mockCore.info).toHaveBeenCalledWith("Checking out branch: feature-branch"); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "feature-branch"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); expect(mockCore.info).toHaveBeenCalledWith("โœ… Successfully checked out branch: feature-branch"); expect(mockCore.setFailed).not.toHaveBeenCalled(); @@ -266,8 +271,8 @@ If the pull request is still open, verify that: const script = require("./checkout_pr_branch.cjs"); await script.main(); - expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.getExecOutput).not.toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"], expect.anything()); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head"], { silent: true, ignoreReturnCode: true }); + expect(mockExec.getExecOutput).not.toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], expect.anything()); expect(mockCore.setFailed).not.toHaveBeenCalled(); }); @@ -288,8 +293,8 @@ If the pull request is still open, verify that: await runScript(); - expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "feature-branch"]); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "true"); expect(mockCore.setFailed).not.toHaveBeenCalled(); }); @@ -303,8 +308,8 @@ If the pull request is still open, verify that: await runScript(); - expect(mockExec.getExecOutput).not.toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"], expect.anything()); - expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["checkout", "feature-branch"]); + expect(mockExec.getExecOutput).not.toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], expect.anything()); + expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "false"); expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("requires write or higher")); }); @@ -318,8 +323,8 @@ If the pull request is still open, verify that: expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith(expect.objectContaining({ username: "Copilot" })); expect(mockCore.info).toHaveBeenCalledWith("Runtime safety check passed for actor 'Copilot' with 'write' permission"); expect(mockCore.setFailed).not.toHaveBeenCalled(); - expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "feature-branch"]); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); }); it.each([null, undefined])("should preserve collaborator fallback when the bot event repository is %s", async repository => { @@ -420,8 +425,8 @@ If the pull request is still open, verify that: expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalled(); expect(mockGithub.rest.users.getByUsername).toHaveBeenCalledWith({ username: "Copilot" }); - expect(mockExec.getExecOutput).not.toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"], expect.anything()); - expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["checkout", "feature-branch"]); + expect(mockExec.getExecOutput).not.toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], expect.anything()); + expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "false"); expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("bot/app actor 'Copilot' repository permission could not be verified")); }); @@ -440,8 +445,8 @@ If the pull request is still open, verify that: expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalled(); expect(mockGithub.rest.users.getByUsername).toHaveBeenCalledWith({ username: "real-user" }); - expect(mockExec.getExecOutput).not.toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"], expect.anything()); - expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["checkout", "feature-branch"]); + expect(mockExec.getExecOutput).not.toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], expect.anything()); + expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "false"); expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("is not a collaborator")); }); @@ -970,7 +975,7 @@ If the pull request is still open, verify that: // Verify git fetch refs/pull/N/head is used expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); expect(mockExec.getExecOutput).not.toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"], expect.anything()); expect(mockCore.exportVariable).toHaveBeenCalledWith("GH_AW_PR_HEAD_BASE_BRANCH", "feature-branch"); expect(mockCore.exportVariable).toHaveBeenCalledWith("GH_AW_PR_HEAD_BASE_REPO", "test-owner/test-repo"); @@ -992,9 +997,9 @@ If the pull request is still open, verify that: // Verify NOT detected as fork (same full_name) expect(mockCore.info).toHaveBeenCalledWith("Is fork PR: false (same repository)"); - // Verify git fetch + checkout is used (fast path) - expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "feature-branch"]); + // The same-repository fast path also uses the retained PR ref. + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); expect(mockExec.exec).not.toHaveBeenCalledWith("gh", ["pr", "checkout", "123"]); expect(mockCore.setFailed).not.toHaveBeenCalled(); @@ -1008,9 +1013,9 @@ If the pull request is still open, verify that: // Verify non-fork detection expect(mockCore.info).toHaveBeenCalledWith("Is fork PR: false (same repository)"); - // Verify git fetch + checkout is used for non-fork - expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "feature-branch"]); + // Verify the retained PR ref is used for non-fork PRs too. + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); expect(mockExec.exec).not.toHaveBeenCalledWith("gh", ["pr", "checkout", "123"]); }); }); @@ -1035,7 +1040,7 @@ If the pull request is still open, verify that: // Verify git fetch refs/pull/N/head and checkout expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); expect(mockCore.info).toHaveBeenCalledWith("โœ… Successfully checked out PR #123"); expect(mockCore.setFailed).not.toHaveBeenCalled(); @@ -1085,7 +1090,7 @@ If the pull request is still open, verify that: expect(mockCore.warning).not.toHaveBeenCalledWith(expect.stringContaining("Fork PR detected")); // Should successfully checkout expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/456/head:refs/remotes/origin/pr-head", "--depth=4"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "my-feature", "origin/pr-head"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "my-feature", "refs/remotes/origin/pr-head"]); expect(mockCore.setFailed).not.toHaveBeenCalled(); }); @@ -1116,7 +1121,7 @@ If the pull request is still open, verify that: expect(mockCore.warning).toHaveBeenCalledWith("โš ๏ธ Fork PR detected - fetching via refs/pull/N/head from origin"); // Should successfully checkout expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/789/head:refs/remotes/origin/pr-head", "--depth=3"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "fork-feature", "origin/pr-head"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "fork-feature", "refs/remotes/origin/pr-head"]); expect(mockCore.setFailed).not.toHaveBeenCalled(); }); }); @@ -1189,7 +1194,7 @@ If the pull request is still open, verify that: // workflow_dispatch uses git fetch refs/pull + checkout (not the fast pull_request path) expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); // fetchPRDetails must be called with the correct PR number to resolve head ref / commit count expect(mockGithub.rest.pulls.get).toHaveBeenCalledWith(expect.objectContaining({ pull_number: 123 })); @@ -1363,7 +1368,7 @@ If the pull request is still open, verify that: expect(mockCore.info).toHaveBeenCalledWith("Event: pull_request_target"); // pull_request_target uses git fetch refs/pull + checkout expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); }); it("should handle pull_request_review event", async () => { @@ -1374,7 +1379,7 @@ If the pull request is still open, verify that: expect(mockCore.info).toHaveBeenCalledWith("Event: pull_request_review"); // pull_request_review uses git fetch refs/pull + checkout expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); }); it("should handle pull_request_review_comment event", async () => { @@ -1384,7 +1389,7 @@ If the pull request is still open, verify that: // pull_request_review_comment uses git fetch refs/pull + checkout expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); }); }); @@ -1400,7 +1405,7 @@ If the pull request is still open, verify that: await runScript(); expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "refs/remotes/origin/pr-head"]); expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "true"); expect(mockCore.setFailed).not.toHaveBeenCalled(); }); @@ -1442,8 +1447,8 @@ If the pull request is still open, verify that: await runScript(); - expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "feature/new-feature", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "feature/new-feature"]); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature/new-feature", "refs/remotes/origin/pr-head"]); }); it("should handle branches with special characters", async () => { @@ -1451,8 +1456,8 @@ If the pull request is still open, verify that: await runScript(); - expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "fix-issue-#123", "--depth=2"], { silent: true, ignoreReturnCode: true }); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "fix-issue-#123"]); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "fix-issue-#123", "refs/remotes/origin/pr-head"]); }); it("should handle very long branch names", async () => { @@ -1461,7 +1466,7 @@ If the pull request is still open, verify that: await runScript(); - expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", longBranchName, "--depth=2"], { silent: true, ignoreReturnCode: true }); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"], { silent: true, ignoreReturnCode: true }); }); }); @@ -1591,8 +1596,8 @@ If the pull request is still open, verify that: await runScript(); expect(mockCore.startGroup).toHaveBeenCalledWith("๐Ÿ”„ Checkout Strategy"); - expect(mockCore.info).toHaveBeenCalledWith("Strategy: git fetch + checkout"); - expect(mockCore.info).toHaveBeenCalledWith("Reason: pull_request event runs in merge commit context with PR branch available"); + expect(mockCore.info).toHaveBeenCalledWith("Strategy: git fetch refs/pull + checkout"); + expect(mockCore.info).toHaveBeenCalledWith("Reason: same-repository pull_request event; retained PR head ref survives branch deletion"); }); it("should log checkout strategy for pull_request_target events", async () => { @@ -1728,6 +1733,73 @@ If the pull request is still open, verify that: }); describe("race condition - PR merged after workflow trigger", () => { + it.each([ + ["open", true], + ["open", false], + ["closed", true], + ["closed", false], + ])("checks out the retained PR head after branch deletion (payload state %s, shallow %s)", async (state, shallow) => { + const actionsExec = await import("@actions/exec"); + const root = mkdtempSync(join(tmpdir(), "checkout-pr-race-")); + const upstream = join(root, "upstream"); + const workspace = join(root, "workspace"); + const env = { + PATH: process.env.PATH, + HOME: root, + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: "/dev/null", + GIT_TERMINAL_PROMPT: "0", + GIT_AUTHOR_NAME: "Checkout Test", + GIT_AUTHOR_EMAIL: "checkout-test@example.com", + GIT_COMMITTER_NAME: "Checkout Test", + GIT_COMMITTER_EMAIL: "checkout-test@example.com", + }; + const git = (cwd, args) => execFileSync("git", args, { cwd, env, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }).trim(); + try { + git(root, ["init", "--initial-branch=main", upstream]); + writeFileSync(join(upstream, "content.txt"), "base\n"); + git(upstream, ["add", "content.txt"]); + git(upstream, ["commit", "-m", "base"]); + const baseSha = git(upstream, ["rev-parse", "HEAD"]); + git(upstream, ["checkout", "-b", "feature-branch"]); + writeFileSync(join(upstream, "content.txt"), "PR head\n"); + git(upstream, ["commit", "-am", "PR head"]); + const headSha = git(upstream, ["rev-parse", "HEAD"]); + git(upstream, ["update-ref", "refs/pull/123/head", headSha]); + git(upstream, ["checkout", "main"]); + git(upstream, ["update-ref", "-d", "refs/heads/feature-branch"]); + git(root, ["clone", "--branch", "main", ...(shallow ? ["--depth=1"] : []), pathToFileURL(upstream).href, workspace]); + git(workspace, ["branch", "feature-branch"]); + git(workspace, ["update-ref", "refs/remotes/origin/pr-head", baseSha]); + + const deletedBranchFetch = spawnSync("git", ["fetch", "origin", "feature-branch"], { cwd: workspace, env, encoding: "utf8" }); + expect(deletedBranchFetch.status).toBe(128); + expect(deletedBranchFetch.stderr).toContain("couldn't find remote ref feature-branch"); + mockContext.payload.pull_request.state = state; + mockContext.payload.pull_request.head.sha = baseSha; + const fullPR = (await mockGithub.rest.pulls.get()).data; + mockGithub.rest.pulls.get.mockResolvedValue({ data: { ...fullPR, state: "closed", merged: true } }); + mockExec.getExecOutput.mockImplementation((cmd, args, options) => actionsExec.getExecOutput(cmd, args, { ...options, cwd: workspace, env })); + mockExec.exec.mockImplementation((cmd, args) => actionsExec.exec(cmd, args, { cwd: workspace, env, silent: true })); + + await runScript(); + + expect(mockCore.setFailed).not.toHaveBeenCalled(); + expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "true"); + expect(mockCore.setOutput).not.toHaveBeenCalledWith("checkout_pr_invalidated", "true"); + expect(git(workspace, ["rev-parse", "HEAD"])).toBe(headSha); + expect(git(workspace, ["branch", "--show-current"])).toBe("feature-branch"); + expect(git(workspace, ["rev-parse", "refs/remotes/origin/pr-head"])).toBe(headSha); + expect(git(workspace, ["rev-parse", "--is-shallow-repository"])).toBe(String(shallow)); + expect(readFileSync(join(workspace, "content.txt"), "utf8")).toBe("PR head\n"); + expect(mockCore.exportVariable).toHaveBeenCalledWith("GH_AW_PR_HEAD_BASE_SHA", headSha); + expect(mockCore.exportVariable).toHaveBeenCalledWith("GH_AW_PR_HEAD_BASE_BRANCH", "feature-branch"); + expect(git(upstream, ["for-each-ref", "--format=%(refname)", "refs/heads/feature-branch"])).toBe(""); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + it.each([ ["pull_request", "Authentication failed"], ["pull_request", "Permission denied"], @@ -1752,9 +1824,9 @@ If the pull request is still open, verify that: expect(mockExec.exec).not.toHaveBeenCalledWith("git", expect.arrayContaining(["checkout"])); }); it("marks confirmed lifecycle invalidation but fails the step to stop the agent", async () => { - // PR was "open" in webhook payload, but branch was deleted after merge + // Even the retained PR head ref is unavailable after closure. mockContext.payload.pull_request.state = "open"; - mockFetch.mockResolvedValueOnce({ exitCode: 128, stdout: "", stderr: "fatal: couldn't find remote ref feature-branch" }); + mockFetch.mockResolvedValueOnce({ exitCode: 128, stdout: "", stderr: "fatal: couldn't find remote ref refs/pull/123/head" }); // Non-fork pull_request uses fast path (no fetchPRDetails call). // Only the error handler re-check calls pulls.get โ†’ returns closed. mockGithub.rest.pulls.get.mockResolvedValueOnce({ @@ -1781,7 +1853,7 @@ If the pull request is still open, verify that: it("uses real @actions/exec output to detect missing-ref evidence", async () => { const actionsExec = await import("@actions/exec"); mockGithub.rest.pulls.get.mockResolvedValue({ data: { state: "closed" } }); - mockFetch.mockImplementation((_cmd, _args, options) => actionsExec.getExecOutput(process.execPath, ["-e", `process.stderr.write("fatal: couldn't find remote ref feature-branch\\n"); process.exit(128);`], options)); + mockFetch.mockImplementation((_cmd, _args, options) => actionsExec.getExecOutput(process.execPath, ["-e", `process.stderr.write("fatal: couldn't find remote ref refs/pull/123/head\\n"); process.exit(128);`], options)); await runScript(); expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_invalidated", "true"); expect(mockCore.setFailed).toHaveBeenCalled(); @@ -1789,14 +1861,14 @@ If the pull request is still open, verify that: it("does not classify a non-fetch error containing missing-ref text as invalidation", async () => { mockGithub.rest.pulls.get.mockResolvedValue({ data: { state: "closed" } }); - mockExec.exec.mockRejectedValueOnce(new Error("fatal: couldn't find remote ref feature-branch")); + mockExec.exec.mockRejectedValueOnce(new Error("fatal: couldn't find remote ref refs/pull/123/head")); await runScript(); expect(mockCore.setOutput).not.toHaveBeenCalledWith("checkout_pr_invalidated", "true"); expect(mockCore.setFailed).toHaveBeenCalled(); }); it("does not classify a missing head ref as invalidation while the PR remains open", async () => { - mockFetch.mockResolvedValueOnce({ exitCode: 128, stdout: "", stderr: "fatal: couldn't find remote ref feature-branch" }); + mockFetch.mockResolvedValueOnce({ exitCode: 128, stdout: "", stderr: "fatal: couldn't find remote ref refs/pull/123/head" }); await runScript(); expect(mockCore.setOutput).not.toHaveBeenCalledWith("checkout_pr_invalidated", "true"); expect(mockCore.setFailed).toHaveBeenCalled(); @@ -1804,7 +1876,7 @@ If the pull request is still open, verify that: it("does not classify missing-ref text on stdout as fetch failure evidence", async () => { mockGithub.rest.pulls.get.mockResolvedValue({ data: { state: "closed" } }); - mockFetch.mockResolvedValueOnce({ exitCode: 128, stdout: "fatal: couldn't find remote ref feature-branch", stderr: "unknown git error" }); + mockFetch.mockResolvedValueOnce({ exitCode: 128, stdout: "fatal: couldn't find remote ref refs/pull/123/head", stderr: "unknown git error" }); await runScript(); expect(mockCore.setOutput).not.toHaveBeenCalledWith("checkout_pr_invalidated", "true"); expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("unknown git error")); diff --git a/docs/src/content/docs/reference/checkout.md b/docs/src/content/docs/reference/checkout.md index a0768ec5459..76e11c9283f 100644 --- a/docs/src/content/docs/reference/checkout.md +++ b/docs/src/content/docs/reference/checkout.md @@ -174,7 +174,13 @@ checkout: false This is equivalent to omitting the checkout step from the agent job. Custom dev-mode steps (such as "Checkout actions folder") are unaffected. -## Disabling the PR Branch Checkout +## PR Head Checkout and Branch Deletion + +PR head checkout fetches the repository's retained `refs/pull//head` +ref, including for same-repository PRs. A queued review can therefore check out +the PR head even if the PR merges and its source branch is deleted before the +agent starts. The local branch keeps the PR's source branch name; checkout does +not recreate the remote branch or fall back to the base branch. A failed PR checkout stops the agent job; it never continues on the previous base checkout. If an originally open PR closes and its remote ref disappears @@ -191,6 +197,8 @@ read access to the triggering PR; a failed state lookup is an error, not proof o closure. PR-branch pushes also refuse to recreate deleted target branches. These checks cannot make a PR-state lookup and a later GitHub write atomic. +## Disabling the PR Branch Checkout + To keep the normal repository checkout but suppress the compiler-generated "Checkout PR branch" step and its associated base-config snapshot/restore steps, set `pull-request: false` under `checkout`: ```yaml wrap diff --git a/specs/compiler-threat-detection-changelog.md b/specs/compiler-threat-detection-changelog.md index 17c6b42dd8b..6614b12f80f 100644 --- a/specs/compiler-threat-detection-changelog.md +++ b/specs/compiler-threat-detection-changelog.md @@ -16,6 +16,7 @@ This changelog records the version history and the dated mapping audits for `spe | Version | Change | |---|---| +| 1.0.67 | Audit-only clarification of retained PR-head checkout after source-branch deletion. Maps same-repository `pull_request` ref isolation and actual checked-out baseline export to RS-05a, preserves fatal checkout errors and independent safe-output lifecycle checks, and leaves CTR-008/028/030 unchanged. No new threat identifier, permission grant, or lock-file schema change. | | 1.0.66 | Extended CTR-005 with custom-job ordering and detection gates. The compiler checks the final graph, rejects unordered and pre-detection downstream jobs, and adds non-overridable successful-job and clean-output guards to downstream custom jobs, including reusable workflow calls. Failure, cancellation, skips, missing results, and warn-mode findings block execution; disabled detection remains exempt. Mapped the transitive Actions/AW `custom-job-detection` subset, registry metadata, graph/condition regressions, and regenerated lock files. No new threat identifier or lock-file schema change. | | 1.0.65 | Added `otlp-secret-leak`, an Actions/AW AST lint subset of CTR-017 that detects secret-tainted OTLP span/resource attributes and modeled Actions environment flows with value-free diagnostics. Documented that the check is advisory and cannot identify hard-coded credentials, arbitrary runtime values, or collector trust. No new threat identifier or lock-file schema change. | | 1.0.64 | Adds the compromised-intermediate-CA threat and maps bootstrap/runtime controls to security architecture ยง6.8: independently authenticated AWF/Copilot checksum manifests, literal frontmatter manifest-pin overrides bound to explicit versions, compiler-pinned Compose bytes, authenticated apt signing key, HTTPS-only redirects, actual Go release-toolchain verification and bounded runtime inventory. Overrides propagate through matching detection/eval installations; Copilot overrides bypass toolcache activation. Preserves CTR-011 destination authorization as a separate boundary and records broader download enforcement as a candidate. Unreviewed release downloads without a supplied pin fail closed; no new threat identifier or lock-file schema change. | @@ -62,6 +63,35 @@ This changelog records the version history and the dated mapping audits for `spe ## Mapping Audits +### Mapping Audit (2026-10-22) + +Reviewed the shared PR checkout change in +[`PR #71210`](https://github.com/github/gh-aw/pull/71210), +`actions/setup/js/checkout_pr_branch.cjs`, its regression suite, and Security +Architecture Specification RS-05a. The same-repository `pull_request` fast path +now fetches `+refs/pull//head:refs/remotes/origin/pr-head` and resets the +local source-named branch from that fetched ref, matching the existing +fork/other-event path. A deleted source branch no longer prevents checkout of +the retained PR head; stale local branches and event SHAs cannot select the +baseline. This is runtime ref isolation, not a new compiler-detectable threat. + +The real-Git regressions in `actions/setup/js/checkout_pr_branch.test.cjs` +first reproduce the missing-source-branch fetch failure, then verify exact PR +content, local branch, fetched ref, actual checked-out baseline, and unchanged +shallow/full-clone status for open and closed payloads. They also verify that +the deleted remote branch is not recreated. These isolated local tests do not +prove GitHub's ref-retention policy, live API behavior, or atomicity between +PR-state checks and later effects. + +Existing missing-retained-ref and error regressions preserve the distinction +between live-state-confirmed lifecycle invalidation and authentication, +permission, network, or unknown failures. Failed checkout still stops the +agent rather than continuing on base content. Successful retained-head +checkout does not bypass the safe-output manager's live-state checks; +intentional closed-event workflows remain supported. Actor trust, CTR-008 +trusted checkout policy, CTR-028/030 configuration provenance, permissions, +conformance test identifiers, and generated lock formats are unchanged. + ### Mapping Audit (2026-10-19) Reviewed staged skill registration in `actions/setup/js/copilot_sdk_session.cjs`, diff --git a/specs/compiler-threat-detection-spec.md b/specs/compiler-threat-detection-spec.md index 6ef30502d08..3bd5e79abc4 100644 --- a/specs/compiler-threat-detection-spec.md +++ b/specs/compiler-threat-detection-spec.md @@ -7,7 +7,7 @@ sidebar: # GitHub Actions Compiler Threat Detection Specification -**Version**: 1.0.66 +**Version**: 1.0.67 **Status**: Candidate Recommendation **Latest Version**: https://github.com/github/gh-aw/blob/main/specs/compiler-threat-detection-spec.md **Editors**: GitHub Next (GitHub, Inc.) @@ -32,6 +32,7 @@ Each version maps to the minimum compatible binary. A version change MUST update | Versions | Minimum gh-aw | Compatibility | |---|---:|---| +| `1.0.67` | Unreleased | Audit-only; maps retained PR-head checkout after source-branch deletion to runtime ref isolation (RS-05a), separate from compiler checkout provenance and safe-output lifecycle fencing. No new threat identifier, permission grant, or lock-file schema change. | | `1.0.66` | Unreleased | Extends CTR-005 to custom-job detection bypass: transitive ordering checks and compiler-enforced successful, clean detection before downstream custom jobs. Maps the Actions/AW `custom-job-detection` lint subset and disabled-detection exemption. Generated locks require regeneration; no new threat identifier or lock-file schema change. | | `1.0.65` | Unreleased | Adds the `otlp-secret-leak` Actions/AW AST lint subset of CTR-017 for secret-tainted OTLP span and resource attributes, including Actions environment dataflow and value-free diagnostics. It does not infer literal credentials or arbitrary runtime values. No new threat identifier or lock-file schema change. | | `1.0.64` | Unreleased | Separates compromised-CA transport authentication from CTR-011 destination authorization. Maps trusted release digests, HTTPS-only bootstrap redirects, patched release toolchains and runtime inventory to security architecture ยง6.8. Adds optional literal `engine.checksums-sha256` and `sandbox.agent.checksums-sha256` overrides bound to explicit versions, including matching detection/eval installations. Unreviewed downloads without a supplied pin fail closed; no new `CTR-*` identifier or lock-file schema change. | @@ -90,6 +91,8 @@ Safe-output invocation-context repository authorization is also a runtime contro PR-branch checkout authorization in `actions/setup/js/checkout_pr_branch.cjs` is also a runtime trust control, not a compiler-detectable threat. The same-repository bot exception covers `pull_request` and `pull_request_target` `opened`, `synchronize`, `ready_for_review`, and `reopened` events with matching bot sender and actor plus equal positive runtime, PR head, and PR base repository IDs. For `issue_comment`, `pull_request_review_comment`, and `pull_request_review`, an explicitly `on.bots`-allow-listed App may bypass the collaborator check only when `sender.type` is `Bot`, the sender and matching comment or review author logins match exactly, the sender is canonically equivalent to the runtime actor (App slug or `[bot]` form), the runtime repository ID is positive, and the fetched PR head and base repository IDs both match it. Forks, unlisted bots, unverifiable or mismatched identities, and other actors retain the collaborator-permission check. The normative requirements are in the Security Architecture Specification, RS-05a; this control does not add a `CTR-*` rule. +PR-head ref isolation is part of the same runtime boundary. Checkout fetches the exact `refs/pull//head` from the current repository's origin, including for same-repository `pull_request` events, then resets the local source-named branch to the fetched head. This retained ref avoids dependence on a source branch that can disappear after merge; neither an existing local branch nor the previous base checkout is a valid fallback. The exported incremental-patch baseline identifies the commit actually checked out, not an event SHA that may be stale. Successful checkout does not establish that the PR is still open, authorize further effects, or recreate its remote source branch. Existing live-state checks fence obsolete safe-output effects independently, while intentionally closed-event workflows remain supported. If checkout fails, execution stops; only a missing remote ref plus a live state check confirming closure of an originally open PR records lifecycle invalidation. Authentication, permission, network, unknown errors, and unavailable state evidence remain failures. These runtime guarantees do not change CTR-008's trusted `pull_request_target` checkout policy or CTR-028/030's configuration provenance requirements. + Patch/bundle replacement between safe-output validation and Git application is likewise a runtime boundary, not a new compiler threat rule. Safe Outputs [PTI-001โ€“007](../docs/src/content/docs/specs/safe-outputs-specification.md#patch-and-bundle-transport-integrity-requirements) require canonical capture, nonblocking regular-file checks, private snapshot consumption, full byte checks at application/publication boundaries, permanent integrity-error classification, registered local cleanup, and safe diagnostics. The implementation is `actions/setup/js/transport_integrity.cjs`, used by both PR handlers and their publication callbacks; evidence is mapped in the safe-output specification and [`SafeOutputTransport.tla`](workflow-security/SafeOutputTransport.tla). Artifact run/invocation provenance, schema validation, detector gating, and CTR-005 safe-output routing remain independent requirements: none proves that a mutable artifact still contains the bytes validated earlier. This amendment changes no generated workflow construct, GitHub App permission, suppression rule, or `CTR-*` identifier. Sandbox bypass includes the provenance of the agent's own configuration. On pull-request triggers the workspace holds head-branch content that the PR author controls, so agent configuration folders and root instruction files are attacker-controlled inputs until the generated job replaces them with the base-branch snapshot captured before that content was checked out.