diff --git a/python/src/agent_manifest/_canonicalize.py b/python/src/agent_manifest/_canonicalize.py index d52a43c..61e07ce 100644 --- a/python/src/agent_manifest/_canonicalize.py +++ b/python/src/agent_manifest/_canonicalize.py @@ -21,11 +21,16 @@ import hashlib import math -import unicodedata from typing import Any _MAX_DEPTH = 64 # DOS-006: prevent RecursionError from deeply nested JSON +# RFC 8785 §3.2.2.3 routes every number through the ECMAScript Number type, so +# only integers exactly representable as an IEEE 754 double survive the round +# trip. Matches the rfc8785 reference implementation and trace-spec, which hit +# this first (see its changelog: "one signature stands for two records"). +_MAX_SAFE_INTEGER = 9007199254740991 # 2**53 - 1 + def canonicalize(obj: Any, *, exclude_none: bool = True) -> bytes: """Return RFC 8785 canonical JSON bytes for *obj*. @@ -82,6 +87,16 @@ def _serialize(obj: Any, *, exclude_none: bool, depth: int) -> str: # bool check must come before int — bool is a subclass of int in Python return "true" if obj else "false" if isinstance(obj, int): + if not -_MAX_SAFE_INTEGER <= obj <= _MAX_SAFE_INTEGER: + raise ValueError( + f"integer {obj} is outside the safe integer domain RFC 8785 can " + f"serialize (+/-{_MAX_SAFE_INTEGER}). RFC 8785 §3.2.2.3 serializes " + "numbers through the ECMAScript double conversion, which maps " + "9007199254740992 and 9007199254740993 to the same digits: a " + "signature over one would stand for the other. Refusing here keeps " + "this implementation byte-identical to conforming verifiers instead " + "of silently diverging from them." + ) return str(obj) if isinstance(obj, float): return _float_to_str(obj) @@ -120,9 +135,23 @@ def _serialize_dict(d: dict[str, Any], *, exclude_none: bool, depth: int) -> str def _quote(s: str) -> str: """Serialize a Python string as a JSON string per RFC 8785 §3.2.2.2. - Applies NFC normalization (spec Section 4.3) before escaping. + Deliberately does NOT normalize. RFC 8785 has no normalization step, and + spec Section 4.3 scopes NFC to *text artifacts* hashed as raw UTF-8 bytes + ("not as JSON"), which is the module docstring's "use hashlib directly" + case, not this one. Normalizing here was wrong twice over: + + - values: "café" and "café" are distinct JSON strings that + collapsed to the same canonical bytes, so one signature stood for two + documents, and no sibling implementation agreed with the result; + - 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} - invalid JSON that silently drops a + field on re-parse, signed. + + Callers that want NFC must apply it to their input before canonicalizing, + where it is a visible decision about the data rather than a hidden rewrite + of it. """ - s = unicodedata.normalize("NFC", s) buf: list[str] = ['"'] for ch in s: cp = ord(ch) @@ -140,7 +169,13 @@ def _quote(s: str) -> str: buf.append("\\r") elif ch == "\t": buf.append("\\t") - elif cp <= 0x001F or 0x007F <= cp <= 0x009F or cp in (0x2028, 0x2029): + elif cp <= 0x001F: + # Exactly the ECMAScript QuoteJSONString set, which RFC 8785 §3.2.2.2 + # defers to: control code units below 0x20, plus the two literals + # handled above. U+007F-U+009F and U+2028/U+2029 were escaped here + # and are not escaped by any conforming canonicalizer; escaping more + # than the standard is still a divergence, and a signature computed + # over the extra escapes verifies nowhere else. # Control characters and ECMAScript line terminators buf.append(f"\\u{cp:04x}") else: diff --git a/python/tests/test_canonicalize.py b/python/tests/test_canonicalize.py index db54754..ecb4004 100644 --- a/python/tests/test_canonicalize.py +++ b/python/tests/test_canonicalize.py @@ -7,6 +7,7 @@ - @context / @type as ordinary fields """ import hashlib +import json import math import pytest @@ -130,25 +131,88 @@ def test_tab_newline_escaped(): assert canonicalize({"v": "\t\n"}) == b'{"v":"\\t\\n"}' -def test_line_separator_escaped(): - # U+2028 LINE SEPARATOR must be - assert b"\\u2028" in canonicalize({"v": chr(0x2028)}) +def test_escapes_exactly_the_ecmascript_set_and_no_more(): + """RFC 8785 section 3.2.2.2 defers to ECMAScript QuoteJSONString. + + That set is the six two-character escapes, plus \\uXXXX for code units + below 0x20. Nothing else. U+2028, U+2029 and U+007F-U+009F were escaped here + on no recorded rationale: the comment on the old test stopped mid-sentence. + Escaping *more* than the standard is still a divergence, because the extra + escapes change the signed bytes, so the signature verifies under no + conforming implementation. + + Vectors below are the output of the rfc8785 reference implementation. + """ + assert canonicalize({"v": chr(0x2028)}) == b'{"v":"\xe2\x80\xa8"}' + assert canonicalize({"v": chr(0x2029)}) == b'{"v":"\xe2\x80\xa9"}' + assert canonicalize({"v": chr(0x7F)}) == b'{"v":"\x7f"}' + assert canonicalize({"v": chr(0x9F)}) == b'{"v":"\xc2\x9f"}' + # and either side of the boundary that IS escaped + assert canonicalize({"v": chr(0x1F)}) == b'{"v":"\\u001f"}' + assert canonicalize({"v": chr(0x20)}) == b'{"v":" "}' def test_regular_unicode_verbatim(): - # Non-control chars pass through after NFC normalization assert canonicalize({"v": "é"}) == '{"v":"é"}'.encode() # --------------------------------------------------------------------------- -# NFC normalization +# Normalization: the canonicalizer must not do any # --------------------------------------------------------------------------- -def test_nfc_normalization(): - precomposed = "é" # é as single code point - decomposed = "é" # e + combining accent - assert canonicalize({"v": precomposed}) == canonicalize({"v": decomposed}) +def test_does_not_normalize_values(): + """Distinct strings must stay distinct. + + This asserted the opposite until 2026-09. Spec Section 4.3 requires NFC for + *text artifacts* hashed as raw UTF-8 bytes, "not as JSON"; it was applied + inside the JSON canonicalizer, where RFC 8785 has no normalization step at + all. The effect was a signature collision: two different manifests, one set + of canonical bytes, one signature standing for both. + """ + precomposed = "caf\u00e9" # U+00E9 + decomposed = "cafe\u0301" # e + U+0301 combining acute + assert precomposed != decomposed + assert canonicalize({"v": precomposed}) != canonicalize({"v": decomposed}) + assert canonicalize({"v": decomposed}) == b'{"v":"cafe\xcc\x81"}' + + +def test_does_not_normalize_keys_into_a_duplicate(): + """The sharper half of the same bug. + + Keys sort by their pre-normalization UTF-16 encoding but were normalized at + quote time, so two distinct keys emitted the same key twice. That is not + valid JSON: a parser keeps one of the pair, so the canonical bytes did not + round-trip, and the dropped field was signed as though it were present. + """ + doc = {"cafe\u0301": 1, "caf\u00e9": 2} + out = canonicalize(doc) + assert json.loads(out) == doc + assert out == b'{"cafe\xcc\x81":1,"caf\xc3\xa9":2}' + + +# --------------------------------------------------------------------------- +# Integer domain +# --------------------------------------------------------------------------- + + +def test_rejects_integers_outside_the_safe_domain(): + """RFC 8785 routes every number through the ECMAScript double conversion. + + 2**53 and 2**53+1 are distinct Python integers that the conversion maps to + the same digits, so emitting them verbatim diverges from a conforming + verifier, and a verifier that does convert would accept one signature for + two records. trace-spec hit this first; refusing is what it settled on, and + what the rfc8785 reference implementation does. + """ + for n in (2**53, -(2**53), 2**53 + 1, 10**30): + with pytest.raises(ValueError, match="safe integer domain"): + canonicalize({"v": n}) + + +def test_accepts_the_safe_domain_boundary(): + assert canonicalize({"v": 2**53 - 1}) == b'{"v":9007199254740991}' + assert canonicalize({"v": -(2**53 - 1)}) == b'{"v":-9007199254740991}' # --------------------------------------------------------------------------- diff --git a/python/tests/test_memory_assessment_canonicalization_dependency.py b/python/tests/test_memory_assessment_canonicalization_dependency.py index 5242269..595f99f 100644 --- a/python/tests/test_memory_assessment_canonicalization_dependency.py +++ b/python/tests/test_memory_assessment_canonicalization_dependency.py @@ -1,19 +1,8 @@ from __future__ import annotations -import pytest - from agent_manifest._canonicalize import canonicalize -_REMAINING_BLOCKER = pytest.mark.xfail( - strict=True, - reason=( - "agent-manifest#322: current main still over-escapes U+2028, so the " - "shared canonicalizer is not yet fully RFC 8785 conformant" - ), -) - - def test_shared_canonicalizer_now_orders_keys_by_utf16_code_units() -> None: """Keep the resolved #322 dependency visible at the assessment boundary.""" value = {"\ue000": 2, "😀": 1} @@ -25,7 +14,11 @@ def test_shared_canonicalizer_now_normalizes_exponent_leading_zero() -> None: assert canonicalize(1e-7) == b"1e-7" -@_REMAINING_BLOCKER def test_rfc8785_does_not_overescape_line_separator() -> None: - """Retain a strict guard for the unresolved escaping axis of #322.""" + """The last escaping axis of #322, now resolved. + + U+2028 is not escaped by ECMAScript QuoteJSONString, which RFC 8785 + section 3.2.2.2 defers to, so escaping it here diverged from every + conforming implementation. + """ assert canonicalize({"value": "\u2028"}) == '{"value":"\u2028"}'.encode()