diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 532df3b1249..373245a58fd 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -30,7 +30,7 @@ const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { checkFileProtection, checkFileProtectionPostApply } = require("./manifest_file_helpers.cjs"); const { renderTemplateFromFile, renderFilesList, buildProtectedFileList, getPromptPath } = require("./messages_core.cjs"); -const { overridePersistedExtraheader, restorePersistedExtraheader } = require("./git_auth_helpers.cjs"); +const { withGitHubHostToken } = require("./git_auth_helpers.cjs"); const { COPILOT_REVIEWER_BOT, FAQ_CREATE_PR_PERMISSIONS_URL } = require("./constants.cjs"); const { isStagedMode } = require("./safe_output_helpers.cjs"); const { normalizeCommitSHA } = require("./commit_sha_helpers.cjs"); @@ -567,27 +567,18 @@ function enforcePullRequestLimits(patchContent, maxFiles = MAX_FILES) { * @param {string} [options.repo] - Repository name for the deleteRef call. * @param {string} [options.remoteTarget] - Remote name or URL used for remote branch existence checks. * @param {string} [options.remoteToken] - Optional token used for authenticated remote branch checks. + * @param {string} [options.cwd] - Optional working directory for git operations; scopes git config overrides to the correct checkout. * @returns {Promise} The (possibly renamed) branch name to use going forward. */ async function handleRemoteBranchCollision(branchName, preserveBranchName, options = {}) { + const cwd = options.cwd; let remoteBranchExists = false; try { const remoteTarget = options.remoteTarget || "origin"; - const checkRemoteBranch = async () => exec.getExecOutput("git", ["ls-remote", "--heads", remoteTarget, branchName]); + const checkRemoteBranch = async () => exec.getExecOutput("git", ["ls-remote", "--heads", remoteTarget, branchName], cwd ? { cwd } : {}); let checkResult; if (options.remoteToken) { - const githubServerUrl = (process.env.GITHUB_SERVER_URL || "https://github.com").replace(/\/+$/, ""); - let previousExtraheaders = []; - let overrideApplied = false; - try { - previousExtraheaders = await overridePersistedExtraheader(githubServerUrl, options.remoteToken); - overrideApplied = true; - checkResult = await checkRemoteBranch(); - } finally { - if (overrideApplied) { - await restorePersistedExtraheader(githubServerUrl, previousExtraheaders); - } - } + checkResult = await withGitHubHostToken(options.remoteToken, checkRemoteBranch, cwd); } else { checkResult = await checkRemoteBranch(); } @@ -659,7 +650,7 @@ async function handleRemoteBranchCollision(branchName, preserveBranchName, optio const oldBranch = branchName; const renamedBranch = `${branchName}-${extraHex}`; // Rename local branch - await exec.exec("git", ["branch", "-m", oldBranch, renamedBranch]); + await exec.exec("git", ["branch", "-m", oldBranch, renamedBranch], cwd ? { cwd } : {}); core.info(`Renamed branch to ${renamedBranch}`); return renamedBranch; } @@ -1638,7 +1629,8 @@ async function main(config = {}) { // fallback issue can include a compare URL. Genuine push failures are handled in // the catch block below. { - try { + const forkCwd = process.cwd(); + const runBundlePush = async () => { branchName = await handleRemoteBranchCollision(branchName, preserveBranchName, { recreateRef, githubClient: pushGithubClient, @@ -1646,6 +1638,7 @@ async function main(config = {}) { repo: pushRepoParts.repo, remoteTarget: pushRemoteUrl || "origin", remoteToken: headGitHubToken, + cwd: forkCwd, }); await pushSignedCommits({ @@ -1654,7 +1647,7 @@ async function main(config = {}) { repo: pushRepoParts.repo, branch: branchName, baseRef: `origin/${baseBranch}`, - cwd: process.cwd(), + cwd: forkCwd, pushRemoteUrl, pushToken: headGitHubToken, signedCommits, @@ -1662,6 +1655,9 @@ async function main(config = {}) { currentRepo: itemRepo, validationConfig: config, }); + }; + try { + await runBundlePush(); core.info("Changes pushed to branch (from bundle)"); // Count new commits on PR branch relative to base @@ -1683,20 +1679,22 @@ async function main(config = {}) { core.warning("Signed push rejected merge commit topology from bundle; rewriting branch and retrying signed push"); try { await rewriteBundleBranchAsSingleCommit(baseBranch, exec); - await pushSignedCommits({ - githubClient: pushGithubClient, - owner: pushRepoParts.owner, - repo: pushRepoParts.repo, - branch: branchName, - baseRef: `origin/${baseBranch}`, - cwd: process.cwd(), - pushRemoteUrl, - pushToken: headGitHubToken, - signedCommits, - resolvedTemporaryIds, - currentRepo: itemRepo, - validationConfig: config, - }); + const runRetryPush = async () => + pushSignedCommits({ + githubClient: pushGithubClient, + owner: pushRepoParts.owner, + repo: pushRepoParts.repo, + branch: branchName, + baseRef: `origin/${baseBranch}`, + cwd: forkCwd, + pushRemoteUrl, + pushToken: headGitHubToken, + signedCommits, + resolvedTemporaryIds, + currentRepo: itemRepo, + validationConfig: config, + }); + await runRetryPush(); core.info("Changes pushed to branch after bundle rewrite retry"); try { @@ -2025,7 +2023,8 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${getPullRequestHead // fallback issue can include a compare URL. Genuine push failures are handled in // the catch block below. { - try { + const forkCwd = process.cwd(); + const runPatchPush = async () => { branchName = await handleRemoteBranchCollision(branchName, preserveBranchName, { recreateRef, githubClient: pushGithubClient, @@ -2033,6 +2032,7 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${getPullRequestHead repo: pushRepoParts.repo, remoteTarget: pushRemoteUrl || "origin", remoteToken: headGitHubToken, + cwd: forkCwd, }); await pushSignedCommits({ @@ -2041,7 +2041,7 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${getPullRequestHead repo: pushRepoParts.repo, branch: branchName, baseRef: `origin/${baseBranch}`, - cwd: process.cwd(), + cwd: forkCwd, pushRemoteUrl, pushToken: headGitHubToken, signedCommits, @@ -2049,6 +2049,9 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${getPullRequestHead currentRepo: itemRepo, validationConfig: config, }); + }; + try { + await runPatchPush(); core.info("Changes pushed to branch"); // Count new commits on PR branch relative to base, used to restrict @@ -2196,29 +2199,34 @@ ${patchPreview}`; await exec.exec(`git commit --allow-empty -m "Initialize"`); core.info("Created empty commit"); - branchName = await handleRemoteBranchCollision(branchName, preserveBranchName, { - recreateRef, - githubClient: pushGithubClient, - owner: pushRepoParts.owner, - repo: pushRepoParts.repo, - remoteTarget: pushRemoteUrl || "origin", - remoteToken: headGitHubToken, - }); + const forkCwd = process.cwd(); + const runEmptyPush = async () => { + branchName = await handleRemoteBranchCollision(branchName, preserveBranchName, { + recreateRef, + githubClient: pushGithubClient, + owner: pushRepoParts.owner, + repo: pushRepoParts.repo, + remoteTarget: pushRemoteUrl || "origin", + remoteToken: headGitHubToken, + cwd: forkCwd, + }); - await pushSignedCommits({ - githubClient: pushGithubClient, - owner: pushRepoParts.owner, - repo: pushRepoParts.repo, - branch: branchName, - baseRef: `origin/${baseBranch}`, - cwd: process.cwd(), - pushRemoteUrl, - pushToken: headGitHubToken, - signedCommits, - resolvedTemporaryIds, - currentRepo: itemRepo, - validationConfig: config, - }); + await pushSignedCommits({ + githubClient: pushGithubClient, + owner: pushRepoParts.owner, + repo: pushRepoParts.repo, + branch: branchName, + baseRef: `origin/${baseBranch}`, + cwd: forkCwd, + pushRemoteUrl, + pushToken: headGitHubToken, + signedCommits, + resolvedTemporaryIds, + currentRepo: itemRepo, + validationConfig: config, + }); + }; + await runEmptyPush(); core.info("Empty branch pushed successfully"); // Count new commits (will be 1 from the Initialize commit) diff --git a/actions/setup/js/git_auth_helpers.cjs b/actions/setup/js/git_auth_helpers.cjs index 7e1ee292b70..0208fd81e1d 100644 --- a/actions/setup/js/git_auth_helpers.cjs +++ b/actions/setup/js/git_auth_helpers.cjs @@ -144,8 +144,41 @@ async function restorePersistedExtraheader(serverUrl, previousValues, cwd) { core.info(`git_auth_helpers: extraheader restored`); } +/** + * Temporarily override the persisted GitHub extraheader for remote git operations. + * + * Saves the current extraheader value(s), replaces them with the fork token for the + * duration of the callback, then restores the original value(s). This ensures that + * only one Authorization source is active at a time, preventing duplicate-header HTTP 400s + * when a fork token and the checkout-persisted upstream token both apply to the same host. + * + * @template T + * @param {string} token + * @param {() => Promise} callback + * @param {string} [cwd] - Optional working directory; scopes the git config override to the correct checkout + * @returns {Promise} + */ +async function withGitHubHostToken(token, callback, cwd) { + if (!token) { + return callback(); + } + const githubServerUrl = (process.env.GITHUB_SERVER_URL || "https://github.com").replace(/\/+$/, ""); + let previousExtraheaders = []; + let overrideApplied = false; + try { + previousExtraheaders = await overridePersistedExtraheader(githubServerUrl, token, cwd); + overrideApplied = true; + return await callback(); + } finally { + if (overrideApplied) { + await restorePersistedExtraheader(githubServerUrl, previousExtraheaders, cwd); + } + } +} + module.exports = { checkoutHasPersistedExtraheader, overridePersistedExtraheader, restorePersistedExtraheader, + withGitHubHostToken, }; diff --git a/actions/setup/js/git_auth_helpers.test.cjs b/actions/setup/js/git_auth_helpers.test.cjs index b8c5ae3e526..9faafcd4d8c 100644 --- a/actions/setup/js/git_auth_helpers.test.cjs +++ b/actions/setup/js/git_auth_helpers.test.cjs @@ -6,6 +6,7 @@ describe("git_auth_helpers.cjs", () => { let checkoutHasPersistedExtraheader; let overridePersistedExtraheader; let restorePersistedExtraheader; + let withGitHubHostToken; const SERVER_URL = "https://github.com"; const EXTRAHEADER_KEY = "http.https://github.com/.extraheader"; @@ -25,7 +26,7 @@ describe("git_auth_helpers.cjs", () => { global.exec = mockExec; delete require.cache[require.resolve("./git_auth_helpers.cjs")]; - ({ checkoutHasPersistedExtraheader, overridePersistedExtraheader, restorePersistedExtraheader } = require("./git_auth_helpers.cjs")); + ({ checkoutHasPersistedExtraheader, overridePersistedExtraheader, restorePersistedExtraheader, withGitHubHostToken } = require("./git_auth_helpers.cjs")); }); afterEach(() => { @@ -223,4 +224,109 @@ describe("git_auth_helpers.cjs", () => { await expect(restorePersistedExtraheader(SERVER_URL, null)).resolves.toBeUndefined(); }); }); + + // ────────────────────────────────────────────────────── + // withGitHubHostToken + // ────────────────────────────────────────────────────── + + describe("withGitHubHostToken", () => { + it("should call the callback without any git config changes when token is empty", async () => { + let callbackCalled = false; + await withGitHubHostToken("", async () => { + callbackCalled = true; + }); + expect(callbackCalled).toBe(true); + expect(mockExec.exec).not.toHaveBeenCalled(); + }); + + it("should call the callback without any git config changes when token is undefined", async () => { + let callbackCalled = false; + // @ts-expect-error intentional undefined test + await withGitHubHostToken(undefined, async () => { + callbackCalled = true; + }); + expect(callbackCalled).toBe(true); + expect(mockExec.exec).not.toHaveBeenCalled(); + }); + + it("should override extraheader with fork token before calling callback", async () => { + const token = "fork-token"; + const expectedHeader = `Authorization: basic ${Buffer.from(`x-access-token:${token}`).toString("base64")}`; + const execCalls = []; + + mockExec.exec.mockImplementation(async (_cmd, args) => { + execCalls.push(args); + return 0; + }); + + await withGitHubHostToken(token, async () => { + // Inside callback the override should already be applied + const overrideCall = execCalls.find(a => a[1] === "--replace-all"); + expect(overrideCall).toBeDefined(); + expect(overrideCall[3]).toBe(expectedHeader); + }); + }); + + it("should restore the previous extraheader after the callback completes", async () => { + const upstreamHeader = `Authorization: basic ${Buffer.from("x-access-token:upstream").toString("base64")}`; + mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: upstreamHeader + "\n", stderr: "" }); + + await withGitHubHostToken("fork-token", async () => {}); + + // After callback, the last --replace-all should restore the upstream header + const replaceAllCalls = mockExec.exec.mock.calls.filter(c => c[1][1] === "--replace-all"); + expect(replaceAllCalls.length).toBeGreaterThanOrEqual(2); + const restoreCall = replaceAllCalls[replaceAllCalls.length - 1]; + expect(restoreCall[1][3]).toBe(upstreamHeader); + }); + + it("should restore extraheader even when the callback throws", async () => { + const upstreamHeader = `Authorization: basic ${Buffer.from("x-access-token:upstream").toString("base64")}`; + mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: upstreamHeader + "\n", stderr: "" }); + + const callbackError = new Error("push failed"); + await expect( + withGitHubHostToken("fork-token", async () => { + throw callbackError; + }) + ).rejects.toThrow(callbackError); + + // Restore must still run after the callback throws — the last --replace-all is the restore + const replaceAllCalls = mockExec.exec.mock.calls.filter(c => c[1][1] === "--replace-all"); + expect(replaceAllCalls.length).toBeGreaterThanOrEqual(2); + const restoreCall = replaceAllCalls[replaceAllCalls.length - 1]; + expect(restoreCall[1][3]).toBe(upstreamHeader); + }); + + it("should unset extraheader after callback when no previous value existed", async () => { + mockExec.getExecOutput.mockResolvedValue({ exitCode: 1, stdout: "", stderr: "" }); + + await withGitHubHostToken("fork-token", async () => {}); + + const unsetCall = mockExec.exec.mock.calls.find(c => c[1][1] === "--unset-all"); + expect(unsetCall).toBeDefined(); + expect(unsetCall[1][2]).toBe(EXTRAHEADER_KEY); + }); + + it("should return the callback's return value", async () => { + const result = await withGitHubHostToken("fork-token", async () => "expected-result"); + expect(result).toBe("expected-result"); + }); + + it("should pass cwd to overridePersistedExtraheader and restorePersistedExtraheader", async () => { + const cwd = "/some/repo"; + const upstreamHeader = `Authorization: basic ${Buffer.from("x-access-token:upstream").toString("base64")}`; + mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: upstreamHeader + "\n", stderr: "" }); + + await withGitHubHostToken("fork-token", async () => {}, cwd); + + // All git config calls should include cwd + for (const call of mockExec.exec.mock.calls) { + expect(call[2]).toMatchObject({ cwd }); + } + for (const call of mockExec.getExecOutput.mock.calls) { + expect(call[2]).toMatchObject({ cwd }); + } + }); + }); }); diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 2ac9e1e2294..e29228d2487 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -17,7 +17,7 @@ const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { checkFileProtection, checkFileProtectionPostApply } = require("./manifest_file_helpers.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { renderTemplateFromFile, buildProtectedFileList, getPromptPath } = require("./messages_core.cjs"); -const { overridePersistedExtraheader, restorePersistedExtraheader } = require("./git_auth_helpers.cjs"); +const { withGitHubHostToken } = require("./git_auth_helpers.cjs"); const { ensureFullHistoryForBundle, extractBundlePrerequisiteCommits, isShallowOrSparseCheckout, linearizeRangeAsCommit, ensureSafeDirectoryTrust } = require("./git_helpers.cjs"); const { normalizeCommitSHA } = require("./commit_sha_helpers.cjs"); const { findRepoCheckout } = require("./find_repo_checkout.cjs"); @@ -103,33 +103,6 @@ function parsePositiveInteger(value) { return Number.isInteger(parsed) && parsed > 0 ? parsed : null; } -/** - * Temporarily override the persisted GitHub extraheader for remote git operations. - * - * @template T - * @param {string} token - * @param {() => Promise} callback - * @param {string} [cwd] - Optional working directory; scopes the git config override to the correct checkout - * @returns {Promise} - */ -async function withGitHubHostToken(token, callback, cwd) { - if (!token) { - return callback(); - } - const githubServerUrl = (process.env.GITHUB_SERVER_URL || "https://github.com").replace(/\/+$/, ""); - let previousExtraheaders = []; - let overrideApplied = false; - try { - previousExtraheaders = await overridePersistedExtraheader(githubServerUrl, token, cwd); - overrideApplied = true; - return await callback(); - } finally { - if (overrideApplied) { - await restorePersistedExtraheader(githubServerUrl, previousExtraheaders, cwd); - } - } -} - /** * Uses git as the source of truth for the files modified by a fetched bundle ref. *