fix: make canonicalize() RFC 8785 conformant (#322)

[SnehD]

What

canonicalize() is the single RFC 8785 pre-image used for manifest signatures, COSE signing, delegation chains, revocation records, memory deltas, plugin bundles, and TRACE envelopes. It was not RFC 8785 conformant.

Why

Fixes #322. Four conformance bugs, all in _canonicalize.py:

1. Sorted object keys by Unicode code point instead of UTF-16 code unit (RFC 8785 §3.2.3) — diverges when a key contains a supplementary-plane character.
2. Escaped U+007F, U+0080-U+009F, U+2028 and U+2029, which are outside ECMAScript JSON.stringify's escape set (RFC 8785 §3.2.2.2).
3. Float formatting used ad-hoc shortcuts instead of ECMAScript Number::toString (RFC 8785 §3.2.2.3): wrong integer/exponential cutover, wrong exponent padding, wrong small-magnitude threshold.
4. Silently serialized integers beyond the IEEE-754 safe range instead of refusing them.

Spec impact

No normative text changed. Brings the implementation into conformance with the existing RFC 8785 requirement in Section 2.3. Conformance test AM-CRYPTO-001 is directly affected — this fix is required for that test to be meaningful.

Fix

• UTF-16 code-unit sort key for object keys.
• Escape set corrected to quote, reverse solidus, and U+0000-U+001F only.
• Full ECMA-262 Number::toString algorithm via Decimal(repr(f)).normalize().
• Integers with abs(value) > 2**53-1 now raise ValueError.

Test plan

• pytest tests/test_canonicalize.py -q → 57 passed (removed the test asserting the old non-conformant escaping behavior; added tests for UTF-16 key order, the number-formatting table, and the safe-integer bound)
• pytest tests/interop/test_trace_canonicalization_boundary.py -v → passed against 4 real signed vectors vendored from agentrust-io/trace-spec (fetched via git clone, not retyped)
• Load-bearing check: reverted _canonicalize.py only, reran the guard — 03-utf16-key-order.json failed exactly as reported in agent-manifest rejects a TRACE record that trace-spec signed: the canonical form is not RFC 8785 #322; restored, passes
• Full suite: 1096 passed, 6 skipped, 0 failed (no regressions)
• mypy src/agent_manifest — not run in this environment
• ruff check src/ tests/ — not run in this environment
• CHANGELOG.md — not yet updated

Compatibility note

This changes canonical bytes for the affected value classes (supplementary-plane keys, values needing >2^53 or specific exponent ranges, and the four literal characters above). Anything already signed at one of those values stops verifying against itself under the corrected canonicalizer — though it was already unverifiable against any conformant implementation, which is the point.

DCO

• Sign-off not yet added to these commits — will add before merge if required.

[imran-siddique]

This is careful work and three of the four fixes are right and worth landing. The fourth is not conformance, and I want to settle all of it in one pass rather than send you round twice.

The three that are correct

UTF-16 code-unit key order, the escape set narrowed to quote, reverse solidus and U+0000-U+001F, and the full ECMA-262 Number::toString are genuine RFC 8785 requirements and the previous implementation was wrong on each. Vendoring trace-spec's four signed vectors as a cross-repository guard is the right instrument, and proving it load-bearing by reverting _canonicalize.py and watching 03-utf16-key-order.json fail is what makes the claim checkable rather than plausible. Removing test_line_separator_escaped is correct too: U+2028 is a hazard when JSON is pasted into JavaScript source, not a JSON serialization rule, and that test was pinning the bug in place.

The fourth: refusing integers past 2^53-1 is not RFC 8785

I went to the RFC rather than reasoning about it. Appendix B, note (1):

For maximum compliance with the ECMAScript "JSON" object, values that are to be interpreted as true integers SHOULD be in the range -9007199254740991 to 9007199254740991. However, how numbers are used in applications does not affect the JCS algorithm.

Note (2) then puts 2^68 in the reference vector table specifically to say the algorithm does not treat it as special. So JCS serializes these; it does not refuse them. The comment in the diff, "RFC 8785 numbers must round-trip through a double", is not something the RFC says.

And the implementation disagrees with itself on the RFC's own published value. I ran your _canonicalize.py against exactly the Appendix B entry:

json.loads('{"v":295147905179352830000}')   ->  Python int
  canonicalize(...)  ->  ValueError: exceeds the IEEE-754 safe integer range

the same value as the double behind Appendix B's hex 4430000000000000
  canonicalize(...)  ->  b'{"v":295147905179352830000}'   (correct)

Same number from the spec's own test data, opposite outcomes, decided by which Python type it happened to arrive as. That inconsistency exists independently of whether you agree with me about the RFC.

The practical cost is not hypothetical either: a nanosecond epoch timestamp is about 1.79e18 today, so an ordinary field value now raises inside the single pre-image used for every signature in this SDK.

If you want a safe-range restriction as policy, that is a defensible position, and it belongs as an explicit opt-in with its own name and its own reason, not inside the function that claims to implement JCS. Please drop it from this PR.

The ADR hunk is stale, which is why this conflicts

Both this and #339 say "These are the only two Merkle hash operations selected by this ADR." #335 landed on main yesterday and added a third, the composite policy sub-bundle leaf in Section 3.2.2, which carries raw digest bytes rather than a JSON descriptor. The sentence is now false and the file has moved under you. Please rebase and re-word against the three operations that exist.

The rest of that hunk, pointing Sections 3.2.3 and 3.2.5.1 at the normative definitions and scoping RFC 8785 to where those sections define JSON as a hash input, is good and I would keep it.

What I would like back

The three conformance fixes, the vendored vectors, and the corrected ADR wording, rebased onto current main, with the integer refusal removed. That is mergeable and I will take it. Thank you for filing #322 as an issue first and asking about timing rather than opening this cold, that was the right instinct on a change at the signature layer.

[imran-siddique]

You took the ruling and the code is right. One mechanical problem is blocking it, and it is separable from everything you fixed.

The canonicalization fix is verified correct

Ran it rather than reading it:

• isinstance(obj, int): return str(obj). The refusal is gone.
• The RFC 8785 Appendix B value that raised yesterday now serializes: canonicalize(json.loads('{"v":295147905179352830000}')) returns b'{"v":295147905179352830000}'. That is the reference vector's own expected output.
• The three fixes that were always right survived: safe-range integers serialize unchanged, and UTF-16 key order still sorts U+10000 before U+FFFD.
• tests/test_canonicalize.py plus tests/interop/test_trace_canonicalization_boundary.py: 58 passed.

Thank you for taking the correction on the integer domain without argument. Three of four right on a change to the signing pre-image is a good PR, and the fourth was subtle enough that it took reading Appendix B's notes rather than its body to settle.

What is blocking: the ADR file has committed conflict markers

docs/adr/0003-rfc9162-merkle-domain-separation.md on this head contains nine conflict-marker lines, including a <<<<<<< HEAD nested inside another, from two separate merges (a43da11 and 3bb3ee5a). Both the two-operation and three-operation sentences are present, unresolved, alongside two copies of the leaf-data paragraph.

There is also a corrupted line 60, a half-paste of the intact line 66:

id`. Composite policy sub-bundles sorted by policy language identifer (`cear`, `rego`, `yaml-agt), per Section 3.2.2

