diff --git a/scripts/asvs/scorecard.py b/scripts/asvs/scorecard.py index 35ad1c22..663ad95d 100644 --- a/scripts/asvs/scorecard.py +++ b/scripts/asvs/scorecard.py @@ -77,10 +77,22 @@ class Absence: absence claims survived for weeks. So an absence claim is only admissible with a ``positive_control`` that must still match; if the control goes quiet the search has gone blind and the claim is void, regardless of what the pattern returns. + + ``mutation`` closes the hole the control does not: it is the realistic REINTRODUCTION this pattern + claims to exclude, and the pattern must actually fire on it. A whole chapter of claims was once + authored as prose narrations of shell commands — ``"rg -n 'tar.extractall' -> exit 1 (zero hits)"`` + where the field wanted ``tar\\.extractall\\(``. The field's type is ``str`` and prose is a valid + ``str``, so the shape permitted it and only a detector caught it. Requiring the pattern to match a + stated reintroduction makes prose *unwritable* rather than merely detectable. + + Do NOT derive ``mutation`` from ``pattern``. A value generated from the thing it validates + satisfies the check by construction, which would make this the most authoritative-looking vacuous + gate in the file — the same defect class it exists to close, arriving through the fix. """ pattern: str positive_control: str + mutation: str @dataclass(frozen=True) @@ -171,6 +183,21 @@ def load_scorecard(path: Path) -> list[Cell]: "recording the reason for non-applicability is the one MUST in ASVS 5.0's assessment " "chapter (docs/ASVS-ASSESSMENT-METHOD.md §1)" ) + for a in raw.get("absence", []): + if not str(a.get("mutation", "")).strip(): + raise ScorecardError( + # NO LITERAL CODE EXAMPLE HERE. This module is inside the corpus that absence + # patterns are searched over, so an illustrative call in this string becomes a + # real corpus hit and reads as FALSE. The first draft used one and broke two + # live claims (5.2.5, 5.3.3) the moment they were backfilled: the guidance for + # a check contaminated the check. The Absence docstring carries the example in + # escaped-regex form, which cannot self-match. + f"cell {raw.get('id')!r}: absence claim {a.get('pattern')!r} has no `mutation` — " + "state the realistic reintroduction this pattern excludes, so the pattern can " + "be proved capable of firing. Author it from what the code would look like if " + "the thing came back; do NOT derive it from the pattern, which makes the check " + "vacuous. See the Absence docstring for a worked example" + ) cells.append( Cell( id=str(raw["id"]), @@ -186,7 +213,13 @@ def load_scorecard(path: Path) -> list[Cell]: for e in raw.get("evidence", []) ), absence=tuple( - Absence(pattern=str(a["pattern"]), positive_control=str(a["positive_control"])) + Absence( + pattern=str(a["pattern"]), + positive_control=str(a["positive_control"]), + # No default. A missing mutation must be authored, not inferred — see the + # Absence docstring on why deriving one from the pattern is worse than none. + mutation=str(a["mutation"]), + ) for a in raw.get("absence", []) ), ) @@ -231,14 +264,45 @@ def check_completeness(cells: list[Cell], corpus: dict[str, int]) -> list[str]: want = corpus.get(c.id) if want is not None and c.level != want: problems.append(f"{c.id}: level {c.level} but the corpus says L{want}") + + # A DECIDED verdict with no evidence at all is the conflation this whole tool exists to prevent: + # a guess wearing a verdict's clothes. The method is explicit — every non-`unverified` cell carries + # at least one anchor — but nothing enforced it, because `check_anchors` iterates the evidence a + # cell HAS. A cell with none is not checked and fails nothing; the gate could only ever validate + # evidence that existed, never assert that it must. Measured when this landed: 14 of 59 decided + # cells carried zero anchors AND zero absence claims, several inherited from the prose lineage + # where the verdict was reached but never anchored. + unevidenced = sorted( + (c.id for c in cells if c.verdict in DECIDED_VERDICTS and not c.evidence and not c.absence), + key=_sort_key, + ) + if unevidenced: + problems.append( + f"evidence: {len(unevidenced)} decided cell(s) carry NO anchor and NO absence claim, so " + "nothing about them is verified — either anchor them or return them to `unverified`, " + f"which is what an unevidenced verdict actually is: {', '.join(unevidenced)}" + ) return problems def check_anchors(cells: list[Cell], root: Path, findings: Findings) -> None: - """Open every evidence anchor and assert its token still resolves. + """Open every evidence anchor and assert its token still resolves, and resolves UNAMBIGUOUSLY. When the code moves, this reds a test — instead of the sentence rotting in place and the next session funding work that is already done. + + **Uniqueness is not pedantry; it is what makes the resolution mean anything.** An ``expect`` that + occurs many times in its file resolves from almost anywhere: with ``await conn.rollback()`` + appearing 101 times in one module, *any* line number in that file lands within ±40 of some + occurrence, so the anchor cannot fail and certifies nothing. Two such anchors sat in this scorecard + as evidence for weeks. + + It also closes a defect in the REPAIR path rather than the detection path. When code moves, the + check correctly reports it — but a re-anchor to the nearest occurrence can silently install a + *stale-but-resolving* anchor that passes forever. That happened live: after ADR 0154 landed, + ``UPDATE sessions SET revoked_at=`` had two occurrences 19 lines apart — one the keep-N revoke, one + a different method entirely — each inside the other's window, so the check would have accepted the + wrong one. A repair is exactly where suspicion lapses, because the tool has just proved it works. """ for c in cells: for a in c.evidence: @@ -246,10 +310,19 @@ def check_anchors(cells: list[Cell], root: Path, findings: Findings) -> None: if not target.is_file(): findings.problems.append(f"{c.id}: evidence path {a.path} does not exist") continue - lines = target.read_text(encoding="utf-8", errors="replace").splitlines() + text = target.read_text(encoding="utf-8", errors="replace") + lines = text.splitlines() lo = max(0, a.line - 1 - ANCHOR_WINDOW) hi = min(len(lines), a.line + ANCHOR_WINDOW) findings.checked_anchors += 1 + occurrences = text.count(a.expect) + if occurrences > 1: + findings.problems.append( + f"{c.id}: {a.path}:{a.line} anchor is AMBIGUOUS — {a.expect!r} occurs " + f"{occurrences} times in the file, so the line number is not load-bearing and a " + "re-anchor cannot be checked. Cite a longer token that appears exactly once" + ) + continue if a.expect in "\n".join(lines[lo:hi]): continue where = " (found elsewhere in the file)" if a.expect in "\n".join(lines) else "" @@ -265,6 +338,14 @@ def check_absences(cells: list[Cell], root: Path, findings: Findings) -> None: for c in cells: for a in c.absence: findings.checked_absences += 1 + # Before asking what the corpus says, ask whether the pattern is a pattern at all. A prose + # narration greps to nothing and is indistinguishable from a true absence. + if not re.search(a.pattern, a.mutation): + findings.problems.append( + f"{c.id}: absence claim is INERT — {a.pattern!r} does not match its own stated " + f"reintroduction {a.mutation!r}, so it would stay quiet if the thing came back" + ) + continue control = _grep_count(a.positive_control, corpus_files) if control == 0: findings.problems.append( @@ -328,6 +409,17 @@ def check_pinning(scorecard: Path, corpus: Path) -> list[str]: "pinning: [scorecard].asvs_version is missing — bare requirement ids re-point across " "ASVS versions, so the scorecard must say which version its ids mean" ) + anchor = str(meta.get("anchor_commit", "")).strip() + # actions/checkout resolves `ref:` as a BRANCH OR TAG name unless it is a full 40-char hash, so an + # abbreviated anchor fails with "A branch or tag with the name ... could not be found" -- a gate + # failing for a reason with nothing to do with what it measures. Caught in CI twice; refused here. + if anchor and not re.fullmatch(r"[0-9a-f]{40}", anchor): + problems.append( + f"pinning: [scorecard].anchor_commit {anchor!r} must be a FULL 40-character SHA -- an " + "abbreviated hash is not resolvable by actions/checkout, which treats a short ref as a " + "branch or tag name" + ) + declared = str(meta.get("corpus_sha256", "")).strip() actual = corpus_digest(corpus) if not declared: diff --git a/tests/test_asvs_scorecard.py b/tests/test_asvs_scorecard.py index 03a0f39e..ebd6502a 100644 --- a/tests/test_asvs_scorecard.py +++ b/tests/test_asvs_scorecard.py @@ -52,7 +52,23 @@ def _scorecard_file(tmp_path: Path, body: str) -> Path: def _cells(*specs: tuple[str, int, str]) -> list[Cell]: - return [Cell(id=i, level=lv, verdict=v) for i, lv, v in specs] # type: ignore[arg-type] + """Fixture cells for the id / level / count checks. + + Decided verdicts carry a placeholder anchor because `check_completeness` now refuses an + unevidenced one. These fixtures are about ids and levels, so the anchor keeps them focused rather + than tripping a rule they do not test; `check_anchors` is never called on them. + """ + decided = {"pass", "partial", "fail", "na"} + return [ + Cell( + id=i, + level=lv, + verdict=v, # type: ignore[arg-type] + evidence=(Anchor("messagefoundry/m.py", 1, "x"),) if v in decided else (), + residual="rationale" if v == "na" else "", + ) + for i, lv, v in specs + ] # --- completeness: the check whose absence cost ten cells --------------------------------------- @@ -165,7 +181,10 @@ def test_absence_holds_when_pattern_is_quiet_and_control_speaks(tmp_path: Path) ) cells = [ Cell( - id="1.1.1", level=1, verdict="fail", absence=(Absence("clamav|clamd", "ScanRejected"),) + id="1.1.1", + level=1, + verdict="fail", + absence=(Absence("clamav|clamd", "ScanRejected", "import clamd"),), ) ] f = Findings() @@ -180,7 +199,12 @@ def test_absence_is_rejected_as_BLIND_when_the_positive_control_matches_nothing( (tmp_path / "messagefoundry").mkdir() (tmp_path / "messagefoundry" / "m.py").write_text("unrelated\n", encoding="utf-8") cells = [ - Cell(id="1.1.1", level=1, verdict="fail", absence=(Absence("clamav", "ScanRejected"),)) + Cell( + id="1.1.1", + level=1, + verdict="fail", + absence=(Absence("clamav", "ScanRejected", "import clamav"),), + ) ] f = Findings() check_absences(cells, tmp_path, f) @@ -193,12 +217,71 @@ def test_absence_is_rejected_as_FALSE_when_the_thing_now_exists(tmp_path: Path) (tmp_path / "messagefoundry" / "m.py").write_text( "import clamd\nclass ScanRejected: pass\n", encoding="utf-8" ) - cells = [Cell(id="1.1.1", level=1, verdict="fail", absence=(Absence("clamd", "ScanRejected"),))] + cells = [ + Cell( + id="1.1.1", + level=1, + verdict="fail", + absence=(Absence("clamd", "ScanRejected", "import clamd"),), + ) + ] f = Findings() check_absences(cells, tmp_path, f) assert not f.ok and "FALSE" in f.problems[0] +def test_absence_is_rejected_as_INERT_when_the_pattern_is_prose_not_a_pattern( + tmp_path: Path, +) -> None: + """The real defect: an entire chapter was authored as narrations of shell commands. + + The field's type is ``str`` and prose is a valid ``str``, so the shape permitted it. Prose greps + to nothing, which is indistinguishable from a true absence -- nine claims would have shipped + proving nothing. Requiring the pattern to fire on a stated reintroduction makes prose unwritable. + """ + (tmp_path / "messagefoundry").mkdir() + (tmp_path / "messagefoundry" / "m.py").write_text( + "class ScanRejected: pass\n", encoding="utf-8" + ) + prose = "rg -n 'tar.extractall' messagefoundry/ -> exit 1 (zero hits)" + cells = [ + Cell( + id="1.1.1", + level=1, + verdict="fail", + absence=(Absence(prose, "ScanRejected", "tar.extractall(dest)"),), + ) + ] + f = Findings() + check_absences(cells, tmp_path, f) + assert not f.ok and "INERT" in f.problems[0] + + +def test_absence_INERT_is_decided_before_the_corpus_is_consulted(tmp_path: Path) -> None: + """A live positive control must not launder a pattern that cannot fire. + + BLIND and INERT are different failures: BLIND means the search could not have seen the thing, + INERT means the pattern could not have matched it. A prose claim whose control happens to speak + would otherwise pass every existing check while measuring nothing. + """ + (tmp_path / "messagefoundry").mkdir() + (tmp_path / "messagefoundry" / "m.py").write_text( + "class ScanRejected: pass\n", encoding="utf-8" + ) + cells = [ + Cell( + id="1.1.1", + level=1, + verdict="fail", + absence=(Absence("zero hits for pyclamd", "ScanRejected", "import pyclamd"),), + ) + ] + f = Findings() + check_absences(cells, tmp_path, f) + assert not f.ok and "INERT" in f.problems[0] + assert not any("BLIND" in p for p in f.problems) + + # --- fail closed, never skip ---------------------------------------------------------------------- @@ -255,6 +338,10 @@ def test_verify_end_to_end_clean(tmp_path: Path) -> None: level = 3 verdict = "partial" residual = "ships off" +[[cell.evidence]] +path = "messagefoundry/m.py" +line = 1 +expect = "tls_cert_file" """, ) findings = verify(sc, corpus, tmp_path) @@ -332,3 +419,139 @@ def test_pinning_passes_when_version_and_digest_are_declared(tmp_path: Path) -> f'[scorecard]\nasvs_version = "5.0.0"\ncorpus_sha256 = "{corpus_digest(corpus)}"\n', ) assert check_pinning(sc, corpus) == [] + + +def test_pinning_refuses_an_abbreviated_anchor_sha(tmp_path: Path) -> None: + """actions/checkout resolves a short ref as a BRANCH OR TAG name, so an abbreviated anchor fails + with "A branch or tag with the name ... could not be found" -- a gate failing for a reason with + nothing to do with what it measures. Caught in CI twice before it was refused here.""" + corpus = _corpus_file(tmp_path) + sc = _scorecard_file( + tmp_path, + '[scorecard]\nasvs_version = "5.0.0"\nanchor_commit = "8f01cef8"\n' + f'corpus_sha256 = "{corpus_digest(corpus)}"\n', + ) + assert any("must be a FULL 40-character SHA" in p for p in check_pinning(sc, corpus)) + + +def test_pinning_accepts_a_full_anchor_sha(tmp_path: Path) -> None: + corpus = _corpus_file(tmp_path) + sc = _scorecard_file( + tmp_path, + '[scorecard]\nasvs_version = "5.0.0"\n' + 'anchor_commit = "28d186b5d85b10c8e0ce3fc35adc01b7269bcb28"\n' + f'corpus_sha256 = "{corpus_digest(corpus)}"\n', + ) + assert check_pinning(sc, corpus) == [] + + +def test_load_refuses_an_absence_claim_with_no_mutation(tmp_path: Path) -> None: + """Authored, never inferred: a mutation derived from its pattern would pass by construction.""" + sc = _scorecard_file( + tmp_path, + '[[cell]]\nid = "1.1.1"\nlevel = 1\nverdict = "fail"\n' + ' [[cell.absence]]\n pattern = "clamd"\n' + ' positive_control = "ScanRejected"\n', + ) + with pytest.raises(ScorecardError, match="has no `mutation`"): + load_scorecard(sc) + + +def test_the_module_does_not_contaminate_the_corpus_it_scans() -> None: + """scorecard.py lives INSIDE the corpus that absence patterns are searched over. + + A literal code example in this module therefore becomes a real corpus hit. The first draft of + the `mutation` guidance carried one, and it broke two live absence claims (5.2.5, 5.3.3) the + moment they were backfilled -- the documentation OF a check contaminated the check. + + This is an INSTANCE lock, not a class lock. It pins the one literal that has actually bitten; + the general rule -- write examples in escaped-regex form, which cannot self-match -- is stated + in the Absence docstring and is not mechanically enforceable. + """ + import scripts.asvs.scorecard as mod + + src = Path(mod.__file__).read_text(encoding="utf-8") + for literal in (".extractall(", "unpack_archive("): + assert literal not in src, ( + f"scorecard.py contains the literal {literal!r}. This module is inside the scanned " + "corpus, so that literal is a corpus hit and will read as FALSE for any absence claim " + "excluding it. Write the example in escaped-regex form instead." + ) + + +def test_anchor_is_rejected_as_AMBIGUOUS_when_the_token_is_not_unique(tmp_path: Path) -> None: + """A token occurring many times resolves from almost anywhere, so it certifies nothing. + + Measured on the real scorecard: 46 of 292 anchors (15%) had a non-unique token, and the worst + occurred 101 times in one file -- meaning ANY line number in that file landed within the window. + Those were not anchors at risk of going hollow; they were already hollow, and passing. + """ + (tmp_path / "messagefoundry").mkdir() + (tmp_path / "messagefoundry" / "m.py").write_text( + "await conn.rollback()" + chr(10) + "x = 1" + chr(10) + "await conn.rollback()" + chr(10), + encoding="utf-8", + ) + cells = [ + Cell( + id="1.1.1", + level=1, + verdict="pass", + evidence=(Anchor("messagefoundry/m.py", 1, "await conn.rollback()"),), + ) + ] + f = Findings() + check_anchors(cells, tmp_path, f) + assert not f.ok and "AMBIGUOUS" in f.problems[0] and "occurs 2 times" in f.problems[0] + + +def test_anchor_holds_when_the_token_is_unique(tmp_path: Path) -> None: + """The uniqueness rule must not reject a legitimate anchor -- proved alongside its negative.""" + (tmp_path / "messagefoundry").mkdir() + (tmp_path / "messagefoundry" / "m.py").write_text( + "a" + chr(10) + "tls_cert_file = None" + chr(10) + "b" + chr(10), encoding="utf-8" + ) + cells = [ + Cell( + id="1.1.1", + level=1, + verdict="pass", + evidence=(Anchor("messagefoundry/m.py", 2, "tls_cert_file"),), + ) + ] + f = Findings() + check_anchors(cells, tmp_path, f) + assert f.ok and f.checked_anchors == 1 + + +def test_completeness_catches_a_decided_verdict_with_no_evidence_at_all() -> None: + """The gate could only validate evidence that EXISTED, never assert that it must. + + check_anchors iterates the anchors a cell has, so a cell with none is skipped and fails nothing. + Measured when this landed: 14 of 59 decided cells carried zero anchors and zero absence claims -- + verdicts inherited from the prose lineage that were reached but never anchored. That is precisely + the guess-wearing-a-verdict conflation this tool exists to prevent. + """ + cells = [ + Cell(id="1.1.1", level=1, verdict="pass", evidence=(Anchor("m.py", 1, "x"),)), + Cell(id="1.1.2", level=2, verdict="partial"), # no anchor, no absence + Cell(id="2.1.1", level=3, verdict="unverified"), # unverified needs none + ] + problems = check_completeness(cells, CORPUS) + hit = [p for p in problems if "carry NO anchor" in p] + assert len(hit) == 1, problems + assert "1.1.2" in hit[0] and "1.1.1" not in hit[0] and "2.1.1" not in hit[0] + + +def test_completeness_accepts_a_decided_cell_evidenced_only_by_an_absence_claim() -> None: + """A `fail` is often proved by absence, not presence -- the rule must not demand a presence anchor.""" + cells = [ + Cell(id="1.1.1", level=1, verdict="pass", evidence=(Anchor("m.py", 1, "x"),)), + Cell( + id="1.1.2", + level=2, + verdict="fail", + absence=(Absence("clamd", "ScanRejected", "import clamd"),), + ), + Cell(id="2.1.1", level=3, verdict="unverified"), + ] + assert not [p for p in check_completeness(cells, CORPUS) if "carry NO anchor" in p]