From 808b7f1c381cf55aa657cdd8127b821ef5a45a59 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 9 Aug 2026 00:18:56 -0500 Subject: [PATCH 1/3] feat(asvs): promote the scorecard writer to scripts/asvs/apply.py with tests P0 of the ASVS tracking rework. The only tool that WRITES the record of record lived at docs/security/asvs-apply-cells.py in the vault: hardcoded absolute path, no argparse, zero tests, zero references, and outside the CI path filter. THE OLD LOCATION WAS WORSE THAN UNTESTED. ci.yml's docs-only detector treats ^docs/ as non-code, so a PR touching ONLY the writer set code=false and skipped install, lint, type-check and the entire pytest suite. The tool that can silently un-close an owner-closed cell was classified as documentation. Verified by running the workflow's own regex against all three paths. The hardcoded path pointed at the SHARED vault checkout, which several sessions edit at once, so running it from a worktree rewrote a record the operator was not looking at -- I hit that this session and worked around it with a patched copy. --scorecard is now REQUIRED with no default: the one thing a writer must never guess is which record it is rewriting. BOTH SHIPPED INVARIANTS KEPT VERBATIM, as directed, and now proved rather than asserted: - The non-allowlist. render() enumerates only what it ORDERS; every other key on the live cell survives by default. Test: a payload omitting decision_closed and decision_closed_by leaves both intact. That is the 7818991d incident, where an ALLOWLIST silently un-closed two owner-closed cells while every gate stayed green, because an absent decision_closed is a valid False. - The set(was) - set(now) backstop, MUTATION-PROVED: render is replaced with one that drops the decision_* keys, reproducing the historical defect in the one function that could reintroduce it, and the write must be refused. The test asserts the refusal names those keys, because a non-zero exit is not evidence -- several guards return 1, and a mutation proof that trips an unrelated one proves nothing about the invariant it claims to test. Also proved to fire: anchor_repair byte-identity on prose and verdict, the owner-closed rescore refusal, the glyph fail-closed check (which fired for real on 13.3.4 this session), unknown-cell refusal, and dry-run-by-default. Two mypy errors were sitting in this file and are fixed here -- it had never been type-checked, because of the path filter above. Not in scope: deleting the vault copy. That is the two-repo consolidation and it needs the mirror settled first. --- scripts/asvs/apply.py | 269 +++++++++++++++++++++++++++++++++++ tests/test_asvs_apply.py | 300 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 569 insertions(+) create mode 100644 scripts/asvs/apply.py create mode 100644 tests/test_asvs_apply.py diff --git a/scripts/asvs/apply.py b/scripts/asvs/apply.py new file mode 100644 index 00000000..36f70ef2 --- /dev/null +++ b/scripts/asvs/apply.py @@ -0,0 +1,269 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Apply re-verified ASVS cells into the scorecard TOML, replacing whole [[cell]] blocks. + +Rewrites only the named cells and leaves every other byte of the file alone, because the vault +working tree is shared and a whole-file re-emit would silently reformat another session's work. + +Input JSON: [ {id, level, verdict, residual, evidence:[{path,line,expect}], + absence:[{pattern,positive_control,mutation}]}, ... ] +""" + +from __future__ import annotations + +import argparse +import json +import re +import tomllib +from pathlib import Path +from typing import Any + +VERDICTS = {"pass", "partial", "fail", "na", "needs-review", "unverified"} + +#: The banner alphabet and the general emoji planes. CLAUDE.md section 11 bans these in prose; the +#: only sanctioned holdout is docs/BACKLOG.md, which this file is not. Fail closed rather than +#: writing one into a security record where a later reader would copy the vocabulary forward. +_BANNED = re.compile( + "[" + "\u26a0\u26d4\u2705\u2b50\u274c\u2714\u2716\u2717\u2718" # warning, no-entry, check, star, crosses + "\U0001f300-\U0001faff" # emoji planes + "\U0001f000-\U0001f2ff" + "\u2190-\u21ff" # arrows + "\u2022" # bullet + "\ufe0f\ufe0e" # variation selectors + "]" +) + + +def toml_str(s: str) -> str: + """A TOML basic string. JSON escaping is a strict subset of TOML's, so json.dumps is safe.""" + return json.dumps(s, ensure_ascii=False) + + +#: Scalar keys this writer knows how to emit. ANY OTHER scalar key found on the live cell is carried +#: through verbatim rather than dropped. +#: +#: This list was an ALLOWLIST once, and it silently deleted `decision_closed`, `decision_closed_verdict`, +#: `decision_closed_on` and `decision_closed_by` from the two owner-closed cells during an anchor +#: repair -- un-closing them. The gate passed, because an absent `decision_closed` is a valid False. +#: A green gate cannot distinguish PRESERVED from DROPPED, so the writer must never enumerate what it +#: keeps; it enumerates only what it ORDERS, and everything else survives by default. +_ORDERED = ("id", "level", "verdict", "residual", "last_verified", "verified_at", "reviewed_by") + +#: Every field that can carry free text. anchor_repair must hold ALL of these byte-identical, not just +#: the one the glyph check reads -- otherwise the exemption is a bypass with a narrow mouth. +_PROSE_FIELDS = ( + "residual", + "reviewed_by", + "decision_closed_by", + "decision_reopen_requires", + "decision_permits_without_owner", +) +_SUBTABLES = ("evidence", "absence") + + +def _scalar(key: str, value: object) -> str: + if isinstance(value, bool): + return f"{key} = {str(value).lower()}" + if isinstance(value, int): + return f"{key} = {value}" + return f"{key} = {toml_str(str(value))}" + + +def render(cell: dict[str, Any], live: dict[str, Any] | None = None) -> str: + out = ["[[cell]]", f'id = "{cell["id"]}"', f"level = {int(cell['level'])}"] + out.append(f'verdict = "{cell["verdict"]}"') + if cell.get("residual"): + out.append(f"residual = {toml_str(cell['residual'])}") + out.append(f'last_verified = "{cell["last_verified"]}"') + out.append(f'verified_at = "{cell["verified_at"]}"') + if cell.get("reviewed_by"): + out.append(f"reviewed_by = {toml_str(cell['reviewed_by'])}") + # Carry through every other scalar the live cell had -- decision_closed and friends, and anything + # a future schema adds that this writer has never heard of. + for key, value in (live or {}).items(): + if key in _ORDERED or key in _SUBTABLES or key in cell: + continue + out.append(_scalar(key, value)) + for a in cell.get("evidence") or []: + out.append(" [[cell.evidence]]") + out.append(f" path = {toml_str(a['path'])}") + out.append(f" line = {int(a['line'])}") + out.append(f" expect = {toml_str(a['expect'])}") + for a in cell.get("absence") or []: + out.append(" [[cell.absence]]") + out.append(f" pattern = {toml_str(a['pattern'])}") + out.append(f" positive_control = {toml_str(a['positive_control'])}") + out.append(f" mutation = {toml_str(a['mutation'])}") + return "\n".join(out) + "\n" + + +def block_spans(text: str) -> dict[str, tuple[int, int]]: + """Map cell id -> (start, end) character offsets of its whole top-level [[cell]] block.""" + starts = [m.start() for m in re.finditer(r"^\[\[cell\]\]$", text, re.M)] + spans: dict[str, tuple[int, int]] = {} + for i, s in enumerate(starts): + e = starts[i + 1] if i + 1 < len(starts) else len(text) + m = re.search(r'^id = "([^"]+)"$', text[s:e], re.M) + if not m: + raise SystemExit(f"a [[cell]] block at offset {s} has no id") + spans[m.group(1)] = (s, e) + return spans + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser( + description="Apply re-verified ASVS cells into the scorecard TOML (ADR 0156).", + ) + ap.add_argument("payload", type=Path, help="JSON array of cells to write") + # REQUIRED, and deliberately not defaulted. This was a hardcoded absolute path into the SHARED + # vault checkout -- a tree several sessions edit at once -- so running the writer from a worktree + # silently rewrote a record the operator was not looking at. A default here would restore that + # failure with a nicer spelling: the one thing a writer must never guess is WHICH record it is + # rewriting. + ap.add_argument("--scorecard", type=Path, required=True, help="path to asvs-scorecard.toml") + ap.add_argument( + "--apply", + action="store_true", + help="write. Omitted, the run is a dry run and the file is not touched.", + ) + args = ap.parse_args(argv) + SCORECARD = args.scorecard + payload = json.loads(args.payload.read_text(encoding="utf-8")) + dry = not args.apply + + live_text = SCORECARD.read_text(encoding="utf-8") + live_cells = {x["id"]: x for x in tomllib.loads(live_text)["cell"]} + + problems: list[str] = [] + for c in payload: + live = live_cells.get(c.get("id"), {}) + # An ANCHOR REPAIR re-points citations after the code moved; it must not touch anything else. + # Declaring it lets two guards relax in a way that is strictly more conservative than the + # alternative: the residual passes through BYTE-IDENTICAL, so no retired glyph can enter the + # record that was not already in it, and an existing empty `reviewed_by` is preserved rather + # than invented. Any difference in verdict or residual takes it out of this mode immediately. + anchor_repair = bool(c.get("anchor_repair")) + if anchor_repair: + # Assert byte-identity on EVERY prose-bearing field, not just the two the glyph check + # reads. Holding only verdict+residual was sound by argument -- the writer never rewrites + # the others -- but an argument is worth less than a check, and it left the next reader to + # reconstruct why two were sufficient. + for f in _PROSE_FIELDS: + if c.get(f, live.get(f, "")) != live.get(f, ""): + problems.append( + f"{c.get('id')}: declared anchor_repair but {f!r} differs from the record; " + "that is a rescore, not a repair" + ) + if c.get("verdict") != live.get("verdict"): + problems.append( + f"{c.get('id')}: declared anchor_repair but the verdict differs from the " + "record; that is a rescore, not a repair" + ) + required: tuple[str, ...] = ("id", "level", "verdict", "last_verified", "verified_at") + if not anchor_repair: + required = required + ("reviewed_by",) + for field in required: + if not c.get(field) and c.get(field) != 0: + problems.append(f"{c.get('id')}: missing {field}") + if c.get("verdict") not in VERDICTS: + problems.append(f"{c.get('id')}: bad verdict {c.get('verdict')!r}") + if c.get("verdict") == "na" and not (c.get("residual") or "").strip(): + problems.append(f"{c['id']}: verdict 'na' requires a written rationale in residual") + if c.get("verdict") in {"pass", "partial", "fail"} and not ( + c.get("evidence") or c.get("absence") + ): + problems.append(f"{c['id']}: {c['verdict']} needs at least one anchor or absence claim") + blob = "" if anchor_repair else " ".join(str(v) for v in (c.get("residual", ""),)) + hit = _BANNED.search(blob) + if hit: + # Report the codepoint, never the character: echoing it to a cp1252 console raises + # UnicodeEncodeError and the refusal turns into a traceback that hides its own reason. + problems.append( + f"{c['id']}: residual contains a banned glyph U+{ord(hit.group()):04X} " + f"at offset {hit.start()}" + ) + if problems: + print("REFUSING TO APPLY:") + for p in problems: + print(" " + p) + return 1 + + text = SCORECARD.read_text(encoding="utf-8") + spans = block_spans(text) + + edits = [] + for c in payload: + if c["id"] not in spans: + print(f"REFUSING: cell {c['id']} not present in the scorecard") + return 1 + s, e = spans[c["id"]] + old = text[s:e] + if "decision_closed = true" in old: + # The method permits exactly ONE change to a closed cell without the owner: repairing a + # broken evidence anchor, re-anchored by content. So allow it only when the verdict and + # the residual are byte-identical to what is already recorded -- i.e. anchors only. + import tomllib as _t + + live = {x["id"]: x for x in _t.loads(text)["cell"]}[c["id"]] + if c["verdict"] != live["verdict"] or c.get("residual", "") != live.get("residual", ""): + print( + f"REFUSING: cell {c['id']} is decision_closed and this edit changes its " + "verdict or residual; only an anchor repair is permitted without the owner" + ) + return 1 + print( + f" note: {c['id']} is decision_closed - anchor-only repair, verdict and residual unchanged" + ) + edits.append((s, e, render(c, live_cells.get(c["id"], {})), old)) + + new_text = text + for s, e, rendered, _old in sorted(edits, key=lambda t: -t[0]): + new_text = new_text[:s] + rendered + new_text[e:] + + # Parse before writing: a scorecard that does not load is worse than one not updated. + parsed = tomllib.loads(new_text) + by_id = {c["id"]: c for c in parsed["cell"]} + for c in payload: + got = by_id[c["id"]]["verdict"] + if got != c["verdict"]: + print(f"REFUSING: round-trip mismatch on {c['id']}: {got!r} != {c['verdict']!r}") + return 1 + if len(parsed["cell"]) != len(spans): + print(f"REFUSING: cell count changed {len(spans)} -> {len(parsed['cell'])}") + return 1 + + # FIELD-PRESERVATION INVARIANT. A rewrite must never silently DROP a key, and the anchor gate + # cannot see that: an absent `decision_closed` is a valid False, so un-closing an owner-closed + # cell reads as green. Assert cardinality too - a repair that deletes working anchors also passes + # a resolution check, because fewer anchors that all resolve is a passing state. + for c in payload: + was, now = live_cells[c["id"]], by_id[c["id"]] + lost = set(was) - set(now) + if lost: + print(f"REFUSING: cell {c['id']} would LOSE field(s) {sorted(lost)}") + return 1 + for sub in ("evidence", "absence"): + if len(now.get(sub, [])) < len(was.get(sub, [])): + print( + f"REFUSING: cell {c['id']} {sub} count would DROP " + f"{len(was.get(sub, []))} -> {len(now.get(sub, []))}" + ) + return 1 + + print(f"{len(edits)} cell blocks re-rendered; file parses; {len(parsed['cell'])} cells intact") + for c in payload: + print( + f" {c['id']:<8} -> {c['verdict']:<12} " + f"({len(c.get('evidence') or [])} anchors, {len(c.get('absence') or [])} absence)" + ) + if dry: + print("\nDRY RUN. Re-run with --apply to write.") + return 0 + SCORECARD.write_text(new_text, encoding="utf-8", newline="") + print(f"\nWROTE {SCORECARD}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_asvs_apply.py b/tests/test_asvs_apply.py new file mode 100644 index 00000000..28ee2d7a --- /dev/null +++ b/tests/test_asvs_apply.py @@ -0,0 +1,300 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The ASVS scorecard WRITER (ADR 0156) — every refusal proved to fire. + +This file had none. It lived at `docs/security/asvs-apply-cells.py` in the vault, outside the +`scripts/asvs/**` CI path filter, with a hardcoded absolute path and zero tests — while being the only +thing that writes the record of record. Its guards were sound and entirely unverified, which is the +combination that lets a guard rot silently. + +**The tests that matter here are the ones asserting a REFUSAL, and each is mutation-proved: the guard +is removed and the test must go red.** A refusal nobody has watched fire is indistinguishable from a +refusal that cannot. +""" + +from __future__ import annotations + +import json +import tomllib +from pathlib import Path + +import pytest + +from scripts.asvs.apply import main + +#: A two-cell record. `5.4.3` is owner-CLOSED, mirroring the real one, because the closed-cell guards +#: are the ones with the worst failure mode: an un-closing is invisible to every downstream check. +FIXTURE = """[scorecard] +asvs_version = "5.0.0" + +[[cell]] +id = "1.1.1" +level = 1 +verdict = "partial" +residual = "a control exists but ships off" +last_verified = "2026-08-09" +verified_at = "1111111111111111111111111111111111111111" +reviewed_by = "fixture" + [[cell.evidence]] + path = "messagefoundry/m.py" + line = 10 + expect = "tls_cert_file" + [[cell.evidence]] + path = "messagefoundry/m.py" + line = 20 + expect = "verify_mode" +[[cell]] +id = "5.4.3" +level = 2 +verdict = "na" +residual = "enterprise-provided control, outside the declared scope" +last_verified = "2026-08-02" +verified_at = "2222222222222222222222222222222222222222" +reviewed_by = "owner" +decision_closed = true +decision_closed_by = "owner" + [[cell.evidence]] + path = "messagefoundry/m.py" + line = 30 + expect = "_no_scan" +""" + + +def _record(tmp_path: Path) -> Path: + p = tmp_path / "asvs-scorecard.toml" + p.write_text(FIXTURE, encoding="utf-8") + return p + + +def _payload(tmp_path: Path, cells: list[dict]) -> Path: + p = tmp_path / "payload.json" + p.write_text(json.dumps(cells), encoding="utf-8") + return p + + +def _cell_111(**over: object) -> dict: + base: dict = { + "id": "1.1.1", + "level": 1, + "verdict": "partial", + "residual": "a control exists but ships off", + "last_verified": "2026-08-09", + "verified_at": "3333333333333333333333333333333333333333", + "reviewed_by": "test", + "evidence": [ + {"path": "messagefoundry/m.py", "line": 11, "expect": "tls_cert_file"}, + {"path": "messagefoundry/m.py", "line": 21, "expect": "verify_mode"}, + ], + } + base.update(over) + return base + + +# --- the happy path, so the refusals below are not passing vacuously ------------------------------ + + +def test_a_dry_run_does_not_touch_the_file(tmp_path: Path) -> None: + """DEFAULT IS DRY. The writer that rewrites the security record must not do so by accident.""" + rec = _record(tmp_path) + before = rec.read_bytes() + rc = main([str(_payload(tmp_path, [_cell_111()])), "--scorecard", str(rec)]) + assert rc == 0 + assert rec.read_bytes() == before + + +def test_apply_rewrites_only_the_named_cell(tmp_path: Path) -> None: + rec = _record(tmp_path) + rc = main( + [ + str(_payload(tmp_path, [_cell_111(residual="rewritten")])), + "--scorecard", + str(rec), + "--apply", + ] + ) + assert rc == 0 + got = {c["id"]: c for c in tomllib.loads(rec.read_text(encoding="utf-8"))["cell"]} + assert got["1.1.1"]["residual"] == "rewritten" + # The untouched cell keeps every byte of its metadata, including the closure keys. + assert got["5.4.3"]["decision_closed"] is True + assert got["5.4.3"]["decision_closed_by"] == "owner" + + +# --- REFUSALS. each of these is the guard the writer exists for ------------------------------------ + + +def _naked_543() -> dict: + """A payload for the owner-closed cell that OMITS every closure key. + + This is byte-for-byte the shape the pre-`7818991d` writer emitted, and the shape any caller + produces who did not know the keys existed -- which is the realistic case, since nothing in the + payload schema mentions them. + """ + return { + "id": "5.4.3", + "level": 2, + "verdict": "na", + "residual": "enterprise-provided control, outside the declared scope", + "last_verified": "2026-08-09", + "verified_at": "4444444444444444444444444444444444444444", + "reviewed_by": "test", + "evidence": [{"path": "messagefoundry/m.py", "line": 30, "expect": "_no_scan"}], + } + + +def test_omitted_keys_are_carried_through_rather_than_dropped(tmp_path: Path) -> None: + """THE 7818991d INCIDENT, and the design that answers it. + + An earlier writer enumerated the keys it kept as an ALLOWLIST, so `decision_closed`, + `decision_closed_by` and friends were silently deleted from the two owner-closed cells during an + anchor repair -- un-closing them. Every downstream check stayed green, because an ABSENT + `decision_closed` is a valid False, and a gate cannot distinguish PRESERVED from DROPPED. + + The fix is structural rather than a check: the writer enumerates only what it ORDERS, and every + other key on the live cell survives by default. So the payload below omits the closure keys and + they are still there afterwards. This asserts the PRESERVATION, which is the property that makes + the record safe; the next test proves the backstop that fires if this ever breaks. + """ + rec = _record(tmp_path) + rc = main([str(_payload(tmp_path, [_naked_543()])), "--scorecard", str(rec), "--apply"]) + assert rc == 0 + got = {c["id"]: c for c in tomllib.loads(rec.read_text(encoding="utf-8"))["cell"]}["5.4.3"] + assert got["decision_closed"] is True + assert got["decision_closed_by"] == "owner" + + +def test_the_preservation_backstop_fires_when_carry_through_is_broken( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """MUTATION PROOF of the `set(was) - set(now)` invariant. + + The test above proves the carry-through works TODAY. This proves the record is still defended if + someone breaks it -- by breaking it. `render` is replaced with one that drops exactly the keys + `7818991d` dropped, reproducing the historical defect in the one function that could reintroduce + it, and the write must be REFUSED. + + Without this, the preservation invariant is a line of code nobody has watched work, guarding + against a defect that has already happened once. + """ + import scripts.asvs.apply as mod + + real_render = mod.render + + def dropping_render(cell: dict, live: dict | None = None) -> str: + stripped = {k: v for k, v in (live or {}).items() if not k.startswith("decision_")} + return real_render(cell, stripped) + + monkeypatch.setattr(mod, "render", dropping_render) + rec = _record(tmp_path) + before = rec.read_bytes() + rc = main([str(_payload(tmp_path, [_naked_543()])), "--scorecard", str(rec), "--apply"]) + assert rc == 1, "the writer dropped decision_* and the preservation invariant did not fire" + assert rec.read_bytes() == before, "refused, but wrote anyway" + # It must refuse for THIS reason. A non-zero exit is not evidence on its own -- several other + # guards in this writer also return 1, and a mutation proof that passes because it tripped an + # unrelated check proves nothing about the invariant it claims to be testing. + out = capsys.readouterr().out + assert "would LOSE field(s)" in out + assert "decision_closed" in out and "decision_closed_by" in out + + +def test_it_refuses_to_shrink_the_evidence_list(tmp_path: Path) -> None: + """Fewer anchors that all resolve is a PASSING state for the verifier. + + So anchor-count loss is invisible downstream exactly like field loss, and for the same reason: the + reader of a green gate cannot tell a repair from a deletion. + """ + rec = _record(tmp_path) + one_anchor = _cell_111( + evidence=[{"path": "messagefoundry/m.py", "line": 11, "expect": "tls_cert_file"}] + ) + rc = main([str(_payload(tmp_path, [one_anchor])), "--scorecard", str(rec), "--apply"]) + assert rc == 1 + + +def test_anchor_repair_refuses_a_residual_edit(tmp_path: Path) -> None: + """`anchor_repair` relaxes the glyph and reviewed_by guards, so it must buy that with byte-identity. + + Otherwise the exemption is a bypass with a narrow mouth: declare a repair, edit the prose, and the + checks that exist to police prose have been told not to look. + """ + rec = _record(tmp_path) + sneaky = _cell_111(anchor_repair=True, residual="quietly different") + rc = main([str(_payload(tmp_path, [sneaky])), "--scorecard", str(rec), "--apply"]) + assert rc == 1 + + +def test_anchor_repair_refuses_a_verdict_move(tmp_path: Path) -> None: + rec = _record(tmp_path) + rc = main( + [ + str(_payload(tmp_path, [_cell_111(anchor_repair=True, verdict="pass")])), + "--scorecard", + str(rec), + "--apply", + ] + ) + assert rc == 1 + + +def test_it_refuses_to_rescore_an_owner_closed_cell(tmp_path: Path) -> None: + """The method permits exactly ONE change to a closed cell without the owner: an anchor repair.""" + rec = _record(tmp_path) + reopened = { + "id": "5.4.3", + "level": 2, + "verdict": "fail", # the move the closure exists to prevent + "residual": "enterprise-provided control, outside the declared scope", + "last_verified": "2026-08-09", + "verified_at": "5555555555555555555555555555555555555555", + "reviewed_by": "test", + "decision_closed": True, + "decision_closed_by": "owner", + "evidence": [{"path": "messagefoundry/m.py", "line": 30, "expect": "_no_scan"}], + } + rc = main([str(_payload(tmp_path, [reopened])), "--scorecard", str(rec), "--apply"]) + assert rc == 1 + got = {c["id"]: c for c in tomllib.loads(rec.read_text(encoding="utf-8"))["cell"]} + assert got["5.4.3"]["verdict"] == "na" + + +def test_it_refuses_a_glyph_in_a_residual(tmp_path: Path) -> None: + """CLAUDE.md section 11, enforced against the record itself. + + This fired for real on 13.3.4, whose carried residual was full of banner glyphs: the cell could + not be rewritten until they were converted to words. The check reports the CODEPOINT rather than + echoing the character, because echoing it to a cp1252 console raises UnicodeEncodeError and the + refusal turns into a traceback that hides its own reason. + """ + rec = _record(tmp_path) + rc = main( + [ + str(_payload(tmp_path, [_cell_111(residual="WARNING ⛔ do not")])), + "--scorecard", + str(rec), + "--apply", + ] + ) + assert rc == 1 + + +def test_it_refuses_a_cell_that_is_not_in_the_record(tmp_path: Path) -> None: + rec = _record(tmp_path) + rc = main( + [str(_payload(tmp_path, [_cell_111(id="9.9.9")])), "--scorecard", str(rec), "--apply"] + ) + assert rc == 1 + + +# --- the CLI contract ------------------------------------------------------------------------------ + + +def test_scorecard_path_is_required_and_has_no_default(tmp_path: Path) -> None: + """It used to be a hardcoded absolute path into the SHARED vault checkout. + + Several sessions edit that tree at once, so running the writer from a worktree rewrote a record the + operator was not looking at. A default would restore that with a nicer spelling. + """ + with pytest.raises(SystemExit) as e: + main([str(_payload(tmp_path, [_cell_111()]))]) + assert e.value.code == 2 # argparse usage error, not a silent fallback From e57433289196188bdd4b2981702cbb1cf9f771a2 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 9 Aug 2026 00:22:33 -0500 Subject: [PATCH 2/3] feat(asvs): refuse a verdict move unless it is declared The one refusal in this writer against a WELL-FORMED payload. Every other guard rejects malformed input; this rejects input that is valid and means more than its author intended -- a verdict moving during a pass whose stated purpose was mechanical (an anchor repair, a re-render, a bulk transform). That is this writer's whole failure mode, so the safe thing is now the default and the dangerous thing is explicit: --allow-verdict-change. The refusal names the cell and BOTH verdicts, per review. A refusal that says only 'verdict changed' leaves the operator's actual next question -- which cell, and to what -- unanswered, and an unanswerable refusal gets re-run with the override reflexively, which converts the guard into a speed bump. BOTH HALVES TESTED, because the first commit of this guard passed all 11 existing tests while nothing exercised it. Adding a guard no test drives is how a guard that cannot fire ships looking green. So: the move is refused and the message names 1.1.1 and 'partial' -> 'pass'; AND the flag actually lifts it. Without the second, the flag could be misspelled, unwired or shadowed and the refusal test would still pass. --- scripts/asvs/apply.py | 26 +++++++++++++++++++++++ tests/test_asvs_apply.py | 45 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/scripts/asvs/apply.py b/scripts/asvs/apply.py index 36f70ef2..b43b85a2 100644 --- a/scripts/asvs/apply.py +++ b/scripts/asvs/apply.py @@ -127,7 +127,17 @@ def main(argv: list[str] | None = None) -> int: action="store_true", help="write. Omitted, the run is a dry run and the file is not touched.", ) + ap.add_argument( + "--allow-verdict-change", + action="store_true", + help=( + "permit a payload to move a cell's verdict. Refused by default: a verdict move is an " + "assessor decision, and this writer's failure mode is making one during a pass whose " + "stated purpose was mechanical." + ), + ) args = ap.parse_args(argv) + allow_verdict_change = args.allow_verdict_change SCORECARD = args.scorecard payload = json.loads(args.payload.read_text(encoding="utf-8")) dry = not args.apply @@ -168,6 +178,22 @@ def main(argv: list[str] | None = None) -> int: problems.append(f"{c.get('id')}: missing {field}") if c.get("verdict") not in VERDICTS: problems.append(f"{c.get('id')}: bad verdict {c.get('verdict')!r}") + # A VERDICT MOVE IS AN ASSESSOR ACT AND MUST BE DECLARED. This writer's whole failure mode is + # silent verdict movement during a pass whose stated purpose was mechanical: an anchor repair, + # a re-render, a bulk transform. Everything else here is a refusal against malformed input; + # this is the one refusal against a WELL-FORMED payload that means more than its author + # intended. So the safe thing is the default and the dangerous thing is explicit. + # + # The message names the cell and BOTH verdicts on purpose. A refusal that says only "verdict + # changed" leaves the operator's actual next question -- which cell, and to what -- unanswered, + # and an unanswerable refusal gets re-run with the override flag reflexively, which converts + # the guard into a speed bump. + if live and c.get("verdict") != live.get("verdict") and not allow_verdict_change: + problems.append( + f"{c['id']}: verdict would change {live.get('verdict')!r} -> {c.get('verdict')!r}. " + "That is an assessor decision, not a mechanical edit. Re-run with " + "--allow-verdict-change if you mean it" + ) if c.get("verdict") == "na" and not (c.get("residual") or "").strip(): problems.append(f"{c['id']}: verdict 'na' requires a written rationale in residual") if c.get("verdict") in {"pass", "partial", "fail"} and not ( diff --git a/tests/test_asvs_apply.py b/tests/test_asvs_apply.py index 28ee2d7a..64bd5353 100644 --- a/tests/test_asvs_apply.py +++ b/tests/test_asvs_apply.py @@ -286,6 +286,51 @@ def test_it_refuses_a_cell_that_is_not_in_the_record(tmp_path: Path) -> None: assert rc == 1 +def test_a_verdict_move_is_refused_by_default( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The one refusal here against a WELL-FORMED payload. + + Every other guard rejects malformed input. This one rejects input that is valid and means more + than its author intended -- a verdict moving during a pass whose stated purpose was mechanical. + That is this writer's whole failure mode, so the safe thing is the default. + """ + rec = _record(tmp_path) + before = rec.read_bytes() + rc = main( + [str(_payload(tmp_path, [_cell_111(verdict="pass")])), "--scorecard", str(rec), "--apply"] + ) + assert rc == 1 + assert rec.read_bytes() == before + out = capsys.readouterr().out + # It must answer the operator's actual next question -- WHICH cell, and TO WHAT. A refusal that + # says only "verdict changed" gets re-run with the override reflexively, which turns the guard + # into a speed bump. + assert "1.1.1" in out + assert "'partial' -> 'pass'" in out + + +def test_the_verdict_flag_actually_unlocks_the_move(tmp_path: Path) -> None: + """Guard-the-guard: a refusal that cannot be lifted is a bug, not a control. + + Without this, `--allow-verdict-change` could be misspelled, unwired, or shadowed and the test + above would still pass -- it only asserts the refusal. This asserts the other half. + """ + rec = _record(tmp_path) + rc = main( + [ + str(_payload(tmp_path, [_cell_111(verdict="pass")])), + "--scorecard", + str(rec), + "--apply", + "--allow-verdict-change", + ] + ) + assert rc == 0 + got = {c["id"]: c for c in tomllib.loads(rec.read_text(encoding="utf-8"))["cell"]} + assert got["1.1.1"]["verdict"] == "pass" + + # --- the CLI contract ------------------------------------------------------------------------------ From ef1401e5dc69fff4945bdf3facb6dd239b7a0fdc Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 9 Aug 2026 17:35:14 -0500 Subject: [PATCH 3/3] fix(asvs): collapse the banned-glyph emoji planes into one contiguous range CodeQL flagged the class as an overly permissive range: it analyses the character class in UTF-16, where 1f000-1f2ff and 1f300-1faff both decompose to ranges sharing a high surrogate, and reads that as an overlap. The two ranges were adjacent (0x1F2FF + 1 == 0x1F300), so their union is exactly 1f000-1faff. Proved rather than asserted: compared old against new over every one of the 1,112,064 non-surrogate codepoints -- zero behavioural differences, both matching the same 2,940 -- with the comparison itself guarded against passing degenerately. The seam is now a test. A later edit that re-splits the range and mistypes a bound, or truncates it, leaves a hole exactly where the two halves used to meet, and a hole in a FAIL-CLOSED guard goes red nowhere: nothing fails, a glyph simply starts getting written into the security record. Mutation-proved in both directions -- truncating the range trips the inside-bounds assertion, widening it trips the outside-bounds one. The first mutation run was a false green. A heredoc collapsed the double backslash, so the search string became the literal U+1F000 CHARACTER while the source carries the escape SEQUENCE; the replace matched nothing, the file was never mutated, and the test "passed" over unmodified code. The harness now refuses a no-op replacement and prints the before/after line, because a mutation test that cannot prove its mutation landed is not evidence. --- scripts/asvs/apply.py | 7 +++++-- tests/test_asvs_apply.py | 21 ++++++++++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/scripts/asvs/apply.py b/scripts/asvs/apply.py index b43b85a2..d4960afe 100644 --- a/scripts/asvs/apply.py +++ b/scripts/asvs/apply.py @@ -26,8 +26,11 @@ _BANNED = re.compile( "[" "\u26a0\u26d4\u2705\u2b50\u274c\u2714\u2716\u2717\u2718" # warning, no-entry, check, star, crosses - "\U0001f300-\U0001faff" # emoji planes - "\U0001f000-\U0001f2ff" + # ONE range, not the adjacent pair 1f000-1f2ff + 1f300-1faff it replaces. Those are contiguous, + # so the union is identical (asserted at the seam by + # test_the_banned_class_is_one_contiguous_emoji_range); splitting them read as an overlapping + # range to CodeQL, which analyses the class in UTF-16 where both halves share a high surrogate. + "\U0001f000-\U0001faff" # emoji planes "\u2190-\u21ff" # arrows "\u2022" # bullet "\ufe0f\ufe0e" # variation selectors diff --git a/tests/test_asvs_apply.py b/tests/test_asvs_apply.py index 64bd5353..7ee2ec61 100644 --- a/tests/test_asvs_apply.py +++ b/tests/test_asvs_apply.py @@ -20,7 +20,7 @@ import pytest -from scripts.asvs.apply import main +from scripts.asvs.apply import _BANNED, main #: A two-cell record. `5.4.3` is owner-CLOSED, mirroring the real one, because the closed-cell guards #: are the ones with the worst failure mode: an un-closing is invisible to every downstream check. @@ -278,6 +278,25 @@ def test_it_refuses_a_glyph_in_a_residual(tmp_path: Path) -> None: assert rc == 1 +def test_the_banned_class_is_one_contiguous_emoji_range() -> None: + """The emoji planes are ONE range, not the adjacent pair `1f000-1f2ff` + `1f300-1faff`. + + That pair was contiguous, so collapsing it is a pure refactor — CodeQL read the split as an + overlapping range because it analyses the class in UTF-16, where both halves share a high + surrogate. The rewrite is only safe while the seam stays covered, so the seam is what this + asserts: a later edit that re-splits the range and mistypes a bound, or truncates it, leaves a + hole exactly here. A hole in a FAIL-CLOSED guard is invisible — nothing goes red, a glyph simply + starts getting written into the security record, which is the failure this guard exists to stop. + + The outside-bounds assertions matter too: a range widened to `\\U0001f000-\\U0001ffff` would pass + every inside check while quietly banning codepoints nobody reviewed. + """ + for cp in (0x1F000, 0x1F2FF, 0x1F300, 0x1FAFF): # both ends, and both sides of the old seam + assert _BANNED.search(chr(cp)), f"U+{cp:04X} escaped the banned class" + for cp in (0x1EFFF, 0x1FB00): # immediately outside, both ends + assert not _BANNED.search(chr(cp)), f"U+{cp:04X} was banned but is outside the range" + + def test_it_refuses_a_cell_that_is_not_in_the_record(tmp_path: Path) -> None: rec = _record(tmp_path) rc = main(