Development record
scanner leaves a second back-to-back secret token undetected and unredacted, and the fail-closed residual re-scan reports clean anyway. Fixed-length token patterns (e.g. github_pat_ at patterns.go:93) require a leading \b; when two same-family tokens are concatenated with no separator, the first regex match consumes bytes up through the second token's first byte, leaving word/word at the junction so the second token's \b can never match. ScanText (scanner.go:274) therefore returns only one finding. Redact's fingerprintSpan keeps 2 raw alnum bytes at the tail of the masked span (scanner.go:393), so the junction stays word/word after redaction too — the second token survives completely raw in the output. Because the boundary never recovers, history.Capture's stage-two residual check (history.go:159, blockingResidual) re-scans the redacted text, finds nothing, and writes the record to disk with a live secret still in it, directly violating history's documented fail-closed guarantee ('a stored record can never contain a live secret'). Distinct from iss-65 (Finding.MarshalJSON snippet cross-leak in serialized findings, already fixed) — this is a ScanText/Redact detection gap in the scan pass itself. Reproducing test: internal/adapter/scanner/, TestConcatenatedSecretsBothDetected.
scanner leaves a second back-to-back secret token undetected and unredacted, and the fail-closed residual re-scan reports clean anyway. Fixed-length token patterns (e.g. github_pat_ at patterns.go:93) require a leading \b; when two same-family tokens are concatenated with no separator, the first regex match consumes bytes up through the second token's first byte, leaving word/word at the junction so the second token's \b can never match. ScanText (scanner.go:274) therefore returns only one finding. Redact's fingerprintSpan keeps 2 raw alnum bytes at the tail of the masked span (scanner.go:393), so the junction stays word/word after redaction too — the second token survives completely raw in the output. Because the boundary never recovers, history.Capture's stage-two residual check (history.go:159, blockingResidual) re-scans the redacted text, finds nothing, and writes the record to disk with a live secret still in it, directly violating history's documented fail-closed guarantee ('a stored record can never contain a live secret'). Distinct from iss-65 (Finding.MarshalJSON snippet cross-leak in serialized findings, already fixed) — this is a ScanText/Redact detection gap in the scan pass itself. Reproducing test: internal/adapter/scanner/, TestConcatenatedSecretsBothDetected.
Frontmatter
| id | iss-185 |
|---|---|
| type | issue |
| lifecycle | resolved |
| severity | critical |
.abcd/work/issues/resolved/iss-185-scanner-adjacent-secret-boundary-bypass.md
open on GitHub ↗ · commit history ↗
mentions8
- dcn-26080500000000062026-08-05 — bug-hunt loop round 1: iss-184 (critical, guard tokenizer heredoc misparse) fixed at root cause, in two passes after a pre-PR adversarial security review caught the first pass narrowing the hole rather than closing it. `internal/core/guard/tokenize.go`'s `<<` handling already special-cased the literal-digit arithmetic-shift form (`$((1<<20))` is not a heredoc), but `isDelimStart` accepts any identifier-shaped word, so `$((1<<shift))` — an identifier operand — still read as a heredoc delimiter, and `skipHeredocBodies` then silently consumed every remaining line of the command looking for a "shift" terminator line that never comes, dropping later commands (a force-push, an `rm -rf`, anything) from ever reaching command position — a silent allow, not the loud fail-open the hook's own doc comment promises for anything the tokenizer cannot answer confidently. The first pass made `skipHeredocBodies` report whether it actually found each pending heredoc's terminator, turning a miss into `ErrUnparsableCommand`. The security review then constructed the exploit this missed: an attacker supplying a later line that happens to equal the misread "delimiter" (e.g. appending a bare `shift`) still finds a match, so the swallow still succeeds silently — the fix only closed the *unterminated* half of the class, not the *coincidentally-terminated* half, which is the one an attacker actually controls. The real fix is classification, not detection-of-failure: a heredoc delimiter word immediately followed by a bare `(` or `)` with no separator is never a real heredoc — its body and terminator line have to come first, so nothing legitimate places a paren directly against the delimiter word, and `$((expr<<ident))` produces exactly that shape. `<<` in that position is now read as the arithmetic operator at tokenize time, same as the literal-digit case, so the guarded line is reached and matched normally regardless of what any later line contains. The unterminated-body-to-error fix from the first pass is kept as defense in depth for genuinely malformed heredocs (`cat <<EOF` with no closing `EOF` line), a distinct, narrower gap the same swallow also covered — an existing test that pinned that silent-swallow as intended behaviour is updated to assert the corrected `ErrUnparsableCommand` contract. Detectors: `TestArithmeticShiftByIdentifierIsNotAHeredoc`, `TestArithmeticShiftCoincidentalDelimiterStillBlocks` (the adversarial payload the security review demonstrated), and `TestTokenizeRejectsUnterminatedHeredoc` — each watched failing on pre-fix code for the claimed reason and passing after. Three more bugs surfaced by the same round's multi-angle sweep and independently verified with failing tests are captured but not fixed here — iss-185 (critical, scanner leaves a second back-to-back secret token undetected and unredacted, defeating history's fail-closed residual re-scan), iss-186 (minor, a failed source-unlink in a capture ledger transition can permanently strand an issue id across two status directories), iss-187 (minor, `rules.Merge` panics on a nil-Domains base — currently unreachable through any live caller, so a latent exported-API contract defect) — left open for a future round, highest severity first.
- dcn-26080500000000092026-08-05 — iss-185 (bug-hunt loop, round 3): `ScanText`'s leading-`\b`-anchored secret patterns could never detect a second fixed-length token immediately abutting a first with no separator — the byte before the second token is itself a word character (the first token's own last byte), a word/word transition where `\b` never holds, so `FindAllStringIndex` silently returns only the first match. `Redact` then left the second token's whole body raw, and `history`'s stage-two fail-closed residual re-scan reported the redacted text clean anyway, so a concatenated pair of secrets reached disk with one still live. Landed after three rounds of pre-PR/pre-merge adversarial review, each catching a real problem in the pass before it: (1) the first pass's own-pattern-only probe missed a mixed pair of different fixed-length patterns, and double-counted a `google_api_key` match whose dash-terminated 35th char already satisfied `\b` on its own; (2) the fix for that, an unanchored `FindStringIndex` probe, cost O(matches × patterns × remaining line length) — measured at 14–49s on large single-line input; (3) anchoring the probe with `\A` fixed the *restart-at-every-offset* cost but not the cost of one attempt against two patterns (`net_lan_hostname`, `net_device_hostname`) that carry their own unbounded internal quantifier, still quadratic on many back-to-back fixed-length tokens. The landed shape: `adjacencyProbe` compiles an `\A`-anchored, `\b`-stripped (past any inline flag group) variant of each pattern; `scanAllPatterns` probes every pattern's variant, within a small fixed window (`maxAdjacencyProbeWindow`, 512 bytes) so one attempt can never cost more than a constant regardless of what follows, at the byte offset immediately after each already-found match, deduplicated against matches already found. A pattern with an open-ended quantifier can still greedily consume into a following token before this recovery ever runs — a separate, broader gap, captured as iss-188 rather than folded in here. Repro: `internal/adapter/scanner/adjacency_test.go`, `TestConcatenatedSecretsBothDetected` (the original bug), `TestConcatenatedDifferentFixedLengthSecretsBothDetected` and `TestGoogleAPIKeyDashJunctionNotDoubleCounted` (review round 1), `TestAdjacencyProbeStaysLinearOnLongLines` and `TestAdjacencyProbeWindowIsBounded` (review rounds 2 and 3).
- dcn-26080500000000102026-08-05 — iss-188 (bug-hunt loop, follow-on to iss-185): a secret pattern whose quantifier is open-ended (`\bghp_[A-Za-z0-9]{36,}` and the seven other `{n,}` families) greedily consumes a following token's own leading bytes when those fall inside its trailing character class, so the true junction sits BEFORE the match's reported end and iss-185's forward adjacency probe — which only ever looks at that end — never runs where the second token actually starts. `"ghp_"+36×a+"ghp_"+36×b` reported one finding covering `ghp_aaa…aaaghp` and left `_bbb…bbb` raw through `Redact`, with `history`'s fail-closed residual re-scan reporting the output clean: the same live-secret-reaches-disk violation iss-185 closed for the fixed-length family. Fixed in `internal/adapter/scanner/scanner.go:465` (`stolenJunctions`): before a match's reported end is accepted as final, a bounded backward search walks candidate cuts and reports each one where the shortened prefix is STILL a whole match for its own pattern AND a token can begin, and `scanAllPatterns` probes those cuts exactly as it probes the forward end. Open-ended is detected from the compiled regexp — a match that cannot still match one byte shorter has a rigid length and is skipped after one test — not from hand-annotated metadata, so a pattern added later inherits the classification. Three things keep the cost per match rather than per match LENGTH, which the ledger entry named as the trap (an unbounded backward scan would trade the leak for a resource-exhaustion cliff on exactly the huge single-line input a scanner must handle): that one-test early-out, `maxAdjacencyBacktrack` (512, mirroring `maxAdjacencyProbeWindow`), and a single combined `junctionProbe` alternation over the whole pattern set that generates candidate cuts in one linear pass instead of one pass per pattern — measured 40s unbounded against 0.3s bounded on a 200KB line. The over-long match is deliberately left untrimmed: it only over-reports the first token's span, and `sealLine` already forces every overlap byte to `*`. Repro: `internal/adapter/scanner/adjacency_test.go`, `TestConcatenatedOpenEndedSecretsBothDetected` (the ledger entry's literal reproducer) and `TestOpenEndedSecretSwallowingDifferentFamilyBothDetected` (a whole `AKIA` key swallowed by an alnum class run), both watched failing on pre-fix code for the claimed reason; cost guard `TestJunctionBacktrackIsBounded`.
- dcn-26080500000000112026-08-05 — iss-188 follow-up (two independent adversarial reviews of the fix above, both landing on the same defect): the bounded backward search in `internal/adapter/scanner/scanner.go` (`stolenJunctions`) could step OVER the junction it was looking for. `junctionProbe` is unanchored, so one of its hits can SPAN a real junction — begin before it and end after it — without beginning at it; the loop nevertheless resumed at that hit's END (`off += loc[1]`) even when the hit's own offset had just been REJECTED by `wholeMatch`, skipping every byte in between, the true junction among them. Nothing revisits a skipped range, so the search returned fewer cuts than it should and iss-188's own failure mode reopened through a narrower trigger: `"ghp_"+32×a+"AIza"+4×b+"ghp_"+36×c` reported one finding and left the second token's tail raw, and `"ghp_"+10×a+"AIza"+22×z+"sk-proj-"+40×i` — where the skipped-over decoy is itself a syntactically valid secret — left 48 raw bytes of an OpenAI project key through `Redact` with the fail-closed residual re-scan reporting the output clean. Fixed by always resuming at `cut + 1`, dropping the jump-to-hit-end fast path entirely: a rejected candidate can hide a real junction one byte later, and an accepted one is not worth a special case. This stays bounded for the reason the whole feature exists: the loop is capped at `maxAdjacencyBacktrack` (512) iterations per match regardless of how long the match is, and each iteration's search is capped at `maxAdjacencyBacktrack + maxAdjacencyProbeWindow` bytes by RE2's linear guarantee — so the per-match cost is a constant factor independent of the REST of the line, and the whole scan stays linear in line length rather than quadratic. Measured: doubling and quadrupling an adversarial 247KB line doubled and quadrupled the time (2.3s / 4.6s / 9.2s), which is the property that matters. Repro: `internal/adapter/scanner/adjacency_test.go`, `TestStolenJunctionSearchDoesNotSkipPastRejectedCandidate` and `TestStolenJunctionSearchSkipsPastValidDecoySecret`, both watched failing on pre-fix code for the claimed reason; cost guard `TestJunctionBacktrackIsBounded`, extended with `dense_rejected_candidates_in_backtrack_window` — every match's window packed with candidates that all fail validation, the worst case for advancing one byte at a time. Two further gaps the same reviews found were CAPTURED, not folded in: iss-190 (a recovered match longer than `maxAdjacencyProbeWindow` is truncated, and the misaligned artificial end breaks the chain that would recover a THIRD abutting token — the pre-existing iss-185 window trade-off reached through a new path, whose real fix is an adaptive window that risks reintroducing the unbounded per-match cost) and iss-191 (`junctionProbe`'s `(?s).` compile fallback would validate once per byte instead of once per candidate, unreachable with the bundled pattern set). Two comment claims were also corrected as inaccurate: cost is bounded independently of the rest of the line but IS proportional to the match's own length, since `wholeMatch` re-runs the probe over the prefix; and a window-exceeding recovery can miss a token ENTIRELY, not merely truncate it, when the pattern's required structural markers (`jwt_shaped`'s two `.` separators) both fall outside the window.
- iss-65scanner serialized-finding secret cross-leak (BLOCK) + scan hygiene: Finding.MarshalJSON rebuilds the snippet from the full source line but masks only THIS finding own token, so a second secret on the same line (minified JSON, collapsed .env, a=X; b=Y) is serialized verbatim (finding.go:124). Also isText caps the UTF-8 sniff at 8192 bytes and utf8.Valid fails when the cut splits a multibyte rune, misclassifying a valid >8KB text file as binary and skipping the scan (scanner.go:338); ScanBundle drops an unreadable file with a bare continue instead of surfacing it in Unscanned (scanner.go:269); per-repo pii.json skip-lists have a severity floor but no coverage floor (scanner.go:128). Detector: two-secrets-one-line redaction test; mid-rune isText case; unreadable-file-surfaced case; skip-list coverage floor. Corpus: sweep C14/C17, C15, C18, P10.
- iss-188scanner adjacency recovery misses greedy over-consumption in open-ended-quantifier secret patterns. iss-185's fix (scanAllPatterns, scanner.go) recovers a second token that immediately abuts a first with no separator ONLY when the first pattern is fixed-length (its match always ends exactly where the second token begins, so probing every pattern's boundary-free variant at that one byte offset is sufficient). A pattern whose quantifier is open-ended (e.g. `\bghp_[A-Za-z0-9]{36,}`, `\bsk-ant-[A-Za-z0-9_-]{40,}`, `\bsk_live_[A-Za-z0-9]{20,}`) instead greedily consumes as many class-matching bytes as are available, including a following token's own leading bytes when those happen to fall inside the same character class — shifting the true junction earlier than the reported match end, before scanAllPatterns' adjacency probe ever runs. Concrete repro: `"ghp_" + strings.Repeat("a",36) + "ghp_" + strings.Repeat("b",36)` — the first `ghp_` pattern's `{36,}` class run consumes the 36 a's AND the second token's leading "ghp" (all alnum), stopping only at the second token's `_` (not in `[A-Za-z0-9]`); ScanText reports exactly ONE finding covering `ghp_aaaa...aaaghp`, and the second token's `_bbbb...bbb` tail survives Redact completely raw with the fail-closed residual re-scan reporting clean — the same "can never contain a live secret" violation iss-185 was filed for, just for the open-ended-quantifier pattern family instead of the fixed-length one. Distinct from iss-185 (closed): that fix's mechanism (probe every pattern at a match's reported end) is necessary but not sufficient — this needs a bounded backward search (shrink the greedy match's end toward its start, checking at each candidate cut whether the shortened match is still valid for its own pattern AND some pattern's boundary-free variant matches there) without becoming O(line length) per match, since these patterns' matches can legitimately be very long (base64 blobs, minified files) and an unbounded backward scan across every byte would be a new resource-exhaustion risk on exactly the kind of large single-line input a secret scanner must handle. Surfaced by an adversarial pre-PR security review of iss-185's fix (bug-hunt loop round 3), not by the round's own hunt; not fixed in that PR to keep its diff scoped to the exact bug iss-185 named and to avoid shipping an unbounded-cost search under review pressure. Affected pattern families: github_pat (ghp_/ghs_/gho_/ghu_/ghr_), anthropic, openai_project, openai_svcacct, stripe_live, stripe_test, slack, jwt_shaped — every bundled pattern with an open (`{n,}`) rather than fixed (`{n}`) quantifier.
- iss-189adjacency probe's trailing \b can be satisfied by the window edge instead of real content, causing a spurious over-redaction. scanAllPatterns (scanner.go) slices the line to a bounded window (line[end:min(end+512,len(line))]) before running each pattern's adjacencyProbe, to keep one probe attempt's cost constant regardless of what follows (iss-185's fix). Some probes retain an internal trailing \b of their own (e.g. net_lan_hostname's `\b` at the very end of its alternation) which normally requires a real non-word character (or true end of string) after the match. But when the probe is truncated to the window, "end of window" is treated by the regex engine exactly like "end of string" — so if the window happens to end immediately after what LOOKS like a complete hostname/token, the trailing \b is satisfied by the artificial window boundary rather than by real surrounding content, even though the FULL, untruncated line would not have matched there (the real next byte continues the token). Concrete repro: a line built as `"AKIA"+16*"Q"` (a confirmed match, establishing the probe's junction) + 506 filler bytes + ".local" + "zzz" — the window (512 bytes from the junction) ends exactly after "local", so net_lan_hostname's probe matches "b...local" (satisfied by end-of-window) even though the real line continues with "zzz" right after ".local", meaning the FULL line's own un-windowed net_lan_hostname pattern would NOT match there at all (no real word boundary follows in the actual text). Impact: a spurious net:lan_hostname finding (warn severity only, not hard_fail) causes Redact to needlessly rewrite up to 512 bytes of legitimate content as fingerprinted/starred output — not a security leak (over-redaction, not under-redaction) but a correctness/usability defect: legitimate file content gets mangled on a contrived but reachable input shape. Suggested fix (from the reviewer that found this): when a probe match's end lands exactly at the window's limit AND limit < len(line) (i.e. the window was actually truncated, not naturally at the line's end), re-run that one probe against the full, untruncated line[end:] before accepting the match — paying the larger cost only in this rare edge case rather than on every probe. Found during a merge-gate correctness review of iss-185's PR (bugfix/iss-185-scanner-adjacent-secret-boundary-bypass); non-blocking (contrived input, warn-severity-only impact), so not fixed inline — captured for its own verify-and-fix cycle.
- iss-190scanner adjacency recovery is capped by maxAdjacencyProbeWindow, so a recovered token longer than the window is truncated — and the truncation can break the recovery CHAIN, hiding a third abutting token entirely. probeAt (scanAllPatterns, internal/adapter/scanner/scanner.go) runs its anchored probe against line[at:at+maxAdjacencyProbeWindow] (512 bytes), so a recovered open-ended match longer than that is recorded with an artificial end at the window edge. Two manifestations. (a) Known and accepted since iss-185: the recovered token's far tail past the window stays raw in Redact's output; the token's prefix is still masked, so this is a partial-redaction trade, not a total miss. (b) NEW, found by two independent adversarial reviews of iss-188's fix: because the artificial end does not align with the token's real end, the next iteration's probeAt(m.end) and stolenJunctions both start from a byte offset that is inside the token rather than at its junction, so the chain that would otherwise unwind a run of three or more abutting tokens breaks — the third token becomes entirely invisible, not merely truncated. Repro: line := "ghp_" + strings.Repeat("a",36) + "ghp_" + strings.Repeat("m",600) + "ghp_" + strings.Repeat("z",36) yields exactly 2 token:github_pat findings where an unbounded search would find 3; the third token's 36 raw bytes survive Redact and the fail-closed residual re-scan reports the output clean. A sharper variant of (a) affects jwt_shaped specifically: its regex requires two literal '.' separators, so a JWT-shaped token whose header and payload together exceed the window has NEITHER separator inside the probe's slice and the anchored probe finds no match at all — the token is missed entirely rather than truncated, e.g. a fixed-length match immediately followed by a JWT whose first two segments total more than ~502 bytes. This is NOT a defect introduced by iss-188's backward search; it is the same pre-existing maxAdjacencyProbeWindow trade-off from iss-185, now reachable through a second code path because the backward search feeds more candidates into probeAt. Fixing it properly needs a different mechanism (a growing or adaptive window, or re-probing chained from a recovered match's real end) that must not reintroduce the unbounded per-match cost the window exists to prevent — the exact risk iss-188's own ledger entry warned against. Deliberately not folded into iss-188's fix pass to keep that diff scoped to the confirmed junction-search defect.