Recover from an ambient insteadOf rewrite onto a refused transport

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,

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,

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.<base>.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")
+}

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.<base>.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.<rewrite>.insteadOf
+// = <match> 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,
+	)
+}

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(),