diff --git a/cache.go b/cache.go index eb43ff4..53fbcba 100644 --- a/cache.go +++ b/cache.go @@ -80,7 +80,7 @@ func (c *Cache) EnsureCommit(ctx context.Context, url, commit string) error { if strings.TrimSpace(out) != "true" { return nil } - out, err := policy.Do(ctx, Command{ + out, err := doPinnedURL(ctx, policy, url, Command{ Args: []string{"fetch", "--unshallow", "--quiet", "origin"}, //nolint:goconst // Git argv is clearer with literal subcommands and flags. Label: "fetch", //nolint:goconst // Retry notices use the literal Git subcommand. Dir: cacheSrc, diff --git a/ensure.go b/ensure.go index eb8dfe5..ba1772b 100644 --- a/ensure.go +++ b/ensure.go @@ -48,7 +48,7 @@ func ensure(ctx context.Context, retry Retry, url, dst, ref string, full bool) e return err } if _, err := os.Stat(filepath.Join(dst, ".git")); err == nil { - return fetchRef(ctx, retry, dst, ref, full) + return fetchRef(ctx, retry, url, dst, ref, full) } if err := os.MkdirAll(filepath.Dir(dst), dirPerm); err != nil { return err @@ -64,7 +64,7 @@ func ensure(ctx context.Context, retry Retry, url, dst, ref string, full bool) e args = append(args, "--depth", "1") } args = append(args, "--", url, dst) - out, err := retry.Do(ctx, Command{ + out, err := doPinnedURL(ctx, retry, url, Command{ Label: "clone", Env: remoteEnv(), Args: args, @@ -74,12 +74,15 @@ func ensure(ctx context.Context, retry Retry, url, dst, ref string, full bool) e return fmt.Errorf("%s: %w", strings.TrimSpace(out), err) } if ref != "" { - return fetchRef(ctx, retry, dst, ref, full) + return fetchRef(ctx, retry, url, dst, ref, full) } return nil } -func fetchRef(ctx context.Context, retry Retry, dst, ref string, full bool) error { +// fetchRef takes url only so it can be pinned against ambient insteadOf +// rewriting: the fetch addresses the remote by name, but Git resolves origin's +// stored URL -- the validated one clone recorded -- through the same rules. +func fetchRef(ctx context.Context, retry Retry, url, dst, ref string, full bool) error { policy := retry.Resolved() target := ref if target == "" { @@ -93,7 +96,7 @@ func fetchRef(ctx context.Context, retry Retry, dst, ref string, full bool) erro } } args = append(args, "--", "origin", target) - out, err := policy.Do(ctx, Command{ + out, err := doPinnedURL(ctx, policy, url, Command{ Label: "fetch", //nolint:goconst // Retry notices use the literal Git subcommand. Env: remoteEnv(), Args: args, diff --git a/insteadof.go b/insteadof.go new file mode 100644 index 0000000..c3b4fe2 --- /dev/null +++ b/insteadof.go @@ -0,0 +1,66 @@ +package clone + +import ( + "context" + "strings" +) + +// doPinnedURL runs cmd, and retries it once with url pinned to itself when Git +// refused the transport the command ended up on. +// +// Git applies url..insteadOf rewriting after ValidateURL has already +// approved an https:// input, so a global ~/.gitconfig such as +// +// [url "ssh://git@github.com/"] +// insteadOf = https://github.com/ +// +// silently changes the transport of every github.com URL a caller passes in. +// remoteEnv's GIT_ALLOW_PROTOCOL whitelist then refuses the rewritten +// transport, and the caller is left with "fatal: transport 'ssh' not allowed" +// for a URL it never asked to have rewritten. The whitelist is doing its job -- +// the rewrite really did move the request off the URL that was validated -- but +// failure is the only outcome it can offer, on a machine whose Git config is +// otherwise perfectly ordinary. +// +// Git resolves insteadOf by longest match, so mapping the whole URL to itself +// outranks any prefix rule and restores the validated URL. That is deliberately +// narrower than GIT_CONFIG_GLOBAL=os.DevNull, which would also discard the +// proxy, CA-bundle and credential configuration a user legitimately keeps in +// the same file. +// +// The pin is applied only after Git has already refused, which is what keeps it +// from changing any working setup: +// +// - A rewrite between https:// URLs, the usual internal-mirror case, is never +// refused, so it is never touched. +// - A rewrite onto a transport the caller allowed through GIT_ALLOW_PROTOCOL +// is not refused either, so a deliberate opt-in still wins. +// +// Letting Git decide also avoids second-guessing its rewrite rules here: the +// scp-like shorthand, longest-match ordering and protocol naming stay Git's to +// interpret. The retry costs one extra invocation, and only on a command that +// has already failed: a refusal matches no transient marker, so TransientFailure +// treats it as permanent and the first Do returns after a single attempt with no +// backoff. +func doPinnedURL(ctx context.Context, retry Retry, url string, cmd Command) (string, error) { + out, err := retry.Do(ctx, cmd) + if err == nil || url == "" || !transportRefused(out) { + return out, err + } + pinned := cmd + pinned.Args = append(pinArgs(url), cmd.Args...) + return retry.Do(ctx, pinned) +} + +// pinArgs returns the Git configuration that maps url to itself, so Git's +// longest-match rule prefers it over any ambient prefix rewrite. +func pinArgs(url string) []string { + return []string{"-c", "url." + url + ".insteadOf=" + url} +} + +// transportRefused reports whether out is Git refusing a transport that the +// GIT_ALLOW_PROTOCOL whitelist does not list, as opposed to any other failure. +func transportRefused(out string) bool { + _, rest, ok := strings.Cut(out, "transport '") + return ok && strings.Contains(rest, "' not allowed") +} diff --git a/insteadof_test.go b/insteadof_test.go new file mode 100644 index 0000000..1c037d6 --- /dev/null +++ b/insteadof_test.go @@ -0,0 +1,214 @@ +package clone + +import ( + "context" + "errors" + "os/exec" + "path/filepath" + "slices" + "strings" + "testing" +) + +const refusalOutput = "fatal: transport 'ssh' not allowed\n" + +func refusedOnce(calls *[][]string, out string) Runner { + return func(_ context.Context, _ string, _ []string, args ...string) (string, error) { + *calls = append(*calls, append([]string(nil), args...)) + if len(*calls) == 1 { + return refusalOutput, errors.New("exit status 128") + } + return out, nil + } +} + +// An ambient url..insteadOf in the user's Git config rewrites a URL that +// ValidateURL already approved, and GIT_ALLOW_PROTOCOL then refuses the +// transport it landed on. The retry pins the URL to itself so the validated +// URL is the one Git contacts. +func TestRemoteBranchesRetriesWithPinnedURLAfterTransportRefusal(t *testing.T) { + const url = "https://example.com/repo" + var calls [][]string + retry := Retry{Run: refusedOnce(&calls, "bbb\trefs/heads/z\naaa\trefs/heads/main\n")} + + branches, err := RemoteBranches(context.Background(), retry, url) + if err != nil { + t.Fatalf("RemoteBranches: %v", err) + } + if !slices.Equal(branches, []string{"main", "z"}) { + t.Errorf("branches = %v, want [main z]", branches) + } + if len(calls) != 2 { + t.Fatalf("git invocations = %d, want 2", len(calls)) + } + unpinned := []string{"-c", "credential.helper=", "ls-remote", "--heads", "--", url} + if !slices.Equal(calls[0], unpinned) { + t.Errorf("first attempt = %v, want %v", calls[0], unpinned) + } + if want := append(pinArgs(url), unpinned...); !slices.Equal(calls[1], want) { + t.Errorf("retry = %v, want %v", calls[1], want) + } +} + +func TestRemoteHeadRetriesWithPinnedURLAfterTransportRefusal(t *testing.T) { + const url = "https://example.com/repo" + var calls [][]string + retry := Retry{Run: refusedOnce(&calls, "deadbeef\tHEAD\n")} + + head, err := RemoteHead(context.Background(), retry, url) + if err != nil { + t.Fatalf("RemoteHead: %v", err) + } + if head != "deadbeef" { + t.Errorf("head = %q, want deadbeef", head) + } + if len(calls) != 2 { + t.Fatalf("git invocations = %d, want 2", len(calls)) + } + if want := append(pinArgs(url), calls[0]...); !slices.Equal(calls[1], want) { + t.Errorf("retry = %v, want %v", calls[1], want) + } +} + +// Ensure clones through the same helper, so a refusal on the clone itself is +// recovered rather than surfacing as an UnreachableError. +func TestEnsureRetriesWithPinnedURLAfterTransportRefusal(t *testing.T) { + const url = "https://example.com/repo" + dst := filepath.Join(t.TempDir(), "checkout") + var calls [][]string + retry := Retry{Run: refusedOnce(&calls, "")} + + if err := Ensure(context.Background(), retry, url, dst, "", false); err != nil { + t.Fatalf("Ensure: %v", err) + } + if len(calls) != 2 { + t.Fatalf("git invocations = %d, want 2", len(calls)) + } + if !slices.Equal(calls[1], append(pinArgs(url), calls[0]...)) { + t.Errorf("retry = %v, want the first attempt with %v prepended", calls[1], pinArgs(url)) + } +} + +// The pin must not appear on a command Git accepted. An insteadOf rewrite +// between two https:// URLs -- the usual internal-mirror setup -- is never +// refused, so pinning it would break a working configuration to fix nothing. +func TestSuccessfulCommandIsNeverPinned(t *testing.T) { + const url = "https://example.com/repo" + var calls [][]string + retry := Retry{ + Run: func(_ context.Context, _ string, _ []string, args ...string) (string, error) { + calls = append(calls, append([]string(nil), args...)) + return "aaa\trefs/heads/main\n", nil + }, + } + if _, err := RemoteBranches(context.Background(), retry, url); err != nil { + t.Fatalf("RemoteBranches: %v", err) + } + if len(calls) != 1 { + t.Fatalf("git invocations = %d, want 1", len(calls)) + } + if slices.Contains(calls[0], pinArgs(url)[1]) { + t.Errorf("args = %v, want no insteadOf pin", calls[0]) + } +} + +// A failure Git did not describe as a refused transport is not something the +// pin can fix, so it must not buy the caller a second round of attempts. +func TestUnrelatedFailureIsNotRetriedWithPin(t *testing.T) { + const url = "https://example.com/repo" + var calls [][]string + retry := Retry{ + Run: func(_ context.Context, _ string, _ []string, args ...string) (string, error) { + calls = append(calls, append([]string(nil), args...)) + return "fatal: repository not found\n", errors.New("exit status 128") + }, + } + if _, err := RemoteBranches(context.Background(), retry, url); err == nil { + t.Fatal("RemoteBranches succeeded, want error") + } + if len(calls) != 1 { + t.Errorf("git invocations = %d, want 1", len(calls)) + } +} + +func TestTransportRefused(t *testing.T) { + cases := []struct { + name string + out string + want bool + }{ + {"ssh refusal", "fatal: transport 'ssh' not allowed", true}, + {"file refusal", "fatal: transport 'file' not allowed", true}, + {"ext refusal", "fatal: transport 'ext' not allowed", true}, + {"repository missing", "fatal: repository 'https://example.com/x' not found", false}, + {"connection refused", "fatal: unable to access: Connection refused", false}, + {"empty", "", false}, + {"quote without refusal", "fatal: transport 'ssh' is fine", false}, + } + for _, test := range cases { + if got := transportRefused(test.out); got != test.want { + t.Errorf("%s: transportRefused(%q) = %v, want %v", test.name, test.out, got, test.want) + } + } +} + +// The retry keys off Git's wording, so pin that wording against the real +// binary rather than a message invented here. Git refuses the transport before +// any network access, so this stays offline. +func TestTransportRefusedMatchesGitsRefusal(t *testing.T) { + requireGit(t) + const url = "https://example.invalid/owner/repo" + // Both spellings a hand-written insteadOf commonly uses: an ssh:// URL and + // Git's scp-like shorthand, which carries no scheme at all. + for _, rewrite := range []string{"ssh://git@example.invalid/", "git@example.invalid:"} { + t.Run(rewrite, func(t *testing.T) { + cmd := exec.Command("git", "ls-remote", "--heads", "--", url) + cmd.Env = append(ambientRewriteEnv(rewrite, "https://example.invalid/"), remoteEnv()...) + out, err := cmd.CombinedOutput() + if err == nil { + t.Fatalf("git accepted the rewritten URL: %s", out) + } + if !transportRefused(string(out)) { + t.Errorf("transportRefused did not match Git's refusal: %q", out) + } + }) + } +} + +// The fix rests on Git preferring the longest matching insteadOf, so prove +// that a whole-URL self-map outranks a prefix rule. --get-url applies the +// rewrite rules and prints the result without contacting the remote. +func TestPinnedURLOutranksAmbientPrefixRewrite(t *testing.T) { + requireGit(t) + const url = "https://example.invalid/owner/repo" + env := ambientRewriteEnv("ssh://git@example.invalid/", "https://example.invalid/") + + resolve := func(args ...string) string { + cmd := exec.Command("git", append(args, "ls-remote", "--get-url", "--", url)...) + cmd.Env = env + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %s: %v", args, out, err) + } + return strings.TrimSpace(string(out)) + } + + if got := resolve(); got == url { + t.Fatalf("fixture did not rewrite the URL: %q", got) + } else if !strings.HasPrefix(got, "ssh://") { + t.Fatalf("fixture rewrote to %q, want an ssh:// URL", got) + } + if got := resolve(pinArgs(url)...); got != url { + t.Errorf("pinned URL resolved to %q, want %q", got, url) + } +} + +// ambientRewriteEnv builds an environment carrying one url..insteadOf +// = rule, standing in for a rule in the user's ~/.gitconfig. +func ambientRewriteEnv(rewrite, match string) []string { + return append(gitTestEnv(), + "GIT_CONFIG_COUNT=1", + "GIT_CONFIG_KEY_0=url."+rewrite+".insteadOf", + "GIT_CONFIG_VALUE_0="+match, + ) +} diff --git a/remote.go b/remote.go index c971e1f..b042f95 100644 --- a/remote.go +++ b/remote.go @@ -13,7 +13,7 @@ func RemoteBranches(ctx context.Context, retry Retry, url string) ([]string, err if err := ValidateURL(url); err != nil { return nil, err } - out, err := retry.Do(ctx, Command{ + out, err := doPinnedURL(ctx, retry, url, Command{ Args: []string{"-c", "credential.helper=", "ls-remote", "--heads", "--", url}, //nolint:goconst // Git argv is clearer with literal subcommands. Label: "ls-remote", //nolint:goconst // Retry notices use the literal Git subcommand. Env: remoteEnv(), @@ -29,7 +29,7 @@ func RemoteHead(ctx context.Context, retry Retry, url string) (string, error) { if err := ValidateURL(url); err != nil { return "", err } - out, err := retry.Do(ctx, Command{ + out, err := doPinnedURL(ctx, retry, url, Command{ Args: []string{"-c", "credential.helper=", "ls-remote", "--", url, "HEAD"}, //nolint:goconst // Git argv is clearer with literal subcommands and refs. Label: "ls-remote", //nolint:goconst // Retry notices use the literal Git subcommand. Env: remoteEnv(),