[drk-ram]

What this changes

Contributor: Raghuvar Durga (@drk-ram)

Closes #555

Enforces consumer field validation and strict UTF-8 / RFC 8259 JSON boundaries in src/agentrust_trace/content_marking.py and spec/content-marking-v1.md:

1. _loads(data, what) strict UTF-8 and RFC 8259 JSON checks:
   • Rejects inputs that are not bytes or bytearray with ContentMarkingError.
   • Decodes data strictly as UTF-8 (data.decode("utf-8")) and rejects a leading U+FEFF UTF-8 byte-order mark (RFC 8259 section 8.1), refusing UTF-16, UTF-32, and UTF-8 BOM-prefixed payloads with ContentMarkingError.
   • Rejects non-RFC-8259 numeric constants (NaN, Infinity, -Infinity) via parse_constant with ContentMarkingError.
   • Propagates ContentMarkingError directly from object_pairs_hook (_no_duplicate_members) and parse_constant.
   • Rejects unpaired UTF-16 surrogates (U+D800 to U+DFFF) in parsed strings and object keys with ContentMarkingError, while preserving valid supplementary Unicode scalars (\U0001f600) and JSON surrogate-pair escapes (\ud83d\ude00).
2. build_assertion and verify_assertion bytes | bytearray contract and field validation:
   • Types record_bytes as bytes | bytearray across _digest, build_assertion, and verify_assertion.
   • In build_assertion, checks isinstance(subject, str) before matching _SUBJECT_RE and rejects UTF-16 surrogate code points in anchor with ContentMarkingError (tools/sweep_public_surface.py now reports '\ud800' refused on content_marking.build_assertion.anchor).
   • In verify_assertion, validates subject (SPIFFE or DID URI), eat_profile (non-empty str), and optional anchor (non-empty str without UTF-16 surrogate code points) on both assertion["data"] and the resolved record with ContentMarkingError before comparing the two for RecordMismatch.
3. Specification and Changelog:
   • Updates spec/content-marking-v1.md section 4 (Verification) to state that record bytes are decoded as strict UTF-8 JSON without a leading byte-order mark, non-finite numeric constants, or unpaired UTF-16 surrogates, and that both the assertion and the resolved record carry a valid SPIFFE or DID subject, a non-empty string eat_profile, and (when present) a non-empty string anchor.
   • Adds a CHANGELOG.md entry under [Unreleased].

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

spec/content-marking-v1.md sections 2 and 4 (src/agentrust_trace/content_marking.py)

Tests for a fix

Added tests/test_content_marking_consumer_boundary.py (51 test cases) and cherry-picked companion regression controls tests/test_content_marking_review_controls.py (24 test cases, co-authored by @dinakarjs).

Validation commands and results:

• pytest tests/test_content_marking*.py tests/test_public_functions_raise_what_they_document.py tests/test_regex_surface_parity.py tests/test_untrusted_input_stays_on_the_documented_error.py: passed (417 passed)
• python tools/review_bar.py --base origin/main --head HEAD: passed (57 fail on behaviour, 0 fail on a missing name or file, 18 pass or skip)
• python tools/sweep_public_surface.py --strict: passed (89 parameters swept, 140 accepted junk values, 0 unfiled leak(s))
• python tools/check_dashes.py: passed (no dashes outside examples/, spec/trace-v0.1.md)
• ruff check src/agentrust_trace/content_marking.py tests/test_content_marking_consumer_boundary.py tests/test_content_marking_review_controls.py: passed
• mypy --strict src/agentrust_trace/content_marking.py tests/test_content_marking_consumer_boundary.py tests/test_content_marking_review_controls.py: passed (Success: no issues found in 3 source files)

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)

[rajnisht7]

Overall the direction looks good, the assertion-side checks, error taxonomy (ContentMarkingError vs RecordMismatch), UTF-8/BOM/NaN/Infinity rejection, and nested surrogate handling all seems correct

a few things that needs to be looked upon:

• the str(exc).startswith(what) logic in _loads is brittle as it rewrites some ContentMarkingErrors (like duplicate members) but not others (like non-finite numbers) based purely on the message text. Just let ContentMarkingError propagate and only wrap the real ValueError/RecursionError cases. Also add a regression test that feeds duplicate-key JSON through the new _loads path.
• contract the inconsistency around bytes vs bytearray. The code accepts both, but the error message says “must be bytes” and the new test is named test_loads_rejects_non_bytes_input, either document + test bytes | bytearray properly, or tighten to pure bytes.
• _has_surrogate currently rejects any code unit in U+D800–U+DFFF which means a valid surrogate pair passed as a Python string (e.g. an emoji constructed that way) gets rejected on the direct anchor= path, even though the JSON-bytes path accepts it after json.loads. Worth an explicit decision + test.
• after a successful hash match, the fetched record only gets presence + equality checks on subject/eat_profile. No type or _SUBJECT_RE re-check. Not a bypass (equality fails against a non-string), but the error framing and symmetry would be cleaner if you validated the record side too.
• BOM rejection is a deliberate library choice (RFC 8259 only says generators must not emit it; parsers may ignore). The companion spec currently has no text about this either note it as a TRACE profile decision or add a short normative line
• test: a couple of tests mix two scenarios, theres no positive bytearray control through the new _loads, and the duplicate-key path isnt exercised by the new suite
• also the CHANGELOG entry is missing

