fix: clear all git config scopes before overriding extraheader to prevent duplicate Authorization headers - #49059
Conversation
…vent duplicate Authorization headers Fixes the duplicate Authorization header issue in fork-backed create_pull_request: Root cause: getExtraheaderValues reads from ALL git config scopes (--get-all), but overridePersistedExtraheader wrote only to the local scope via --replace-all without explicit scope. actions/checkout writes to the global scope, so after override both local (fork CI token) and global (checkout token) were active simultaneously, causing duplicate Authorization headers and HTTP 400 errors. After restore, both global (original) and local (restored original) had the same value, causing getExtraheaderValues to return 2 values on the next read, then 3, 4 — the accumulation bug observed in the issue log. Fix: - Add unsetExtraheaderAllScopes() helper that silently clears from --global and --local scopes before any write operation - overridePersistedExtraheader calls unsetExtraheaderAllScopes before writing fork token to --local scope, ensuring exactly one Authorization source is active - restorePersistedExtraheader calls unsetExtraheaderAllScopes before restoring to --local scope, preventing accumulation across retries - Add regression test that verifies cardinality stays at 1 across override/restore cycles Closes #48952 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates Git authentication handling to prevent duplicate Authorization headers during fork-backed remote operations.
Changes:
- Clears global and local extraheader values before overrides/restoration.
- Uses explicit local scope for temporary and restored credentials.
- Expands unit coverage for scoped configuration calls and retry cycles.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/git_auth_helpers.cjs |
Adds scoped extraheader clearing and updates override/restoration logic. |
actions/setup/js/git_auth_helpers.test.cjs |
Updates scope assertions and adds retry regression coverage. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
actions/setup/js/git_auth_helpers.cjs:150
- Restoring a global value into the current repository's local config is not functionally equivalent to restoring the original state. A global credential originally available to another checkout (or to a later operation outside this
cwd) disappears after this callback, while this repository gains a persisted local credential that it did not previously contain. Snapshot values per scope and restore each scope rather than flattening all effective values into--local.
// 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.
actions/setup/js/git_auth_helpers.cjs:113
- This mutates both saved scopes before the replacement write succeeds. If the following
git config --local --replace-allfails (for example, due to a read-only repository config),overridePersistedExtraheaderrejects before returning the saved values,overrideAppliedremains false, and the callers skip restoration; the original global/local credentials are then permanently removed. Roll back the scoped snapshot when installation fails, or avoid destructive mutation until the override can be applied atomically.
// 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);
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Medium
| const opts = { silent: true, ignoreReturnCode: true, ...(cwd ? { cwd } : {}) }; | ||
| await exec.getExecOutput("git", ["config", scope, "--unset-all", key], opts); | ||
| } catch { | ||
| // Ignore – scope may not be accessible or key may be absent. |
| // Simulate the initial state: git config --get-all returns 1 value (global scope) | ||
| mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { | ||
| if (args[1] === "--get-all") return { exitCode: 0, stdout: upstreamHeader + "\n", stderr: "" }; | ||
| // unset-all calls return success (exit 0) | ||
| return { exitCode: 0, stdout: "", stderr: "" }; |
Triage Summary
Fixes fork PR creation failures (Duplicate Authorization header). High urgency but 2 CI checks currently failing — investigate before merge. Batch B-git-checkout (with #49002, #49003).
|
|
@copilot run pr-finisher skill |
|
@copilot add git integration tests |
…n test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…egression test with stateful mock Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Both review issues addressed in commit 1. 2. Regression test with stateful mock — Replaced the hard-coded |
…h GIT_CONFIG_GLOBAL isolation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Warning threat detection engine error DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. @copilot please run the Unresolved feedback to address (newest first):
Failed checks:
|
…nset-all calls Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
✅ PR finisher complete. Summary:
Next steps: |
PR Triage
Rationale: Fixes duplicate
|
|
Warning threat detection engine error DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. @copilot please run the
|
…cope operations Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
✅ Fixed the failing JS Tests (shard 1/4). Root cause: The Changes made:
All 46 tests now pass locally. PR is ready for CI re-trigger to verify in the full environment. |
Fork-backed
create_pull_requestfails withremote: Duplicate header: "Authorization"HTTP 400 on every attempt, and the restore loop accumulates an extra header value per retry (read 1,read 2,read 3, ...).Root cause
getExtraheaderValuesusesgit config --get-all(reads all scopes), butoverridePersistedExtraheaderusedgit config --replace-allwithout an explicit scope flag (writes local scope only). Sinceactions/checkoutwrites credentials to the global scope, after override both scopes hold an Authorization value simultaneously. After restore, the global value is never cleared so it persists alongside the freshly restored local value — causing--get-allto return N+1 values on each subsequent attempt.Changes
unsetExtraheaderAllScopes(key, cwd)— new helper that silently clears the extraheader key from--globaland--localscopes before any write (system scope skipped; read-only in CI)overridePersistedExtraheader— callsunsetExtraheaderAllScopesbefore writing the fork token to--local, ensuring exactly one Authorization source is active during the callbackrestorePersistedExtraheader— callsunsetExtraheaderAllScopesbefore restoring to--local, breaking the accumulation cycle across retries; cleanup path also uses the new helper--global/--localscope usage; regression test added to verify header cardinality stays at 1 across multiple override/restore cyclesWarning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.