Skip to content

fix(sdk): make canonicalize() RFC 8785 conformant (closes #322) - #404

Merged
imran-siddique merged 1 commit into
mainfrom
fix/jcs-nfc-and-integer-domain
Sep 7, 2026
Merged

fix(sdk): make canonicalize() RFC 8785 conformant (closes #322)#404
imran-siddique merged 1 commit into
mainfrom
fix/jcs-nfc-and-integer-domain

Conversation

@imran-siddique

Copy link
Copy Markdown
Member

Closes #322.

#322 reported that agent-manifest rejects a TRACE record that trace-spec signed. The key-ordering half was fixed at the time. The escaping half stayed open behind a strict=True xfail in test_memory_assessment_canonicalization_dependency.py, which called the canonicalizer "not yet fully RFC 8785 conformant". Closing that half surfaced two more divergences, both with worse consequences than the one on the ticket.

I built a differential harness against the rfc8785 reference implementation and ran every canonicalizer in the org through it. 30,000 randomised documents, over a charset of DEL, C1 controls, U+2028/U+2029, BOM, soft hyphen, combining marks and supplementary-plane characters, now agree byte for byte. Three classes did not before.

1. NFC normalization, removed

_quote() ran unicodedata.normalize("NFC", s) on every JSON string. Spec Section 4.3 requires NFC for text artifacts hashed as raw UTF-8 bytes, "not as JSON" (spec line 1274), and line 1262 says canonical JSON is RFC 8785 and implementations "MUST NOT use any other canonicalization standard". RFC 8785 has no normalization step. The module docstring already stated the correct rule, four lines above the code that broke it.

Two consequences, both demonstrable on main:

Values collide. "café" (U+00E9) and "café" (e + U+0301) are distinct JSON strings that canonicalized to the same bytes, so one signature stood for two documents.

Keys can duplicate. Keys sort by their pre-normalization UTF-16 encoding but were normalized at quote time. Those two as sibling keys emitted:

{"café":1,"café":2}

That is not valid JSON. json.loads keeps one of the pair, so the signed canonical bytes did not round-trip, and the dropped field was covered by the signature as though it were present.

2. Escape set narrowed to the ECMAScript set

U+2028, U+2029 and U+007F to U+009F were escaped. RFC 8785 §3.2.2.2 defers to ECMAScript QuoteJSONString, which escapes the six two-character escapes plus \uXXXX below 0x20, and nothing else. There was no recorded rationale: the comment on the old test stopped mid-sentence at "U+2028 LINE SEPARATOR must be". Escaping more than the standard is still a divergence, because the extra escapes change the signed bytes.

3. Integers above 2**53-1 now refused

RFC 8785 routes every number through the ECMAScript double conversion, which maps 9007199254740992 and 9007199254740993 to the same digits. agent-manifest emitted them verbatim, so it diverged from a conforming verifier, and a verifier that does the conversion would accept one signature for two records. trace-spec hit this first and settled on refusing (its changelog: "one signature stands for two records"). The rfc8785 reference refuses too. ca2a has the same gap and gets the same fix separately.

Compatibility

Documents made only of NFC text, with no C1 or U+2028/9 characters and integers inside the safe domain, canonicalize to exactly the bytes they did before. Existing manifests are unaffected. The bytes that change are the ones that no other implementation agreed with anyway.

Full suite passes unchanged: 1499 passed, 6 skipped, including signature round-trips and golden vectors.

Tests

Two tests asserted the old behaviour and now assert conformance. test_nfc_normalization asserted the collision directly, which is why it never caught it. Added coverage for the duplicate-key case, the integer domain and its boundary, and both sides of the 0x20 escape cutoff. Vectors are rfc8785 output.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XbDBXDWWvMFa7c2jGgyq9t

#322 reported that agent-manifest rejects a TRACE record trace-spec signed.
Key ordering was fixed then; the escaping axis stayed open behind a strict
xfail. Closing it surfaced two more divergences, both worse than the one
being tracked.

Verified against the rfc8785 reference implementation: 30,000 randomised
documents over a charset of DEL, C1, U+2028/9, BOM, soft hyphen, combining
marks and supplementary-plane characters now agree byte for byte. Before this
change, three classes did not.

1. NFC normalization, removed. _quote() normalized every JSON string. Spec
   Section 4.3 requires NFC for *text artifacts* hashed as raw UTF-8 bytes,
   "not as JSON", which the module docstring already said; RFC 8785 has no
   normalization step. Two consequences:

     values: "café" and "café" are distinct JSON strings that
     canonicalized to the same bytes, so one signature stood for two
     documents.

     keys: keys sort by their pre-normalization UTF-16 encoding but were
     normalized at quote time, so those two as sibling keys emitted
     {"café":1,"café":2}. That is not valid JSON. A parser keeps
     one of the pair, so the signed canonical bytes did not round-trip and
     the dropped field was covered as though present.

2. Escape set, narrowed to ECMAScript QuoteJSONString. U+2028, U+2029 and
   U+007F-U+009F were escaped on no recorded rationale; the comment on the
   old test stopped mid-sentence. Escaping more than the standard is still a
   divergence, since the extra escapes change the signed bytes.

3. Integer domain, now refused above 2**53-1. RFC 8785 routes numbers through
   the ECMAScript double conversion, which maps 9007199254740992 and
   9007199254740993 to the same digits. Emitting them verbatim diverged from
   conforming verifiers, and a verifier that does convert would accept one
   signature for two records. trace-spec hit this first and settled on
   refusing, as does the rfc8785 reference.

Documents made only of NFC text, no C1/U+2028/9, and integers inside the safe
domain canonicalize to exactly the bytes they did before, so existing
manifests are unaffected. The full suite (1499 tests, including signature
round-trips and golden vectors) passes unchanged.

Two tests asserted the old behaviour and were rewritten to assert conformance;
one of them, test_nfc_normalization, asserted the collision directly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbDBXDWWvMFa7c2jGgyq9t
@imran-siddique
imran-siddique requested a review from a team as a code owner September 7, 2026 23:37
imran-siddique added a commit to agentrust-io/ca2a that referenced this pull request Sep 7, 2026
canonical.py already refuses floats, on the grounds that serializing them
approximately would break the cross-verifiable signature contract its module
docstring promises with agent-manifest. Unbounded integers break the same
contract by the same mechanism and were accepted.

RFC 8785 section 3.2.2.3 routes every number through the ECMAScript Number
type. 9007199254740992 and 9007199254740993 both serialize to
"9007199254740992" under that conversion, so emitting large integers verbatim
diverges from a conforming verifier, and a verifier that does apply the
conversion would accept one signature for two distinct values.

Refusing above 2**53-1 matches the rfc8785 reference implementation, trace-spec
(which hit this first) and agent-manifest, whose corresponding fix is in
agentrust-io/agent-manifest#404.

564 passed, 2 skipped.


Claude-Session: https://claude.ai/code/session_01XbDBXDWWvMFa7c2jGgyq9t

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imran-siddique
imran-siddique merged commit f883eca into main Sep 7, 2026
15 checks passed
@imran-siddique
imran-siddique deleted the fix/jcs-nfc-and-integer-domain branch September 7, 2026 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

agent-manifest rejects a TRACE record that trace-spec signed: the canonical form is not RFC 8785

1 participant