Skip to content

Commit 92f5137

Browse files
Security: only forward auth to submodules on same host
Addresses credential exfiltration risk where a malicious .gitmodules could point to an attacker-controlled server and receive the parent repo's auth. - Add extractHost() to parse host from standard URLs and SCP-like URLs - Add SameHost() to compare hosts (case-insensitive, ignores port) - Only forward RepoAuth to submodule if hosts match - Log warning when auth is not forwarded due to different host - Add comprehensive tests for SameHost()
1 parent 92783ef commit 92f5137

2 files changed

Lines changed: 137 additions & 3 deletions

File tree

git/git.go

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,32 @@ func ProgressWriter(write func(line string, args ...any)) io.WriteCloser {
439439
// This handles: git@github.com:org/repo, deploy@host:repo, user@10.0.0.5:project
440440
var scpLikeURLRegex = regexp.MustCompile(`^([^@]+)@([^:]+):(.+)$`)
441441

442+
// extractHost extracts the host from a URL, handling both standard URLs and SCP-like URLs.
443+
// Returns empty string if host cannot be determined.
444+
func extractHost(u string) string {
445+
// Try standard URL parsing first
446+
if parsed, err := url.Parse(u); err == nil && parsed.Host != "" {
447+
// Remove port if present
448+
host := parsed.Hostname()
449+
return strings.ToLower(host)
450+
}
451+
452+
// Handle SCP-like URLs: user@host:path
453+
if matches := scpLikeURLRegex.FindStringSubmatch(u); matches != nil {
454+
return strings.ToLower(matches[2])
455+
}
456+
457+
return ""
458+
}
459+
460+
// SameHost checks if two URLs point to the same host.
461+
// Used to determine if credentials should be forwarded to submodules.
462+
func SameHost(url1, url2 string) bool {
463+
host1 := extractHost(url1)
464+
host2 := extractHost(url2)
465+
return host1 != "" && host2 != "" && host1 == host2
466+
}
467+
442468
// RedactURL redacts credentials from a URL for safe logging.
443469
// Handles:
444470
// - Standard URLs with userinfo: https://user:pass@host, https://token@host
@@ -627,6 +653,16 @@ func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktr
627653
return fmt.Errorf("chroot to submodule path: %w", err)
628654
}
629655

656+
// Security: Only forward parent repo auth if submodule is on the same host.
657+
// This prevents credential exfiltration if a malicious .gitmodules points
658+
// to an attacker-controlled server.
659+
var submoduleAuth transport.AuthMethod
660+
if SameHost(opts.RepoURL, resolvedURL) {
661+
submoduleAuth = opts.RepoAuth
662+
} else if opts.RepoAuth != nil {
663+
logf(" ⚠ Not forwarding auth to submodule (different host: %s)", extractHost(resolvedURL))
664+
}
665+
630666
// Check if already cloned
631667
_, err = subFS.Stat(".git")
632668
if err == nil {
@@ -675,7 +711,7 @@ func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktr
675711
// Use SingleBranch=false to fetch all branches so we can find the commit
676712
subRepo, err := git.CloneContext(ctx, gitStorage, subFS, &git.CloneOptions{
677713
URL: resolvedURL,
678-
Auth: opts.RepoAuth,
714+
Auth: submoduleAuth,
679715
Progress: opts.Progress,
680716
InsecureSkipTLS: opts.Insecure,
681717
CABundle: opts.CABundle,
@@ -698,7 +734,7 @@ func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktr
698734
RefSpecs: []config.RefSpec{
699735
config.RefSpec("+" + expectedHash.String() + ":" + expectedHash.String()),
700736
},
701-
Auth: opts.RepoAuth,
737+
Auth: submoduleAuth,
702738
Progress: opts.Progress,
703739
InsecureSkipTLS: opts.Insecure,
704740
CABundle: opts.CABundle,
@@ -709,7 +745,7 @@ func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktr
709745
logf(" Direct fetch failed, fetching all refs...")
710746
err = subRepo.FetchContext(ctx, &git.FetchOptions{
711747
RemoteName: "origin",
712-
Auth: opts.RepoAuth,
748+
Auth: submoduleAuth,
713749
Progress: opts.Progress,
714750
InsecureSkipTLS: opts.Insecure,
715751
CABundle: opts.CABundle,

git/git_test.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,104 @@ func TestRedactURL(t *testing.T) {
680680
}
681681
}
682682

683+
func TestSameHost(t *testing.T) {
684+
t.Parallel()
685+
686+
cases := []struct {
687+
name string
688+
url1 string
689+
url2 string
690+
expect bool
691+
}{
692+
// Same host cases
693+
{
694+
name: "https same host",
695+
url1: "https://github.com/org/repo.git",
696+
url2: "https://github.com/other/submodule.git",
697+
expect: true,
698+
},
699+
{
700+
name: "https and scp same host",
701+
url1: "https://github.com/org/repo.git",
702+
url2: "git@github.com:other/submodule.git",
703+
expect: true,
704+
},
705+
{
706+
name: "scp same host",
707+
url1: "git@github.com:org/repo.git",
708+
url2: "git@github.com:other/submodule.git",
709+
expect: true,
710+
},
711+
{
712+
name: "case insensitive",
713+
url1: "https://GitHub.com/org/repo.git",
714+
url2: "https://github.com/other/submodule.git",
715+
expect: true,
716+
},
717+
{
718+
name: "with port same host",
719+
url1: "https://github.com:443/org/repo.git",
720+
url2: "https://github.com/other/submodule.git",
721+
expect: true,
722+
},
723+
{
724+
name: "ssh scheme same host",
725+
url1: "ssh://git@github.com/org/repo.git",
726+
url2: "https://github.com/other/submodule.git",
727+
expect: true,
728+
},
729+
730+
// Different host cases
731+
{
732+
name: "different hosts",
733+
url1: "https://github.com/org/repo.git",
734+
url2: "https://gitlab.com/other/submodule.git",
735+
expect: false,
736+
},
737+
{
738+
name: "scp different hosts",
739+
url1: "git@github.com:org/repo.git",
740+
url2: "git@evil.com:exfiltrate/creds.git",
741+
expect: false,
742+
},
743+
{
744+
name: "subdomain is different",
745+
url1: "https://github.com/org/repo.git",
746+
url2: "https://api.github.com/other/submodule.git",
747+
expect: false,
748+
},
749+
750+
// Edge cases
751+
{
752+
name: "empty url1",
753+
url1: "",
754+
url2: "https://github.com/other/submodule.git",
755+
expect: false,
756+
},
757+
{
758+
name: "relative url",
759+
url1: "https://github.com/org/repo.git",
760+
url2: "../other/submodule.git",
761+
expect: false,
762+
},
763+
{
764+
name: "file path",
765+
url1: "https://github.com/org/repo.git",
766+
url2: "/local/path/to/repo",
767+
expect: false,
768+
},
769+
}
770+
771+
for _, tc := range cases {
772+
tc := tc
773+
t.Run(tc.name, func(t *testing.T) {
774+
t.Parallel()
775+
got := git.SameHost(tc.url1, tc.url2)
776+
require.Equal(t, tc.expect, got)
777+
})
778+
}
779+
}
780+
683781
func TestResolveSubmoduleURL(t *testing.T) {
684782
t.Parallel()
685783

0 commit comments

Comments
 (0)