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

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:

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}'
 
 
 # ---------------------------------------------------------------------------

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()