From 6a9055176b7dec1592e8890e6c9b71ffc54ac764 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 14 Aug 2026 17:39:40 -0500 Subject: [PATCH 1/2] fix(asvs): compare the type the PAYLOAD stated, not the keys it did not mention (BACKLOG #1242) RETRACTING MY OWN SCOPING, on someone else's measurement. I wrote the type guard's `k not in c` clause and flagged it as the part my own judgment could not check -- a mutation test by the author proves a guard is CONNECTED, never that it is connected to the right thing. The ASVS Tracker measured it and the hole is real. THE HOLE. With the writer's dict branch disabled, a payload OMITTING the key was refused while a payload CARRYING it exited 0 and wrote a Python repr into a TOML string. So the guard stopped looking at the exact moment a cell is rewritten. NOT A CORNER, and this is why it outranks a tidier fix: of 345 cells in the record exactly ONE holds a top-level non-scalar, and the natural payload for rewriting that cell ECHOES the key. The guard covered every cell that cannot be hurt and skipped the one that can. THE FIX KEEPS THE PROPERTY THE SCOPING WAS FOR. The intent was right -- a payload that intentionally retypes a field is an EDIT, and a guard refusing legitimate writes is a guard someone disables. The payload STATES a type, so compare against it rather than declining to look: an intentional retype agrees with its own payload and passes, a writer corruption disagrees in BOTH zones. _ORDERED is excluded because render() coerces those by design (int(cell['level']), the quoted emissions), so a payload stating another type there is NORMALISED, not corrupted -- refusing it would be the cry-wolf failure the scoping exists to avoid. MUTATION-PROVEN, and it changed the change. Four mutants: revert to `k not in c` -> killed always use the LIVE type -> killed drop the _ORDERED exclusion -> SURVIVED, so I wrote the test that kills it drop the _SUBTABLES exclusion -> SURVIVES, and is documented as such The third is the point: 23 tests stayed green while that clause did nothing, which is this item's own defect one level up. _SUBTABLES is left in as belt-and-braces with the reason written down rather than claimed as covered -- evidence and absence render as arrays of tables on both sides, so it cannot fire today, and that is a property of the current writer rather than an invariant. VERIFIED: ruff format + check clean (0.15.22, matching constraints.lock), mypy clean on the changed module, 24 passed in tests/test_asvs_apply.py and 411 passed / 23 skipped across -k asvs. Interpreter resolves messagefoundry to THIS worktree, checked by printing __file__. The venv lacks the x12/xml extras, so pytest printed INCOMPLETE RUN -- none of these is a full-suite claim. THIS DOES NOT CLOSE #1242, and the banner is not mine to write. Co-Authored-By: Claude Opus 5 --- scripts/asvs/apply.py | 48 ++++++++--------- tests/test_asvs_apply.py | 111 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 26 deletions(-) diff --git a/scripts/asvs/apply.py b/scripts/asvs/apply.py index 60634f0f1..0f6ae5983 100644 --- a/scripts/asvs/apply.py +++ b/scripts/asvs/apply.py @@ -410,38 +410,34 @@ def main(argv: list[str] | None = None) -> int: # above; it was written to catch DROPPED KEYS and it does. It is simply blind to this, and a # rewrite that corrupts every value while preserving every key would report green. # - # SCOPED TO KEYS THE PAYLOAD DID NOT TOUCH, deliberately: the corruption is the WRITER - # changing a type nobody asked it to change. A payload that INTENTIONALLY retypes a field -- - # schema evolution, a scalar becoming a table -- is an edit, not damage, and an unscoped - # check would refuse it. A guard that refuses legitimate edits is a guard someone disables. + # COMPARE AGAINST THE TYPE THE PAYLOAD STATED, rather than declining to look at keys it + # carries. The intent behind the original scoping is right and is preserved: a payload that + # INTENTIONALLY retypes a field -- schema evolution, a scalar becoming a table -- is an EDIT, + # not damage, and a guard that refuses legitimate edits is a guard someone disables. # - # THE INTENT ABOVE IS RIGHT AND THIS IMPLEMENTATION OF IT IS KNOWN-INCOMPLETE -- see the open - # BACKLOG #1242. `k not in c` scopes by "the payload did not MENTION this key", which is not - # the same question as "the payload asked for this type". A payload that CARRIES the key is - # skipped entirely, so this guard cannot see a corruption arriving through a mentioned key, - # while the same corruption through an omitted key is refused. That asymmetry is the defect: - # of the whole record exactly one cell holds a top-level non-scalar, and the natural payload - # for rewriting that cell ECHOES the key -- so the guard covers every cell that cannot be - # hurt and stops looking at the one that can. + # RETRACTED AND WHY (#1242): the first version expressed that as `k not in c`, which skipped + # every key the payload carries. Measured by the ASVS Tracker against this author's own + # scoping -- with the writer's dict branch disabled, a payload OMITTING the key was refused + # while a payload CARRYING it exited 0 and wrote a Python repr into a TOML string. So the + # guard stopped looking at the exact moment a cell is rewritten. That is not a corner: of + # 345 cells in the record exactly ONE holds a top-level non-scalar, and the natural payload + # for rewriting that cell ECHOES the key -- the guard covered every cell that cannot be hurt. # - # THE EXPOSURE IS LATENT, NOT LIVE, AND THE DISTINCTION IS LOAD-BEARING. Measured on this - # writer by a seat other than its author: the table is PRESERVED whether the payload carries - # the key or omits it. Corruption requires a BROKEN writer -- and then only when the payload - # carries the key, which is exactly the case this guard does not inspect. So the correct - # sentence is "WOULD fail to catch a regression here", not "corrupts today"; the second reads - # as 12.1.5 being at risk now, and it is not. Written conditionally on purpose: a false - # present-tense claim propagates into severity language and security records, which is the - # defect this comment exists to prevent, one level up. + # The payload IS the record of the type the author asked for, so it can be compared against. + # An intentional retype agrees with its own payload and still passes; a writer corruption + # disagrees whether or not the payload happened to mention the key. # - # This note exists because the paragraph above ARGUES for the boundary and argues well. An - # unguarded gap invites the question; a well-reasoned wrong boundary suppresses it, and an - # auditor reading this function would otherwise find a guard, find a persuasive rationale, - # and stop. Do not read the presence of this check as "the writer's type corruption is - # guarded". Read #1242's open row first. + # _ORDERED is excluded because render() deliberately COERCES those -- `int(cell['level'])` + # and the quoted emissions -- so a payload stating another type there is NORMALISED BY + # DESIGN, and refusing it would be the false-refusal this scoping exists to prevent. + # _SUBTABLES are excluded because they have their own key comparison below. retyped = sorted( k for k in was - if k in now and k not in c and type(was[k]) is not type(now[k]) # noqa: E721 + if k in now + and k not in _ORDERED + and k not in _SUBTABLES + and type(c[k] if k in c else was[k]) is not type(now[k]) # noqa: E721 ) if retyped: print( diff --git a/tests/test_asvs_apply.py b/tests/test_asvs_apply.py index fc989021b..fda6660f7 100644 --- a/tests/test_asvs_apply.py +++ b/tests/test_asvs_apply.py @@ -611,6 +611,117 @@ def test_the_TYPE_guard_does_NOT_refuse_a_payload_that_intentionally_retypes( assert cell["note"] == {"now": "a table"} +def test_the_TYPE_guard_sees_a_corruption_the_payload_ALSO_MENTIONS( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """THE SCOPING HOLE, found by the ASVS Tracker against a scoping this author wrote and could not + check (BACKLOG #1242). + + ``k not in c`` skipped every key the payload carries, so the guard covered the WRITER-only case + and stopped looking at the exact moment a cell is being rewritten. **That is the case that + matters rather than a corner:** measured on the real record, exactly ONE cell of 345 holds a + top-level non-scalar, and the natural payload for rewriting that cell ECHOES the key. So the + guard covered every cell that cannot be hurt and skipped the one that can. + + THE THREE ARMS, and the third is what makes the second attributable: + + ========== ===================================== ================================== + arm setup required + ========== ===================================== ================================== + control payload OMITS the key, writer broken refuse (the sibling test above) + subject payload CARRIES the key, writer broken refuse -- THIS test + sanity payload CARRIES the key, writer sound allow (the retype test above) + ========== ===================================== ================================== + + Carrying the key is not what corrupts the value; the writer regression is. Without the sanity + arm a refusal here would be equally consistent with "the guard now refuses any carried key", + which is the unscoped version this scoping exists to avoid. + + THE FIX COMPARES AGAINST WHAT THE PAYLOAD STATED rather than declining to look. The payload IS + the record of the type the author asked for, so an intentional retype still agrees with its own + payload and passes, while a writer corruption disagrees in BOTH zones. + """ + import scripts.asvs.apply as mod + + rec = _record(tmp_path) + assert ( + main( + [ + str(_payload(tmp_path, [_cell_111(sym_table={"a": 1})])), + "--scorecard", + str(rec), + "--apply", + ] + ) + == 0 + ) + + real_render = mod.render + + def mangling_render(cell: dict, live: dict | None = None) -> str: + text = real_render(cell, live) + return text.replace("sym_table = { a = 1 }", "sym_table = \"{'a': 1}\"") + + monkeypatch.setattr(mod, "render", mangling_render) + before = rec.read_bytes() + # The payload DOES carry sym_table, and carries it as the same dict it already is. Under the + # old scoping this is the silent-corruption path: the key is skipped, the guard never looks, + # and the file comes back holding a Python repr inside a TOML string. + rc = main( + [ + str(_payload(tmp_path, [_cell_111(sym_table={"a": 1})])), + "--scorecard", + str(rec), + "--apply", + ] + ) + + assert rc == 1, "a writer corruption is invisible whenever the payload happens to carry the key" + assert rec.read_bytes() == before, "refused, but wrote anyway" + out = capsys.readouterr().out + assert "would CHANGE the TYPE" in out and "sym_table" in out, out + + +def test_the_TYPE_guard_does_NOT_refuse_a_field_the_writer_COERCES_BY_DESIGN( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """THE FALSE-REFUSAL ARM for the corrected scoping (BACKLOG #1242). + + Comparing the payload's stated type against the output would be wrong for the keys ``render()`` + deliberately NORMALISES: ``level`` goes through ``int()``, and verdict / last_verified / + verified_at are emitted quoted. A payload stating ``level`` as the string ``"1"`` therefore + produces an int in the file **by design**, and refusing that is precisely the cry-wolf failure + the scoping exists to avoid -- a guard that refuses legitimate writes is a guard someone + disables. + + WITHOUT THIS TEST THE ``_ORDERED`` EXCLUSION IS UNPINNED. Measured while writing it: dropping + that clause left all 23 other tests green, so the suite was silent about exactly the region the + clause occupies -- which is this item's own defect one level up (COMMON 4.5.1). + + Its sibling ``_SUBTABLES`` clause is deliberately NOT pinned here and is belt-and-braces rather + than load-bearing: ``evidence`` and ``absence`` render as arrays of tables on both sides, so the + comparison cannot fire for them today. It is kept because that is a property of the current + writer rather than an invariant, and the sub-table entries have their own key check below. + """ + rec = _record(tmp_path) + rc = main( + [ + str(_payload(tmp_path, [_cell_111(level="1")])), + "--scorecard", + str(rec), + "--apply", + ] + ) + + out = capsys.readouterr().out + assert rc == 0, f"a by-design coercion must not read as corruption: {out}" + assert "would CHANGE the TYPE" not in out, out + cell = next( + c for c in tomllib.loads(rec.read_text(encoding="utf-8"))["cell"] if c["id"] == "1.1.1" + ) + assert cell["level"] == 1, "the writer's own int() normalisation still happened" + + # --- BACKLOG #1307: a retirement is a SANCTIONED outcome the writer could not express ------------- # # The shrink guard refuses any payload where an evidence or absence list gets shorter, and it took no From f1144e4e6f989a3a677165685e4191c75b51f1cf Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 21 Aug 2026 21:15:30 -0500 Subject: [PATCH 2/2] docs(asvs): drop the record's cell total from the type-guard comment The figure is vault-derived and this file ships to PyPI. A coverage count over a closed public requirement set discloses the uncovered set by subtraction, which is why the scorecard is vaulted in the first place. `main` already words this clause without a figure. Merging this branch as-is would have brought the number BACK, and `git merge-tree` reports NO CONFLICT because the two sides edited different line ranges of the same comment -- so nothing would have objected. That was the second of two blockers recorded on PR #487. ONLY THE COMMENT MOVES. The guard itself is this branch's fix and is deliberately untouched: main still carries `k not in c`, which skips every key the payload carries, and repairing that is what these commits are for. RECORDED BECAUSE I NEARLY DID THE OPPOSITE. Comparing the two comment blocks, I judged main's better-worded and figure-free version to be the later revision and concluded the fix was to take main's whole block. It is the EARLIER one. Diffing the CODE rather than the prose showed main still has the unrepaired guard, so taking its comment would have shipped a comment describing an implementation that no longer exists -- and would have read as reverting the fix. Prose quality is not version order. A note now sits inline saying the total is omitted on purpose, so a future merge that reintroduces it has something to contradict. --- scripts/asvs/apply.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/asvs/apply.py b/scripts/asvs/apply.py index 0f6ae5983..f53497373 100644 --- a/scripts/asvs/apply.py +++ b/scripts/asvs/apply.py @@ -420,8 +420,12 @@ def main(argv: list[str] | None = None) -> int: # scoping -- with the writer's dict branch disabled, a payload OMITTING the key was refused # while a payload CARRYING it exited 0 and wrote a Python repr into a TOML string. So the # guard stopped looking at the exact moment a cell is rewritten. That is not a corner: of - # 345 cells in the record exactly ONE holds a top-level non-scalar, and the natural payload + # the whole record exactly ONE cell holds a top-level non-scalar, and the natural payload # for rewriting that cell ECHOES the key -- the guard covered every cell that cannot be hurt. + # (The record's cell TOTAL is deliberately not stated here. It is vault-derived, this file + # ships to PyPI, and a coverage count over a closed public requirement set discloses the + # uncovered set by subtraction. `main` already words it this way; the figure is the only + # thing that differs, and it must not come back through a merge.) # # The payload IS the record of the type the author asked for, so it can be compared against. # An intentional retype agrees with its own payload and still passes; a writer corruption