fix(jcs): reject lone surrogates in canonicalization (RFC 8785)#3
Merged
Conversation
A lone/unpaired UTF-16 surrogate has no valid UTF-8 encoding, so RFC 8785 requires rejecting the input. canonicalize_jcs previously returned a surrogate string that crashed later at UTF-8 encoding. Add _assert_no_lone_surrogate (typed JCSCanonicalizationError, a ValueError subclass so fail-closed handlers keep working) on every string value and object key; keys are checked before the utf-16-be sort that itself raises on surrogates. A valid surrogate pair is unchanged. Full suite 725 -> 732 passed (+7 new), 3 skipped unchanged; no existing hash changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…785)
Python already rejects lone surrogates soundly (json.loads preserves lone
surrogate escapes, so the detector sees them). This v2 adds the cross-language
error contract and the consilium's test list; it does not change detection.
- JCSCanonicalizationError gains a stable category ('invalid_unicode') and
reason ('lone_surrogate'), matching the TypeScript and Go SDKs. It still
subclasses ValueError, so existing fail-closed handlers keep working.
- Tests: lone surrogate rejected in nested value, array element, and nested key;
valid-pair-then-lone-low still rejected; error-contract asserts ValueError
catch + category/reason + no leaked offending string; signing-boundary asserts
a real content-id API (compute_action_ref) fails closed.
No detection change; full suite green (738 passed, 3 skipped), no hash changed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins the raw JSON payloads from the Go scanner tests through json.loads + canonicalize_jcs: non-adjacent (space/newline) surrogate escapes, low-first, high-then-literal-low, lone-in-key, lowercase hex, and literal-backslash+lone all reject; valid pair, escaped-backslash and double-backslash literals, and a genuine U+FFFD accept. Python preserves a lone surrogate through parsing and rejects at canonicalization, matching the Go raw-JSON path. Test-only; full suite green (750 passed, 3 skipped). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rejects strings containing an unpaired UTF-16 surrogate during JCS canonicalization, matching RFC 8785 and I-JSON: a lone surrogate has no valid UTF-8 encoding, so canonicalization terminates with a typed, catchable error. A valid surrogate pair and a genuine U+FFFD are unchanged.
Motivation: the three SDK canonicalizers previously diverged on this input (TypeScript emitted a \ud800 escape, Go substituted U+FFFD, Python raised at UTF-8 encode), so the same payload produced different canonical bytes across languages, which breaks the byte-identical property that signing relies on. All three now reject, restoring cross-language agreement.
Coverage:
Tested: adversarial scanner cases (non-adjacent escapes, low-surrogate-first, lone-in-key, case variants, backslash parity), cross-language agreement on both reject and valid inputs, and full suites per repo with no existing valid-input hash changed. Reviewed before opening.
Part of a coordinated fix across the TypeScript, Python, and Go SDKs.