fix(audit): un-mask findings hidden by detector-ID collisions + clearer B2 message#48
Merged
Merged
Conversation
…er B2 Two detector families ship the same IDs (builtin readme R1/R2 vs reconcile R1/R2; a repo's custom plugin may add a third). run_audit/_run_adapters did `result.patterns[id] = entry`, so a later count-0 instance silently overwrote an earlier instance that found something, while total_findings (summed separately) still counted it — a phantom finding invisible in the patterns map and findings(). It masked a real boundary-leak (R2 scrub-target) in a consumer. - AuditResult.add_pattern(): merge on collision (sum count, dedupe samples, max severity, flag `collision`/`collision_sources`); route both detector and adapter registration through it. total_findings is unchanged (computed by pre-storage sum), so --fail-on-findings behavior is byte-identical — the fix only makes already-counted findings visible. - detect_b2: distinguish a content-less artifact (provided + parsed, zero forbidden_names — a secret/data fix) from not-provided (a config fix). Tests: collision merge/no-mask/severity/dedupe; B2 content-less message. Full suite 1123 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ProtocolWarden
added a commit
that referenced
this pull request
Jun 22, 2026
…warning) (#50) #48 un-masked the COUNT when two detectors share an id, but the displayed finding still showed only the first-registered detector's title — so a `.console/task.md` violation surfaced as "[R2] README first H1 does not match repo name" and silently misdirected a consumer (the reviewer escalated it as ci_misconfigured_check; the goal lane stalled for hours chasing a phantom README issue). This makes the collision visible at both points: - audit_kit/result.py: `collision_note(pattern)` — a marker rendered for any merged (collided) pattern, naming the colliding sources and warning that the title may not match the finding (read the samples). Used by both the single-repo (audit.py) and multi-repo (multi.py) finding renderers. - cli/runner.py: `_warn_detector_id_collisions(detectors)` logs a WARNING at load time for every id registered by more than one detector, listing each source's description — so the collision is fixable at the source (rename the custom detector) instead of being discovered the hard way. Non-breaking: no detector ids renamed; the builtin readme R1/R2 vs reconcile R1/R2 families and custom-plugin ids keep working, the collision is just no longer silent. 3 new tests (collision marker on/off, load-time warning fires per duplicate and not for unique ids); 1130 existing tests pass; `ruff check src` clean. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Problem
Two detector families ship the same IDs: builtin
readme(R1=READMEmissing, R2=H1 mismatch) and builtin
reconcile(R1=.consoleline-budget,R2=scrub-target leak), and a repo's custom plugin can add a third R1/R2.
Assembled into one list,
run_audit/_run_adaptersdidresult.patterns[id] = entry— so a later, count-0 instance silentlyoverwrote an earlier instance that found something, while
total_findings(summed independently) still counted it.
Net effect: a phantom finding —
total_findings: 1with every visiblepattern at
count: 0and an emptyfindings(). In the field this masked areal boundary-leak (a reconcile R2 scrub-target hit) behind a custom
plugin's count-0 R2. A security detector firing into a black hole.
Fix
AuditResult.add_pattern(code, entry)— merge on ID collision instead ofoverwrite: sum
count, dedupe-concatsamples, take maxseverity, flagcollision+ recordcollision_sources. Both registration sites (detectorsand tool adapters) route through it.
total_findingsis still computed by thepre-storage sum in
run_audit, so--fail-on-findingsbehaves byte-for-byteidentically. The change only makes already-counted findings visible —
repos that were silently red become diagnosably red; no green repo flips.
detect_b2now distinguishes a content-less artifact (provided +parsed but zero
forbidden_names— a secret/data fix) from not-provided(a config fix). Previously both got the generic "not provided" message, which
sent operators hunting for a missing file that was actually present-but-empty.
Tests
add_patternfirst-write / collision-merge-no-mask / max-severity+dedupe, andthe B2 content-less message. Full suite: 1123 passed, 1 skipped; ruff +
doctor clean.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
🤖 Generated with Claude Code