diff --git a/.kiro/specs/contributor-scan/tasks.md b/.kiro/specs/contributor-scan/tasks.md index 4e5dad8..e6ba84f 100644 --- a/.kiro/specs/contributor-scan/tasks.md +++ b/.kiro/specs/contributor-scan/tasks.md @@ -318,7 +318,7 @@ - _Requirements: 3.1, 8.2_ - _Depends: 2.15_ -- [ ] 2.17 The model's own suspicion band is a pattern match with an opinion attached +- [x] 2.17 The model's own suspicion band is a pattern match with an opinion attached - **Req 5.5's other half.** Task 2.11 made pattern matches proportionate and left this untouched, so vampi still emits 61 sections for a 520-line application -- 38 of them model suspicions, roughly one per fourteen lines of code. diff --git a/benchmarks/measurements/2026-09-09-report-proportionality-model-band.json b/benchmarks/measurements/2026-09-09-report-proportionality-model-band.json new file mode 100644 index 0000000..e5e4d22 --- /dev/null +++ b/benchmarks/measurements/2026-09-09-report-proportionality-model-band.json @@ -0,0 +1,25 @@ +{ + "generated": "2026-09-09", + "measured": { + "libpng": { + "findings": 198, + "sections_after_2_11": 39, + "sections_after_2_17": 30 + }, + "vampi": { + "findings": 60, + "report_lines": "1085 -> 446", + "sections_after": 26, + "sections_before": 61 + } + }, + "reading": "Task 2.11 made static pattern matches proportionate and left the model layer's own suspicions alone, on the reasoning that a proposer had reasoned about them. That reasoning does not survive inspection: a model suspicion is what the enumerator proposed at a site the graph could NOT decide, affirmed by a judge with no coverage there. Its own rationale says so -- 'the model resolved no flow here; its sink table may not cover this API'. It is a pattern match with an opinion attached, and it reads like a reasoned claim only because a model produced it.", + "spec": "contributor-scan 2.17 (Req 5.5) -- the model's own suspicion band", + "the_section_renamed": "'Pattern matches' became 'Proposed, not corroborated', because it now holds two shapes: a static pattern saying an API is present, and a candidate the model could not decide. Model rows group by FAMILY rather than by rule, since their rule is always `model`.", + "what_is_not_grouped": "The obligations checker also sits at `suspicion`, and is NOT grouped. Its output comes from a structural analysis -- an obligated operation with no discharging guard -- rather than from a guess. An earlier attempt at this rule filtered on rung alone and would have collapsed it; six tests failed and they were right to.", + "what_it_costs": { + "caveat": "Three known vulnerabilities in one repository. The rule would hide a real finding that only ever reached model-suspicion, and no such finding exists in the evidence base -- which is a statement about how small the evidence base is as much as about the rule.", + "method": "For each known vulnerability the recipe declares, take every finding row at its location and ask whether any survives the rule. If a known vulnerability appeared ONLY at model-suspicion, this rule hides it, and that is the number that decides whether the rule is right.", + "result": "VAmPI-SQLI, VAmPI-BOLA and VAmPI-BOLA-BOOKS all still found at model_entailed and all three survive the rule. Nothing known is hidden." + } +} diff --git a/src/openultrasast/reports.py b/src/openultrasast/reports.py index ebd411e..5fac6e5 100644 --- a/src/openultrasast/reports.py +++ b/src/openultrasast/reports.py @@ -385,13 +385,8 @@ def _crowded_pattern_rules(findings: Sequence[StaticFinding]) -> list[StaticFind counts: dict[str, int] = {} for finding in findings: if _is_pattern_only(finding): - rule = str(finding.finding_id).split(":", 1)[0] - counts[rule] = counts.get(rule, 0) + 1 - return [ - finding - for finding in findings - if _is_pattern_only(finding) and counts.get(str(finding.finding_id).split(":", 1)[0], 0) > _PATTERN_DETAIL_LIMIT - ] + counts[_rule_key(finding)] = counts.get(_rule_key(finding), 0) + 1 + return [finding for finding in findings if _is_pattern_only(finding) and counts.get(_rule_key(finding), 0) > _PATTERN_DETAIL_LIMIT] def _append_coverage(lines: list[str], coverage: Sequence[Mapping[str, object]]) -> None: @@ -425,30 +420,51 @@ def _append_coverage(lines: list[str], coverage: Sequence[Mapping[str, object]]) def _is_pattern_only(finding: StaticFinding) -> bool: - """Is a text match the only thing behind this finding? + """Was this proposed with nothing corroborating it? - `static_corroboration` is what the static layer emits when a pattern matched and nothing else spoke. A - proposer that reasoned about the site -- the model layer, the obligations checker -- carries `suspicion` - here even when its rung is also suspicion, and that is the distinction: an undecided CLAIM is not the - same as an observation that an API is present. + Two shapes qualify, and the second took a second look to see. + + A **static pattern match** (`static_corroboration` at `suspicion`) says an API is present. + + A **model suspicion** (`model:` at `suspicion`) is what the enumerator proposed at a site the graph could + NOT decide, affirmed by a judge that has no coverage there -- a pattern match with an opinion attached. + Its rationale says so in its own words: "the model resolved no flow here; its sink table may not cover + this API". It reads like a reasoned claim because a model produced it, which is exactly why it needs + naming: on vampi it is 38 of 60 findings, roughly one per fourteen lines of a 520-line application. + + What does NOT qualify is the obligations checker, which also sits at `suspicion`. Its output comes from a + structural analysis -- an obligated operation with no discharging guard -- rather than from a guess, and + collapsing it would hide a detector. """ - return str(finding.evidence_level) == "static_corroboration" and str(finding.rung) == "suspicion" + if str(finding.rung) != "suspicion": + return False + if str(finding.evidence_level) == "static_corroboration": + return True + return str(finding.finding_id).startswith("model:") + + +def _rule_key(finding: StaticFinding) -> str: + """How a proposal is grouped. Model findings key on their FAMILY, since their rule is always `model`.""" + parts = str(finding.finding_id).split(":") + if parts and parts[0] == "model" and len(parts) > 1: + return f"model:{parts[1]}" + return parts[0] if parts else "" def _append_pattern_matches(lines: list[str], patterned: Sequence[StaticFinding]) -> None: """Pattern matches, grouped by rule. Proportionate: few sites are named, many are counted.""" by_rule: dict[str, list[StaticFinding]] = {} for finding in patterned: - by_rule.setdefault(str(finding.finding_id).split(":", 1)[0], []).append(finding) + by_rule.setdefault(_rule_key(finding), []).append(finding) lines.extend( [ - "## Pattern matches", + "## Proposed, not corroborated", "", - f"{len(patterned)} site(s) matched a static pattern with nothing corroborating them. A match says " - "an API is present, not that it is misused; the model reached none of these, and silence from the " - "model means no fact table covered the site rather than that it is safe. All of them are in " - "`findings.json`.", + f"{len(patterned)} site(s) were proposed with nothing corroborating them: a static pattern saying " + "an API is present, or a candidate the model could not decide and a judge affirmed without " + "coverage there. Neither says the site is misused. Silence from the model means no fact table " + "covered it rather than that it is safe, and all of them are in `findings.json`.", "", ] ) diff --git a/tests/test_reports.py b/tests/test_reports.py index 2f96ee8..1bec66e 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -485,7 +485,7 @@ def test_a_pattern_rule_with_many_sites_is_summarised_not_repeated(tmp_path: Pat write_markdown_report(crowded, output) text = output.read_text() - assert "## Pattern matches" in text + assert "## Proposed, not corroborated" in text assert "20 site(s) across 20 file(s)" in text assert text.count("## c-unsafe-copy needs review") == 0, "twenty sections for one rule is the noise" assert "`findings.json`" in text, "counted, never dropped" @@ -499,7 +499,7 @@ def test_a_pattern_rule_with_few_sites_keeps_its_sections(tmp_path: Path) -> Non text = output.read_text() assert "## python-flask-debug needs review" in text - assert "## Pattern matches" not in text + assert "## Proposed, not corroborated" not in text def test_a_reasoned_claim_keeps_its_section_whatever_its_rung(tmp_path: Path) -> None: @@ -552,3 +552,68 @@ def test_a_scan_that_made_no_claims_states_no_limits(tmp_path: Path) -> None: write_markdown_report([_finding()], output) assert "## What was analysed" not in output.read_text() + + +def test_a_model_suspicion_is_grouped_like_a_pattern_match(tmp_path: Path) -> None: + """contributor-scan 2.17, Req 5.5's other half. + + A model suspicion is what the enumerator proposed at a site the graph could NOT decide, affirmed by a + judge with no coverage there -- a pattern match with an opinion attached. It reads like a reasoned claim + because a model produced it, and on vampi it was 38 of 60 findings on a 520-line application. + """ + from dataclasses import replace + + base = _finding() + suspicions = [ + replace( + base, + finding_id=f"model:injection:api/f{i}.py:{i}:jsonify", + evidence_level="suspicion", + rung="suspicion", + path=f"api/f{i}.py", + line=i, + rationale="the model resolved no flow here; its sink table may not cover this API", + ) + for i in range(20) + ] + output = tmp_path / "report.md" + + write_markdown_report(suspicions, output) + + text = output.read_text() + assert "`model:injection`" in text, "grouped by family, since the rule is always `model`" + assert "20 site(s)" in text + assert text.count(f"## {base.title}") == 0 + + +def test_an_obligation_finding_is_not_grouped_away(tmp_path: Path) -> None: + """It sits at `suspicion` too, but it comes from a structural analysis rather than a guess. Collapsing + it would hide a detector -- which an earlier attempt at this rule did, and six tests caught it.""" + from dataclasses import replace + + base = _finding() + obligations = [ + replace(base, finding_id=f"obligation:protected_read:api/u.py:{i}", evidence_level="suspicion", rung="suspicion", line=i) + for i in range(20) + ] + output = tmp_path / "report.md" + + write_markdown_report(obligations, output) + + assert output.read_text().count(f"## {base.title}") == 20 + + +def test_an_entailed_model_finding_keeps_its_section(tmp_path: Path) -> None: + """The rung is the product. Whatever the model layer DECIDED is never summarised away.""" + from dataclasses import replace + + base = _finding() + entailed = [ + replace(base, finding_id=f"model:injection:app.py:{i}:execute", evidence_level="suspicion", rung="model_entailed", line=i) + for i in range(20) + ] + output = tmp_path / "report.md" + + write_markdown_report(entailed, output) + + assert output.read_text().count(f"## {base.title}") == 20