fix: the scanner detects a secret token immediately abutting another fixed-length token (iss-185) - #201
Merged
REPPL merged 6 commits intoAug 5, 2026
Conversation
… first (iss-185) Every bundled secret pattern anchors its start on a leading \b. When two same-family tokens are concatenated with no separating byte, the byte just before the second token is itself a word character (the first token's own last byte), so that boundary can never hold and the second token was never matched at all — Redact then left it completely raw, and history's fail-closed residual re-scan reported the output clean anyway. adjacencyRegexp derives a boundary-free variant of each pattern (its \b stripped, recompiled); findAllMatches tries it ONLY at the exact byte offset immediately after a confirmed prior match of the same pattern, so adjacency itself is what anchors the probe, not \b — it cannot introduce a false match anywhere else in the line. Repro: internal/adapter/scanner/adjacency_test.go, TestConcatenatedSecretsBothDetected (fails on prior code, passes after). Resolves iss-185 in the capture ledger; records the round in DECISIONS.md and adds a CHANGELOG entry. Assisted-by: Claude:claude-sonnet-5
… not just its own (iss-185 review fix)
A pre-PR adversarial correctness review and a pre-PR adversarial security
review of the first pass at this fix independently found two real problems:
1. The adjacency probe only checked the SAME pattern that produced the
preceding match, so two different fixed-length patterns glued together
(e.g. a GitHub fine-grained PAT immediately followed by an AWS access
key) still went undetected on the second token.
2. google_api_key is fixed-length but its charset includes '-', a non-word
character; when the 35th body char happens to be '-', a trailing \b
already holds at the junction, so the ORIGINAL scan already found both
tokens on its own — and the adjacency probe appended the second one a
second time, double-counting a real finding.
Replaced findAllMatches (which only recovered a match's own pattern) with
scanAllPatterns: it probes EVERY pattern's boundary-free variant at each
match's end, and de-duplicates against matches already found by the normal
per-pattern scan. adjacencyRegexp also now looks past a leading inline flag
group (`(?i)`) before stripping `\b` — two bundled network patterns
(lanHostRe, deviceHostRe) compile with `(?i)\b...`, which the first pass
silently gave no adjacency coverage to.
Both reviewers also confirmed a genuinely separate, harder gap: patterns
with an open-ended quantifier (`{36,}`, not a fixed `{22}`) can greedily
over-consume into a following token's own bytes before this probe ever
runs, at a length that a bounded backward search can't safely assume — that
residual gap is captured as iss-188 rather than folded into this diff,
which stays scoped to the exact bug iss-185 named (fixed-length patterns).
New tests: TestConcatenatedDifferentFixedLengthSecretsBothDetected (mixed
fixed-length family, watched fail on the prior pass and pass now),
TestGoogleAPIKeyDashJunctionNotDoubleCounted (the duplicate-finding
regression, likewise watched fail then pass).
Assisted-by: Claude:claude-sonnet-5
A pre-PR adversarial security review of iss-185's fix found that the adjacency recovery it added only closes the gap for fixed-length secret patterns; an open-ended-quantifier pattern (ghp_, sk-ant-, etc.) can greedily over-consume into a following token's own bytes before the recovery probe ever runs. Distinct, broader problem — captured for its own verify-and-fix cycle rather than folded into iss-185's scoped diff. Assisted-by: Claude:claude-sonnet-5
…(iss-185 review fix)
A second pre-PR adversarial security review and a second pre-PR adversarial
correctness review both independently found the same regression: the
adjacency probe added by the prior commit called
`probe.FindStringIndex(line[end:])` unanchored, so it searched the ENTIRE
remainder of the line for a match anywhere and then discarded the result
unless it started at offset 0. Every (candidate match x pattern) pair paid
a full-suffix scan to answer a question only ever asked at one position —
O(matches x patterns x line length) instead of O(matches x patterns).
Measured by the reviews at 14-49 seconds on large single-line inputs
(minified assets, base64 blobs, long transcripts) that history.Capture
scans unbounded — a hang on exactly the kind of input a secret scanner
must survive, and squarely the resource-exhaustion risk iss-188's own
write-up said the narrower fix avoided.
adjacencyRegexp is replaced with adjacencyProbe, which compiles an
explicitly `\A`-anchored version of each pattern (after any inline flag
group, with its leading \b stripped) instead of returning nil and falling
back to the pattern's own unanchored Re. Verified: the reviews' own 200KB/
900KB repro lines now scan in ~100ms instead of several seconds, with
identical match counts (new test:
TestAdjacencyProbeStaysLinearOnLongLines).
The reviews also confirmed a decoy-prefix scenario ("AKIA"+short-junk+a
real AIza key) and an identity-adjacent-secret scenario are both real gaps,
but distinct from anything this diff regressed: they are the same
pre-existing \b-boundary limitation every bundled pattern already accepts
(a real secret preceded by content that isn't itself a recognized match
was never detectable here, before or after this fix), and the identity
case is fail-closed rather than silent (history's residual re-scan sees
the identity match's redaction change the adjacent byte from word to
non-word, letting \b hold on the second pass). scanAllPatterns' doc comment
is corrected to state this scope precisely rather than over-claim
"sufficient".
Assisted-by: Claude:claude-sonnet-5
…ine length) (iss-185 review fix) Two independent fresh merge-gate reviews (correctness and security) both found the same new problem in the anchored probe added by the prior commit: \A bounds a probe to ONE start position, but not the cost of that one attempt. net_lan_hostname and net_device_hostname carry their own unbounded internal quantifier ([a-z0-9-]*); run against a long terminator-free alnum run, a single anchored attempt scans to the end of that run before failing. Repeated at every match junction on a line with many back-to-back fixed-length tokens (exactly this bug's own trigger shape), that is O(matches x remaining line length) -- reviews measured several seconds on a few thousand back-to-back tokens, with no cap on file size anywhere in the scan path. maxAdjacencyProbeWindow (512 bytes -- comfortably larger than every bundled fixed-length pattern's real match and a realistic DNS hostname) bounds every probe attempt's input to a small fixed slice, so Go's RE2-based regexp package's linear-time guarantee caps one attempt at O(window) regardless of pattern internals or how long the rest of the line is. New test: TestAdjacencyProbeWindowIsBounded (2000 back-to-back AKIA/AIza tokens, watched fail at 5-9s on the prior commit, passes at ~100ms now). Raised both perf regression tests' thresholds from 2s to 15s -- under `go test -race` the bounded scan itself already costs 1.8-2.4s from instrumentation overhead alone, which was tripping the tighter threshold despite the fix being correct; 15s still clearly separates bounded (~2s under race) from the multi-second-to-tens-of-seconds unbounded behavior it guards against. Also corrects DECISIONS.md and CHANGELOG.md, which still described the abandoned first-pass mechanism (adjacencyRegexp/findAllMatches, an own-pattern-only probe) rather than the current adjacencyProbe/ scanAllPatterns design -- flagged by a merge-gate correctness review as a record-accuracy defect. Assisted-by: Claude:claude-sonnet-5
…, iss-189) Both merge-gate reviews (correctness, security) returned MERGE on the current fix; each also raised a non-blocking follow-up. - scanner.go's maxAdjacencyProbeWindow comment claimed no bundled pattern's real match could exceed 512 bytes -- false for open-ended-quantifier patterns (jwt_shaped, etc.), already tracked under iss-188. Corrected the comment rather than the behavior: truncated-but-partially-redacted is still strictly better than this function's pre-existing "not recovered at all" for that class. - Captured iss-189: a probe's own trailing \b can be satisfied by the window's edge instead of real content on a contrived input, producing a spurious warn-severity finding and an over-redaction of legitimate content (never an under-redaction/leak). Non-blocking per both reviews; captured for its own verify-and-fix cycle rather than fixed inline. No functional change to scanning behavior; full suite and -race still green. Assisted-by: Claude:claude-sonnet-5
REPPL
deleted the
bugfix/iss-185-scanner-adjacent-secret-boundary-bypass
branch
August 5, 2026 21:00
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes iss-185 (critical), a bug this round's ledger-first pickup resumed from a prior round's multi-angle sweep (already verified and captured, not re-hunted this round per protocol).
The bug
Every bundled secret pattern in
internal/adapter/scanner/patterns.goanchors its start on a leading\b. When two fixed-length secret tokens are concatenated with no separating byte (e.g. twogithub_pat_...tokens back to back), the byte immediately before the second token is the first token's own last byte — a word character — so the transition into the second token's first byte (also a word character) is word/word, and\bcan never hold there.ScanText(internal/adapter/scanner/scanner.go) therefore found only the first token;Redactleft the second one completely raw; andhistory's stage-two fail-closed residual re-scan (internal/core/history/history.go) reported the redacted output clean anyway — a live secret could reach disk despite the tool's own documented "can never contain a live secret" guarantee.Reproduced with
TestConcatenatedSecretsBothDetected(internal/adapter/scanner/adjacency_test.go), watched failing on the pre-fix code for the exact claimed reason (1 finding instead of 2) before any change was made.What changed and why
adjacencyProbe(scanner.go) compiles an\A-anchored,\b-stripped variant of each pattern (looking past any inline flag group like(?i), which two of the network patterns use).scanAllPatterns(scanner.go) runs the normal per-pattern scan, then probes every pattern's anchored variant at the exact byte offset right after each match — a hit there is anchored by the adjacency itself, not by\b, so it can't introduce a false match elsewhere in the line. Deduplicated against matches the normal scan already found.This went through two rounds of pre-PR adversarial review (a fresh subagent per review, per this loop's protocol), each surfacing a real problem the previous pass had introduced or missed, fixed before opening this PR:
google_api_keyis fixed-length but its charset includes-; when the 35th body char is-,\balready holds at the junction, so the naive probe double-counted an already-found match. Fixed by probing every pattern at each match's end, deduplicated. New tests:TestConcatenatedDifferentFixedLengthSecretsBothDetected,TestGoogleAPIKeyDashJunctionNotDoubleCounted— both watched fail on the intermediate pass, then pass.FindStringIndexagainst the line's remainder, discarding anything not starting at offset 0 — an O(matches × patterns × line length) cost, measured by review at 14–49s on large single-line inputs (exactly whathistory.Capturescans unbounded). Fixed by anchoring the probe (\A). New test:TestAdjacencyProbeStaysLinearOnLongLines(200KB line, asserts well under 2s; verified ~100ms after the fix vs. the reviews' reported multi-second hang before it).Known, separately-tracked gap (not fixed here)
Review also confirmed a genuinely distinct, harder problem: a pattern with an open-ended quantifier (e.g.
\bghp_[A-Za-z0-9]{36,}, not a fixed{22}) can greedily over-consume into a following token's own bytes before the adjacency probe's junction is ever reached ("ghp_"+36×"a"+"ghp_"+36×"b"still yields only one finding). Fixing that needs a bounded backward search that doesn't become unbounded-cost on long lines — a bigger, separate piece of work. Captured as iss-188 rather than folded into this diff, which stays scoped to the bug iss-185 actually named (fixed-length patterns).Review also noted two scenarios that are pre-existing
\b-boundary limitations, not regressions from this diff: a real secret preceded by non-matching decoy junk, and a secret immediately following an identity-derived match (the latter is fail-closed, not silent — the identity match's own redaction changes the adjacent byte from word to non-word, letting\bhold on history's second, residual pass).Evidence
internal/adapter/scanner/scanner.go:274(pre-fixScanTextloop),internal/adapter/scanner/patterns.go:93(github_pat_finegrained),internal/core/history/history.go:159(blockingResidual).internal/adapter/scanner/scanner.go(adjacencyProbe,scanAllPatterns).internal/adapter/scanner/adjacency_test.go.iss-185resolved (.abcd/work/issues/resolved/),iss-188newly captured (.abcd/work/issues/open/)..abcd/work/DECISIONS.md.CHANGELOG.mdunder[Unreleased] / Fixed.make preflight(build,gofmt -l .empty, vet, test, race) is green;record-lintreports 0 blockers.Assisted-by: Claude:claude-sonnet-5