Fuzz member types, and reject a wrong-typed member wherever one is read - #353
Conversation
The surface found one instance of the malformed-member class and then could not find its siblings: it mutated text, so it reached a wrong-typed member only by accident and could never introduce an absent one. It now emits the optional structural members and mutates types directly. At the seed that had produced nothing, that found 291 divergences in two shapes. A non-string currentSigningKeyId was compared as a value by the reference and read as missing by Go, and a chain carrying something that is not a link was reported as a key that failed to decode. Both reject as an invalid card now, and the seed is clean. Signed-off-by: Jason Odoom <jason@adastracomputing.com>
Three more members were read as values without a type check. A signing entry whose keyId was not a string was accepted by the reference and rejected by Go, because a type assertion collapses every non-string to the empty string and calls two of them duplicates. An absent cardSignature keyId did the same one level up, matching an entry whose own id was empty. Both are a MUST in the spec, so both are now required. The runner also recorded findings it had disproved: where a minimized case no longer diverged it kept the original claim and wrote the finding anyway. It now re-decides the original and reports an unstable observation instead. The non-ASCII escape range is written as escapes rather than literal characters, which the local hook reads as invisible text. Same range. Signed-off-by: Jason Odoom <jason@adastracomputing.com>
Review found the last instance of the class, one level inside a rotation link. Go collapsed a wrong-typed entry keyId to the empty string and called two of them duplicates, where the reference kept them distinct and reached a signature failure. Both reject as an invalid card now, and Go separates a malformed entry from a key that failed to decode so the reasons agree. The generator emits a chain link with a real entry and can address the members inside it, so the surface reaches this shape rather than dying at the decode. A finding whose minimized case does not reproduce now carries the case that does as its witness, instead of one whose recorded decisions show agreement. Signed-off-by: Jason Odoom <jason@adastracomputing.com>
|
Review verdict was mergeable with one remaining finding, the committed-set keyId collapse inside a rotation link, offered as a follow-up. It is fixed here instead, in 3eabd97, along with the note that a finding whose minimized case does not reproduce should not carry that case as its witness. Two things about the review status on this last commit, for the record. The external reviewer had already given its verdict before 3eabd97 landed, and both reviewers are at their usage caps, so that commit carries only my own review. I verified the part of it that the clean seeds cannot reach. The finding writer only runs when a divergence is found, so five clean seeds never exercise it, and the self-test path does not write findings at all. I injected a temporary fault into the reference, ran 2000 cases, confirmed all 50 written findings carry a witness whose own recorded decisions show the divergence they claim, then reverted the fault and reconfirmed the tree is clean. Final state: 2405 tests, Go suite green, typecheck and lint clean, five seeds at 40000 cases with no divergence and no unstable observations. |
1 participant
Follow-up to #352, which fixed six instances of one bug class: a member that is
present but malformed being read as absent, which selects a weaker path.
Coverage gap
The pre-1.0 audit pointed out that this surface generated a non-array
rotationChainzero times in 363000 cases. The generator never emitted thatmember, and the mutation arm worked on the card's text, so it could only reach a
wrong-typed member by accident and could never introduce one that was absent.
Every instance of this class is a member present with the wrong type. The
surface found the first and could not reach its siblings, so they came out one
at a time under review.
The generator now emits the optional structural members before signing, and a
new arm sets a chosen member to a wrong-typed value, adding it when absent,
across the eleven members whose type decides which branch a verifier takes.
A 20000-case probe generates all four shapes:
keysnon-object 1868,keys.signingnon-array 433,rotationChainnon-array 543,cardSignaturenon-object 421.
Divergences at seed 31
Seed 31 had produced nothing before and now produces 291 divergences in two
shapes, both the same split between a TypeScript truthiness test and a Go type
assertion, one level below the members #352 guarded.
A non-string
currentSigningKeyId: the reference passed an array through itstruthiness test and reported
signer_not_current, while Go's type assertionfailed and reported
missing_current_signing_key_id.A
rotationChaincarrying something that is not a link: the reference read itthrough to the decode step and reported
invalid_key_encoding, which conflatesa malformed card with a key that failed to decode. Go rejected the shape.
Review found three more members of the same kind, one of them an accept/reject
split: a signing entry whose
keyIdis not a string is accepted by thereference and rejected by Go, because a Go type assertion collapses every
non-string to the empty string and calls two of them duplicates. An absent
cardSignature.keyIddoes the same one level up, matching an entry whose own idis empty. The spec makes
keyIdandsignaturea MUST, so both are requirednow rather than merely type-checked.
The two seed-31 divergences are not bypasses, since both sides reject either
way. All are fixed by applying the same rule one level deeper: a present member of the wrong type
rejects as an invalid card, and is never read as absent or read through to a
later step. Seed 31 then goes from 291 divergences to 16 to none.
Harness fixes
An earlier version of this description said a throwing
resignwould have endeda live fuzzing run. That was wrong:
run.mjsalready catches a throwinggenerator and skips the case. The real choice is between skipping such a case
and deciding it with the signature it already had, and the second is worth more,
because a card whose signature no longer covers its bytes is exactly what a
verifier has to reject. Only the canonicalizer's own refusals are caught now, so
a harness fault such as a bad secret still throws instead of leaving every
re-signed case silently failing its signature while the run reports green.
The runner was recording findings it had already disproved. After minimizing a
case it re-decides it, and where the minimized case no longer diverged it fell
back to the original claim and wrote the finding anyway, so an artifact could
show both sides agreeing under a heading saying they did not. That is what sent
this review chasing divergences that would not replay. It now re-decides the
original as well, and where neither reproduces it reports an unstable
observation and records nothing.
Verification
Six seeds at 20000 cases each report no divergence and no unstable
observations. The TypeScript suite passes 2401 tests, the Go package suite
passes, typecheck, lint and check:facts are clean. Exit codes were checked
directly rather than through a pipe that would mask them.