test: cover cross-host auth forwarding for submodule clones#515
Merged
Conversation
Member
|
needs rebase |
johnstcn
approved these changes
May 22, 2026
Add a TestCloneRepoSubmoduleHostAuth case that observes whether the submodule HTTP server received an Authorization header. The same-host subtest expects auth to be forwarded; the cross-host subtest uses 'localhost' for the .gitmodules URL (so SameHost compares as mismatched) while both hosts still resolve to the loopback interface, so the submodule server is reachable but should never see the parent credentials. Covers the security half of mafredri thread PRRT_kwDOJPSNMM5vAH0h.
mafredri
force-pushed
the
mafredri/485-submodule-crosshost-auth-test
branch
from
40ca764 to
9f1a99d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a test that verifies the SameHost auth gating in
cloneSubmoduleactually behaves as intended end-to-end. The submodule HTTP server is wrapped with an observer that records every request carrying anAuthorizationheader; the same-host subtest expects auth to be forwarded, the cross-host subtest expects it to be withheld.The cross-host case uses
localhostfor the URL in.gitmodulesand127.0.0.1for the parent server's URL. Both resolve to the loopback interface so the submodule clone still succeeds, butSameHostcompares hostname strings and reports a mismatch, so the auth must not be sent.Covers the security half of mafredri's thread PRRT_kwDOJPSNMM5vAH0h, where mafredri asked whether the gittest helper should be expanded to exercise the auth-exfiltration guard.
Why this PR and not the changes I considered
I also investigated AMREM-7 (move
cloneSubmoduleover to go-git's nativeSubmodule.Update()). go-git v5.16.5'sSubmodule.Initresolves relative URLs viatransport.NewEndpoint, which absolutizes against CWD, so the v5 implementation is broken forhttps://parent + ../siblingstyle submodules. The v6 fix ingo-git/go-git@0e6f8484lives inWorktree.Submodules()and is not backported. We could pre-resolve URLs ourselves and call nativeUpdate, but in v5SubmoduleUpdateOptionsdoes not surfaceInsecureSkipTLS,CABundle, orProxyOptions, and the internalfetchAndCheckoutonly forwardsAuthandDepthtoFetchContext. Going native silently drops self-signed-cert, corporate-CA, and proxy support for submodule fetches. Keeping the bespoke clone is the right call; the reasoning is the TLS/proxy regression, not the relative-URL bug bjornrobertsson cited in-thread.