Three typos and an unclosed backtick. And the header now lists the composite policy bundle twice, once as "Section 2.2.3".

What I would like

Revert docs/adr/0003-rfc9162-merkle-domain-separation.md to main and push. That is the only change needed; everything else here is ready and I will merge it.

git checkout 02632d53a263 -- docs/adr/0003-rfc9162-merkle-domain-separation.md

The ADR wording is worth having and I still want it, as its own PR against current main. It is a docs change with no dependency on the canonicalization work, and keeping them together means a verified fix to the signing pre-image waits on a merge artifact in a markdown file.

For that separate PR, the substance we agreed: three operations rather than two, and Sections 3.2.2, 3.2.3 and 3.2.5.1 named as the normative definitions with RFC 8785 scoped to where those sections define JSON as a hash input. Your line about the composite sub-bundle leaf using raw digest bytes and no JSON canonicalization is a good addition and I would keep it.

One piece of cruft to sweep while you are in there: _MAX_SAFE_INTEGER at _canonicalize.py:30 is now defined and never referenced.

[devdiv07]

While checking this head for any residual RFC 8785 differences, I found one historical behavior that I’m not sure is still intended.

_quote() still applies unicodedata.normalize("NFC", s), and test_nfc_normalization pins precomposed and decomposed Unicode to the same canonical bytes.

