fix(gate): the security scan's own diff was the seventh unvalidated git ref #5497
gluecron[bot]🤖 botAI Reviewcommented 20d ago
AI review unavailable
The platform's AI balance is exhausted, so AI generation is temporarily unavailable. Nothing was lost. You can queue this as a repair for the internal agent from the repository's Health page, or try again once the balance is restored. The PR is otherwise unchanged.
ccantynzAI Reviewcommented 20d ago
AI Triage
(no summary)
Priority: medium Risk area: mixed
Suggested labels: (no label suggestions) Suggested reviewers: (no reviewer suggestions)
Suggestions only — nothing has been applied. The PR author stays in control.
Cross-repo impact
See what breaks downstream if this PR merges.
⮌ Merged
This pull request was merged into main.
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts
The bug
runAllGateChecksspawnedgit diffwith the range built by template:["git", "diff", `${baseBranch}...${headBranch}`]There is no shell here — it's an argv array — but git treats a leading-dash argument as an OPTION wherever it appears. A branch named
--output=<path>turnsgit diffinto an arbitrary file write as the app user.That is not theoretical.
src/__tests__/git-ref-injection.test.tsrecords the vector as live and verified against real git, reachable unauthenticated throughGET /:owner/:repo/compare/:specon any public repo. The shared guard built in response —isSafeRef/refRange/treeishinsrc/git/repository.ts— carries a comment noting six call sites that shipped this same bug.This was the seventh, added later at the gate chokepoint.
Two things were supposed to catch it. Between them, nothing did.
HIGH unvalidated-git-ref src/lib/gate.ts:566. Nobody was reading — partly because the same report carried a permanently-red CRITICAL that was entirely benign (fixed in #5495).git-ref-injection.test.ts— the suite written for this exact class — is inscripts/ci-test-excludes.txt, so CI never ran it. It passes fine (55 assertions); it's excluded for a subprocess-PATH reason unrelated to what it asserts.A rule with no teeth and a test that doesn't run is how a known-and-fixed vulnerability class reappears in a new file.
The fix
The range now goes through
refRange(base, head), which returnsnullrather than throwing — so an unsafe ref lands in the same "no diff" state the scan already degrades to, with a warning naming the repo and both refs.Verification
high 2 → 1(unvalidated-git-refcleared)git-ref-injection+git-arg-injection: 55 passtsc --noEmitcleanFollow-up (not in this PR)
Re-include
git-ref-injection.test.tsin CI (#213). The rule found this one; the test is what stops the eighth.