fix: clear all git config scopes before overriding extraheader to prevent duplicate Authorization headers

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);

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();
     });
   });

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<void>}
+ */
+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.<serverUrl>/.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,
 };