v1.87.7.0 fix(redact-prepush): four paths where the hook exits 0 on a real credential

[lubosxyz]

Why (in your own words)

bin/gstack-redact-prepush is the last thing standing between a working tree and a pushed
credential. On current main there are four ways to make it exit 0 with the secret going out
anyway. None of them needs an attacker — three are ordinary git usage and the fourth is a
consequence of the scanner's own slicing.

The first two are range-resolution bugs. remotesExclusion() already reasons about which
remote the push targets, but defaultRemoteBranch() kept asking origin regardless, and the two
together choose the scan range. On a repo where HEAD == origin/main, git push publish HEAD:main
to a second remote resolves a merge-base of HEAD against origin/main, which is HEAD..HEAD —
an empty diff. The whole tree ships to that remote with nothing scanned. Separately, the shape
check accepts any 40 plausible hex digits, so when git names a remote tip we do not have, the base
is guessed; when the guess yields an empty diff the hook concludes "nothing to scan" and allows,
while git is about to send whatever the real difference is. A guess that scanned nothing is not
evidence that there is nothing.

The other two come from scanAddedLines(). Its comment argues that overlap between slices is
unnecessary "because every pattern is single-line … a line boundary cannot bisect a detectable
secret". That is true of the match and false of the proximity requirement: five patterns in
lib/redact-patterns.ts carry a nearRegex and fire only when their qualifying label sits within
nearWindow characters — aws.secret_key (HIGH) needs aws_secret_access_key within 100,
gcp.service_account (HIGH) within 300. Put the label at the end of one slice and the secret at the
start of the next and the pattern never fires at all. The zero-width variant is the same hole
measured in the wrong unit: the engine strips zero-width characters before matching, so a budget
counted in raw bytes decides the seam using bytes the engine will never see, and invisible padding
can separate a label from its secret while their normalized distance stays a couple of characters.

Two adjacent defects in the same functions have to land with them. The fallback range used a
hardcoded SHA-1 empty-tree object id, which does not exist in a SHA-256 repository and hard-blocks
every legitimate first push of a new branch there — and the remote-scoping fix above makes that
fallback reachable more often, so it stops being cosmetic. And an over-budget single line (a
minified bundle, a one-line JSON blob) was handed to the engine whole, tripping
engine.input_too_large: blocked, which is safe, but blocked without the content ever being
read. That is the behavior that teaches an operator to reach for the skip switch, and the next
time the gate is right they skip anyway.

