fix(content-marking): build_assertion and verify_assertion crash on non-object record_bytes

[rajnisht7]

What this changes

content_marking.build_assertion() and content_marking.verify_assertion() both call .get(...) on the result of json.loads(record_bytes) without first checking that the parsed value is a dict. Valid JSON is not always an object that is an array, a string, a number, null, and a bool are all valid top-level JSON and record_bytes is exactly the kind of externally-sourced input this is likely to happen to: build_assertion() takes whatever bytes a caller hands it, and verify_assertion()'s own docstring says its record_bytes are "the record bytes actually retrieved from its URL," i.e. a network response the caller does not control. Either function raised an unhandled AttributeError instead of the documented ContentMarkingError.

verify_assertion() had a second, related gap: its second parse of record_bytes (the one after the hash check, used to compare subject and eat_profile) was not wrapped in a try/except the way its build_assertion sibling already is, so bytes that are not valid JSON at all but happen to hash-match the assertion's declared digest raised json.JSONDecodeError instead of ContentMarkingError.

Type of change

• Editorial (typo, link fix, clarification — no normative effect)
• Non-breaking spec change (new optional field, new platform profile, informative addition)
• Breaking spec change (requires 14-day comment period and Project Lead sign-off)
• Schema change
• Example addition

Spec section

None

Checklist

• DCO sign-off on all commits (git commit -s)
• CHANGELOG.md updated (for any normative change)
• Breaking changes marked with <!-- CHANGED: #NNN — description --> in spec text
• Backward compatibility statement included (for breaking changes)

[lywinged]

Approving. I checked the defect, the fix, and the tests separately rather than reading the argument, since a fix for "raises the wrong exception type" is easy to write in a way that passes its own tests without closing the path.

The defect reproduces on main. All seven inputs escape the documented contract:

build_assertion(b"[1,2,3]" / b'"s"' / b"42" / b"null" / b"true")  -> AttributeError
verify_assertion(hash-matching non-object body)                   -> AttributeError
verify_assertion(hash-matching non-JSON body)                     -> JSONDecodeError

The fix closes all seven, each to ContentMarkingError.

The tests are load-bearing. Running this PR's tests/test_content_marking.py against main's source gives 7 failed, 17 passed. They fail for the reason claimed, not incidentally. Ruff clean, 24 pass on the branch.

The docstring on test_verify_assertion_refuses_non_object_record_at_the_url is the part I would keep verbatim. It explains why the path stays reachable after build_assertion refuses to produce such bytes, which is the question a reader asks next, and the answer is the right one: verify_assertion is written against the spec rather than against this module's own producer.

One gap of the same class, in the same function

build_assertion validates the type of record_bytes:

if not isinstance(record_bytes, bytes | bytearray) or not record_bytes:
    raise ContentMarkingError(...)

verify_assertion does not, and on this branch:

record_bytes	result
"a string"	TypeError
None	TypeError
["x"]	TypeError
5	RecordMismatch

The last row is the one worth fixing before the others. bytes(5) is b"\x00\x00\x00\x00\x00", so five zero bytes get hashed and compared, and the caller is told:

the record at https://example.com/r.json does not match the assertion: computed sha256:8855508a... The record changed after the asset was signed, or the URL is serving a different one.

Nothing is wrong with the record or the URL. The caller passed an int. This is worse than the AttributeError this PR removes: a crash says the call was wrong, this says someone else's server is serving a different record, and it says so specifically, with a digest attached.

It is one line, in the function this PR is already editing, and the sibling function already has it.

Nit: the changelog entry does not parse

Valid JSON is not always an object that is an array, a string, a number,
`null`, and a bool are all valid top-level JSON  and `record_bytes` is ...

"an object that is an array" states the opposite of the point. The line contains no dashes and carries a leftover double space at "JSON and", where the sentence evidently read:

... not always an object - an array, a string, a number, `null`, and a bool
are all valid top-level JSON - and `record_bytes` is ...

Worth fixing before it ships in a release.

---

Generated by Claude Code

[imran-siddique]

Same class as #225 and right for the same reason: a documented exception contract that malformed input could route around.

json.loads() returning a non-dict is the case here, and verify_assertion is the one that matters. Its own docstring says record_bytes are "the record bytes actually retrieved from its URL", so the input is a network response the caller does not control. An array, a string, a number, null and a bool are all valid top-level JSON.

Two things worth calling out:

The second parse in verify_assertion, after the hash check, was not wrapped the way its build_assertion sibling already was. So bytes that are not JSON at all but happen to hash-match the declared digest raised JSONDecodeError. Narrow, and exactly the input an attacker who controls the URL would reach for.

The error message distinguishes this from RecordMismatch explicitly: it matched the declared hash, so this is what the record is at that URL, not a mismatch between two records. Getting that boundary right matters more than the fix itself, because RecordMismatch is what a caller acts on differently.

CI green. Tests cover each JSON shape.

[imran-siddique]

Same class as #225 and right for the same reason: a documented exception contract that malformed input could route around.

json.loads() returning a non-dict is the case, and verify_assertion is where it matters. Its own docstring says record_bytes are "the record bytes actually retrieved from its URL", so the input is a network response the caller does not control. An array, a string, a number, null and a bool are all valid top-level JSON.

Two things worth calling out:

The second parse in verify_assertion, after the hash check, was not wrapped the way its build_assertion sibling already was. So bytes that are not JSON at all but happen to hash-match the declared digest raised JSONDecodeError. Narrow, and exactly what someone controlling the URL would reach for.

The error message distinguishes this from RecordMismatch explicitly: it matched the declared hash, so this is what the record is at that URL, not a disagreement between two records. That boundary matters more than the fix, because RecordMismatch is what a caller acts on differently.

I merged main in to clear a CHANGELOG conflict created by #225 landing first, and took main's copy of the shared line so the new no-dashes check passes. Nothing of yours changed.