Skip to content

EXP-23278: Fix duplicate Authorization header on recursive HTTPS clones - #93

Merged
a-olkhovskyi merged 2 commits into
mainfrom
task/EXP-23278-fix-https-recursive-checkout
Jul 30, 2026
Merged

EXP-23278: Fix duplicate Authorization header on recursive HTTPS clones#93
a-olkhovskyi merged 2 commits into
mainfrom
task/EXP-23278-fix-https-recursive-checkout

Conversation

@a-olkhovskyi

@a-olkhovskyi a-olkhovskyi commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

When subrepo auth runs over HTTPS token (GH_USER/GH_TOKEN or S7_GIT_USER/S7_GIT_TOKEN), s7 injects its auth into the child git's environment as GIT_CONFIG_* entries, appending past any GIT_CONFIG_COUNT the environment already carried. Recursing into a nested s7 subrepo spawns a fresh s7 process (via that subrepo's post-checkout hook) that inherits this environment and appended its own copy on top. Because http.<url>.extraheader is multi-valued in git, the child then sent two identical Authorization headers, and GitHub rejected the request with HTTP 400 ("Duplicate header: Authorization") — breaking every nested clone/fetch. Top-level clones were unaffected.

Make the auth-env builder idempotent: stamp S7_GIT_AUTH_INJECTED on the environment it produces and, when an ancestor s7 already stamped it, reuse that environment verbatim instead of layering a second extraheader.

Add tests for the marker stamp, reuse-on-inherited-env, and a multi-level nesting chain asserting a single Authorization header survives.


Note

Medium Risk
Changes HTTPS token auth environment construction for child git processes; behavior is narrowed with an idempotency guard and covered by new tests, but mistakes could still break nested clones or auth in CI.

Overview
Fixes nested HTTPS subrepo clones failing with GitHub HTTP 400 (Duplicate header: Authorization) when each nested s7 process appended another http.https://github.com/.extraheader onto inherited GIT_CONFIG_* env.

gitHubTokenAuthTaskEnvironmentForUser:token:processEnvironment: now stamps S7_GIT_AUTH_INJECTED=1 on the env it builds and, when that marker is already present, returns the inherited env unchanged instead of adding another Authorization header. Comments clarify that appending past existing GIT_CONFIG_COUNT is for foreign CI config, not nested s7 (nested case uses the marker).

Tests expect the marker on first injection, document the CI append case, and add nested reuse plus multi-level chaining checks so only one Basic auth header remains.

Reviewed by Cursor Bugbot for commit dfb7fd4. Bugbot is set up for automated code reviews on this repo. Configure here.

When subrepo auth runs over HTTPS token (GH_USER/GH_TOKEN or
S7_GIT_USER/S7_GIT_TOKEN), s7 injects its auth into the child git's
environment as GIT_CONFIG_* entries, appending past any GIT_CONFIG_COUNT
the environment already carried. Recursing into a nested s7 subrepo spawns
a fresh s7 process (via that subrepo's post-checkout hook) that inherits
this environment and appended its own copy on top. Because
http.<url>.extraheader is multi-valued in git, the child then sent two
identical Authorization headers, and GitHub rejected the request with
HTTP 400 ("Duplicate header: Authorization") — breaking every nested
clone/fetch (e.g. rd2 -> RDPDFKit -> SPFlounder -> Eigen). Top-level
clones were unaffected.

Make the auth-env builder idempotent: stamp S7_GIT_AUTH_INJECTED on the
environment it produces and, when an ancestor s7 already stamped it, reuse
that environment verbatim instead of layering a second extraheader.

Add tests for the marker stamp, reuse-on-inherited-env, and a multi-level
nesting chain asserting a single Authorization header survives.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@a-olkhovskyi
a-olkhovskyi requested a review from pastey July 30, 2026 10:57
pastey
pastey previously approved these changes Jul 30, 2026

@pastey pastey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fix.
One tiny suggestion on the tests. Up to you.

Comment thread system7-tests/gitGitHubTokenAuthTests.m Outdated
Comment on lines +137 to +142
- (void)testStampsInjectedMarker {
NSDictionary<NSString *, NSString *> *const env =
[GitRepository gitHubTokenAuthTaskEnvironmentForUser:@"alice" token:@"abc" processEnvironment:@{}];

XCTAssertEqualObjects(@"1", env[@"S7_GIT_AUTH_INJECTED"]);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, this is an implementation detail. The other two tests are fine – they test the behaviour. This one – I would rather remove it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, removed it 👍

@pastey pastey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@a-olkhovskyi
a-olkhovskyi enabled auto-merge July 30, 2026 11:07
@a-olkhovskyi
a-olkhovskyi merged commit 808ab1a into main Jul 30, 2026
4 checks passed
@a-olkhovskyi
a-olkhovskyi deleted the task/EXP-23278-fix-https-recursive-checkout branch July 30, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants