fix(asvs): compare the type the payload stated, not the keys it did not mention (BACKLOG #1242) - #612
Merged
Merged
Conversation
…ot 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 <noreply@anthropic.com>
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.
wshallwshall
enabled auto-merge (squash)
August 26, 2026 15:12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lifts the two live commits from the stranded PR #487 (
lander/432-resolved), per Cleaner'sgrader-then-refuter grading (
.git/mefor-coord/handoffs/CLEANER-2026-08-26-FOR-LANDER-stranded-pr-grading.md).Everything else on that 45-commit branch is either superseded by what already landed on main or a
drop; this is the two-commit live remainder, cherry-picked onto a fresh branch off current main
rather than resolving PR 487's own 13-hunk conflict.
5e03aa59c-- BACKLOG #1242's own fix: the type-guard compared against the KEYS the payload didnot mention (
k not in c), not the TYPE it stated. A payload that CARRIES a key it is corruptingpassed silently. Found by the ASVS Tracker against a scoping gap the original author flagged as
unable to check themselves.
6dd9a0b2c-- drops a vault-derived cell total from the comment the fix above introduced. Thefigure discloses the uncovered set by subtraction over a closed public requirement set, which is
why the scorecard is vaulted in the first place.
Both cherry-picked clean except one conflict in
scripts/asvs/apply.py+tests/test_asvs_apply.py:main had since landed BACKLOG #1307 (retirement) and #1308 (banned-glyph) tests in the same region.
Not a real conflict -- both sides added independent tests near the same anchor. Resolved by keeping
all of them: the two new #1242 tests slot in immediately after the existing retype test they are
siblings of (the test docstring calls it "the sanity arm"), main's #1307/#1308 tests keep their
position after. Verified no duplicate test functions and no leftover conflict markers before
committing.
Verified before pushing:
test_asvs_apply.py: 33/33 passed (ruff format/check clean, mypy clean).k not in c and type(was[k]) is not type(now[k])guard is what's live today -- the code Cleanerdescribed as already-fixed only appeared mid-conflict because git's 3-way merge had already
auto-applied that non-conflicting hunk; caught and re-verified against
git show origin/main:...directly before trusting it.
scripts/coord/claim.ps1 -Take 1242) before committing; not claimed byanyone else at the time.
Does not touch PR 487 itself, which stays open pending its own precondition/DROP list from the same
grading pass.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com