[dinakarjs]

Independent boundary review of c495cb1. I reran the 385 content-marking/public-error/regex/untrusted-input tests and added 23 local controls, all passing; imports were explicitly pinned to this checkout. Current CI, Review bar, Conformance CI and CodeQL report success.

The additional controls exercised escaped duplicate names (including nested objects inside arrays) through both public APIs, bytes and bytearray with sha256/sha384, raw/escaped supplementary Unicode, an interior U+FEFF, exact-byte hashing across a trailing newline, and malformed payloads with matching versus stale hashes. Duplicate-member refusal remains intact on this head. Malformed bytes with a stale hash correctly raise RecordMismatch before parsing; after updating the hash to those same malformed bytes the parser raises ContentMarkingError rather than accusing the server of a changed record.

One clarification for the existing surrogate discussion: a Python scalar U+1F600 works through the direct anchor path and escaped JSON surrogate pairs decode to that scalar. A Python string containing two explicit surrogate code points is refused on both direct build and verify paths. Keeping that refusal is defensible because that string is not UTF-8 encodable under strict encoding; accepting the pair should be an explicit normalization decision, not an automatic requirement. Please retain separate positive tests for the actual scalar and JSON escape representation whichever contract is chosen.

I found no additional blocking defect in these controls. This is a bounded COMMENT review, not approval of unresolved author follow-ups: Rajnish's existing error-message/contract/profile-documentation points remain for the author and maintainers to resolve. The positive bytearray, nested duplicate and Unicode controls above would be useful committed regressions.

[dinakarjs]

Tests-only companion for the author to adopt on exact head c495cb1: dinakarjs@4e65626 (DCO-signed, one new test file; original PR/implementation ownership retained).

The 22 paired public-API controls commit the useful boundaries from the earlier review: positive bytearray build/verify and exact SHA256/SHA384 hashes; escaped duplicate member names at top level, nested objects and inside arrays through build and hash-matching verify; actual supplementary Unicode scalar versus JSON escape-pair spelling; interior U+FEFF versus a leading UTF-8 BOM; valid direct scalar anchor; trailing-newline exact-byte binding; stale-hash RecordMismatch versus hash-matching malformed-JSON ContentMarkingError.

Validation: 445 related content-marking/public-error/regex/untrusted-input tests passed on Python 3.12.14, including these 22 controls. Import resolution uses this checkout. Ruff, strict mypy on the new file (MYPYPATH=src), repository dash and whitespace checks pass. The published tree matches the tested local tree. Isolated mutants are detected: replacing the duplicate-member hook with dict fails 6 controls; stripping trailing newlines before hashing fails 2; rejecting an interior U+FEFF as a leading BOM fails 2. Positive paired controls remain in each test.

These tests do not prescribe current diagnostic-wrapper wording or normalize Python strings containing explicit surrogate code points. Rajnish's error-propagation, bytes/bytearray documentation, fetched-record validation, BOM profile rationale and changelog follow-ups remain for the author/maintainers. This is a companion handoff, not approval of those unresolved items or a claim that this commit has landed upstream.

[dinakarjs]

Re-reviewed exact head 06ddb48. The author follow-ups from the previous review are addressed: ContentMarkingError propagates by type rather than message prefix, bytes/bytearray are documented and tested consistently, valid UTF-8 scalars/JSON surrogate escapes are distinguished from explicit Python surrogate code points, resolved-record fields are validated before equality classification, and the companion specification/changelog state the strict JSON boundary.

The focused content-marking/public-error/regex/untrusted-input selection passes 417 tests with this head's hash-locked dependencies and checkout imports. It includes the supplementary controls I contributed for nested/escaped duplicate members, both digest algorithms, bytearray positives, scalar versus escaped-byte identity, interior versus leading BOM, trailing newlines, and stale-hash versus hash-matching malformed bytes. I found no remaining blocking defect in this bounded code review.

Ruff, strict mypy on the production module and both changed test modules, dash and whitespace checks pass. CI and Conformance CI succeed; ClusterFuzzLite was still running when checked. I contributed the supplementary tests; this technical approval does not replace another maintainer's assessment of that contribution, any specification-process decision, or full record/asset/hardware verification.

[drk-ram]

Thanks @rajnisht7 and @dinakarjs for the thorough review and companion test suite! All feedback items have been addressed in \4b4de13\ and \