I checked the history before raising it: #3 explicitly required “Unicode strings in NFC normalization”, so this looks like an intentional historical requirement rather than an implementation slip.

The current contract seems different, though. RFC 8785 §3.1 says parsed JSON string data MUST NOT be altered during subsequent serializations, and specifically says JCS does not apply Unicode normalization — implementations must preserve Unicode string data “as is.” ADR-0001/current spec require RFC 8785 for canonical JSON; the NFC rules I could find are for raw text-artifact hashing rather than JSON canonicalization.

I reproduced this through the public API on main (605946b) and this head (1043afb) using:

precomposed: U+00E9
decomposed: U+0065 U+0301

The RFC 8785 reference preserves distinct bytes; agent_manifest.canonicalize() collapses them in both value and key positions.

For the decomposed value:

reference preimage: 7b2276223a2265cc81227d
agent-manifest preimage: 7b2276223a22c3a9227d

Signing the reference preimage verifies over those producer bytes but fails over Agent Manifest’s reconstructed bytes.

I also removed only the NFC-normalization line in a disposable copy of this head: the decomposed case then matched the reference, and the canonicalization suite changed from 57 passed to 56 passed / 1 failed — the only failure was test_nfc_normalization. The TRACE boundary guard added here still passed.

So the observed impact is a fail-closed interoperability/conformance case for non-NFC input, not a signature bypass.

Is #3’s NFC rule still an intentional Agent Manifest profile deviation, or should it be removed as part of the RFC 8785 conformance work here? If the deviation is intentional, it looks like the current spec/ADR may need to state that exception explicitly.

[SnehD]

Thanks for the detailed investigation. I agree that the remaining NFC normalization appears to be a pre-existing behavior introduced in #3 and is separate from the RFC 8785 fixes addressed in #322. Since removing it would change canonical bytes and could affect compatibility with existing signatures, we'd appreciate maintainer guidance on whether this behavior should remain as an intentional project-specific deviation or be removed for strict RFC 8785 compliance.

…

