diff --git a/.kiro/specs/contributor-scan/tasks.md b/.kiro/specs/contributor-scan/tasks.md index e10a8c1..46e424d 100644 --- a/.kiro/specs/contributor-scan/tasks.md +++ b/.kiro/specs/contributor-scan/tasks.md @@ -185,7 +185,7 @@ - _Requirements: 8.1, 8.2_ - _Depends: 2.6_ -- [ ] 2.9 One defect is one finding, however many regions reach it +- [x] 2.9 One defect is one finding, however many regions reach it - libpng's scan produced 26 entailed findings that are all the identical site, `contrib/gregbook/wpng.c:335:main`. Many regions reach one shared sink and each emits its own finding; nothing dedupes by site. A contributor would be shown the same defect twenty-six times. diff --git a/src/openultrasast/cli.py b/src/openultrasast/cli.py index 1a33bed..def0d11 100644 --- a/src/openultrasast/cli.py +++ b/src/openultrasast/cli.py @@ -969,6 +969,17 @@ def _run_model_layer( return findings, _model_payload(result, unjudged_paths=unjudged, coverage=coverage) +def _model_rationale(item: object, witness: str, family: str) -> str: + """The witness, plus how many entry points reach this site when more than one does. + + A site reached from twenty-six entry points is one defect with a wide blast radius, which is worth + saying once. It used to be said twenty-six times. + """ + base = witness or f"the model layer reported a {family} candidate" + reached = int(getattr(item, "reached_from", 1) or 1) + return base if reached <= 1 else f"{base} (reached from {reached} regions)" + + def _finding_from_model(item: object) -> StaticFinding: """A model finding as a StaticFinding, carrying its rung and witness (Req 5.1).""" site = str(getattr(item, "site", "")) @@ -984,7 +995,7 @@ def _finding_from_model(item: object) -> StaticFinding: severity="medium", confidence="medium", evidence_level="suspicion", - rationale=witness or f"the model layer reported a {family} candidate", + rationale=_model_rationale(item, witness, family), line=int(line) if line.isdigit() else None, function_name=function or None, reachability_status="unknown", diff --git a/src/openultrasast/model/pipeline.py b/src/openultrasast/model/pipeline.py index 9894160..b16163a 100644 --- a/src/openultrasast/model/pipeline.py +++ b/src/openultrasast/model/pipeline.py @@ -64,6 +64,9 @@ class ModelFinding: rung: Rung witness: str = "" contradiction: str = "" + # How many regions reached this site. One defect is one finding however many entry points lead to it; + # the count is the useful part of the duplication, and the rows are not. + reached_from: int = 1 def _arbitrate_all( diff --git a/src/openultrasast/model/scan.py b/src/openultrasast/model/scan.py index fff716f..dd72879 100644 --- a/src/openultrasast/model/scan.py +++ b/src/openultrasast/model/scan.py @@ -21,7 +21,7 @@ from __future__ import annotations import logging import time from collections.abc import Mapping, Sequence -from dataclasses import dataclass, field +from dataclasses import dataclass, field, replace from pathlib import Path from typing import Any @@ -206,7 +206,7 @@ def scan_repository( scanned = len(judged) - findings = _ordered(collected) + findings = _ordered(_deduplicated(collected)) return ModelScanResult( findings=findings, by_rung=_tally(findings), @@ -289,6 +289,30 @@ def _spec_for(family: str, language: str) -> ArbiterSpec | None: return taint_specs(language=language).get(family) +def _deduplicated(collected: Sequence[tuple[ModelFinding, float]]) -> list[tuple[ModelFinding, float]]: + """One finding per (site, family), keeping the strongest rung and counting the regions that reached it. + + Task 2.4 let a region's question follow the call graph, and the moment it did, many entry points could + reach one shared sink -- libpng reported the same site twenty-six times. A contributor shown the same + defect twenty-six times learns to scroll past it, and "reached from 26 entry points" is the useful half + of that observation. + + Two families at one site stay two findings: `echo $_GET[...]` is an injection question and an output + encoding question, and they have different answers. + """ + best: dict[tuple[str, str], tuple[ModelFinding, float]] = {} + for finding, rank in collected: + key = (finding.site, finding.family) + current = best.get(key) + if current is None: + best[key] = (finding, rank) + continue + kept, kept_rank = current + stronger = finding if _RUNG_ORDER[finding.rung] < _RUNG_ORDER[kept.rung] else kept + best[key] = (replace(stronger, reached_from=kept.reached_from + finding.reached_from), max(rank, kept_rank)) + return list(best.values()) + + def _ordered(collected: Sequence[tuple[ModelFinding, float]]) -> tuple[ModelFinding, ...]: """Rung first, then the region's rank, then the site — a total order, so two runs agree.""" return tuple(finding for finding, _ in sorted(collected, key=lambda pair: (_RUNG_ORDER.get(pair[0].rung, 9), -pair[1], pair[0].site))) diff --git a/tests/test_model_scan.py b/tests/test_model_scan.py index 6ae1c7e..4e24ae1 100644 --- a/tests/test_model_scan.py +++ b/tests/test_model_scan.py @@ -327,3 +327,40 @@ def test_the_scan_reports_query_time_per_kind() -> None: assert set(result.query_seconds_by_kind) == {"taint", "config"} assert sum(result.query_seconds_by_kind.values()) <= result.query_seconds + 0.05 + + +def test_one_defect_is_one_finding_however_many_regions_reach_it(tmp_path) -> None: + """contributor-scan 2.9. Task 2.4 let a region's question follow the call graph, and the moment it did, + many entry points could reach one shared sink: libpng reported the same site twenty-six times. A + contributor shown the same defect twenty-six times learns to scroll past it.""" + from openultrasast.model.ladder import Rung + from openultrasast.model.pipeline import ModelFinding + from openultrasast.model.scan import _deduplicated + + same = [ + (ModelFinding(site="wpng.c:335:main", family="memory", rung=Rung.SUSPICION, witness="w"), 0.2), + (ModelFinding(site="wpng.c:335:main", family="memory", rung=Rung.ENTAILED, witness="w"), 1.0), + (ModelFinding(site="wpng.c:335:main", family="memory", rung=Rung.SUSPICION, witness="w"), 0.5), + ] + + kept = _deduplicated(same) + + assert len(kept) == 1 + finding, rank = kept[0] + assert finding.rung is Rung.ENTAILED, "the strongest rung survives" + assert finding.reached_from == 3, "and the count is what the duplication was worth" + assert rank == 1.0 + + +def test_two_families_at_one_site_stay_two_findings() -> None: + """`echo $_GET[...]` is an injection question and an output-encoding question, with different answers.""" + from openultrasast.model.ladder import Rung + from openultrasast.model.pipeline import ModelFinding + from openultrasast.model.scan import _deduplicated + + both = [ + (ModelFinding(site="app.php:9:show", family="injection", rung=Rung.ENTAILED), 1.0), + (ModelFinding(site="app.php:9:show", family="output_encoding", rung=Rung.ENTAILED), 1.0), + ] + + assert len(_deduplicated(both)) == 2