fix: reject malformed nested discriminator values - #85
Conversation
Guard string-valued discriminators before set membership in TR-POL, TR-RTE, and TR-SIG so arrays and objects produce findings rather than terminating verification. Apply the same boundary to TR-SCA while excluding JSON booleans from Python's integer membership semantics. Extend the malformed-record matrix through the runner and preserve every JSON Schema integer SLSA level. Signed-off-by: Yatsuiii <battyrises@gmail.com>
lywinged
left a comment
lywinged
left a comment
There was a problem hiding this comment.
Approving, with a measurement the PR body could not make about itself: for the class of wrong value this suite already tests against, these four fields are all of them, not four instances of many.
Naming the input class rather than saying "malformed", because widening it changes the answer, and the second half of this comment is what widening it found.
The four are all of them, for the nine values
I swept every nested path in the four valid_* vectors, replacing each with the nine values tests/test_modules_never_raise.py carries in JUNK, and called all seven modules on every mutant.
baseline 46c9c0e2 |
this branch | |
|---|---|---|
| vectors | 4 | 4 |
| nested paths swept | 144 | 144 |
| values per path | 9 | 9 |
| distinct field paths where a module raises | 4 | 0 |
The four at baseline, each raising in every vector that carries the field:
build_provenance.slsa_level tr_sca:TypeError
cnf.jwk.kty tr_sig:TypeError
policy.enforcement_mode tr_pol:TypeError
runtime.platform tr_rte:TypeError
Exactly the four this PR guards, and on this branch not one of their fourteen vector-and-field instances raises. One further path does, at baseline and still on this branch: trace itself in valid_cmcp_runtime.json, which is the library-caller case test_modules_never_raise.py discloses in its own docstring and deliberately leaves open. Everything else is closed.
The boolean false PASS, and no regression on valid levels
slsa_level |
at 46c9c0e2 |
on this branch |
|---|---|---|
True |
PASS, "slsa_level is valid (True)" | FAIL, TR-SCA-001 |
False |
PASS, "slsa_level is valid (False)" | FAIL, TR-SCA-001 |
3 |
PASS | PASS |
0 |
PASS | PASS |
4 |
FAIL | FAIL |
The claim holds and the fix is narrow: only booleans move. 402 passed, 5 xpassed reproduces.
On the disclosed layer above, which I confirmed rather than found
I checked the claim in that docstring that carries the weight. loader.py:38 is if not isinstance(data.get("trace"), dict): guarding a LoadError, and all four non-dict envelopes I tried are refused before extract_trace runs. The disclosure is exact, the CLI path is closed, and the residual is the library path it names.
Recording it because I arrived at it from the other end, through the sweep, and spent a while thinking I had found something before reading the docstring that already had it.
Widening the value set found a different escape, and it is mine
Not a gap in this PR, and not something to fix here. It belongs in this thread because it is the same contract, and because the sweep above would have reported 0 for it too.
I ran the same sweep with eleven further values, among them 10**20, float("nan"), float("inf") and a lone surrogate. On the baseline and on this branch alike, 44 further paths raise, every one of them tr_sig letting rfc8785.IntegerDomainError, FloatDomainError or CanonicalizationError out of _canonical_json. Both call sites are exposed: line 129 in check_cmcp_runtime, over the whole envelope, and line 183 in check, over the trace fields.
The correlation is exact in both directions. Three of the eight vectors carry a signature and all three show escapes, 44 and 20 and 23 paths; the five without a signature show none. That is the only condition under which _canonical_json is reached at all.
Three of those values are ordinary JSON, so this is reachable from a file rather than only from a library caller:
$ trace-tests verify --record bigint.json --level 0 # tool_calls_total: 10**20
rfc8785._impl.IntegerDomainError: 100000000000000000000 exceeds safe integer domain
$ trace-tests verify --record inf.json --level 0 # a 1e400 literal
rfc8785._impl.FloatDomainError: inf is not representable in JCS
$ trace-tests verify --record surrogate.json --level 0 # a "\ud800" escape
rfc8785._impl.CanonicalizationError: input contains non-UTF-8 codepoints
json.loads accepts all three and load_record has no reason to refuse any of them. runner.py contains no try at all, so each ends the run where a verdict belongs. report fails the same way, so it is both commands rather than one. All three exit 1, which is also what an honest FAIL exits, so a caller reading the status code cannot tell a rejected record from a crashed tool. A control record carrying an ordinary wrong value at the same path returns Result: FAIL on every tree I ran it on, so the probe is reaching the case rather than breaking the tool generally.
This came in with #66, which is mine, and that is measured rather than inferred. At c725bbb^ all three records verify to Result: FAIL (7 checks, 2 failures, 0 skipped). At c725bbb they traceback. That commit replaced json.dumps(sort_keys=True, ...) with rfc8785.dumps for a good reason, and §3.2.2 requires exactly that, but json.dumps serialises all three of these without complaint: it emits NaN, Infinity and large integers as they are. Making the canonicalizer correct made it strict, and strict inside a function whose callers may not raise is a failure mode the correctness argument did not cover.
Nothing caught it, and no amount of re-running the existing test would have. All nine values in JUNK serialise cleanly through rfc8785, checked one at a time, so test_modules_never_raise.py cannot see this class by construction. The lens had to change, not the number of passes through it.
I will open it separately, with a fix and a vector per class, so this PR keeps its scope.
Generated by Claude Code
…g on it `runner.run` calls every module with no `try`, and `tests/test_modules_never_raise.py` states the contract in its first line: a malformed record must produce a finding, never an exception. `tr_sig` breaks it while canonicalizing. `_canonical_json` is `rfc8785.dumps`, which raises on three classes of value JCS has no form for: an integer outside the safe range, a non-finite float, and a string carrying a lone surrogate. All three are ordinary JSON. `json.loads` accepts them and `load_record` has no reason to refuse them, so both CLI commands reached them as a traceback, exiting 1, which is also what an honest FAIL exits. A caller reading the status code could not tell a rejected record from a crashed tool. Both call sites were exposed, `check_cmcp_runtime` over the whole envelope and `check` over the trace fields. Across the eight vectors, 20 wrong-typed values and 240 nested paths, 87 paths raised this way; now none do. The 30 that remain are the `TypeError` class agentrust-io#85 addresses plus the library-caller path `test_modules_never_raise.py` discloses, and neither is touched here. `_canonical_json` still raises, because `test_canonicalization_boundary.py` compares its bytes directly and a wrapper that swallowed the error would hide a real serializer defect. `_canonical_body` is the boundary: callers that owe a verdict use it, and it returns the reason rather than only the failure. All five `raise` sites in `rfc8785._impl` are `CanonicalizationError` or a subclass, so one clause covers every way it can refuse. The finding says the record has no canonical form and does not say verification failed, because those are different facts and a consumer acting on the second would go looking for a key problem that is not there. This came in with agentrust-io#66, and that is measured rather than inferred: at `c725bbb^` all three records verify to `Result: FAIL`; at `c725bbb` they traceback. That change was correct and §3.2.2 requires it, but `json.dumps` emitted `NaN`, `Infinity` and large integers without complaint. Making the canonicalizer correct made it strict, inside a function whose callers may not raise. Nothing caught it and no number of runs would have. Every one of the nine values `JUNK` carried serializes through `rfc8785` cleanly, so that test could not see the class by construction. The three are added to it first, which is the general guard; `tests/test_canonicalization_refusals.py` is the specific one and asserts the verdict rather than only the absence of an exception. Each of its four vectors is checked for still provoking its own error before anything else is asserted about it, so a vector edited into serializability fails loudly instead of passing over a record that exercises nothing. With the fix reverted, 8 assertions in that file fail and the widened `JUNK` takes the general guard red alongside them. 384 to 397 passed, the 13 added here; ruff and mypy unchanged at 57 and 3, all pre-existing and none in the files touched. Signed-off-by: Louielunz <48041247+lywinged@users.noreply.github.com>
2 participants
Summary
Closes #84.
Malformed arrays or objects in four nested discriminator fields previously reached
Python set membership and raised
TypeError, terminating verification before therunner could return findings. Boolean SLSA levels also passed because Python treats
TrueandFalseas integers1and0.This change adds explicit JSON type boundaries in the modules that own those fields:
policy.enforcement_modeTR-POL-002runtime.platformTR-RTE-001build_provenance.slsa_levelTR-SCA-001cnf.jwk.ktyTR-SIG-004Regression evidence
The existing malformed-record harness now mutates all four nested fields. Separate
runner-level tests require arrays and objects to produce the owning FAIL finding rather
than an exception. Boolean SLSA levels have their own runner regression.
Before the guards were applied, the new tests produced 14 failures:
TypeErrorexceptions;With this commit:
The branch adds 18 tests over the 384-test base.
Compatibility boundary
There is no schema change, new finding code, or verdict-policy change. Valid string
discriminators retain their existing behavior. TR-SCA continues to accept levels 0
through 3, including zero-fraction JSON numbers such as
1.0, matching JSON Schemainteger semantics. Only booleans and malformed JSON shapes change outcome.
Verification
Ruff reports the same 16 existing findings on the four source files at both main and
this branch. The new and modified test files are clean.
The commit is DCO signed as
Yatsuiii <battyrises@gmail.com>.