Scan reachable history for secrets, not just the working tree

[ThinkOffApp]

The failure mode this closes

scripts/check-stageable-secrets.mjs reads files from disk with readFileSync and answers one question: would git add -A stage a credential right now. That is a useful question and it is not the dangerous one.

A key committed on Tuesday and deleted on Wednesday is gone from the working tree, gone from git status, gone from the file listing - and still sits in the pack file, one git log -p away from anyone who clones. To a working-tree scanner that repo looks spotless. It is not: it is leaking. This repo is PUBLIC, and a snapshot pushed into it is world-readable the instant it lands, with no private staging period in which to notice.

scripts/scan-history-for-secrets.mjs walks every blob reachable from all refs (git rev-list --objects --all, read through git cat-file --batch), so a deleted-but-reachable blob is examined like any other. It takes an optional repo path and defaults to the current repo.

Why the patterns are shared, not duplicated

The pattern list, the binary-extension skip list and the size cap moved to src/secret-patterns.mjs, imported by both scanners.

Two lists is not two scanners, it is one scanner and one decoy: the day somebody adds a new key format to the list they happen to be looking at, the other one keeps passing and keeps looking healthy while missing the exact shape that was just added. That is the same defect the header of check-stageable-secrets.mjs already describes - the rule that existed stayed correct, so nothing looked broken, and it was always the sibling nobody thought to name. A test fails if either script grows a private copy, and a second test compares both scripts' --print-rules output at runtime.

Two consequences worth calling out:

• The shared matcher returns a rule name, a line and a length, never the matched text. So check-stageable-secrets.mjs no longer prints a 6 character prefix of the match. A prefix of a live key in a CI transcript is still a prefix of a live key.
• Two formats the bash .githooks/pre-commit already knew about and this list did not are now covered: antfarm_ room keys and xai- keys. (.githooks/pre-commit keeps its own bash list on purpose: it is dependency-free grep so it works in a clone with no node_modules. Unifying that one is a separate change.)

Fail closed

An error while scanning is not a pass. Distinct, documented exit codes:

code	verdict	meaning
0	clean	every reachable blob examined, nothing matched
1	found	at least one match (where, never what)
2	usage	bad arguments, or not a git repository
3	incomplete	unreadable object, undecodable blob, blob over the size cap, git error, timeout
4	shallow	.git/shallow or a --depth clone

Precedence when several apply: 1 > 3 > 4 > 0. "I could not check" never renders as "clean", which is the single most repeated bug in this codebase, so it gets its own exit code rather than a log line.

Shallow and depth honesty

A --depth clone's history is not absent, it is UNEXAMINED, and the blobs that were cut off are the old ones - precisely where a deleted-but-reachable key lives. Shallow exits 4 and prints a banner at the top of the report, never "clean".

Every run reports how many commits, refs and blobs it actually examined, how many bytes, how long it took, and how many blobs were skipped or unexamined. A one-commit history gets an explicit note that a history scan of a fresh snapshot repo proves almost nothing: it has no deleted past to hide a key in. --json gives the same report for machines.

Never print the value

Findings report path, line, blob sha, introducing commit and rule name. That is enough to act on and it does not copy the secret into a log, a CI transcript or a chat message. A test plants a distinctive synthetic token and greps stdout, stderr and the JSON for the whole value, for every prefix of it from 8 characters up, and for a distinctive fragment.

Tests

test/scan-history-for-secrets.test.mjs, node:test style: 33 tests, all passing. Full suite on the rebased branch: 567 tests, 567 pass, exit 0 (the known flaky gui adapter runs the idle guard before the script passed on this run; it had failed on an earlier full-suite run and passes in isolation).

The central one builds a throwaway git repo in a temp dir, commits a synthetic secret, DELETES it in a later commit, asserts the working tree is clean, and proves the scanner still finds it and attributes it to the oops commit. Also: a genuinely clean repo exits 0 (the tool is capable of saying yes), a shallow clone of a leaking repo is reported shallow and is caught once unshallowed, genuinely undecodable objects (invalid UTF-8 sequences) and over-cap blobs are could-not-complete, a file with a stray NUL and a secret after it is scanned and FOUND rather than refused, an exhausted time budget is could-not-complete, findings win over incomplete, a one-commit repo is told it proved little, --help documents every exit code, and the two scanners cannot diverge.

Fixtures are assembled at runtime from harmless pieces, so this test file contains no credential-shaped literal. A "fake" key in a fixture in a public repo is still a key-shaped thing in a public clone - and the scanner would find it in its own history forever.

Proven negative controls (break it, watch it fail, restore, watch it pass)

1. Walk only the tip commit (rev-list --objects --all narrowed to --max-count=1 HEAD): the deleted-secret test went from pass to expected FOUND, got 0 - the broken build reported a leaking repo as clean, which is the exact false confidence this tool exists to prevent. 3 of 12 failed. Restored: 12/12.

2. Silently skip undecodable blobs (undecodableReason returns null, the old return null habit): expected INCOMPLETE, got 0 - could-not-check rendered as clean. 2 of 12 failed. Restored: 12/12.

3. Print a "redacted" 10 character prefix of the match: the no-leak test failed with 8-char prefix leaked. 2 of 12 failed. Restored: 12/12.

4. Reinstate the old "contains a NUL, therefore binary" heuristic: the NUL regression test failed with a file that decodes must be scanned, not refused; got 3 - could-not-complete where the answer was found. 1 of 13 failed. Restored: 13/13. See "Decode, do not sniff" below.

5. Reinstate the invoked-directly guard on the CLI: a scanner invoked through a symlink must not silently produce nothing. 2 of 18 failed. Restored: 18/18.

6. Revert one swept file to the hand-rolled comparison: the sweep test failed naming scripts/local-agent.mjs. 1 of 18 failed. Restored: 18/18. Worth recording that this control failed to fail on the first attempt - the sweep test only checked whether a file mentioned isMainModule anywhere, and the reverted file still had the (now unused) import sitting above the broken guard. A check that cannot fail is not a check, so the test was rewritten to look for process.argv[1] within 200 characters of import.meta.url in code, ignoring whole-line comments.

7. Remove the JWT rule: a service_role JWT must not read as clean; got 0 - the state this branch shipped in, which is how a live production database key went unreported. 5 of 24 failed. Restored: 24/24.

8. Have the JWT decoder return the token alongside the claims: the token leaked. 1 of 24 failed. Restored: 24/24.

9. Make the stageable gate skip undecodable files again (the state codexmb reviewed): a stray byte must not hide a credential from the commit gate, exit 0 where 1 is required. 1 of 27 failed. Restored: 27/27.

10. Let the extension filter decide before the content is read: text content named .png must still be scanned. 1 of 27 failed. Restored: 27/27.

11. Print any claim value under 65 characters (the reviewed state): 3 of 33 failed, including a secret planted in ANY emitted claim does not reach the output. Restored: 33/33.

12. Let the filename decide the fate of an undecodable blob: a media-looking name must not hide undecodable text; got 0. 1 of 33 failed. Restored: 33/33.

13. Hand-roll the comparison in a migrated file, using the CORRECT idiom: the sweep still flags it, now with a message that says it is a drift risk rather than a bug. 1 of 33 failed. Restored: 33/33.

A fourteenth failure was found by the tests rather than planted: a timeout during the initial rev-parse probe was reported as exit 2 "not a git repository", turning an unanswered question into an answer. Fixed - a timed-out probe now exits 3.

Result against this repo

Run against ide-agent-kit's own reachable history: 562 commits, 1146 of 1146 reachable blobs in 1.4 s, 0 unexamined, 0 errors, exit 1, measured on this branch's tip.

Verdict: not clean. It found credential-shaped blobs in reachable history, most of them reachable from origin/main. Paths, commits and blob shas have been reported privately to the repo owner and are deliberately not listed here: this PR is public, and a map to the objects is most of the work. The values are not printed anywhere, by design.

That is the point of the change. The working-tree scanner passes on this repo today.

Decode, do not sniff

The first version of this branch refused any blob containing a NUL byte and reported the refusal as could-not-complete. It flagged exactly one blob in this repo: bin/iak-pending.mjs, 26,507 bytes with a single NUL at offset 12,684, used as a deliberate field separator between a host and an id so neither half can forge a collision in the key. It is valid UTF-8, node --check passes, and it is ordinary JavaScript.

So the scanner skipped 26 kB of readable source. A credential after that byte would have been missed, and the miss would have been dressed up as a scanning error rather than a finding. The fail-closed rule did its job by surfacing it; the classifier under it was wrong.

decodeUtf8() now answers one question with TextDecoder in fatal mode: do these bytes decode. Anything that decodes is scanned. Invalid sequences, over-cap blobs, unreadable objects and timeouts stay could-not-complete. check-stageable-secrets.mjs had the same conflation (readFileSync utf8, then look for a NUL, which cannot tell a real binary from text either) and now reads bytes through the same helper.

An entry point that cannot run must not report success

The scanner had the usual invoked-directly guard. It produced no output and exit 0 through a symlinked path, because node always realpath-resolves import.meta.url and never resolves process.argv[1]. macOS /tmp IS a symlink to /private/tmp, so every scratch dir, every worktree under /tmp, every ~/bin symlink and any CI checkout in a symlinked workspace was affected. --help printed nothing too. For a security tool, exit 0 means clean.

The fix is not a better comparison, it is no comparison:

• src/history-scan.mjs is the engine and is pure - no argv, no printing, no process.exit. Importing it starts nothing.
• scripts/scan-history-for-secrets.mjs is the command line and calls main() unconditionally. Nothing imports it, so the guard bought nothing. There is no path on which the tool does not run.
• package.json gains an iak-scan-history bin entry so the intended invocation is unambiguous.

