fix: reading third-party chain data must be lenient (#576) - #587
Merged
Conversation
pyrxd silently refused about a quarter of live mainnet glyph payloads. Two write-side limits fired on the READ path, and each discarded the ENTIRE token: - `GlyphMedia.__post_init__` capped media at 100,000 bytes - `_cbor_str` raised on a wrong-typed or over-long field `GlyphInspector.extract_reveal_metadata` catches Exception and returns None, so the user saw `metadata: NONE` — not a refusal naming a field they could act on. MEASURED against an independent verifier on live mainnet: 6 of 25 sampled `gly` payloads were refused by pyrxd and decoded fine. Four were webp images of 153,650 / 178,608 / 236,726 bytes. Two were Photonic-minted relationship glyphs carrying `loc` as an INTEGER where our own spec says text — and Photonic is the reference implementation, so the chain wins. The media cap guarded nothing pyrxd writes: the DECODER is the only code in the tree that constructs a GlyphMedia. It also contradicted its own outer bound — `_MAX_CBOR_PAYLOAD_BYTES` was deliberately raised to 256 KB to admit a real 65,569-byte payload, and the inner 100 KB cap made that headroom unreachable for any media-bearing token. BOTH SECURITY PROPERTIES SURVIVE, and are still asserted. The threat MEDIUM-5 named was type confusion — "must raise, not silently become str(42)='42'" — and dropping does not coerce. The length caps exist so nothing oversized reaches a display path, and an empty string satisfies that exactly as a refusal did. What raising bought was neither; it cost availability, on somebody else's data. The security-audit tests are CONVERTED, not deleted: each now asserts the property it was written to protect (not coerced / not propagated) against the new failure mode, plus a case proving one bad field no longer discards the token, plus a case pinning that the DoS bound still fires at the payload cap. docs/reference/glyph-token-protocol-spec.md §4.4-4.5 corrected. An earlier revision said pyrxd "decodes media it would refuse to construct" — which was RIGHT — and a previous change rewrote the spec to match the code instead of fixing the code. A test even recorded doing so in its docstring. The spec was made to follow the defect; it leads again. Plant-verified both directions: restoring the media cap fails 3 tests, restoring the raise fails 4. CI-equivalent: 10,923 passed, 192 skipped, 1 xfailed. Read-path leniency does not extend to writing: nothing on the encode path calls either helper, and the 256 KB payload cap is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <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.
Closes #576.
pyrxd silently refused about a quarter of live mainnet glyph payloads. Two write-side limits fired on the READ path, and each discarded the entire token — then
extract_reveal_metadatacaught the exception and returnedNone, so the user sawmetadata: NONErather than a refusal naming a field.Measured against an independent verifier on live mainnet: 6 of 25 sampled
glypayloads refused by pyrxd, decoded fine by the other implementation.GlyphMedia100 KB caplocas an integer_cbor_strtype raisePhotonic is the reference implementation, so on
locthe chain wins over our spec.The media cap guarded nothing we write
The decoder is the only code in the tree that constructs a
GlyphMedia. It also contradicted its own outer bound:_MAX_CBOR_PAYLOAD_BYTESwas deliberately raised to 256 KB to admit a real 65,569-byte payload, and the inner 100 KB cap made that headroom unreachable for any media-bearing token.Both security properties survive, and are still asserted
The threat MEDIUM-5 named was type confusion — "must raise, not silently become
str(42)='42'". Dropping does not coerce. The length caps exist so nothing oversized reaches a display path, and an empty string satisfies that exactly as a refusal did. What raising bought was neither; it cost availability, on somebody else's data.The security-audit tests are converted, not deleted — each asserts the property it was written to protect against the new failure mode, plus a case proving one bad field no longer discards the token, plus one pinning that the DoS bound still fires at the payload cap.
The spec was made to follow the defect
glyph-token-protocol-spec.md§4.4 once said pyrxd "decodes media it would refuse to construct" — which was right. A previous change rewrote the spec to match the code rather than fixing the code, and a test docstring recorded doing exactly that. Both are corrected; the spec leads again.Verification
Plant-verified both directions — restoring the media cap fails 3 tests, restoring the raise fails 4. CI-equivalent locally: 10,923 passed, 192 skipped, 1 xfailed, no unexpected skips.
Read-path leniency does not extend to writing: nothing on the encode path calls either helper, and the 256 KB payload cap is unchanged.
🤖 Generated with Claude Code