fix(redact-prepush): close the ext-diff, header-lookalike, and ref-parse bypasses

Three ways the pushed diff escaped scanning: (1) a user-level diff.external
or textconv driver replaced the diff with its own output — zero '+' lines,
so the scan saw nothing (now --no-ext-diff --no-textconv); (2) an added
content line whose text begins with "++" renders as "+++…" and the blanket
header skip dropped it (now hunk-aware header detection); (3) a pre-push
ref line that failed to parse was silently skipped, leaving that ref
unscanned (now fails closed with the offending line named).

Minimal reimplementation of the two confirmed bypasses from PR #2498 by
@lubosxyz (the full PR overlaps the chunked-scan work absorbed separately),
plus the unparseable-ref hardening.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

bin/gstack-redact-prepush

@@ -140,12 +140,28 @@ function addedLinesFor(localSha: string, remoteSha: string): string {
   // +++ file header. Unified diff added lines start with a single '+'.
   // Strict (#1946): a failed diff used to return "" and the push sailed
   // through unscanned — fail open on the exact path the guard exists for.
-  const diff = gitStrict(["diff", "--unified=0", "--no-color", range]);
+  //
+  // --no-ext-diff: a user's `diff.external` driver replaces the entire diff
+  // with its own output — with one set, `git diff` emits zero '+' lines, so an
+  // unhardened scanner reads an empty diff and exits 0 on a push full of
+  // secrets. Reachable from ordinary user config, not hypothetical. (#2498)
+  // --no-textconv: a .gitattributes textconv driver can likewise rewrite
+  // content before we ever see it. (#2498)
+  const diff = gitStrict([
+    "diff", "--unified=0", "--no-color", "--no-ext-diff", "--no-textconv",
+    range,
+  ]);
   const added: string[] = [];
+  // Hunk-aware header skip (#2498): `+++ ` is only a FILE HEADER outside a
+  // hunk. Inside a hunk, an added content line whose text begins with "++"
+  // renders as "+++<content>" — the old blanket startsWith("+++") skip
+  // silently dropped exactly those lines from the scan.
+  let inHunk = false;
   for (const line of diff.split("\n")) {
-    if (line.startsWith("+") && !line.startsWith("+++")) {
-      added.push(line.slice(1));
-    }
+    if (line.startsWith("diff --git")) { inHunk = false; continue; }
+    if (line.startsWith("@@")) { inHunk = true; continue; }
+    if (!inHunk && (line.startsWith("+++") || line.startsWith("---"))) continue;
+    if (line.startsWith("+")) added.push(line.slice(1));
   }
   return added.join("\n");
 }
@@ -239,8 +255,23 @@ function main() {
   const allHigh: Finding[] = [];
   let mediumCount = 0;
 
-  for (const [, localSha, , remoteSha] of refs) {
-    if (!localSha || ZERO.test(localSha)) continue; // branch delete → nothing pushed
+  for (const fields of refs) {
+    // Fail CLOSED on a ref line we cannot parse (#2498): git hands pre-push
+    // exactly "<local ref> <local sha> <remote ref> <remote sha>" — anything
+    // else means we cannot tell WHAT is being pushed, and silently skipping
+    // it would leave that ref unscanned.
+    const [, localSha, , remoteSha] = fields;
+    const shaShaped = (s: string | undefined) => !!s && /^[0-9a-f]{40,64}$/i.test(s);
+    if (fields.length !== 4 || !shaShaped(localSha) || !shaShaped(remoteSha)) {
+      process.stderr.write(
+        "\n⛔ gstack-redact-prepush BLOCKED the push — could not parse a pre-push ref line, " +
+          "so its content cannot be scanned.\n" +
+          `  line: ${JSON.stringify(fields.join(" "))}\n` +
+          "Bypass if you're sure: GSTACK_REDACT_PREPUSH=skip git push   (or git push --no-verify)\n",
+      );
+      process.exit(1);
+    }
+    if (ZERO.test(localSha!)) continue; // branch delete → nothing pushed
     let added: string;
     try {
       added = addedLinesFor(localSha, remoteSha || "0");

test/redact-prepush-hook.test.ts

@@ -324,3 +324,33 @@ describe("base resolution when the default branch is neither main nor master", (
     expect(stderr).toContain("aws.access_key");
   });
 });
+
+describe("diff-extraction bypasses (#2498, minimal reimplementation)", () => {
+  test("a diff.external driver cannot blank the scanned diff", () => {
+    // With diff.external set, plain `git diff` emits the driver's output —
+    // typically zero '+' lines — so an unhardened scanner reads an empty diff
+    // and allows a push full of secrets. --no-ext-diff must neutralize it.
+    const head = commit("leak.txt", "AKIA1234567890ABCDEF\n", "secret behind ext driver");
+    git(["config", "diff.external", "/usr/bin/true"]);
+    const { code, stderr } = runHook(`refs/heads/feat ${head} refs/heads/feat ${ZERO}\n`);
+    git(["config", "--unset", "diff.external"]);
+    expect(code).toBe(1);
+    expect(stderr).toContain("aws.access_key");
+  });
+
+  test("an added content line starting with ++ is still scanned", () => {
+    // Content "++AKIA…" renders in the diff as "+++AKIA…", which a blanket
+    // startsWith('+++') header skip silently dropped from the scan.
+    const head = commit("notes.txt", "++AKIA1234567890ABCDEF\n", "content line looks like a header");
+    const { code, stderr } = runHook(`refs/heads/feat ${head} refs/heads/feat ${ZERO}\n`);
+    expect(code).toBe(1);
+    expect(stderr).toContain("aws.access_key");
+  });
+
+  test("an unparseable pre-push ref line fails closed", () => {
+    commit("ok.txt", "clean\n", "clean commit");
+    const { code, stderr } = runHook(`refs/heads/feat not-a-sha\n`);
+    expect(code).toBe(1);
+    expect(stderr).toContain("could not parse");
+  });
+});