This is a follow-up to #2498, which you closed as absorbed by v1.67.0.0. Five of those six paths
are indeed closed on main today — ++-prefixed lines, unparseable stdin, diff.external, the
shape check on both shas, and localSha="0". My gate confirms all five still hold. These four are
the ones that survived, plus two the slicing introduced afterwards. The change is deliberately
targeted and leaves your architecture alone: addedLinesFromNewCommits() (#2592/#2573),
remotesExclusion()'s push-target scoping, and the 768 KiB slicing are all preserved.

Live evidence

test/redact-prepush-fail-open.sh is new in this PR: 32 scenarios, each driving the real scanner
through real git repositories with synthetic credentials assembled at runtime from halves that
match nothing on their own. It defaults to the in-tree scanner and takes an optional path, so the
same gate runs against any build.

Against the scanner on main (v1.87.5.0, bin/gstack-redact-prepush sha256
f9c85d22cdd13803ebaaf8cfbe9aa2469524bc47b51367a6cdb41ae219e5f209), the rows that differ:

$ bash test/redact-prepush-fail-open.sh /path/to/main/bin/gstack-redact-prepush
12 push to a non-origin remote                   BLOCK(aws.access_key)      ALLOW                      FAIL
16 missing-but-shaped remote sha                 BLOCK(aws.access_key)      ALLOW                      FAIL
17 zero-width across slice boundary              BLOCK(aws.secret_key)      ALLOW                      FAIL
18 proximity split by slice cut                  BLOCK(aws.secret_key)      ALLOW                      FAIL
20 absent named tip, non-empty wrong guess       BLOCK(aws.access_key)      ALLOW                      FAIL
E5 URL push does not borrow origin's base        BLOCK(aws.access_key)      ALLOW                      FAIL
19 label at the end of an over-overlap line      BLOCK(aws.secret_key)      ALLOW                      FAIL
21 short line between label line and secret     BLOCK(aws.secret_key)      ALLOW                      FAIL
22 several short lines between them             BLOCK(aws.secret_key)      ALLOW                      FAIL
E8 sha256 repo, new branch                       BLOCK(aws.access_key)      BLOCK(unscannable)         FAIL
E8b clean sha256 first push allowed              ALLOW                      BLOCK(unscannable)         FAIL

GATE: FAIL        (21/32 pass; exit 1)

Nine of those are fail-open — main exits 0 with the credential in the push. The last two are the
SHA-256 pair: main hard-blocks a clean first push there on a diff error, which is unusable rather
than safe. The other 21 rows pass identically on both builds, and that is the point of including
them: they are the no-detection-regression check.

Against this PR's head:

$ bash test/redact-prepush-fail-open.sh
 1 clean 2 MiB, no credential                    ALLOW                      ALLOW                      ok
 2 small + AWS key                               BLOCK(aws.access_key)      BLOCK(aws.access_key)      ok
 3 AWS key buried in 3 MiB                       BLOCK(aws.access_key)      BLOCK(aws.access_key)      ok
 4 PEM straddling window cut                     BLOCK(pem.private_key)     BLOCK(pem.private_key)     ok
 5 AWS key exactly at window cut                 BLOCK(aws.access_key)      BLOCK(aws.access_key)      ok
 6 unreadable range (fail-closed)                BLOCK(unscannable)         BLOCK(unscannable)         ok
 7 credential on a '++...' line                  BLOCK(aws.access_key)      BLOCK(aws.access_key)      ok
 8 malformed stdin (fail-closed)                 BLOCK(unscannable)         BLOCK(unscannable)         ok
 9 gcp key spanning the window cut               BLOCK(pem.private_key)     BLOCK(pem.private_key)     ok
10 diff.external set + real key                  BLOCK(aws.access_key)      BLOCK(aws.access_key)      ok
11 credential in a 2 MiB one-liner               BLOCK                      BLOCK(aws.access_key)      ok
12 push to a non-origin remote                   BLOCK(aws.access_key)      BLOCK(aws.access_key)      ok
13 junk REMOTE sha                               BLOCK(unscannable)         BLOCK(unscannable)         ok
14 zero-width padding at the seam                BLOCK(aws.secret_key)      BLOCK(aws.secret_key)      ok
15 localSha="0" is not a delete                  BLOCK(unscannable)         BLOCK(unscannable)         ok
16 missing-but-shaped remote sha                 BLOCK(aws.access_key)      BLOCK(aws.access_key)      ok
17 zero-width across slice boundary              BLOCK(aws.secret_key)      BLOCK(aws.secret_key)      ok
18 proximity split by slice cut                  BLOCK(aws.secret_key)      BLOCK(aws.secret_key)      ok
19 label at the end of an over-overlap line      BLOCK(aws.secret_key)      BLOCK(aws.secret_key)      ok
20 absent named tip, non-empty wrong guess       BLOCK(aws.access_key)      BLOCK(aws.access_key)      ok
21 short line between label line and secret     BLOCK(aws.secret_key)      BLOCK(aws.secret_key)      ok
22 several short lines between them             BLOCK(aws.secret_key)      BLOCK(aws.secret_key)      ok
E1 new branch off origin/main stays narrow       ALLOW                      ALLOW                      ok
E2 origin/HEAD -> trunk resolves, stays narrow   ALLOW                      ALLOW                      ok
E3 fetched non-origin remote anchors on its own  ALLOW                      ALLOW                      ok
E4 branch delete skipped                         ALLOW                      ALLOW                      ok
E5 URL push does not borrow origin's base        BLOCK(aws.access_key)      BLOCK(aws.access_key)      ok
E5b clean URL push still allowed                 ALLOW                      ALLOW                      ok
E6 long-line slicer survives multi-byte text     BLOCK                      BLOCK(aws.access_key)      ok
E7 ordinary push of clean content                ALLOW                      ALLOW                      ok
E8 sha256 repo, new branch                       BLOCK(aws.access_key)      BLOCK(aws.access_key)      ok
E8b clean sha256 first push allowed              ALLOW                      ALLOW                      ok

GATE: PASS (32/32)   exit 0

Every test that references the scanner, on this head:

$ bun test test/*redact*.test.ts test/ship-template-redaction.test.ts
 283 pass
 0 fail          (15 files)

$ bun test test/egress-receipt-wiring.test.ts
 10 pass
 0 fail

$ bun test test/redact-prepush-fail-open.test.ts      # runs the 32-scenario gate
 1 pass
 0 fail

$ shellcheck --severity=error test/redact-prepush-fail-open.sh
# no output, exit 0

Free suite. The full local bun run test fails in test/pty-screen.test.ts,
test/relink.test.ts and the Chromium and compiled-CSO lanes — they need artifacts CI builds first
(build:gates, build:cso, vendor:xterm, the browse node server) which I did not build here, and
CONTRIBUTING notes dev boxes see flakes in that lane while the required CI free lane turns on its
retry pass. Following CLAUDE.md's blame protocol, I measured it rather than asserting it: the same
two files, same machine, runs serialized — main (35dd014) 62 pass / 8 fail, this branch
62 pass / 8 fail, and a diff of the sorted failing-test lists is identical. This branch
introduces none of them.

What changed after review

A first version of this branch closed the four classes only partially, and your review process
should see that rather than discover it. An independent cross-family review found three shapes the
first version still let through with exit 0, and one gate row that could not have caught a broken
fix. All four are fixed in a separate commit so the delta is readable:

• URL push (the most important one). pushRemote() returns null for a URL, and
  defaultRemoteBranch() then fell back to origin; remotesExclusion() fell back to a bare
  --remotes. With the credential already on origin/main and a harmless local commit, a first
  push to a new empty URL remote anchored on origin, excluded everything origin had seen, scanned
  the harmless commit only, and shipped the credential. My gate's E5 row asserted ALLOW for
  exactly that shape — it encoded the hole as expected behaviour. The push target is now a
  three-way value (configured remote / URL / unknown): a URL anchors on nothing and excludes
  nothing; an unknown target (no argv, stdin or CLI) keeps the historical origin-shaped behaviour.
  E5 now expects the block, and E5b pins that a clean URL push is still allowed.
• Absent tip, non-empty guess. The first version blocked only when the guessed range was
  EMPTY. A stale tracking ref holding the credential plus one harmless commit gives a non-empty,
  credential-free guess, and it passed. An absent named tip now gets no local narrowing: the whole
  reachable history is scanned. That is slower on that rare path and affordable only because every
  scan is sliced under the engine's cap — which is exactly why the old code avoided it. A new branch
  (zero remote sha) is deliberately not treated this way, since git named no tip there and the
  tracking refs are not contradicted. Row 20.
• Over-overlap line. The inter-slice overlap carried whole lines, so a single line longer than
  16 KiB contributed nothing to the seam. A ~770 KB line ending in aws_secret_access_key =
  followed by the key never fired. The overlap now carries such a line's suffix. Row 19.
• E8 could not discriminate. It carried a credential, so a build that cannot resolve the
  SHA-256 fallback range still "blocked" — with a diff error instead of a finding. E8b is a clean
  SHA-256 first push that must be allowed, and main fails it.

A second review round found that my overlap fix was itself incomplete: it carried a long line's
suffix only when the tail was otherwise empty, so one short line between the long label line and
the secret landed in the tail first and the label was dropped again. The overlap is now filled from
the seam backwards to its exact budget, taking a partial line wherever a whole one no longer fits.
Rows 21 and 22 pin the one-short-line and several-short-lines forms; main fails both.

One upstream test changes, and I want to be explicit about it because it reverses a choice you
made. In test/redact-prepush-scan-range.test.ts, the S1 case "an unconfigured name (URL push)
also falls back rather than erroring" asserted exit 0. Its fixture merges a commit whose secret
was pushed only to a private other remote, then pushes to a URL whose claimed tip does not
contain it — so that push really delivers the secret to the URL. The S1 comment accepts this as
"falling back scans LESS than the narrowed form would, but never less than the hook historically
did". That trade is the same fail-open as E5. I kept the test's stated purpose — it still asserts
the hook does not error on an unconfigured name — and changed the outcome assertion to the block.
The updated test fails on main and passes here. If you would rather keep the historical leniency
for URL pushes, this is the one assertion to revert, and E5 goes with it.

Why S1's URL case changes but its no-argv neighbour does not, since the two share a fixture and
now carry opposite verdicts. The URL case is a real push: git hands the hook a destination, and that
destination is described by no tracking ref, so nothing may be excluded. The no-argv case is the
documented contract for invoking the hook without arguments — stdin or a direct CLI call, where no
destination exists at all — and real pushes always carry the name. There is nothing to protect
there, so it keeps the historical behaviour, and pushTarget() models it as its own unknown state
rather than as a URL.

Known limitation, not addressed here

The hook scans the net diff of the range it computes, never each commit. A credential added in
one commit and removed in a later one within the same push is absent from that diff, so it is not
seen — yet git delivers both commits. I measured this on main and on this branch, across the three
push shapes that pick different ranges:

                                         main        this branch
new branch (zero remote sha)             ALLOW       ALLOW
absent named remote tip                  ALLOW       ALLOW
existing branch (remote tip present)     ALLOW       ALLOW

It is identical on both, including the ordinary existing-branch push, so this branch neither causes
nor widens it; it is a property of diff-based scanning. Closing it means scanning each commit's own
diff in the range, which changes the hook's cost profile on every push and deserves its own PR and
its own review rather than riding in on this one. The code comment on the absent-tip path now says
this plainly instead of claiming to scan "everything reachable".

Scope

• Changed: bin/gstack-redact-prepush (pushRemote() added; defaultRemoteBranch() scoped to
  the push target and nullable; unknownRemoteTipBase() falls through on a null default;
  addedLinesFor() blocks when a guessed base scanned nothing; emptyTree() asks git for the
  object id; SCAN_OVERLAP_BYTES + LINE_BUDGET_BYTES + zero-width stripping on ingest +
  sliceLongLine() in scanAddedLines(), with finding dedup on id+preview because overlap
  deliberately re-scans the seam). New: test/redact-prepush-fail-open.sh,
  test/redact-prepush-fail-open.test.ts; the S1 URL-push assertion in test/redact-prepush-scan-range.test.ts (see above). VERSION → 1.87.7.0 and a CHANGELOG.md entry.
• Verified live by: the 32-scenario gate above, run against both the main scanner and this
  head; bun run test; shellcheck --severity=error; and a spawn-based bun test so the gate is
  part of the free suite rather than a script nobody runs.
• Did NOT test: a real git push to a real second remote over the network (the gate drives the
  hook's stdin protocol against local repositories instead, which is what the hook actually sees);
  Windows; a genuinely huge repository for wall-clock regression beyond the gate's fixtures.
• Deliberately not ported: streaming the diff instead of spawnSync, and a wall-clock budget.
  Both would mean rewriting addedLinesFor()/addedLinesFromNewCommits() and throwing away your
  range narrowing, and the remaining 64 MiB maxBuffer limit is fail-closed (BLOCK(unscannable)),
  not a hole. One thing stays open and I am not claiming otherwise: a stale tracking ref for the
  target remote. If local <remote>/<branch> survives a reset of the remote and the local commit
  is its descendant, the range is computed against that stale tip and the real content is not
  scanned. Fixing that honestly needs git ls-remote — a network call inside a pre-push hook —
  which is your call, not something to append here. Scenario 16 does not cover it: 16 is about a tip
  git named that we do not have, not a tip we have wrongly.

Liveness proof (required for external contributors)

I have not attached a GSTACK PR screenshot, and I would rather say why than quietly skip the box.
This change was prepared with agent assistance on my own machine and under my own account, so an
image produced from that session would be me manufacturing the human-presence attestation the gate
exists to check. GitHub also has no API for attaching an image to a PR body — only drag-drop in the
web UI — so it is not something that session could have produced honestly anyway.

What I can give you is the provenance the template asks agents for:

$ gh api user --jq .login
lubosxyz

If you want the screenshot before reviewing, say so and I will add it myself in the web UI.

Checklist

• Liveness screenshot attached (GSTACK PR typed live into a real surface) or PR author is @garrytan (owner exemption) — not attached; see the section above, gh api user --jq .login → lubosxyz, happy to add on request
• This is not a generated-file-only diff (I edited the source/template and regenerated)
• No ETHOS.md edits, and no changes to voice / founder perspective / YC references
• New public command / external service / host adapter has an accepted issue linked (or N/A) — N/A, no new command or service
• Linked issue or reproduction: follow-up to fix(redact-prepush): six paths where the hook exits 0 on a real credential #2498 (closed as absorbed by v1.67.0.0); the reproduction is test/redact-prepush-fail-open.sh, which fails on main and passes here

[trunk-io]

Merging to main in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[capy-ai]

Thank you @lubosxyz for the detailed destination/range and normalized-proximity reproductions, including the follow-up negative controls. Your PR is credited as diagnostic source in #2942, merged as b9706f3 for v1.88.1.0.

The independently implemented repair binds scans to the actual destination name and effective URL, treats absent advertised objects conservatively, computes the correct empty tree for SHA-256 repositories, and preserves normalized proximity context across bounded scan windows. The implementation is in bin/gstack-redact-prepush; installed-hook regressions exercise real pushes to disposable local remotes, including seam failures and clean controls.

Closing this PR as superseded, not cherry-picked or merged. One deliberate difference: a line or required context over the existing 1 MiB cap is still refused, rather than claiming it was fully scanned. This closure also makes no new claim about the separately documented net-diff/transient-secret or stale-tracking-ref limitations. Thank you for identifying both the security failures and the boundaries of your proposed fix.