diff --git a/actions/setup/js/extra_empty_commit.integration.test.cjs b/actions/setup/js/extra_empty_commit.integration.test.cjs index b8687196755..3c9ce1f69d0 100644 --- a/actions/setup/js/extra_empty_commit.integration.test.cjs +++ b/actions/setup/js/extra_empty_commit.integration.test.cjs @@ -105,6 +105,11 @@ describe("extra_empty_commit git integration", () => { if (args[0] === "config" && args[1] === "--get-all") { return { exitCode: 1, stdout: "", stderr: "" }; } + // Handle git config --global/--local --unset-all calls from unsetExtraheaderAllScopes + // Return exit code 5 (key absent) since this test simulates no pre-existing extraheaders + if (args[0] === "config" && (args[1] === "--global" || args[1] === "--local") && args[2] === "--unset-all") { + return { exitCode: 5, stdout: "", stderr: "" }; + } // Return the actual HEAD SHA for rev-parse (needed for GraphQL createCommitOnBranch) if (args[0] === "rev-parse" && args[1] === "HEAD") { const result = execGit(["rev-parse", "HEAD"], repoDir); diff --git a/actions/setup/js/extra_empty_commit.test.cjs b/actions/setup/js/extra_empty_commit.test.cjs index 984396480b3..0669ef6993f 100644 --- a/actions/setup/js/extra_empty_commit.test.cjs +++ b/actions/setup/js/extra_empty_commit.test.cjs @@ -26,7 +26,15 @@ describe("extra_empty_commit.cjs", () => { // Default exec mock: resolves successfully, no stdout output mockExec = { exec: vi.fn().mockResolvedValue(0), - getExecOutput: vi.fn().mockResolvedValue({ stdout: "", stderr: "", exitCode: 1 }), + getExecOutput: vi.fn().mockImplementation(async (_cmd, args) => { + // Handle git config --global/--local --unset-all calls from unsetExtraheaderAllScopes + // Return exit code 5 (key absent) since most tests simulate no pre-existing extraheaders + if (args && args[0] === "config" && (args[1] === "--global" || args[1] === "--local") && args[2] === "--unset-all") { + return { exitCode: 5, stdout: "", stderr: "" }; + } + // Default: return exit code 1 (generic failure) for other getExecOutput calls + return { stdout: "", stderr: "", exitCode: 1 }; + }), }; global.core = mockCore; @@ -177,25 +185,34 @@ describe("extra_empty_commit.cjs", () => { const execCalls = mockExec.exec.mock.calls; // Use findIndex so the assertion stays correct even if restorePersistedExtraheader // also emits a --replace-all call (e.g. when restoring previous values). - const replaceIdx = execCalls.findIndex(c => c[0] === "git" && c[1] && c[1][0] === "config" && c[1][1] === "--replace-all"); + const replaceIdx = execCalls.findIndex(c => c[0] === "git" && c[1] && c[1][0] === "config" && c[1][1] === "--local" && c[1][2] === "--replace-all"); expect(replaceIdx).toBeGreaterThanOrEqual(0); const replaceCall = execCalls[replaceIdx]; - expect(replaceCall[1][2]).toBe("http.https://github.com/.extraheader"); - expect(replaceCall[1][3]).toBe(`Authorization: basic ${Buffer.from("x-access-token:ghp_test_token_123").toString("base64")}`); + expect(replaceCall[1][3]).toBe("http.https://github.com/.extraheader"); + expect(replaceCall[1][4]).toBe(`Authorization: basic ${Buffer.from("x-access-token:ghp_test_token_123").toString("base64")}`); const pushIdx = execCalls.findIndex(c => c[0] === "git" && c[1] && c[1][0] === "push"); expect(pushIdx).toBeGreaterThan(-1); expect(replaceIdx).toBeLessThan(pushIdx); - const unsetCall = execCalls.find(c => c[0] === "git" && c[1] && c[1][0] === "config" && c[1][1] === "--unset-all"); + // After push, restoration clears credentials via getExecOutput (unsetExtraheaderAllScopes) + // When there are no previous values to restore, only the unset happens. + const getExecOutputCalls = mockExec.getExecOutput.mock.calls; + const unsetCall = getExecOutputCalls.find(c => c[0] === "git" && c[1] && c[1][0] === "config" && c[1][2] === "--unset-all"); expect(unsetCall).toBeDefined(); - expect(execCalls.indexOf(unsetCall)).toBeGreaterThan(pushIdx); }); it("should restore a previously-set extraheader after push", async () => { const prevHeader = `Authorization: basic ${Buffer.from("x-access-token:old_token").toString("base64")}`; // Override getExecOutput so it returns the previous header when reading the extraheader key. - mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: prevHeader + "\n", stderr: "" }); + mockExec.getExecOutput.mockImplementation(async (cmd, args) => { + // Handle git config --global/--local --unset-all calls + if (args && args[0] === "config" && (args[1] === "--global" || args[1] === "--local") && args[2] === "--unset-all") { + return { exitCode: 5, stdout: "", stderr: "" }; + } + // Return previous header for --get-all + return { exitCode: 0, stdout: prevHeader + "\n", stderr: "" }; + }); await pushExtraEmptyCommit({ branchName: "feature-branch", @@ -207,10 +224,10 @@ describe("extra_empty_commit.cjs", () => { const pushIdx = execCalls.findIndex(c => c[0] === "git" && c[1] && c[1][0] === "push"); expect(pushIdx).toBeGreaterThan(-1); - // After push, there should be a --replace-all that restores the old header. - const restoreCall = execCalls.find((c, i) => i > pushIdx && c[0] === "git" && c[1] && c[1][0] === "config" && c[1][1] === "--replace-all"); + // After push, there should be a --local --replace-all that restores the old header. + const restoreCall = execCalls.find((c, i) => i > pushIdx && c[0] === "git" && c[1] && c[1][0] === "config" && c[1][1] === "--local" && c[1][2] === "--replace-all"); expect(restoreCall).toBeDefined(); - expect(restoreCall[1][3]).toBe(prevHeader); + expect(restoreCall[1][4]).toBe(prevHeader); }); it("should restore extraheader even when push throws", async () => { @@ -232,13 +249,14 @@ describe("extra_empty_commit.cjs", () => { const execCalls = mockExec.exec.mock.calls; - // The override (--replace-all with CI token) must have happened before the failed push. - const overrideIdx = execCalls.findIndex(c => c[0] === "git" && c[1] && c[1][0] === "config" && c[1][1] === "--replace-all"); + // The override (--local --replace-all with CI token) must have happened before the failed push. + const overrideIdx = execCalls.findIndex(c => c[0] === "git" && c[1] && c[1][0] === "config" && c[1][1] === "--local" && c[1][2] === "--replace-all"); expect(overrideIdx).toBeGreaterThanOrEqual(0); // After the failed push, the finally block must restore by unsetting the key - // (default mock returns no previous extraheader). - const unsetCall = execCalls.find((c, i) => i > overrideIdx && c[0] === "git" && c[1] && c[1][0] === "config" && c[1][1] === "--unset-all"); + // (default mock returns no previous extraheader). Check getExecOutput calls. + const getExecOutputCalls = mockExec.getExecOutput.mock.calls; + const unsetCall = getExecOutputCalls.find(c => c[0] === "git" && c[1] && c[1][0] === "config" && c[1][2] === "--unset-all"); expect(unsetCall).toBeDefined(); }); @@ -396,6 +414,10 @@ describe("extra_empty_commit.cjs", () => { if (cmd === "git" && args && args[0] === "rev-parse" && args[1] === "HEAD") { return { exitCode: 0, stdout: TEST_HEAD_OID + "\n", stderr: "" }; } + // Handle git config --global/--local --unset-all calls + if (args && args[0] === "config" && (args[1] === "--global" || args[1] === "--local") && args[2] === "--unset-all") { + return { exitCode: 5, stdout: "", stderr: "" }; + } // No pre-existing extraheader return { exitCode: 1, stdout: "", stderr: "" }; }); @@ -529,11 +551,12 @@ describe("extra_empty_commit.cjs", () => { }); const execCalls = mockExec.exec.mock.calls; - // Override was applied before the API commit - const overrideCall = execCalls.find(c => c[0] === "git" && c[1] && c[1][0] === "config" && c[1][1] === "--replace-all"); + // Override was applied before the API commit (--local --replace-all) + const overrideCall = execCalls.find(c => c[0] === "git" && c[1] && c[1][0] === "config" && c[1][1] === "--local" && c[1][2] === "--replace-all"); expect(overrideCall).toBeDefined(); - // After the API commit, --unset-all restores to empty (no previous header) - const unsetCall = execCalls.find(c => c[0] === "git" && c[1] && c[1][0] === "config" && c[1][1] === "--unset-all"); + // After the API commit, unsetExtraheaderAllScopes is called (via getExecOutput) + const getExecOutputCalls = mockExec.getExecOutput.mock.calls; + const unsetCall = getExecOutputCalls.find(c => c[0] === "git" && c[1] && c[1][0] === "config" && c[1][2] === "--unset-all"); expect(unsetCall).toBeDefined(); }); }); diff --git a/actions/setup/js/git_auth_helpers.cjs b/actions/setup/js/git_auth_helpers.cjs index 0208fd81e1d..81c5585eaf3 100644 --- a/actions/setup/js/git_auth_helpers.cjs +++ b/actions/setup/js/git_auth_helpers.cjs @@ -16,6 +16,37 @@ function normalizeServerUrl(serverUrl) { return serverUrl.replace(/\/+$/, ""); } +/** + * Unset the given git config key from all writable scopes (global and local). + * Errors are silently ignored because the key may be absent in some scopes + * or a scope may not be accessible (e.g. read-only system scope on CI runners). + * + * This is necessary because `actions/checkout` typically writes credentials to + * the global scope, and a plain `--replace-all` without an explicit scope flag + * only removes values from the local scope, leaving the global value in place + * and causing duplicate Authorization header HTTP 400 errors. + * + * @param {string} key - Full git config key (e.g. "http.https://github.com/.extraheader") + * @param {string} [cwd] - Optional working directory for local-scope operations + * @returns {Promise} + */ +async function unsetExtraheaderAllScopes(key, cwd) { + // Clear both the global scope (typically written by actions/checkout) and the + // local scope (written by this helper). The system scope is intentionally + // skipped because it is usually read-only in CI environments. + const scopes = ["--global", "--local"]; + for (const scope of scopes) { + const opts = { silent: true, ignoreReturnCode: true, ...(cwd ? { cwd } : {}) }; + const result = await exec.getExecOutput("git", ["config", scope, "--unset-all", key], opts); + // Exit code 5 means the key was absent in this scope — expected and safe to ignore. + // Any other non-zero exit code indicates a real failure (e.g., permission denied, + // write lock). Throw so the caller knows the credential may still be effective. + if (result.exitCode !== 0 && result.exitCode !== 5) { + throw new Error(`git config ${scope} --unset-all ${key} failed (exit ${result.exitCode}): ${result.stderr.trim()}`); + } + } +} + /** * Get all configured values for http./.extraheader. * Throws if `exec.getExecOutput` itself throws (e.g. git not available). @@ -75,10 +106,18 @@ async function overridePersistedExtraheader(serverUrl, token, cwd) { } core.info(`git_auth_helpers: overriding http.${normalizedUrl}/.extraheader with CI trigger token`); const tokenBase64 = Buffer.from(`x-access-token:${token.trim()}`).toString("base64"); + const authHeader = `Authorization: basic ${tokenBase64}`; + + // Clear from ALL writable scopes before writing our token to prevent duplicate + // Authorization headers. actions/checkout writes to the global scope; without + // this step a plain --replace-all only writes to the local scope, leaving the + // global value in place and causing duplicate-header HTTP 400 errors. + await unsetExtraheaderAllScopes(`http.${normalizedUrl}/.extraheader`, cwd); + if (cwd) { - await exec.exec("git", ["config", "--replace-all", `http.${normalizedUrl}/.extraheader`, `Authorization: basic ${tokenBase64}`], { cwd }); + await exec.exec("git", ["config", "--local", "--replace-all", `http.${normalizedUrl}/.extraheader`, authHeader], { cwd }); } else { - await exec.exec("git", ["config", "--replace-all", `http.${normalizedUrl}/.extraheader`, `Authorization: basic ${tokenBase64}`]); + await exec.exec("git", ["config", "--local", "--replace-all", `http.${normalizedUrl}/.extraheader`, authHeader]); } core.info(`git_auth_helpers: extraheader override applied`); return previousValues; @@ -94,48 +133,43 @@ async function overridePersistedExtraheader(serverUrl, token, cwd) { */ async function restorePersistedExtraheader(serverUrl, previousValues, cwd) { const key = `http.${normalizeServerUrl(serverUrl)}/.extraheader`; + + // Clear from ALL writable scopes first. This removes the override written by + // overridePersistedExtraheader and prevents values from accumulating across + // multiple override/restore cycles (which happens when global scope values + // are not cleared and get re-read by getExtraheaderValues on each attempt). + await unsetExtraheaderAllScopes(key, cwd); + if (!previousValues || previousValues.length === 0) { - core.info(`git_auth_helpers: no previous extraheader values — unsetting ${key}`); - try { - if (cwd) { - await exec.exec("git", ["config", "--unset-all", key], { cwd }); - } else { - await exec.exec("git", ["config", "--unset-all", key]); - } - } catch { - // Nothing to restore/unset. - } + core.info(`git_auth_helpers: no previous extraheader values — ${key} cleared`); return; } core.info(`git_auth_helpers: restoring ${previousValues.length} previous extraheader value(s) for ${key}`); - // --replace-all removes any existing values for the key (including the CI-token - // entry) and writes previousValues[0]. Subsequent --add calls stack the remaining - // previous values onto the same key without removing any already written. - // If any --add call fails after --replace-all has already run, git config is left + // Restore to the local scope. Even when the original values were written to + // the global scope by actions/checkout, restoring to local is functionally + // equivalent: with the global scope cleared the restored local value is the + // only source, so no duplicate headers can arise on subsequent retries. + // If any call fails after --replace-all has already run, git config is left // in a partially-restored state. In that case we log a warning and attempt a - // best-effort cleanup by unsetting the key entirely, then re-throw so the caller - // is aware that restoration failed. + // best-effort cleanup, then re-throw so the caller is aware that restoration + // failed. try { if (cwd) { - await exec.exec("git", ["config", "--replace-all", key, previousValues[0]], { cwd }); + await exec.exec("git", ["config", "--local", "--replace-all", key, previousValues[0]], { cwd }); for (const value of previousValues.slice(1)) { - await exec.exec("git", ["config", "--add", key, value], { cwd }); + await exec.exec("git", ["config", "--local", "--add", key, value], { cwd }); } } else { - await exec.exec("git", ["config", "--replace-all", key, previousValues[0]]); + await exec.exec("git", ["config", "--local", "--replace-all", key, previousValues[0]]); for (const value of previousValues.slice(1)) { - await exec.exec("git", ["config", "--add", key, value]); + await exec.exec("git", ["config", "--local", "--add", key, value]); } } } catch (err) { core.warning(`git_auth_helpers: partial extraheader restore for ${key} — attempting cleanup: ${getErrorMessage(err)}`); try { - if (cwd) { - await exec.exec("git", ["config", "--unset-all", key], { cwd }); - } else { - await exec.exec("git", ["config", "--unset-all", key]); - } + await unsetExtraheaderAllScopes(key, cwd); } catch { // Best-effort only; ignore secondary failure. } @@ -180,5 +214,6 @@ module.exports = { checkoutHasPersistedExtraheader, overridePersistedExtraheader, restorePersistedExtraheader, + unsetExtraheaderAllScopes, withGitHubHostToken, }; diff --git a/actions/setup/js/git_auth_helpers.integration.test.cjs b/actions/setup/js/git_auth_helpers.integration.test.cjs new file mode 100644 index 00000000000..8393f1324e4 --- /dev/null +++ b/actions/setup/js/git_auth_helpers.integration.test.cjs @@ -0,0 +1,350 @@ +// Integration tests for git_auth_helpers.cjs. +// +// These tests use real git repositories so that --get-all, --unset-all, +// --replace-all, and --add operations interact with the same on-disk state. +// Global git config is isolated via GIT_CONFIG_GLOBAL so no test can pollute +// the developer's real ~/.gitconfig. +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import { spawnSync } from "child_process"; +import { createRequire } from "module"; + +const require = createRequire(import.meta.url); + +const SERVER_URL = "https://github.com"; +const EXTRAHEADER_KEY = `http.${SERVER_URL}/.extraheader`; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** + * Run a real git command with GIT_CONFIG_GLOBAL pointing at the isolated + * config file. Returns the spawnSync result; never throws on non-zero exit. + */ +function runGit(args, repoDir, globalConfigPath) { + const env = { ...process.env, GIT_CONFIG_GLOBAL: globalConfigPath }; + const result = spawnSync("git", args, { encoding: "utf8", cwd: repoDir, env }); + if (result.error) throw result.error; + return result; +} + +/** + * Create a temporary directory containing: + * - a bare global gitconfig file (.gitconfig-global) + * - an initialised git repository (repo/) + * + * Returns { repoDir, globalConfigPath }. + */ +function createIsolatedRepo(prefix) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + const globalConfigPath = path.join(root, ".gitconfig-global"); + // Minimal global config so git commits work + fs.writeFileSync(globalConfigPath, "[user]\n\tname = Test User\n\temail = test@example.com\n"); + const repoDir = path.join(root, "repo"); + fs.mkdirSync(repoDir); + runGit(["init", "-q"], repoDir, globalConfigPath); + fs.writeFileSync(path.join(repoDir, "README.md"), "init\n"); + runGit(["add", "."], repoDir, globalConfigPath); + runGit(["commit", "-q", "-m", "init"], repoDir, globalConfigPath); + return { root, repoDir, globalConfigPath }; +} + +/** + * Build the exec API passed to git_auth_helpers.cjs. + * Every git subprocess it spawns carries GIT_CONFIG_GLOBAL so that --global + * operations hit the isolated config file rather than the real user config. + */ +function createExecApi(repoDir, globalConfigPath) { + function spawnGit(args, apiOptions = {}) { + const cwd = apiOptions.cwd || repoDir; + const env = { ...process.env, GIT_CONFIG_GLOBAL: globalConfigPath }; + return spawnSync("git", args, { encoding: "utf8", cwd, env }); + } + return { + async exec(cmd, args = [], options = {}) { + if (cmd !== "git") throw new Error(`unexpected command: ${cmd}`); + const r = spawnGit(args, options); + if (r.error) throw r.error; + if (r.status !== 0) { + throw new Error(`git ${args.join(" ")} failed (${r.status}): ${r.stderr}`); + } + return r.status; + }, + async getExecOutput(cmd, args = [], options = {}) { + if (cmd !== "git") throw new Error(`unexpected command: ${cmd}`); + const r = spawnGit(args, options); + if (r.error) throw r.error; + if (r.status !== 0 && !options.ignoreReturnCode) { + throw new Error(`git ${args.join(" ")} failed (${r.status}): ${r.stderr}`); + } + return { exitCode: r.status, stdout: r.stdout, stderr: r.stderr }; + }, + }; +} + +/** + * Read all values for key from a specific scope (or all scopes if scopeFlag + * is omitted) using a real git process. + */ +function readConfigValues(key, scopeFlag, repoDir, globalConfigPath) { + const args = scopeFlag ? ["config", scopeFlag, "--get-all", key] : ["config", "--get-all", key]; + const r = runGit(args, repoDir, globalConfigPath); + // exit 5 = key absent; any other non-zero = error + if (r.status !== 0) return []; + return r.stdout.trim().split("\n").filter(Boolean); +} + +/** + * Write a single value to the given scope. + */ +function writeConfigValue(key, value, scopeFlag, repoDir, globalConfigPath) { + const r = runGit(["config", scopeFlag, "--add", key, value], repoDir, globalConfigPath); + if (r.status !== 0) throw new Error(`git config write failed: ${r.stderr}`); +} + +// --------------------------------------------------------------------------- +// Suite +// --------------------------------------------------------------------------- + +describe("git_auth_helpers.cjs git integration", () => { + let repoDir; + let globalConfigPath; + let root; + let mockCore; + let overridePersistedExtraheader; + let restorePersistedExtraheader; + let unsetExtraheaderAllScopes; + let withGitHubHostToken; + + let origGithubServerUrl; + + beforeEach(() => { + ({ root, repoDir, globalConfigPath } = createIsolatedRepo("git-auth-helpers-it-")); + + origGithubServerUrl = process.env.GITHUB_SERVER_URL; + process.env.GITHUB_SERVER_URL = SERVER_URL; + + mockCore = { + info: vi.fn(), + warning: vi.fn(), + }; + + global.core = mockCore; + global.exec = createExecApi(repoDir, globalConfigPath); + + delete require.cache[require.resolve("./git_auth_helpers.cjs")]; + ({ overridePersistedExtraheader, restorePersistedExtraheader, unsetExtraheaderAllScopes, withGitHubHostToken } = require("./git_auth_helpers.cjs")); + }); + + afterEach(() => { + if (root && fs.existsSync(root)) { + fs.rmSync(root, { recursive: true, force: true }); + } + if (origGithubServerUrl !== undefined) { + process.env.GITHUB_SERVER_URL = origGithubServerUrl; + } else { + delete process.env.GITHUB_SERVER_URL; + } + delete global.core; + delete global.exec; + vi.clearAllMocks(); + }); + + // ────────────────────────────────────────────────────── + // unsetExtraheaderAllScopes + // ────────────────────────────────────────────────────── + + describe("unsetExtraheaderAllScopes", () => { + it("clears a value from the global scope", async () => { + writeConfigValue(EXTRAHEADER_KEY, "Authorization: basic abc", "--global", repoDir, globalConfigPath); + expect(readConfigValues(EXTRAHEADER_KEY, "--global", repoDir, globalConfigPath)).toHaveLength(1); + + await unsetExtraheaderAllScopes(EXTRAHEADER_KEY, repoDir); + + expect(readConfigValues(EXTRAHEADER_KEY, "--global", repoDir, globalConfigPath)).toHaveLength(0); + }); + + it("clears a value from the local scope", async () => { + writeConfigValue(EXTRAHEADER_KEY, "Authorization: basic abc", "--local", repoDir, globalConfigPath); + expect(readConfigValues(EXTRAHEADER_KEY, "--local", repoDir, globalConfigPath)).toHaveLength(1); + + await unsetExtraheaderAllScopes(EXTRAHEADER_KEY, repoDir); + + expect(readConfigValues(EXTRAHEADER_KEY, "--local", repoDir, globalConfigPath)).toHaveLength(0); + }); + + it("clears values from both global and local scopes simultaneously", async () => { + writeConfigValue(EXTRAHEADER_KEY, "Authorization: basic global", "--global", repoDir, globalConfigPath); + writeConfigValue(EXTRAHEADER_KEY, "Authorization: basic local", "--local", repoDir, globalConfigPath); + // Both scopes have a value; --get-all should return 2 + expect(readConfigValues(EXTRAHEADER_KEY, null, repoDir, globalConfigPath)).toHaveLength(2); + + await unsetExtraheaderAllScopes(EXTRAHEADER_KEY, repoDir); + + expect(readConfigValues(EXTRAHEADER_KEY, "--global", repoDir, globalConfigPath)).toHaveLength(0); + expect(readConfigValues(EXTRAHEADER_KEY, "--local", repoDir, globalConfigPath)).toHaveLength(0); + expect(readConfigValues(EXTRAHEADER_KEY, null, repoDir, globalConfigPath)).toHaveLength(0); + }); + + it("does not throw when the key is absent in both scopes (exit code 5)", async () => { + // No value written — key is absent in all scopes. + await expect(unsetExtraheaderAllScopes(EXTRAHEADER_KEY, repoDir)).resolves.toBeUndefined(); + }); + }); + + // ────────────────────────────────────────────────────── + // overridePersistedExtraheader + // ────────────────────────────────────────────────────── + + describe("overridePersistedExtraheader", () => { + it("removes the global token and writes only the fork token to local scope", async () => { + const upstreamHeader = `Authorization: basic ${Buffer.from("x-access-token:upstream").toString("base64")}`; + const forkToken = "fork-token-123"; + const expectedForkHeader = `Authorization: basic ${Buffer.from(`x-access-token:${forkToken}`).toString("base64")}`; + + writeConfigValue(EXTRAHEADER_KEY, upstreamHeader, "--global", repoDir, globalConfigPath); + + await overridePersistedExtraheader(SERVER_URL, forkToken, repoDir); + + // Global must be empty + expect(readConfigValues(EXTRAHEADER_KEY, "--global", repoDir, globalConfigPath)).toHaveLength(0); + // Local must have exactly the fork token + expect(readConfigValues(EXTRAHEADER_KEY, "--local", repoDir, globalConfigPath)).toEqual([expectedForkHeader]); + // get-all must see exactly one header + expect(readConfigValues(EXTRAHEADER_KEY, null, repoDir, globalConfigPath)).toEqual([expectedForkHeader]); + }); + + it("returns the upstream header that was in the global scope before the override", async () => { + const upstreamHeader = `Authorization: basic ${Buffer.from("x-access-token:upstream").toString("base64")}`; + writeConfigValue(EXTRAHEADER_KEY, upstreamHeader, "--global", repoDir, globalConfigPath); + + const previous = await overridePersistedExtraheader(SERVER_URL, "fork-token", repoDir); + + expect(previous).toEqual([upstreamHeader]); + }); + + it("returns an empty array and still writes fork token when no extraheader exists", async () => { + const forkToken = "fork-only"; + const expectedForkHeader = `Authorization: basic ${Buffer.from(`x-access-token:${forkToken}`).toString("base64")}`; + + const previous = await overridePersistedExtraheader(SERVER_URL, forkToken, repoDir); + + expect(previous).toEqual([]); + expect(readConfigValues(EXTRAHEADER_KEY, null, repoDir, globalConfigPath)).toEqual([expectedForkHeader]); + }); + }); + + // ────────────────────────────────────────────────────── + // restorePersistedExtraheader + // ────────────────────────────────────────────────────── + + describe("restorePersistedExtraheader", () => { + it("removes the fork token from local and restores the upstream token to local", async () => { + const upstreamHeader = `Authorization: basic ${Buffer.from("x-access-token:upstream").toString("base64")}`; + const forkHeader = `Authorization: basic ${Buffer.from("x-access-token:fork").toString("base64")}`; + + // Simulate the state after an override: global is empty, local has the fork token. + writeConfigValue(EXTRAHEADER_KEY, forkHeader, "--local", repoDir, globalConfigPath); + + await restorePersistedExtraheader(SERVER_URL, [upstreamHeader], repoDir); + + // Global must remain empty + expect(readConfigValues(EXTRAHEADER_KEY, "--global", repoDir, globalConfigPath)).toHaveLength(0); + // Local must have the restored upstream token + expect(readConfigValues(EXTRAHEADER_KEY, "--local", repoDir, globalConfigPath)).toEqual([upstreamHeader]); + // get-all must see exactly one header + expect(readConfigValues(EXTRAHEADER_KEY, null, repoDir, globalConfigPath)).toEqual([upstreamHeader]); + }); + + it("clears all scopes and writes nothing when previousValues is empty", async () => { + const forkHeader = `Authorization: basic ${Buffer.from("x-access-token:fork").toString("base64")}`; + writeConfigValue(EXTRAHEADER_KEY, forkHeader, "--local", repoDir, globalConfigPath); + + await restorePersistedExtraheader(SERVER_URL, [], repoDir); + + expect(readConfigValues(EXTRAHEADER_KEY, "--global", repoDir, globalConfigPath)).toHaveLength(0); + expect(readConfigValues(EXTRAHEADER_KEY, "--local", repoDir, globalConfigPath)).toHaveLength(0); + }); + }); + + // ────────────────────────────────────────────────────── + // withGitHubHostToken — multi-cycle regression + // ────────────────────────────────────────────────────── + + describe("withGitHubHostToken", () => { + it("does not accumulate extraheader values across multiple override/restore cycles", async () => { + // Simulates the exact production failure: actions/checkout writes the upstream + // token to the global scope. Without the scope-clearing fix, each restore would + // leave the global value intact and add a new local copy, so --get-all returns + // N+1 values on each subsequent cycle. + const upstreamHeader = `Authorization: basic ${Buffer.from("x-access-token:upstream").toString("base64")}`; + writeConfigValue(EXTRAHEADER_KEY, upstreamHeader, "--global", repoDir, globalConfigPath); + + const capturedCounts = []; + mockCore.info.mockImplementation(msg => { + const m = msg.match(/read (\d+) existing extraheader value/); + if (m) capturedCounts.push(Number(m[1])); + }); + + // Two full override/restore cycles (simulates two retries) + for (let i = 0; i < 2; i++) { + await withGitHubHostToken("fork-token", async () => {}, repoDir); + } + + // Without the fix: [1, 2] — second cycle sees global + local copies. + // With the fix: [1, 1] — global is always cleared before each write. + expect(capturedCounts).toEqual([1, 1]); + }); + + it("leaves exactly one extraheader value active inside the callback", async () => { + const upstreamHeader = `Authorization: basic ${Buffer.from("x-access-token:upstream").toString("base64")}`; + const forkToken = "fork-callback-check"; + const expectedForkHeader = `Authorization: basic ${Buffer.from(`x-access-token:${forkToken}`).toString("base64")}`; + writeConfigValue(EXTRAHEADER_KEY, upstreamHeader, "--global", repoDir, globalConfigPath); + + let valuesInsideCallback; + await withGitHubHostToken( + forkToken, + async () => { + valuesInsideCallback = readConfigValues(EXTRAHEADER_KEY, null, repoDir, globalConfigPath); + }, + repoDir + ); + + // Exactly one Authorization header must be active during the callback + expect(valuesInsideCallback).toEqual([expectedForkHeader]); + }); + + it("restores exactly one extraheader value after the callback completes", async () => { + const upstreamHeader = `Authorization: basic ${Buffer.from("x-access-token:upstream").toString("base64")}`; + writeConfigValue(EXTRAHEADER_KEY, upstreamHeader, "--global", repoDir, globalConfigPath); + + await withGitHubHostToken("fork-token", async () => {}, repoDir); + + // After restore, exactly the upstream token should be in local scope (global was cleared) + const allValues = readConfigValues(EXTRAHEADER_KEY, null, repoDir, globalConfigPath); + expect(allValues).toEqual([upstreamHeader]); + }); + + it("restores the config even when the callback throws", async () => { + const upstreamHeader = `Authorization: basic ${Buffer.from("x-access-token:upstream").toString("base64")}`; + writeConfigValue(EXTRAHEADER_KEY, upstreamHeader, "--global", repoDir, globalConfigPath); + + await expect( + withGitHubHostToken( + "fork-token", + async () => { + throw new Error("simulated callback failure"); + }, + repoDir + ) + ).rejects.toThrow("simulated callback failure"); + + const allValues = readConfigValues(EXTRAHEADER_KEY, null, repoDir, globalConfigPath); + expect(allValues).toEqual([upstreamHeader]); + }); + }); +}); diff --git a/actions/setup/js/git_auth_helpers.test.cjs b/actions/setup/js/git_auth_helpers.test.cjs index 9faafcd4d8c..8303f146bfe 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 unsetExtraheaderAllScopes; let withGitHubHostToken; const SERVER_URL = "https://github.com"; @@ -19,14 +20,16 @@ describe("git_auth_helpers.cjs", () => { mockExec = { exec: vi.fn().mockResolvedValue(0), - getExecOutput: vi.fn().mockResolvedValue({ exitCode: 1, stdout: "", stderr: "" }), + // Exit code 5 = key absent (the most common default state). + // This is safe for both --get-all (returns []) and --unset-all (key not found, ignored). + getExecOutput: vi.fn().mockResolvedValue({ exitCode: 5, stdout: "", stderr: "" }), }; global.core = mockCore; global.exec = mockExec; delete require.cache[require.resolve("./git_auth_helpers.cjs")]; - ({ checkoutHasPersistedExtraheader, overridePersistedExtraheader, restorePersistedExtraheader, withGitHubHostToken } = require("./git_auth_helpers.cjs")); + ({ checkoutHasPersistedExtraheader, overridePersistedExtraheader, restorePersistedExtraheader, unsetExtraheaderAllScopes, withGitHubHostToken } = require("./git_auth_helpers.cjs")); }); afterEach(() => { @@ -66,21 +69,70 @@ describe("git_auth_helpers.cjs", () => { }); }); + // ────────────────────────────────────────────────────── + // unsetExtraheaderAllScopes + // ────────────────────────────────────────────────────── + + describe("unsetExtraheaderAllScopes", () => { + it("should unset from global and local scopes", async () => { + await unsetExtraheaderAllScopes(EXTRAHEADER_KEY); + + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["config", "--global", "--unset-all", EXTRAHEADER_KEY], expect.objectContaining({ ignoreReturnCode: true })); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["config", "--local", "--unset-all", EXTRAHEADER_KEY], expect.objectContaining({ ignoreReturnCode: true })); + }); + + it("should pass cwd to both scope unset calls", async () => { + const cwd = "/some/repo"; + await unsetExtraheaderAllScopes(EXTRAHEADER_KEY, cwd); + + for (const call of mockExec.getExecOutput.mock.calls) { + expect(call[2]).toMatchObject({ cwd }); + } + }); + + it("should not throw when key is absent (exit code 5)", async () => { + mockExec.getExecOutput.mockResolvedValue({ exitCode: 5, stdout: "", stderr: "" }); + + await expect(unsetExtraheaderAllScopes(EXTRAHEADER_KEY)).resolves.toBeUndefined(); + }); + + it("should throw when a scope unset fails with an unexpected non-zero exit code", async () => { + // Exit code 4 = file write error (permission denied, config lock, etc.) + mockExec.getExecOutput.mockResolvedValue({ exitCode: 4, stdout: "", stderr: "error: could not lock config file" }); + + await expect(unsetExtraheaderAllScopes(EXTRAHEADER_KEY)).rejects.toThrow(/--unset-all.*failed \(exit 4\)/); + }); + }); + // ────────────────────────────────────────────────────── // overridePersistedExtraheader // ────────────────────────────────────────────────────── describe("overridePersistedExtraheader", () => { - it("should replace the extraheader with the CI token", async () => { + it("should clear all scopes before writing the new token", async () => { + const token = "ghp_test_token"; + + await overridePersistedExtraheader(SERVER_URL, token); + + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["config", "--global", "--unset-all", EXTRAHEADER_KEY], expect.objectContaining({ ignoreReturnCode: true })); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["config", "--local", "--unset-all", EXTRAHEADER_KEY], expect.objectContaining({ ignoreReturnCode: true })); + }); + + it("should replace the extraheader with the CI token using --local scope", async () => { const token = "ghp_test_token"; + const expectedHeader = `Authorization: basic ${Buffer.from(`x-access-token:${token}`).toString("base64")}`; await overridePersistedExtraheader(SERVER_URL, token); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--replace-all", EXTRAHEADER_KEY, `Authorization: basic ${Buffer.from(`x-access-token:${token}`).toString("base64")}`]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--local", "--replace-all", EXTRAHEADER_KEY, expectedHeader]); }); it("should return empty array when no previous extraheader exists", async () => { - mockExec.getExecOutput.mockResolvedValue({ exitCode: 1, stdout: "", stderr: "" }); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + // Only the --get-all read returns empty; unset-all calls use ignoreReturnCode + if (args[1] === "--get-all") return { exitCode: 1, stdout: "", stderr: "" }; + return { exitCode: 5, stdout: "", stderr: "" }; // key not found + }); const previous = await overridePersistedExtraheader(SERVER_URL, "ghp_test_token"); @@ -89,7 +141,10 @@ describe("git_auth_helpers.cjs", () => { it("should return previous extraheader values when one exists", async () => { const prevHeader = `Authorization: basic ${Buffer.from("x-access-token:old_token").toString("base64")}`; - mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: prevHeader + "\n", stderr: "" }); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[1] === "--get-all") return { exitCode: 0, stdout: prevHeader + "\n", stderr: "" }; + return { exitCode: 5, stdout: "", stderr: "" }; + }); const previous = await overridePersistedExtraheader(SERVER_URL, "ghp_new_token"); @@ -99,7 +154,10 @@ describe("git_auth_helpers.cjs", () => { it("should return multiple previous values when multi-value extraheader exists", async () => { const header1 = `Authorization: basic ${Buffer.from("x-access-token:tok1").toString("base64")}`; const header2 = `Authorization: basic ${Buffer.from("x-access-token:tok2").toString("base64")}`; - mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: `${header1}\n${header2}\n`, stderr: "" }); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[1] === "--get-all") return { exitCode: 0, stdout: `${header1}\n${header2}\n`, stderr: "" }; + return { exitCode: 5, stdout: "", stderr: "" }; + }); const previous = await overridePersistedExtraheader(SERVER_URL, "ghp_new_token"); @@ -107,14 +165,17 @@ describe("git_auth_helpers.cjs", () => { }); it("should warn and fall back to empty array when reading previous values fails", async () => { - mockExec.getExecOutput.mockRejectedValue(new Error("git read error")); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[1] === "--get-all") throw new Error("git read error"); + return { exitCode: 5, stdout: "", stderr: "" }; + }); const previous = await overridePersistedExtraheader(SERVER_URL, "ghp_test_token"); expect(previous).toEqual([]); expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("could not read existing extraheader")); // Override should still proceed despite read failure - expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--replace-all", EXTRAHEADER_KEY, expect.any(String)]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--local", "--replace-all", EXTRAHEADER_KEY, expect.any(String)]); }); it("should trim the token before base64-encoding", async () => { @@ -123,12 +184,15 @@ describe("git_auth_helpers.cjs", () => { await overridePersistedExtraheader(SERVER_URL, token); const expected = `Authorization: basic ${Buffer.from("x-access-token:ghp_padded_token").toString("base64")}`; - expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--replace-all", EXTRAHEADER_KEY, expected]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--local", "--replace-all", EXTRAHEADER_KEY, expected]); }); it("should log the number of existing values before overriding", async () => { const header = `Authorization: basic ${Buffer.from("x-access-token:tok").toString("base64")}`; - mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: header + "\n", stderr: "" }); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[1] === "--get-all") return { exitCode: 0, stdout: header + "\n", stderr: "" }; + return { exitCode: 5, stdout: "", stderr: "" }; + }); await overridePersistedExtraheader(SERVER_URL, "new_token"); @@ -136,7 +200,9 @@ describe("git_auth_helpers.cjs", () => { }); it("should strip trailing slash from server URL when reading previous values", async () => { - mockExec.getExecOutput.mockResolvedValue({ exitCode: 1, stdout: "", stderr: "" }); + // Use exit code 5 (key absent) for all calls so --unset-all is treated as "not found", + // which is the expected state when testing URL normalization in isolation. + mockExec.getExecOutput.mockResolvedValue({ exitCode: 5, stdout: "", stderr: "" }); await overridePersistedExtraheader("https://github.com/", "ghp_test_token"); @@ -145,7 +211,7 @@ describe("git_auth_helpers.cjs", () => { // Using the literal normalized key makes the assertion explicit. const normalizedKey = "http.https://github.com/.extraheader"; expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["config", "--get-all", normalizedKey], expect.anything()); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--replace-all", normalizedKey, expect.any(String)]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--local", "--replace-all", normalizedKey, expect.any(String)]); }); }); @@ -154,68 +220,71 @@ describe("git_auth_helpers.cjs", () => { // ────────────────────────────────────────────────────── describe("restorePersistedExtraheader", () => { - it("should unset the extraheader key when previousValues is empty", async () => { + it("should clear all scopes when previousValues is empty", async () => { await restorePersistedExtraheader(SERVER_URL, []); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--unset-all", EXTRAHEADER_KEY]); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["config", "--global", "--unset-all", EXTRAHEADER_KEY], expect.objectContaining({ ignoreReturnCode: true })); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["config", "--local", "--unset-all", EXTRAHEADER_KEY], expect.objectContaining({ ignoreReturnCode: true })); + // Should not call exec.exec (no values to write back) + expect(mockExec.exec).not.toHaveBeenCalled(); }); - it("should not throw when unset-all fails (key already absent)", async () => { - mockExec.exec.mockRejectedValue(new Error("key not found")); + it("should not throw when key is absent (exit code 5) when clearing scopes", async () => { + mockExec.getExecOutput.mockResolvedValue({ exitCode: 5, stdout: "", stderr: "" }); await expect(restorePersistedExtraheader(SERVER_URL, [])).resolves.toBeUndefined(); }); - it("should use --replace-all to restore a single previous value", async () => { + it("should use --local --replace-all to restore a single previous value", async () => { const prevHeader = `Authorization: basic ${Buffer.from("x-access-token:old").toString("base64")}`; await restorePersistedExtraheader(SERVER_URL, [prevHeader]); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--replace-all", EXTRAHEADER_KEY, prevHeader]); + expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--local", "--replace-all", EXTRAHEADER_KEY, prevHeader]); expect(mockExec.exec).not.toHaveBeenCalledWith("git", expect.arrayContaining(["--add"])); }); - it("should restore multiple values using --replace-all then --add", async () => { + it("should restore multiple values using --local --replace-all then --local --add", async () => { const header1 = `Authorization: basic ${Buffer.from("x-access-token:tok1").toString("base64")}`; const header2 = `Authorization: basic ${Buffer.from("x-access-token:tok2").toString("base64")}`; await restorePersistedExtraheader(SERVER_URL, [header1, header2]); const calls = mockExec.exec.mock.calls; - const replaceCall = calls.find(c => c[1][1] === "--replace-all"); - const addCall = calls.find(c => c[1][1] === "--add"); + const replaceCall = calls.find(c => c[1][1] === "--local" && c[1][2] === "--replace-all"); + const addCall = calls.find(c => c[1][1] === "--local" && c[1][2] === "--add"); expect(replaceCall).toBeDefined(); - expect(replaceCall[1][3]).toBe(header1); + expect(replaceCall[1][4]).toBe(header1); expect(addCall).toBeDefined(); - expect(addCall[1][3]).toBe(header2); + expect(addCall[1][4]).toBe(header2); // --replace-all must come before --add expect(calls.indexOf(replaceCall)).toBeLessThan(calls.indexOf(addCall)); }); - it("should attempt cleanup and re-throw when --add fails mid-restore", async () => { + it("should attempt cleanup via unsetExtraheaderAllScopes and re-throw when --add fails mid-restore", async () => { const header1 = `Authorization: basic ${Buffer.from("x-access-token:tok1").toString("base64")}`; const header2 = `Authorization: basic ${Buffer.from("x-access-token:tok2").toString("base64")}`; const addError = new Error("git config --add failed"); mockExec.exec.mockImplementation(async (_cmd, args) => { - if (args[1] === "--add") throw addError; + if (args[1] === "--local" && args[2] === "--add") throw addError; return 0; }); await expect(restorePersistedExtraheader(SERVER_URL, [header1, header2])).rejects.toThrow(addError); expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("partial extraheader restore")); - // Cleanup --unset-all should have been attempted - const calls = mockExec.exec.mock.calls; - const unsetCall = calls.find(c => c[1][1] === "--unset-all"); - expect(unsetCall).toBeDefined(); + // Cleanup should use unsetExtraheaderAllScopes (getExecOutput calls for --unset-all) + const unsetCalls = mockExec.getExecOutput.mock.calls.filter(c => c[1][2] === "--unset-all"); + expect(unsetCalls.length).toBeGreaterThan(0); }); it("should strip trailing slash from server URL for the config key", async () => { await restorePersistedExtraheader("https://github.com/", []); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["config", "--unset-all", EXTRAHEADER_KEY]); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["config", "--global", "--unset-all", EXTRAHEADER_KEY], expect.anything()); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["config", "--local", "--unset-all", EXTRAHEADER_KEY], expect.anything()); }); it("should not throw when previousValues is null/undefined (treated as empty)", async () => { @@ -261,28 +330,34 @@ describe("git_auth_helpers.cjs", () => { await withGitHubHostToken(token, async () => { // Inside callback the override should already be applied - const overrideCall = execCalls.find(a => a[1] === "--replace-all"); + const overrideCall = execCalls.find(a => a[2] === "--replace-all"); expect(overrideCall).toBeDefined(); - expect(overrideCall[3]).toBe(expectedHeader); + expect(overrideCall[4]).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: "" }); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[1] === "--get-all") return { exitCode: 0, stdout: upstreamHeader + "\n", stderr: "" }; + return { exitCode: 5, stdout: "", 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"); + const replaceAllCalls = mockExec.exec.mock.calls.filter(c => c[1][2] === "--replace-all"); expect(replaceAllCalls.length).toBeGreaterThanOrEqual(2); const restoreCall = replaceAllCalls[replaceAllCalls.length - 1]; - expect(restoreCall[1][3]).toBe(upstreamHeader); + expect(restoreCall[1][4]).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: "" }); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[1] === "--get-all") return { exitCode: 0, stdout: upstreamHeader + "\n", stderr: "" }; + return { exitCode: 5, stdout: "", stderr: "" }; + }); const callbackError = new Error("push failed"); await expect( @@ -292,20 +367,29 @@ describe("git_auth_helpers.cjs", () => { ).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"); + const replaceAllCalls = mockExec.exec.mock.calls.filter(c => c[1][2] === "--replace-all"); expect(replaceAllCalls.length).toBeGreaterThanOrEqual(2); const restoreCall = replaceAllCalls[replaceAllCalls.length - 1]; - expect(restoreCall[1][3]).toBe(upstreamHeader); + expect(restoreCall[1][4]).toBe(upstreamHeader); }); - it("should unset extraheader after callback when no previous value existed", async () => { - mockExec.getExecOutput.mockResolvedValue({ exitCode: 1, stdout: "", stderr: "" }); + it("should clear all scopes after callback when no previous value existed", async () => { + // Exit code 5 = key absent (the expected state when no extraheader is configured). + // Using 1 here would cause unsetExtraheaderAllScopes to throw as of the fixed implementation. + mockExec.getExecOutput.mockResolvedValue({ exitCode: 5, 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); + // Restore should call unsetExtraheaderAllScopes (global + local unset-all) + const unsetCalls = mockExec.getExecOutput.mock.calls.filter(c => c[1][2] === "--unset-all"); + expect(unsetCalls.length).toBeGreaterThanOrEqual(2); + const keys = unsetCalls.map(c => c[1][3]); + expect(keys).toContain(EXTRAHEADER_KEY); + // Should NOT call exec.exec for restore (no values to write back) + const restoreExecCalls = mockExec.exec.mock.calls.filter(c => c[1][2] === "--replace-all"); + // First --replace-all is the override; no second one since previousValues is [] + // Check only one --replace-all call (the override itself) + expect(restoreExecCalls.length).toBe(1); }); it("should return the callback's return value", async () => { @@ -316,7 +400,10 @@ describe("git_auth_helpers.cjs", () => { 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: "" }); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[1] === "--get-all") return { exitCode: 0, stdout: upstreamHeader + "\n", stderr: "" }; + return { exitCode: 5, stdout: "", stderr: "" }; + }); await withGitHubHostToken("fork-token", async () => {}, cwd); @@ -328,5 +415,72 @@ describe("git_auth_helpers.cjs", () => { expect(call[2]).toMatchObject({ cwd }); } }); + + it("should not accumulate extraheader values across multiple override/restore cycles", async () => { + // Regression test: verifies header cardinality stays at 1 across retries. + // + // Simulates real git config state with independent global and local scopes so that + // --unset-all, --replace-all, and --get-all operations interact with the same in-memory + // state. Without this, --get-all is hard-coded to one value regardless of what the other + // commands do, meaning the test would pass even with the old accumulating implementation. + // + // Bug: getExtraheaderValues reads ALL scopes (--get-all), but the old --replace-all + // without an explicit scope only wrote to local, leaving the global value intact. + // After each restore both scopes held a copy, so the next --get-all returned N+1 values. + const upstreamHeader = `Authorization: basic ${Buffer.from("x-access-token:upstream").toString("base64")}`; + + // Simulate git config state: actions/checkout writes the upstream token to global scope. + let globalValues = [upstreamHeader]; + let localValues = []; + + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[1] === "--get-all") { + // git config --get-all reads from all scopes combined + const all = [...globalValues, ...localValues]; + return all.length > 0 ? { exitCode: 0, stdout: all.join("\n") + "\n", stderr: "" } : { exitCode: 5, stdout: "", stderr: "" }; + } + if (args[2] === "--unset-all") { + if (args[1] === "--global") { + const hadValues = globalValues.length > 0; + globalValues = []; + return { exitCode: hadValues ? 0 : 5, stdout: "", stderr: "" }; + } + if (args[1] === "--local") { + const hadValues = localValues.length > 0; + localValues = []; + return { exitCode: hadValues ? 0 : 5, stdout: "", stderr: "" }; + } + } + return { exitCode: 5, stdout: "", stderr: "" }; + }); + + mockExec.exec.mockImplementation(async (_cmd, args) => { + if (args[1] === "--local" && args[2] === "--replace-all") { + localValues = [args[4]]; + return 0; + } + if (args[1] === "--local" && args[2] === "--add") { + localValues.push(args[4]); + return 0; + } + return 0; + }); + + let capturedPreviousValueCounts = []; + mockCore.info.mockImplementation(msg => { + const m = msg.match(/read (\d+) existing extraheader value/); + if (m) capturedPreviousValueCounts.push(Number(m[1])); + }); + + // Run two override/restore cycles (simulating a retry scenario) + for (let i = 0; i < 2; i++) { + await withGitHubHostToken("fork-token", async () => {}); + } + + // Both cycles should read exactly 1 value — no accumulation. + // Without the fix (no --global unset before writing), the second cycle would + // see both global and local copies of the token and return 2. + expect(capturedPreviousValueCounts).toEqual([1, 1]); + }); }); });