Swept every entry point in bin/, scripts/ and src/: 9 compare argv[1] against import.meta.url (7 at the time of the sweep, plus bin/iak-pending.mjs and bin/model-picker.mjs which arrived with #122 and #119), 6 of them broken, in 3 different shapes (file://${argv[1]}, pathToFileURL(argv[1]).href, path.resolve(argv[1])). scripts/team-watchdog.mjs already had the correct realpath idiom. All 7 now call isMainModule() from src/common/entrypoint.mjs, the single implementation, and a test fails if a new entry point hand-rolls it again.

check-stageable-secrets.mjs never had the guard - it runs at import - but it is a pre-commit gate, so a no-op there would let a commit carrying a credential through the hook. There is now a test asserting it fires identically through a symlink.

JWTs, and why the pattern list was still incomplete

A Supabase service_role key is a JWT. This list had no JWT rule, so the scanner reported a repo as having 14 findings while saying nothing about a non-expiring full-access production database credential sitting in a file that is deleted from the working tree and reachable from origin/main - the exact failure mode this PR is about.

The root cause was not a missing regex, it was a partial port. This list was lifted out of check-stageable-secrets.mjs and called the single source of truth while .githooks/pre-commit still held a bash list with four formats it lacked: JWT, Moltbook secret key, AgentMail key, Discord bot token. The port took two of them and left the rest. All four are now present, and a test asserts one synthetic sample per format is caught, so the next omission fails the suite instead of passing quietly.

A JWT hit is triaged by its claims. describeJwt decodes header and payload and reports an allowlist - alg typ kid iss aud role ref scope iat nbf exp - plus whether exp has passed, how many days remain, and whether there is no exp claim at all, which is worse news rather than better. Claims are metadata, not the secret, and they are the difference between "some JWT" and "a service_role key for production, live until 2036". The token, its segments and its signature are never printed, in any mode; a test asserts every prefix from 8 characters up is absent from stdout, stderr and the JSON. An allowlist rather than a denylist, because a custom claim could hold anything.

Example JWTs in a README or a .d.ts are found rather than suppressed. A noisy true positive is cheap when the claims are right there; a silent miss is what got us here.

matchSecret became matchSecrets: every rule that fires is reported, not just the first. A rule high in the list used to shadow everything below it in the same blob, so a file with an API key on line 3 and a service_role JWT on line 40 reported one finding and hid the other - the same family as a silent miss. Overlapping matches of the same value are still reported once, by the most specific rule, so const apiKey = "sk-..." does not count twice.

Counts moved as a result: this repo went from 18 findings to 32 (9 Moltbook keys no rule caught before, plus room keys that were being shadowed).

Second review round: nothing the scanner does not control may decide what it reports

P1: a JWT's claims are attacker-controlled free text, and the report printed them. A synthetic secret placed in iss came back verbatim. Allowlisting claim NAMES constrains nothing about claim VALUES, and this report exists to be shared - so anyone who can get a token into a scanned repo could choose what our output says.

A value is printed only where the value itself is constrained to a vocabulary we defined:

claim	printed	why
alg, typ	verbatim if in the JWT spec's registered names	closed vocabulary
role	verbatim if a known platform role	closed vocabulary
exp, iat, nbf	as a parsed date	a number, rendered
ref	verbatim only if ^[a-z]{20}$	argued below
everything else	iss=<present, 8 chars>	free text

The argued exception is ref: it is the Supabase project identifier, it appears in the project's own public URL, it is not a credential, and it answers "which project is this key for" - the difference between an alarming finding and an actionable one. The shape restricts the channel to 20 lowercase letters. Delete SUPABASE_REF_SHAPE and it degrades to presence-and-length like the rest.

P2, which the previous round did not fix. The same blob reachable as both a.png and z.txt, containing an ASCII credential plus one 0xff byte, still exited 0 / clean. I had moved where the filename decided, not whether it decided: the media check ran later, on the representative path, and skipped the blob before matching. The oversize policy was representative-dependent too. No filename now takes part in any decision - binary media is recognised by magic bytes, an oversize blob is unexamined whatever it is called, and check-stageable-secrets.mjs lost its identical extension skip.

Two more instances, found by going looking rather than waiting for a fourth review:

• A path is attacker-controlled free text that both scanners print. It can carry ANSI escapes or a carriage return to forge report lines, and it can be a credential - keys/sk-live-xxx.txt is a path, and printing it leaks exactly what we refuse to print from inside a file. Paths are sanitized, and a path matching a secret pattern is withheld.
• PASS: no credential-shaped data in 0 reachable blob(s) was printable. Every reachable blob must now land in exactly one bucket - examined, recognised media, or unexamined - and a mismatch is could-not-complete. The human report says NOTHING SCANNED when it read nothing.

Review fixes (codexmb, first security review of this PR)

P1, and it was this branch's own fix reintroducing the bug class the fix was for. After the decode-not-sniff change, check-stageable-secrets.mjs skipped any file that failed strict UTF-8 decoding. A stageable config.txt holding a plain ASCII credential beside one stray 0xff byte reported PASS, exit 0; the pre-branch baseline at 9b8aa8c reports FAIL, exit 1 on the same file. Verified both ways before touching anything. The encoding check made the gate worse than the code it replaced.

decodeForScanning() now always returns text - exact when the bytes are valid UTF-8, lossy otherwise, with a strict flag saying which. Credential formats are ASCII and a lossy decode preserves ASCII byte for byte, so detection survives.

I chose lossy scanning over "undecodable stageable file exits non-zero", and the reason is specific to this scanner: its job is to block a commit, not to classify encodings. Stageable binaries are routine, and a gate that blocks every commit touching one gets switched off - which is the failure mode that file's own header warns about. It prints a visible NOTE naming the files it had to read lossily, so the limitation is stated rather than swallowed. The history scanner, which reports rather than blocks, does both: an undecodable blob is scanned lossily and listed as unexamined, so an ASCII key inside it is found while the verdict still cannot be clean.

P2: the extension filter decided a blob's fate from a name git handed us by chance. rev-list --objects emits each blob once, under whichever path it met first, so content committed as both logo.png and notes.txt arrives named logo.png and the filter excluded readable text from the scan. Collecting every path cannot fix it - git does not give us the others - so the name no longer decides anything on its own. Every blob within the size cap is read, anything that decodes is scanned whatever it is called, and a media extension now only decides the outcome for a blob that did not decode, so a real image is skipped quietly rather than reported as a scanning failure.

Counts are unchanged by both fixes - ide-agent-kit 32, groupmind 16 - so the false pass is gone without a false positive taking its place.

Already fixed, not re-fixed. The review was against head 005daf3 and reproduced the symlink entry-point no-op as a third P1. That was fixed in 7fb97a7 by removing the guard entirely, and swept across every entry point; see the section above.

The review is also right that neither JS scanner is the live commit gate. .githooks/pre-commit is the bash scanner, and this PR does not change it. So the regression described above could not have affected commits happening now. That is a mitigation, not an excuse: the bash list is exactly where the four missing formats came from, and the JS gate is what npm-driven workflows would reach for.

A false alarm is a trust problem too

After the rebase the entry-point sweep fired on bin/iak-pending.mjs and bin/model-picker.mjs and said they "no-op through a symlink". They do not. Both use the correct realpath idiom, and both were measured behaving identically direct and through a symlink (563 and 3985 bytes, exit 0 either way).

The check matches process.argv[1] and import.meta.url within 200 characters, which the correct idiom also does - so it enforces "use the shared helper" and cannot tell a correct copy from a broken one. It must not claim to. As written it told two other PRs' authors their working code was broken, which is how a useful test gets deleted by the next person who hits it.

That is the day's pattern in its mild form: the check fires on the right policy while claiming the wrong fact. It fails loudly rather than passing silently, but a false alarm that overstates its finding erodes trust the same way a false pass does.

The message now says what it knows - these hand-roll the comparison instead of calling isMainModule(), a drift risk rather than a bug, and a hand-rolled copy may well be correct today - and points at what to check if the copy is not realpathing both sides without asserting that it is not. The behaviour is proven separately by the isMainModule test and the two symlink parity tests. Then both files were routed through isMainModule(), so the policy is satisfied rather than argued with; their output is byte-identical before and after.

Not wired into anything yet

This adds npm run scan:history and an iak-scan-history bin entry, and nothing else. It is not in .githooks/pre-commit and not in CI - a history scan is seconds, not milliseconds, so where it belongs is a separate decision.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-20T07:56:35.758984Z	a7fab64	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7fab64420

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Fail closed when media blobs are skipped

When a reachable blob's path ends in one of these extensions, this branch records it as skipped but does not add it to unexamined, so the verdict logic can still return exit 0. For example, a plaintext leak.png containing a matching key is reported as clean with zero blobs examined. Because the skip is based only on the filename rather than verified content, renamed or misnamed secret files bypass the history scan; skipped blobs should prevent a clean verdict or be inspected sufficiently to establish that they are binary.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Recheck the deadline while scanning each batch

The deadline is checked only when git is invoked, while all decoding and regex matching in the callback occur afterward without another check. If the final cat-file call completes just before the budget expires, processing its roughly 32 MiB chunk can exceed --max-seconds and still return clean; a 0.3-second scan can, for example, report a clean duration well above 0.8 seconds. Check the deadline during or immediately after batch processing so exceeding the documented wall-clock budget produces incomplete.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Accept bare repositories as valid scan targets

For a valid repository created with git clone --bare, git rev-parse --show-toplevel fails because there is no working tree, so this catch reports the target as “not a git repository” and exits with usage status 2. History scanning only needs the object database, and bare repositories are common locations for mirrors and server-side scans; resolve the Git directory or otherwise recognize bare repositories instead of requiring a worktree root.

Useful? React with 👍 / 👎.