diff --git a/.kiro/specs/contributor-scan/tasks.md b/.kiro/specs/contributor-scan/tasks.md index fd79379..4e5dad8 100644 --- a/.kiro/specs/contributor-scan/tasks.md +++ b/.kiro/specs/contributor-scan/tasks.md @@ -294,7 +294,7 @@ - _Requirements: 4.1, 4.4_ - _Depends: 2.12_ -- [ ] 2.16 Say what `c/memory` actually covers, or retire it +- [x] 2.16 Say what `c/memory` actually covers, or retire it - Its score so far is zero true positives and every finding a false one: 26 from a substring match, 3 from a bound it could not see, 1 from an allocation sized to fit. Each was a real defect in the engine and each is now fixed, but the family has never once been right, and 2.15 explains why -- it models string @@ -304,6 +304,17 @@ but it should be a recorded decision with a number behind it, not an assumption. - Observable: either a measured true positive on the 2.15 checkout, or the family's declared scope narrowed in the facts and the docs so no reader takes a silent C scan for a clean bill of health. + - **Done 2026-09-09 by the second route: kept, scoped and stated.** `strcpy` misuse is real in plenty of C + and the family earns its place for that; what it could not do was claim more. Its description said + "bounds, lifetime, arithmetic", which is exactly what it does not decide -- the description was itself + the quiet-failure risk. It now names the APIs it models, and a new `limits` field says what it cannot: + an overflow expressed as an index, a pointer offset, or a memcpy whose size is computed. + - The report renders it as **What was analysed**, for every family offered to a region whether or not it + found anything -- a family that ran and reported nothing is the case a reader is most likely to misread. + The counts are derived from the fact tables the arbiter used, so they cannot drift from what the engine + really looked for; only the limit sentence is authored. + - Retiring the family was the alternative and would have been wrong: it would remove the disclosure along + with the coverage, leaving a C scan silent for a reason nobody could read. - _Requirements: 3.1, 8.2_ - _Depends: 2.15_ diff --git a/src/openultrasast/cli.py b/src/openultrasast/cli.py index 6603c27..1a33bed 100644 --- a/src/openultrasast/cli.py +++ b/src/openultrasast/cli.py @@ -3,7 +3,7 @@ from __future__ import annotations import argparse import json import tempfile -from collections.abc import Sequence +from collections.abc import Mapping, Sequence from dataclasses import dataclass, replace from pathlib import Path from time import perf_counter @@ -538,6 +538,10 @@ def _run_scan(path: Path, config_path: Path, mode: str, fail_on: str) -> ScanOut wrote_verdicts = True plan = record_completed(plan, Stage.REGRESS) worth_fixing_payload = _worth_fixing_payload(verdict_records) if wrote_verdicts else None + # Req 5.6: what each family could see, for the report. Absent when the model layer did not run -- + # and a scan with no model layer states no limits because it made no claims. + coverage_rows = model_payload.get("coverage") if isinstance(model_payload, dict) else None + coverage_for_report = coverage_rows if isinstance(coverage_rows, list) and coverage_rows else None runtime.run_stage( "report", lambda: write_markdown_report( @@ -550,6 +554,7 @@ def _run_scan(path: Path, config_path: Path, mode: str, fail_on: str) -> ScanOut overlay=overlay_records if wrote_overlay else None, obligations=obligations_cited or None, obligations_summary=obligations_payload or None, + coverage=coverage_for_report, ), ) runtime.run_stage( @@ -872,7 +877,9 @@ def _model_skipped(reason: str) -> dict[str, object]: return {**_model_payload(None), "skipped": reason} -def _model_payload(result: object, *, unjudged_paths: tuple[str, ...] = ()) -> dict[str, object]: +def _model_payload( + result: object, *, unjudged_paths: tuple[str, ...] = (), coverage: Sequence[Mapping[str, object]] = () +) -> dict[str, object]: """What the scan records about the model layer. `unjudged_sample` names the regions the budget did not reach, lowest-ranked first. Counting them is the @@ -892,6 +899,7 @@ def _model_payload(result: object, *, unjudged_paths: tuple[str, ...] = ()) -> d "query_seconds": getattr(result, "query_seconds", 0.0), "query_seconds_by_kind": dict(getattr(result, "query_seconds_by_kind", {}) or {}), "arbitrate_seconds": getattr(result, "arbitrate_seconds", 0.0), + "coverage": list(coverage), } @@ -929,6 +937,12 @@ def _run_model_layer( if not regions: return [], _model_skipped("no_regions") + # Req 5.6: what each family could see, recorded whether or not it found anything. A family that ran + # and reported nothing is the case a reader is most likely to misread. + from .model.report import coverage_rows + + coverage = coverage_rows(regions) + resolved = resolve_chat_endpoint(config) client = resolved[0] if resolved else None if client is None: @@ -952,7 +966,7 @@ def _run_model_layer( # The regions the budget did not reach, weakest first -- the evidence an exclusion list is built from. unjudged = tuple(region.path for region in regions[result.regions_scanned :][:10]) findings = [_finding_from_model(item) for item in result.findings] - return findings, _model_payload(result, unjudged_paths=unjudged) + return findings, _model_payload(result, unjudged_paths=unjudged, coverage=coverage) def _finding_from_model(item: object) -> StaticFinding: diff --git a/src/openultrasast/model/report.py b/src/openultrasast/model/report.py index 9d11814..6dbf8d1 100644 --- a/src/openultrasast/model/report.py +++ b/src/openultrasast/model/report.py @@ -82,3 +82,59 @@ def render_findings(findings: Sequence[StaticFinding]) -> str: __all__ = ["SliceRow", "overfitting_gap", "render", "render_findings"] + + +def coverage_rows(regions: Sequence[object]) -> list[dict[str, object]]: + """What each family actually modelled for the languages scanned, and what it cannot decide (Req 5.6). + + Derived, not asserted: the count is the size of the fact table the arbiter used, so it cannot drift from + what the engine really looked for. The `limits` string is authored, reviewed and committed in + `families.toml`, because "cannot decide an overflow expressed as an index" is a statement about an + abstraction and no table can produce it. + + Reported for every family that was OFFERED to a region, not only those that produced findings -- a family + that ran and found nothing is exactly the case a reader is most likely to misread, and the one Req 5.6 + exists for. + """ + from .specs import config_specs, dominance_specs, taint_specs + from .taxonomy import load_families + + taxonomy = load_families() + limits = {family.id: family.limits for family in taxonomy.families} + described = {family.id: family.description for family in taxonomy.families} + + offered: dict[tuple[str, str], int] = {} + for region in regions: + language = str(getattr(region, "language", "")) + for family in getattr(region, "families", ()): + offered.setdefault((language, str(family)), 0) + offered[(language, str(family))] += 1 + + rows: list[dict[str, object]] = [] + for (language, family), region_count in sorted(offered.items()): + modelled = _modelled_count(language, family, taint_specs, dominance_specs, config_specs) + rows.append( + { + "language": language, + "family": family, + "regions": region_count, + "modelled": modelled, + "description": described.get(family, ""), + "limits": limits.get(family, ""), + } + ) + return rows + + +def _modelled_count(language: str, family: str, taint_specs, dominance_specs, config_specs) -> int: # type: ignore[no-untyped-def] + """How many operations the arbiter for this family can even see in this language.""" + spec = taint_specs(language=language).get(family) + if spec is not None: + return len(spec.sinks) + spec = dominance_specs(language=language).get(family) + if spec is not None: + return len(spec.operations) + spec = config_specs(language=language).get(family) + if spec is not None: + return len(spec.settings) + return 0 diff --git a/src/openultrasast/model/taxonomy.py b/src/openultrasast/model/taxonomy.py index e98857c..1c1c324 100644 --- a/src/openultrasast/model/taxonomy.py +++ b/src/openultrasast/model/taxonomy.py @@ -31,7 +31,7 @@ VERIFIER_KINDS = ("canary", "static", "none") Relation = Literal["same", "parent_child", "lateral", "fabricated"] DEFAULT_FAMILIES_PATH = Path(__file__).resolve().parents[1] / "ruleset" / "families.toml" -_FAMILY_FIELDS = frozenset({"id", "description", "cwes", "mechanisms", "verifier", "parent"}) +_FAMILY_FIELDS = frozenset({"id", "description", "cwes", "mechanisms", "verifier", "parent", "limits"}) _TOP_FIELDS = frozenset({"version", "family"}) @@ -47,6 +47,10 @@ class Family: mechanisms: frozenset[str] verifier: str # VERIFIER_KINDS parent: str | None = None # a family this one specializes; partial credit when one side is the other's parent + # What this family's abstraction CANNOT decide, in the words a reader needs. Empty means no limit has + # been stated, which is not the same as no limit existing -- and a family that has one and does not say + # so lets a silent scan read as a clean bill of health (Req 5.6). + limits: str = "" @dataclass(frozen=True) @@ -153,6 +157,7 @@ def _family(source: Path, entry: dict[str, object]) -> Family: mechanisms=frozenset(mechanisms), verifier=verifier, parent=str(parent) if parent else None, + limits=str(entry.get("limits", "") or "").strip(), ) diff --git a/src/openultrasast/reports.py b/src/openultrasast/reports.py index e3459c5..ebd411e 100644 --- a/src/openultrasast/reports.py +++ b/src/openultrasast/reports.py @@ -22,6 +22,7 @@ def write_markdown_report( mechanisms: Mapping[str, Mapping[str, object]] | None = None, obligations: Mapping[str, Mapping[str, object]] | None = None, obligations_summary: Mapping[str, object] | None = None, + coverage: Sequence[Mapping[str, object]] | None = None, ) -> None: path.parent.mkdir(parents=True, exist_ok=True) verification_by_id = _verification_by_id(verifications or []) @@ -86,6 +87,8 @@ def write_markdown_report( cited_obligations[finding.finding_id] = info_o _append_obligation_lines(lines, info_o, (mechanisms or {}).get(str(info_o.get("known_fix") or ""))) lines.extend(["", finding.rationale, ""]) + if coverage: + _append_coverage(lines, coverage) if patterned: _append_pattern_matches(lines, patterned) if cited: @@ -391,6 +394,36 @@ def _crowded_pattern_rules(findings: Sequence[StaticFinding]) -> list[StaticFind ] +def _append_coverage(lines: list[str], coverage: Sequence[Mapping[str, object]]) -> None: + """What was looked for, and what could not be decided (Req 5.6). + + A scan that reports nothing has established that nothing it MODELS was found. That is not the same as + finding nothing, and the difference is the whole reason this section exists: a silent report read as a + clean bill of health is worse than a noisy one, because the reader acts on it. + """ + lines.extend( + [ + "## What was analysed", + "", + "Each family below was offered to at least one region. **A family that reported nothing has " + "established only that nothing it models was found** — silence here means no fact table covered " + "the code, not that the code is safe.", + "", + "| language | family | regions | operations modelled |", + "| --- | --- | --- | --- |", + ] + ) + for row in coverage: + lines.append(f"| {row.get('language')} | `{row.get('family')}` | {row.get('regions')} | {row.get('modelled')} |") + lines.append("") + + stated = [row for row in coverage if str(row.get("limits") or "")] + if stated: + lines.extend(["### Stated limits", ""]) + for row in stated: + lines.extend([f"**`{row.get('family')}` ({row.get('language')})** — {row.get('limits')}", ""]) + + def _is_pattern_only(finding: StaticFinding) -> bool: """Is a text match the only thing behind this finding? diff --git a/src/openultrasast/ruleset/families.toml b/src/openultrasast/ruleset/families.toml index 1f11d8a..9f7311e 100644 --- a/src/openultrasast/ruleset/families.toml +++ b/src/openultrasast/ruleset/families.toml @@ -86,7 +86,8 @@ verifier = "none" [[family]] id = "memory" -description = "Memory safety in C and C++: bounds, lifetime, arithmetic. Measured on the vendored slice, never gated; specialized systems own this class." +description = "Misuse of unbounded string and format APIs in C and C++: a tainted value reaching gets, strcpy, strcat, sprintf or a non-literal format string." +limits = """Models CALLS, not arithmetic. It cannot decide an overflow expressed as an index, a pointer offset, or a memcpy whose size is computed -- which is how most real C overflows are written, and how every one of libpng's is. A scan of C code that reports nothing from this family has established that no tainted value reached a modelled string API, and nothing at all about its buffer arithmetic.""" cwes = [ "CWE-119", "CWE-120", diff --git a/tests/test_model_taxonomy.py b/tests/test_model_taxonomy.py index c3510a8..d0a5a17 100644 --- a/tests/test_model_taxonomy.py +++ b/tests/test_model_taxonomy.py @@ -122,3 +122,15 @@ def test_the_shipped_file_must_declare_the_whole_closed_set_in_order(tmp_path: P (tmp_path / "families.toml").write_text(GOOD) with pytest.raises(FamiliesError, match="in that order"): load_families(tmp_path / "families.toml") + + +def test_the_memory_family_states_what_it_cannot_decide() -> None: + """contributor-scan 2.16. Its description used to claim "bounds, lifetime, arithmetic", which is exactly + what it does not do -- the description was itself the quiet-failure risk.""" + from openultrasast.model.taxonomy import load_families + + memory = next(family for family in load_families().families if family.id == "memory") + + assert "arithmetic" in memory.limits, "the gap is stated" + assert "arithmetic" not in memory.description, "and no longer claimed" + assert "strcpy" in memory.description, "what it does cover is named" diff --git a/tests/test_reports.py b/tests/test_reports.py index 0744853..2f96ee8 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -517,3 +517,38 @@ def test_a_reasoned_claim_keeps_its_section_whatever_its_rung(tmp_path: Path) -> write_markdown_report(reasoned, output) assert output.read_text().count(f"## {base.title}") == 20 + + +def test_the_report_says_what_was_looked_for_and_what_cannot_be_decided(tmp_path: Path) -> None: + """contributor-scan 2.16, Req 5.6. `c/memory` scored zero true positives across 89k lines of C for a + structural reason -- it models string CALLS and libpng overflows through arithmetic. A silent scan + reading as a clean bill of health is worse than a noisy one, because the reader acts on it.""" + output = tmp_path / "report.md" + coverage = [ + { + "language": "c", + "family": "memory", + "regions": 121, + "modelled": 9, + "description": "Misuse of unbounded string and format APIs", + "limits": "Models CALLS, not arithmetic. It cannot decide an overflow expressed as an index.", + } + ] + + write_markdown_report([], output, coverage=coverage) + + text = output.read_text() + assert "## What was analysed" in text + assert "| c | `memory` | 121 | 9 |" in text, "what it looked for, counted from the fact table itself" + assert "### Stated limits" in text + assert "cannot decide an overflow expressed as an index" in text + assert "silence here means no fact table covered the code, not that the code is safe" in text + + +def test_a_scan_that_made_no_claims_states_no_limits(tmp_path: Path) -> None: + """A report with no model layer has nothing to disclaim; the section would be noise.""" + output = tmp_path / "report.md" + + write_markdown_report([_finding()], output) + + assert "## What was analysed" not in output.read_text()