Skip to content

Recover from an ambient insteadOf rewrite onto a refused transport - #2

Merged
andrew merged 1 commit into
git-pkgs:mainfrom
charlie-morrison:fix-ambient-insteadof-rewrite
Aug 6, 2026
Merged

Recover from an ambient insteadOf rewrite onto a refused transport#2
andrew merged 1 commit into
git-pkgs:mainfrom
charlie-morrison:fix-ambient-insteadof-rewrite

Conversation

@charlie-morrison

Copy link
Copy Markdown
Contributor

Fixes the fatal: transport 'ssh' not allowed reported in alpha-omega-security/scrutineer#820.

What is actually happening

This is not a transport bug — it is remoteEnv's whitelist doing exactly what its comment says it does:

GIT_ALLOW_PROTOCOL is a hard whitelist that overrides protocol.*.allow config, so an ambient url.<base>.insteadOf that rewrites an https:// URL to file://, ssh://, or ext:: is refused after ValidateURL has already approved the input.

The reporter's ~/.gitconfig is the documented case:

[url "ssh://git@github.com/"]
	insteadOf = https://github.com/

The rewrite really did move the request off the URL that was validated, and refusing it is right. The gap is that failure is the only outcome offered, on a machine whose Git config is otherwise perfectly ordinary — forcing SSH for a forge is a common setup.

So I did not add ssh to the whitelist. That would let any ambient rule silently redirect a validated URL, including onto ext::, which executes a command. The loud failure is better than that.

The fix

Retry once with the URL pinned to itself. Git resolves insteadOf by longest match, so a whole-URL self-map outranks any prefix rule:

git -c url.https://github.com/o/r.insteadOf=https://github.com/o/r ls-remote ...

Measured against the reporter's exact config, real git, no stubs:

$ git ls-remote --heads -- https://github.com/alpha-omega-security/scrutineer
fatal: transport 'ssh' not allowed

$ git -c url.<URL>.insteadOf=<URL> ls-remote --heads -- <URL>
e2eae551317a77e6b74413a3224b9bf711e177e9	refs/heads/add-sonnet-5-model
...

This is deliberately narrower than GIT_CONFIG_GLOBAL=os.DevNull, which would also discard the proxy, CA-bundle and credential settings a user legitimately keeps in the same file.

Why it reacts to the refusal instead of probing first

My first cut probed every URL with ls-remote --get-url before running the real command. I dropped it for two reasons, the second of which is a bug I caught by testing rather than reasoning:

  1. It bypassed the injected Runner, so it shelled out to real git even under a stubbed Retry — the existing exact-argv tests would have started depending on the developer's own ~/.gitconfig.
  2. It reimplemented Git's rewrite rules, and got them wrong. Deciding "is the rewritten URL allowed?" in Go means classifying the result, and insteadOf targets are very often Git's scp-like shorthand (git@github.com:o/r) which has no scheme at all. Git calls that ssh; a naive url.Parse does not.

Reacting to Git's own refusal makes the shorthand, longest-match ordering and protocol naming stay Git's to interpret.

It also gives the change no regression surface, which I checked rather than assumed:

  • https → https mirror rewrites are never refused, so never pinned. I verified pinning would break them: with insteadOf = https://github.com/https://gitlab.com/, --get-url gives https://gitlab.com/foo/bar normally and https://github.com/foo/bar once pinned. A blanket pin would silently bypass an operator's mirror. Reacting to a refusal cannot, because that config never fails.
  • A transport the caller opted into via GIT_ALLOW_PROTOCOL is not refused either. This is what keeps this package's own file://-via-insteadOf fixtures working — they pass GIT_ALLOW_PROTOCOL=https:file, and the full suite is green.

Cost: one extra invocation, only on a command that 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.

Coverage

All four URL-bearing remote paths: RemoteBranches, RemoteHead, Ensure's clone, fetchRef, and Cache.EnsureCommit's unshallow. fetchRef gained a url parameter for this — the fetch addresses the remote by name, but Git resolves origin's stored URL through the same rewrite rules, so fixing only the clone would leave the next scan broken.

Tests, all offline (Git refuses before any network access):

  • Three proving the recovery, one per entry point. Each fails on the unfixed code with the reported error — Ensure: repository unreachable https://example.com/repo: fatal: transport 'ssh' not allowed.
  • Two regression guards that pass before and after: a successful command is never pinned, and a non-transport failure does not buy a second round of attempts.
  • TestTransportRefusedMatchesGitsRefusal runs real git for both spellings — ssh://git@host/ and the scp-like git@host: — because the retry keys off Git's wording, and that is the fragile part of this design. Better to pin it against the binary than against a message I invented.
  • TestPinnedURLOutranksAmbientPrefixRewrite proves the longest-match premise itself with real git via --get-url.

go build, go vet, go test, go test -race and gofmt all clean.

Droppable, if you disagree

The Cache.EnsureCommit and fetchRef call sites are the ones I would cut first if you want this minimal — #820 only reports the add-repo path. I included them because a user who hits this hits it on every subsequent fetch too, but they are independent one-line changes.

I also considered failing with an actionable message instead of recovering ("your gitconfig rewrites this URL to ssh://, which this tool does not permit") and can switch to that if you would rather the ambient config never be overridden silently. I went with recovery because ValidateURL has already established the caller's intent, but it is your call.

A url.<base>.insteadOf rule in the user's Git config is applied after
ValidateURL has approved an https:// input, so it can move a request off
the URL the caller validated. remoteEnv's GIT_ALLOW_PROTOCOL whitelist
then refuses the transport it landed on, and the caller sees
"fatal: transport 'ssh' not allowed" for a URL it never asked to have
rewritten.

Retry such a command once with the URL pinned to itself. Git resolves
insteadOf by longest match, so a whole-URL self-map outranks any prefix
rule and restores the validated URL, without discarding the proxy, CA
bundle and credential settings that GIT_CONFIG_GLOBAL=os.DevNull would
also take out.

Reacting to Git's refusal rather than probing beforehand keeps working
setups untouched: an https-to-https mirror rewrite and a transport the
caller allowed through GIT_ALLOW_PROTOCOL are never refused, so they are
never pinned. A refusal matches no transient marker, so the first attempt
returns without backoff and the recovery costs one extra invocation on a
command that had already failed.
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