On Thu, Aug 27, 2026 at 10:06 PM Divyansh Shukla ***@***.***> wrote: *devdiv07* left a comment (agentrust-io/agent-manifest#338) <#338 (comment)> While checking this head for any residual RFC 8785 differences, I found one historical behavior that I’m not sure is still intended. _quote() still applies unicodedata.normalize("NFC", s), and test_nfc_normalization pins precomposed and decomposed Unicode to the same canonical bytes. I checked the history before raising it: #3 <#3> explicitly required “Unicode strings in NFC normalization”, so this looks like an intentional historical requirement rather than an implementation slip. The current contract seems different, though. RFC 8785 §3.1 says parsed JSON string data MUST NOT be altered during subsequent serializations, and specifically says JCS does not apply Unicode normalization — implementations must preserve Unicode string data “as is.” ADR-0001/current spec require RFC 8785 for canonical JSON; the NFC rules I could find are for raw text-artifact hashing rather than JSON canonicalization. I reproduced this through the public API on main (605946b <605946b>) and this head (1043afb <1043afb>) using: precomposed: U+00E9 decomposed: U+0065 U+0301 The RFC 8785 reference preserves distinct bytes; agent_manifest.canonicalize() collapses them in both value and key positions. For the decomposed value: reference preimage: 7b2276223a2265cc81227d agent-manifest preimage: 7b2276223a22c3a9227d Signing the reference preimage verifies over those producer bytes but fails over Agent Manifest’s reconstructed bytes. I also removed only the NFC-normalization line in a disposable copy of this head: the decomposed case then matched the reference, and the canonicalization suite changed from 57 passed to 56 passed / 1 failed — the only failure was test_nfc_normalization. The TRACE boundary guard added here still passed. So the observed impact is a fail-closed interoperability/conformance case for non-NFC input, not a signature bypass. Is #3 <#3>’s NFC rule still an intentional Agent Manifest profile deviation, or should it be removed as part of the RFC 8785 conformance work here? If the deviation is intentional, it looks like the current spec/ADR may need to state that exception explicitly. — Reply to this email directly, view it on GitHub <#338?email_source=notifications&email_token=AVNQU6LBS4KC2I44NHAZQET5MBPQLA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBUGIZDAMJUGE4KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5442201418>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVNQU6J4PGDUFOGSWXGWL735MBPQLAVCNFSNUABGKJSXA33TNF2G64TZHMYTENJYGY2DSNBWG45US43TOVSTWNJSGM3TOMJYHEZDRILWAI> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/AVNQU6JT5TP5C5Y2DPZ6EF35MBPQLA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBUGIZDAMJUGE4KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android <https://github.com/notifications/mobile/android/AVNQU6JLDTID2E3GCRFDVE35MBPQLA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBUGIZDAMJUGE4KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today! You are receiving this because you authored the thread.Message ID: ***@***.***>

[imran-siddique]

@SnehD Update on this, and a change of shape rather than a change of verdict.

Your canonicalization work is right. My 25 August review already confirmed the integer domain was fixed and 58 tests passed. Nothing has changed about that. What has kept this PR open for four days is not the code, it is the nine committed conflict markers in docs/adr/0003-rfc9162-merkle-domain-separation.md.

In the meantime #352 arrived with the same two fixes, scoped to _canonicalize.py and its test file only. I ran both implementations against the RFC 8785 Appendix B table from the IEEE-754 hex values before deciding anything: you both score 10 of 10, identically, including 5e-324, 2^68 as 295147905179352830000, and both boundary pairs. You differ only in how you express the UTF-16 sort, yours building a code-unit tuple and theirs encoding to UTF-16BE. Both correct.

I have merged #352, because it was unblocked and because a conformance defect in the signing pre-image should not wait on an ADR file. That is not a judgement between the two implementations and it is worth saying plainly that you reached this answer first, on 24 August.

What I would like from this PR is the part #352 does not carry, which is the more valuable half and is entirely yours:

• docs/adr/0003-rfc9162-merkle-domain-separation.md, with the markers resolved
• python/tests/interop/vectors/canonicalization-boundary/*.json, all four
• python/tests/interop/test_trace_canonicalization_boundary.py

Rebase onto current main and drop python/src/agent_manifest/_canonicalize.py and python/tests/test_canonicalize.py from the diff; those two are now redundant and will conflict. The float and UTF-16 assertions in test_canonicalize.py are the ones that became duplicative. Your interop vectors are not duplicative and I want them: they are the only thing in either PR that pins the boundary as data rather than as assertions in a test body.

On the ADR itself, the substance we agreed still stands: three operations rather than two, Sections 3.2.2, 3.2.3 and 3.2.5.1 named as the normative definitions, and your line about the composite sub-bundle leaf using raw digest bytes with no JSON canonicalization. Rewriting the file from your intended text will be faster than untangling the nested markers.

_MAX_SAFE_INTEGER at _canonicalize.py:30 is now gone from main with the merge, so that piece of cruft is handled.

[devdiv07]

@imran-siddique One small scope check before _canonicalize.py is dropped from #338. I may be missing a follow-up change, but on current main after #352, _quote() still has:

  elif cp <= 0x001F or 0x007F <= cp <= 0x009F or cp in (0x2028, 0x2029):

#338 is the change that narrows this to cp <= 0x001F, and your earlier review identified that escape-set correction as one of the RFC 8785 fixes.

So I just wanted to check whether dropping _canonicalize.py entirely from #338 would leave that part of the fix behind.

Separately, the NFC normalization discussed above also still appears to remain on current main:

    s = unicodedata.normalize("NFC", s)

Since SnehD already noted that the NFC behavior may be compatibility-sensitive and needs maintainer guidance, would the intended split be to keep the escape-set correction in #338, and handle the NFC question separately once the project decides whether #3 remains an intentional deviation?

Just checking the intended scope before the rebase.

[devdiv07]

@SnehD heads-up on the rebased head aa8867e --> I think “drop these from the diff” may have landed as “delete the files.”

The new interop test in this PR imports:

from agent_manifest._canonicalize import canonicalize

but _canonicalize.py is absent from the branch tree, so that test cannot collect.

Against current main, both files are showing as deletions:

 D  python/src/agent_manifest/_canonicalize.py
 D  python/tests/test_canonicalize.py

Main still has both of the #352 versions, so I think the intended state was to carry those files over unchanged and have them simply disappear from the PR diff, rather than delete them from the branch.

That would leave #338 carrying the ADR clarification plus the interop vectors/test, which I think matches the scope requested above.

Flagging it because the current head's interop test depends on a module that the same head removes.