From f2b01404c0ef3b03494b0034b9c3d0b3bc454d7e Mon Sep 17 00:00:00 2001 From: Don Syme Date: Tue, 14 Apr 2026 11:18:46 +1000 Subject: [PATCH] fix: replace gh pr checkout with git fetch refs/pull to avoid GH_HOST issues Replace `gh pr checkout` with pure git operations (`git fetch origin +refs/pull/N/head` + `git checkout -B`) for PR checkout in non-fork pull_request_target, issue_comment, and other PR event handlers. GitHub exposes refs/pull/N/head for all PRs including forks, so this works universally. Git operations use remote URLs directly and are unaffected by GH_HOST overrides from the DIFC proxy, eliminating the need for the getGhEnvBypassingIntegrityFilteringForGitOps helper. Also removes the now-unused getGitHubHost() and getGhEnvBypassingIntegrityFilteringForGitOps() from git_helpers.cjs. --- actions/setup/js/checkout_pr_branch.cjs | 69 ++---- actions/setup/js/checkout_pr_branch.test.cjs | 208 ++++++------------- actions/setup/js/git_helpers.cjs | 32 --- 3 files changed, 87 insertions(+), 222 deletions(-) diff --git a/actions/setup/js/checkout_pr_branch.cjs b/actions/setup/js/checkout_pr_branch.cjs index 2ffddab8411..0991afc1ae8 100644 --- a/actions/setup/js/checkout_pr_branch.cjs +++ b/actions/setup/js/checkout_pr_branch.cjs @@ -12,12 +12,12 @@ * * 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 - * - Must use `gh pr checkout` to fetch from the fork + * - Uses refs/pull/N/head to fetch from origin (works for forks too) * - Has write permissions (be cautious with untrusted code) * * 3. Other PR events (issue_comment, pull_request_review, etc.): * - Also run in base repository context - * - Must use `gh pr checkout` to get PR branch + * - Uses refs/pull/N/head to fetch PR branch * * NOTE: This handler operates within the PR context from the workflow event * and does not support cross-repository operations or target-repo parameters. @@ -26,7 +26,6 @@ */ const { getErrorMessage } = require("./error_helpers.cjs"); -const { getGhEnvBypassingIntegrityFilteringForGitOps } = require("./git_helpers.cjs"); const { renderTemplateFromFile } = require("./messages_core.cjs"); const { detectForkPR } = require("./pr_helpers.cjs"); const { ERR_API } = require("./error_codes.cjs"); @@ -79,10 +78,10 @@ function logPRContext(eventName, pullRequest) { } /** - * Fetch full PR commit count from the GitHub API. - * Used when the commit count is not available in the webhook payload. + * Fetch PR details from the GitHub API. + * Returns head ref and commit count needed for checkout. */ -async function fetchPRCommitCount(prNumber) { +async function fetchPRDetails(prNumber) { const { data } = await github.rest.pulls.get({ owner: context.repo.owner, repo: context.repo.repo, @@ -115,7 +114,7 @@ async function main() { number: context.payload.issue.number, state: context.payload.issue.state || "open", }; - core.info(`Detected ${eventName} event on PR #${pullRequest.number}, will use gh pr checkout`); + core.info(`Detected ${eventName} event on PR #${pullRequest.number}, will fetch PR ref`); } if (!pullRequest) { @@ -159,60 +158,36 @@ async function main() { } else { // For pull_request_target, fork pull_request events, and other PR events, // we run in base repository context. - // IMPORTANT: For fork PRs, the head branch doesn't exist in the base repo - // We must use `gh pr checkout` which handles fetching from forks + // Use refs/pull/N/head which GitHub makes available for all PRs (including forks) + // so we don't need `gh pr checkout` and avoid GH_HOST / DIFC proxy issues. const prNumber = pullRequest.number; const strategyReason = eventName === "pull_request_target" - ? "pull_request_target runs in base repo context; for fork PRs, head branch doesn't exist in origin" + ? "pull_request_target runs in base repo context; fetching via refs/pull/N/head" : eventName === "pull_request" && isFork - ? "pull_request event from fork repository; head branch exists only in fork, not in origin" - : `${eventName} event runs in base repo context; must fetch PR branch`; + ? "pull_request event from fork repository; fetching via refs/pull/N/head" + : `${eventName} event runs in base repo context; fetching via refs/pull/N/head`; - logCheckoutStrategy(eventName, "gh pr checkout", strategyReason); + logCheckoutStrategy(eventName, "git fetch refs/pull + checkout", strategyReason); if (isFork) { - core.warning("⚠️ Fork PR detected - gh pr checkout will fetch from fork repository"); + core.warning("⚠️ Fork PR detected - fetching via refs/pull/N/head from origin"); } - core.info(`Checking out PR #${prNumber} using gh CLI`); + // Get PR details from API to determine head ref name and commit count + const { commitCount, headRef } = await fetchPRDetails(prNumber); + const fetchDepth = (commitCount || 1) + 1; // +1 to include the merge base - // Override GH_HOST with the real GitHub hostname so gh pr checkout can resolve - // the repository from git remotes. The DIFC proxy may have set GH_HOST to - // localhost:18443 which doesn't match any remote. - await exec.exec("gh", ["pr", "checkout", prNumber.toString()], { - env: getGhEnvBypassingIntegrityFilteringForGitOps(), - }); + core.info(`Fetching PR #${prNumber} head via refs/pull/${prNumber}/head (depth: ${fetchDepth} for ${commitCount} PR commit(s))`); + await exec.exec("git", ["fetch", "origin", `+refs/pull/${prNumber}/head:refs/remotes/origin/pr-head`, `--depth=${fetchDepth}`]); - // Log the resulting branch after checkout - let currentBranch = ""; - await exec.exec("git", ["branch", "--show-current"], { - listeners: { - stdout: data => { - currentBranch += data.toString(); - }, - }, - }); - currentBranch = currentBranch.trim(); - - // Deepen history to cover all PR commits (gh pr checkout fetches --depth=1 by default) - try { - const { commitCount, headRef } = await fetchPRCommitCount(prNumber); - const fetchDepth = commitCount + 1; // +1 to include the merge base - const branchRef = headRef || currentBranch; - core.info(`Deepening history for PR #${prNumber}: fetching ${fetchDepth} commits (${commitCount} PR commit(s) + merge base)`); - if (branchRef) { - await exec.exec("git", ["fetch", "origin", branchRef, `--depth=${fetchDepth}`]); - } else { - await exec.exec("git", ["fetch", `--depth=${fetchDepth}`]); - } - } catch (depthError) { - core.warning(`Could not deepen PR history: ${getErrorMessage(depthError)}`); - } + const branchName = headRef || `pr-${prNumber}`; + core.info(`Checking out branch: ${branchName}`); + await exec.exec("git", ["checkout", "-B", branchName, "origin/pr-head"]); core.info(`✅ Successfully checked out PR #${prNumber}`); - core.info(`Current branch: ${currentBranch || "detached HEAD"}`); + core.info(`Current branch: ${branchName}`); } // Set output to indicate successful checkout diff --git a/actions/setup/js/checkout_pr_branch.test.cjs b/actions/setup/js/checkout_pr_branch.test.cjs index 54b6a8119c4..d9b758bd155 100644 --- a/actions/setup/js/checkout_pr_branch.test.cjs +++ b/actions/setup/js/checkout_pr_branch.test.cjs @@ -4,6 +4,7 @@ describe("checkout_pr_branch.cjs", () => { let mockCore; let mockExec; let mockContext; + let mockGithub; beforeEach(() => { // Mock core actions methods @@ -66,6 +67,19 @@ describe("checkout_pr_branch.cjs", () => { global.core = mockCore; global.exec = mockExec; global.context = mockContext; + + // Mock GitHub API for fetchPRDetails (used in the else branch for non-fork PR events) + mockGithub = { + rest: { + pulls: { + get: vi.fn().mockResolvedValue({ + data: { state: "open", commits: 1, head: { ref: "feature-branch" } }, + }), + }, + }, + }; + global.github = mockGithub; + process.env.GITHUB_TOKEN = "test-token"; process.env.GITHUB_SERVER_URL = "https://github.com"; }); @@ -153,9 +167,6 @@ If the pull request is still open, verify that: if (module === "./error_codes.cjs") { return require("./error_codes.cjs"); } - if (module === "./git_helpers.cjs") { - return require("./git_helpers.cjs"); - } throw new Error(`Module ${module} not mocked in test`); }; @@ -229,7 +240,7 @@ If the pull request is still open, verify that: expect(mockCore.setFailed).toHaveBeenCalledWith(`${ERR_API}: Failed to checkout PR branch: git checkout failed`); }); - it("should use gh pr checkout for fork PR in pull_request event", async () => { + it("should use git fetch refs/pull for fork PR in pull_request event", async () => { // Set up fork PR: head repo is different from base repo mockContext.payload.pull_request.head.repo.full_name = "fork-owner/test-repo"; mockContext.payload.pull_request.head.repo.owner.login = "fork-owner"; @@ -240,14 +251,15 @@ If the pull request is still open, verify that: // Verify fork is detected expect(mockCore.info).toHaveBeenCalledWith("Is fork PR: true (different repository names)"); - expect(mockCore.warning).toHaveBeenCalledWith("⚠️ Fork PR detected - gh pr checkout will fetch from fork repository"); + expect(mockCore.warning).toHaveBeenCalledWith("⚠️ Fork PR detected - fetching via refs/pull/N/head from origin"); - // Verify strategy is gh pr checkout, not git fetch - expect(mockCore.info).toHaveBeenCalledWith("Strategy: gh pr checkout"); - expect(mockCore.info).toHaveBeenCalledWith("Reason: pull_request event from fork repository; head branch exists only in fork, not in origin"); + // Verify strategy is git fetch refs/pull + checkout + expect(mockCore.info).toHaveBeenCalledWith("Strategy: git fetch refs/pull + checkout"); + expect(mockCore.info).toHaveBeenCalledWith("Reason: pull_request event from fork repository; fetching via refs/pull/N/head"); - // Verify gh pr checkout is used instead of git fetch, with GH_HOST override - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "github.com" }) })); + // Verify git fetch refs/pull/N/head is used + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"]); expect(mockCore.setFailed).not.toHaveBeenCalled(); @@ -291,7 +303,7 @@ If the pull request is still open, verify that: mockContext.eventName = "issue_comment"; }); - it("should checkout PR using gh pr checkout", async () => { + it("should checkout PR using git fetch refs/pull", async () => { await runScript(); expect(mockCore.info).toHaveBeenCalledWith("Event: issue_comment"); @@ -302,19 +314,18 @@ If the pull request is still open, verify that: // Verify strategy logging expect(mockCore.startGroup).toHaveBeenCalledWith("🔄 Checkout Strategy"); - expect(mockCore.info).toHaveBeenCalledWith("Strategy: gh pr checkout"); - - expect(mockCore.info).toHaveBeenCalledWith("Checking out PR #123 using gh CLI"); + expect(mockCore.info).toHaveBeenCalledWith("Strategy: git fetch refs/pull + checkout"); - // GH_HOST is overridden with value derived from GITHUB_SERVER_URL to avoid proxy/stale values - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "github.com" }) })); + // Verify git fetch refs/pull/N/head and checkout + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); expect(mockCore.info).toHaveBeenCalledWith("✅ Successfully checked out PR #123"); expect(mockCore.setFailed).not.toHaveBeenCalled(); }); - it("should handle gh pr checkout errors", async () => { - mockExec.exec.mockRejectedValueOnce(new Error("gh pr checkout failed")); + it("should handle git fetch errors for PR ref", async () => { + mockExec.exec.mockRejectedValueOnce(new Error("git fetch failed")); await runScript(); @@ -323,22 +334,10 @@ If the pull request is still open, verify that: const summaryCall = mockCore.summary.addRaw.mock.calls[0][0]; expect(summaryCall).toContain("Failed to Checkout PR Branch"); - expect(summaryCall).toContain("gh pr checkout failed"); + expect(summaryCall).toContain("git fetch failed"); expect(summaryCall).toContain("pull request has been closed"); - expect(mockCore.setFailed).toHaveBeenCalledWith(`${ERR_API}: Failed to checkout PR branch: gh pr checkout failed`); - }); - - it("should pass GH_HOST derived from GITHUB_SERVER_URL to gh command", async () => { - // GH_HOST is always derived from GITHUB_SERVER_URL to avoid stale/proxy values - process.env.CUSTOM_VAR = "custom-value"; - - await runScript(); - - // Verify exec is called with GH_HOST derived from GITHUB_SERVER_URL - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "github.com" }) })); - - delete process.env.CUSTOM_VAR; + expect(mockCore.setFailed).toHaveBeenCalledWith(`${ERR_API}: Failed to checkout PR branch: git fetch failed`); }); }); @@ -381,8 +380,9 @@ If the pull request is still open, verify that: await runScript(); expect(mockCore.info).toHaveBeenCalledWith("Event: pull_request_target"); - // pull_request_target uses gh pr checkout with GH_HOST override - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "github.com" }) })); + // pull_request_target uses git fetch refs/pull + checkout + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); }); it("should handle pull_request_review event", async () => { @@ -391,8 +391,9 @@ If the pull request is still open, verify that: await runScript(); expect(mockCore.info).toHaveBeenCalledWith("Event: pull_request_review"); - // pull_request_review uses gh pr checkout with GH_HOST override - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "github.com" }) })); + // pull_request_review uses git fetch refs/pull + checkout + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); }); it("should handle pull_request_review_comment event", async () => { @@ -400,8 +401,9 @@ If the pull request is still open, verify that: await runScript(); - // pull_request_review_comment uses gh pr checkout with GH_HOST override - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "github.com" }) })); + // pull_request_review_comment uses git fetch refs/pull + checkout + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); }); }); @@ -468,6 +470,8 @@ If the pull request is still open, verify that: expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "true"); expect(mockCore.setFailed).not.toHaveBeenCalled(); + // Verify git operations were used (not gh CLI) + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"]); }); it("should set output to false on checkout failure", async () => { @@ -500,8 +504,8 @@ If the pull request is still open, verify that: // Verify fork detection logging with reason expect(mockCore.info).toHaveBeenCalledWith("Is fork PR: true (different repository names)"); - expect(mockCore.warning).toHaveBeenCalledWith("⚠️ Fork PR detected - gh pr checkout will fetch from fork repository"); - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "github.com" }) })); + expect(mockCore.warning).toHaveBeenCalledWith("⚠️ Fork PR detected - fetching via refs/pull/N/head from origin"); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"]); }); it("should NOT detect fork when repo has fork flag but same full_name", async () => { @@ -516,8 +520,8 @@ If the pull request is still open, verify that: // Same full_name = not a fork PR expect(mockCore.info).toHaveBeenCalledWith("Is fork PR: false (same repository)"); expect(mockCore.warning).not.toHaveBeenCalledWith(expect.stringContaining("Fork PR detected")); - // Still uses gh pr checkout because pull_request_target always does - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "github.com" }) })); + // Still uses git fetch refs/pull because pull_request_target always does + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"]); }); it("should detect non-fork PRs in pull_request_target events", async () => { @@ -543,7 +547,7 @@ If the pull request is still open, verify that: // Verify deleted fork detection expect(mockCore.warning).toHaveBeenCalledWith("⚠️ Head repo information not available (repo may be deleted)"); expect(mockCore.info).toHaveBeenCalledWith("Is fork PR: true (head repository deleted (was likely a fork))"); - expect(mockCore.warning).toHaveBeenCalledWith("⚠️ Fork PR detected - gh pr checkout will fetch from fork repository"); + expect(mockCore.warning).toHaveBeenCalledWith("⚠️ Fork PR detected - fetching via refs/pull/N/head from origin"); }); it("should log detailed PR context with startGroup/endGroup", async () => { @@ -574,23 +578,13 @@ If the pull request is still open, verify that: await runScript(); expect(mockCore.startGroup).toHaveBeenCalledWith("🔄 Checkout Strategy"); - expect(mockCore.info).toHaveBeenCalledWith("Strategy: gh pr checkout"); - expect(mockCore.info).toHaveBeenCalledWith("Reason: pull_request_target runs in base repo context; for fork PRs, head branch doesn't exist in origin"); + expect(mockCore.info).toHaveBeenCalledWith("Strategy: git fetch refs/pull + checkout"); + expect(mockCore.info).toHaveBeenCalledWith("Reason: pull_request_target runs in base repo context; fetching via refs/pull/N/head"); }); - it("should log current branch after successful gh pr checkout", async () => { + it("should log current branch after successful refs/pull checkout", async () => { mockContext.eventName = "issue_comment"; - // Mock the git branch command to return a branch name - mockExec.exec.mockImplementation((cmd, args, options) => { - if (cmd === "git" && args[0] === "branch" && args[1] === "--show-current") { - if (options?.listeners?.stdout) { - options.listeners.stdout(Buffer.from("feature-branch\n")); - } - } - return Promise.resolve(0); - }); - await runScript(); expect(mockCore.info).toHaveBeenCalledWith("Current branch: feature-branch"); @@ -661,16 +655,16 @@ If the pull request is still open, verify that: expect(mockCore.setFailed).not.toHaveBeenCalled(); }); - it("should treat checkout failure as warning for closed PR (gh pr checkout)", async () => { + it("should treat checkout failure as warning for closed PR (refs/pull checkout)", async () => { mockContext.eventName = "issue_comment"; mockContext.payload.pull_request.state = "closed"; - mockExec.exec.mockRejectedValueOnce(new Error("gh pr checkout failed - PR closed")); + mockExec.exec.mockRejectedValueOnce(new Error("git fetch failed - PR closed")); await runScript(); // Should log as warning, not error expect(mockCore.startGroup).toHaveBeenCalledWith("⚠️ Closed PR Checkout Warning"); - expect(mockCore.warning).toHaveBeenCalledWith("Checkout failed (expected for closed PR): gh pr checkout failed - PR closed"); + expect(mockCore.warning).toHaveBeenCalledWith("Checkout failed (expected for closed PR): git fetch failed - PR closed"); // Should NOT fail the step expect(mockCore.setFailed).not.toHaveBeenCalled(); @@ -735,31 +729,12 @@ If the pull request is still open, verify that: }); describe("race condition - PR merged after workflow trigger", () => { - let mockGithub; - - beforeEach(() => { - // Default mock: PR is still open (API confirms what payload says) - mockGithub = { - rest: { - pulls: { - get: vi.fn().mockResolvedValue({ - data: { state: "open", commits: 1, head: { ref: "feature-branch" } }, - }), - }, - }, - }; - global.github = mockGithub; - }); - - afterEach(() => { - delete global.github; - }); - it("should treat checkout failure as warning when PR was merged after workflow triggered", async () => { // PR was "open" in webhook payload, but branch was deleted after merge mockContext.payload.pull_request.state = "open"; mockExec.exec.mockRejectedValueOnce(new Error("fatal: couldn't find remote ref feature-branch")); - // API re-check reveals PR is now closed + // 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({ data: { state: "closed", commits: 1, head: { ref: "feature-branch" } }, }); @@ -796,10 +771,7 @@ If the pull request is still open, verify that: it("should still fail when PR is still open and checkout fails", async () => { mockContext.payload.pull_request.state = "open"; mockExec.exec.mockRejectedValueOnce(new Error("network error")); - // API re-check confirms PR is still open - mockGithub.rest.pulls.get.mockResolvedValueOnce({ - data: { state: "open", commits: 1, head: { ref: "feature-branch" } }, - }); + // Non-fork pull_request: error handler re-check confirms still open (default mock) await runScript(); @@ -813,7 +785,7 @@ If the pull request is still open, verify that: it("should still fail when API re-check itself fails", async () => { mockContext.payload.pull_request.state = "open"; mockExec.exec.mockRejectedValueOnce(new Error("fetch failed")); - // API re-check fails + // Non-fork pull_request: error handler re-check fails const apiError = new Error("API rate limited"); apiError.status = 429; mockGithub.rest.pulls.get.mockRejectedValueOnce(apiError); @@ -832,6 +804,7 @@ If the pull request is still open, verify that: it("should include HTTP status code in API re-check failure warning", async () => { mockContext.payload.pull_request.state = "open"; mockExec.exec.mockRejectedValueOnce(new Error("fetch failed")); + // Non-fork pull_request: error handler re-check fails with 404 const apiError = new Error("Not Found"); apiError.status = 404; mockGithub.rest.pulls.get.mockRejectedValueOnce(apiError); @@ -844,6 +817,7 @@ If the pull request is still open, verify that: it("should omit HTTP status suffix when API error has no status code", async () => { mockContext.payload.pull_request.state = "open"; mockExec.exec.mockRejectedValueOnce(new Error("fetch failed")); + // Non-fork pull_request: error handler re-check fails without status mockGithub.rest.pulls.get.mockRejectedValueOnce(new Error("network timeout")); await runScript(); @@ -858,6 +832,7 @@ If the pull request is still open, verify that: it("should call the GitHub API with the correct PR number and repo", async () => { mockContext.payload.pull_request.state = "open"; mockExec.exec.mockRejectedValueOnce(new Error("fetch failed")); + // Non-fork pull_request: error handler re-check returns closed mockGithub.rest.pulls.get.mockResolvedValueOnce({ data: { state: "closed", commits: 1, head: { ref: "feature-branch" } }, }); @@ -871,14 +846,13 @@ If the pull request is still open, verify that: }); }); - it("should handle race condition for gh pr checkout path (issue_comment event)", async () => { + it("should handle race condition for refs/pull checkout path (issue_comment event)", async () => { mockContext.eventName = "issue_comment"; mockContext.payload.pull_request.state = "open"; - mockExec.exec.mockRejectedValueOnce(new Error("gh pr checkout failed - PR closed")); - // API re-check shows PR was merged - mockGithub.rest.pulls.get.mockResolvedValueOnce({ - data: { state: "closed", commits: 1, head: { ref: "feature-branch" } }, - }); + // First pulls.get (fetchPRDetails): succeeds + // Second pulls.get (re-check): shows PR was merged + mockGithub.rest.pulls.get.mockResolvedValueOnce({ data: { state: "open", commits: 1, head: { ref: "feature-branch" } } }).mockResolvedValueOnce({ data: { state: "closed", commits: 1, head: { ref: "feature-branch" } } }); + mockExec.exec.mockRejectedValueOnce(new Error("git fetch failed - PR closed")); await runScript(); @@ -888,56 +862,4 @@ If the pull request is still open, verify that: expect(mockCore.setFailed).not.toHaveBeenCalled(); }); }); - - describe("GH_HOST override for gh pr checkout", () => { - it("should override DIFC proxy GH_HOST (localhost:18443) with actual GitHub host", async () => { - const previousGhHost = process.env.GH_HOST; - - try { - // Simulate active DIFC proxy that set GH_HOST=localhost:18443 in env - process.env.GH_HOST = "localhost:18443"; - process.env.GITHUB_SERVER_URL = "https://github.com"; - mockContext.eventName = "issue_comment"; - - await runScript(); - - // GH_HOST should be overridden to github.com, not localhost:18443 - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "github.com" }) })); - } finally { - if (previousGhHost === undefined) { - delete process.env.GH_HOST; - } else { - process.env.GH_HOST = previousGhHost; - } - } - }); - - it("should use GHE host from GITHUB_SERVER_URL for gh pr checkout", async () => { - process.env.GITHUB_SERVER_URL = "https://myorg.ghe.com"; - mockContext.eventName = "pull_request_target"; - - await runScript(); - - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "myorg.ghe.com" }) })); - }); - - it("should strip https:// protocol from GITHUB_SERVER_URL when deriving GH_HOST", async () => { - process.env.GITHUB_SERVER_URL = "https://github.com"; - mockContext.eventName = "pull_request_target"; - - await runScript(); - - // Should not include the protocol in GH_HOST - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "github.com" }) })); - }); - - it("should default to github.com when GITHUB_SERVER_URL is not set", async () => { - delete process.env.GITHUB_SERVER_URL; - mockContext.eventName = "issue_comment"; - - await runScript(); - - expect(mockExec.exec).toHaveBeenCalledWith("gh", ["pr", "checkout", "123"], expect.objectContaining({ env: expect.objectContaining({ GH_HOST: "github.com" }) })); - }); - }); }); diff --git a/actions/setup/js/git_helpers.cjs b/actions/setup/js/git_helpers.cjs index 29feca1924b..70becf63bb2 100644 --- a/actions/setup/js/git_helpers.cjs +++ b/actions/setup/js/git_helpers.cjs @@ -114,39 +114,7 @@ function execGitSync(args, options = {}) { return result.stdout; } -/** - * Derive the real GitHub hostname from GITHUB_SERVER_URL. - * - * When the DIFC proxy is active, GH_HOST is overridden to localhost:18443 - * in GITHUB_ENV. This causes `gh` CLI commands that resolve the repository - * from git remotes (e.g. `gh pr checkout`) to fail because the proxy address - * doesn't match any remote. Use this helper to get the actual GitHub host - * (e.g. "github.com" or "myorg.ghe.com") for per-call GH_HOST overrides. - * - * @returns {string} The GitHub hostname (e.g. "github.com") - */ -function getGitHubHost() { - const serverUrl = process.env.GITHUB_SERVER_URL || "https://github.com"; - return serverUrl.replace(/^https?:\/\/|\/+$/g, ""); -} - -/** - * Build environment variables for a `gh` CLI exec call with the correct GH_HOST. - * - * Spreads process.env and any additional environment variables, then enforces - * GH_HOST to the real GitHub hostname derived from GITHUB_SERVER_URL. Use this - * for any `gh` CLI call that needs to bypass a DIFC proxy GH_HOST override. - * - * @param {Object} [extraEnv] - Additional environment variables to set (e.g. { GH_TOKEN: token }) - * @returns {Object} Environment object suitable for exec.exec options - */ -function getGhEnvBypassingIntegrityFilteringForGitOps(extraEnv) { - return { ...process.env, ...extraEnv, GH_HOST: getGitHubHost() }; -} - module.exports = { execGitSync, - getGhEnvBypassingIntegrityFilteringForGitOps, getGitAuthEnv, - getGitHubHost, };