[rajnisht7]

Reviewed the head 06ddb48, the changes looks good,there are two things worth fixing before merge:

• bytearray TOCTOU: The dual-read (hash vs parse on the same object) already existed on main. But the latest commits (06ddb48 especially) made bytes | bytearray the official public contract and added tests for it. So the risk became real and documented
• Missing fields -> RecordMismatch inconsistency: The old presence check (RecordMismatch) was already there and this PR adds the new type/format checks that raise ContentMarkingError which creates the asymmetry.

nit:

• kindly remove the issue and PR number from the docstring, top of the test files

[dinakarjs]

Follow-up to the current-head mutable-input/error-classification review: a DCO-signed companion is ready for author adoption on exact head 06ddb481bdf776adfd78c1a4d9567544df9730a8:

dinakarjs@5495993

Both public entry points now freeze one immutable record-bytes snapshot before their first parse/hash read, so a caller mutation between those reads cannot bind one version and inspect another. Deterministic mutation-at-the-seam controls cover build and verify for SHA-256 and SHA-384, with valid twins and stale-hash refusals.

The companion also changes hash-matching records missing subject/eat_profile to ContentMarkingError, consistent with other malformed fields; true hash or well-formed field disagreement remains RecordMismatch. This deliberately changes the specific exception subclass for missing fields, so please assess that compatibility choice before adopting. The old test now asserts the intended malformed-input distinction, the new cases cover absent/null fields under both algorithms, and the changelog documents it. Issue/PR numbers are removed from the two test-module docstrings.

Validation on Linux/Python 3.12.14 with the current head's hash-pinned development dependencies: 4,271 root tests passed, 48 skipped; 429 focused tests passed; repository Ruff/mypy, targeted strict mypy, dash and whitespace checks passed. Removing only the build snapshot causes 2 assertion failures; removing only the verify snapshot causes 2; restoring the old missing-field exception causes 4 failures while the 4 null-field controls pass. The repository review-bar run against the parent reports 10 failures on behavior, 0 on missing names/files, and 120 passes/skips. Published tree dcf2b1c8a0bb6af0405e857baf9227d75e5c98f8 equals the tested tree.

This supersedes my earlier no-blocker conclusion specifically for these newly identified mutable-input/error boundaries. Author incorporation, current-head CI/re-review and maintainer disposition remain open; this companion does not change the upstream PR head or merge it. Implemented with OpenAI Codex assistance.

[drk-ram]

Thanks @rajnisht7 and @dinakarjs! Adopted \7948259\ (cherry-picked from \5495993358d978eac55bfdf10455782612b99f92):

1. Single immutable \�ytes(record_bytes)\ snapshot: Both \�uild_assertion\ and \�erify_assertion\ now snapshot caller-supplied \�ytes | bytearray\ once before parsing and hashing, eliminating any TOCTOU window across the parse/hash seam (covered by \ ests/test_content_marking_snapshot.py).
2. Consistent \ContentMarkingError\ classification for hash-matching records missing required fields: A hash-matching record missing \subject\ or \�at_profile\ now raises \ContentMarkingError\ directly (matching other malformed record fields), while hash mismatches and well-formed field disagreements continue to raise \RecordMismatch.
3. Test docstrings: Removed issue and PR numbers from the top-level docstrings of \ ests/test_content_marking_consumer_boundary.py\ and \ ests/test_content_marking_review_controls.py.

[dinakarjs]

Rechecked exact head 7948259 after adoption of companion 5495993. Both public entry points take one immutable record-bytes snapshot before their parse/hash reads. Hash-matching records missing required fields now raise ContentMarkingError, consistent with other malformed fields; hash disagreement and well-formed field disagreement retain RecordMismatch. The changelog and test docstrings also reflect the intended correction.

The 429 focused content-marking/public-error/regex/untrusted-input tests pass on Linux/Python 3.12.14 with this head's hash-locked development dependencies and imports pinned to this checkout, including the deterministic build/verify seam controls for SHA-256 and SHA-384 and missing/null-field controls. All returned current-head technical workflows succeed, including ClusterFuzzLite (Dependabot auto-merge is skipped).

This closes my companion-adoption/recheck action for these two boundaries. I authored the adopted correction and tests, so this is a COMMENT rather than approval of my own contribution. Rajnish and an independent maintainer should assess the exception-subclass compatibility choice and final head before merge. No merge, release, or broader record/asset/hardware acceptance is claimed.

[imran-siddique]

Thanks @drk-ram and @dinakarjs. Approving as sponsoring maintainer for the steps 3 and 5 wording in content-marking-v1. Both of @rajnisht7's points from 18 October are closed in 7948259: build and verify now read one immutable copy of the record bytes, and a hash-matching record missing a required field raises ContentMarkingError like any other malformed field. Rejecting unpaired surrogates here matches the anchor rule in #537.