security: warn and self-test when CBOR_C_EXTENSION=1#496
Open
OscarOzaine wants to merge 1 commit into
Open
Conversation
PyCardano anchors transaction identity in CBOR bytes (tx.id = blake2b of the tx body's CBOR) and defaults to the pure-Python cbor2pure backend so decode->re-encode stays byte-stable. The C cbor2 extension does not guarantee preservation of map/set element order on decode (cbor2 issue Python-Cardano#311), so enabling CBOR_C_EXTENSION=1 can silently change a re-serialized transaction's id and cause a node to reject or misattribute it. This was a latent, entirely silent risk: an operator setting the flag "for speed" got no signal. This change adds two layered guard rails when the flag is set: - Emit a RuntimeWarning and a logging.warning (package "PyCardano" logger) at import, so the hazard surfaces in both dev/test harnesses and production log pipelines, once per process. - Run a dependency-free import-time round-trip self-test against a tiny golden tag-258 set vector, mirroring PyCardano's real config (258 semantic decoder removed). If the active cbor2 build is not byte-stable it raises PyCardanoException so the process fails fast instead of silently producing wrong tx ids. Gated behind PYCARDANO_CBOR_SELFTEST (default on when the C extension is requested; opt out with 0/false/no). Default behavior (flag unset) is byte-identical to before: no warning, no self-test, no public API change. Implements docs/specs/spec-03-cbor-c-extension-warning.md. Co-Authored-By: Claude Opus 4.8 <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
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.
Background
A transaction's id is the blake2b hash of its body's CBOR bytes, so those bytes must be reproducible. The C
cbor2extension does not guarantee map/set element order is preserved on decode, so decoding and re-serializing a transaction can change its id — and a node would then reject or misattribute it.That's why PyCardano defaults to the pure-Python
cbor2purebackend. But users can still opt into the faster C extension withCBOR_C_EXTENSION=1, with no warning that it's unsafe for transaction workflows.What this adds
When
CBOR_C_EXTENSION=1is set, PyCardano now:RuntimeWarningand aloggingwarning explaining the hazard and pointing to the README.[3, 1, 2]in non-sorted order) is decoded then re-encoded. A correct backend returns the exact same bytes; an order-losing build does not. If the round-trip isn't byte-stable, it raisesPyCardanoExceptionso the process fails fast instead of silently producing wrong transaction ids.The self-test can be disabled with
PYCARDANO_CBOR_SELFTEST=0for users who have verified their workflow never decodes-then-reserializes.What does not change
With the flag unset (the default), behavior is identical — no warnings, no self-test, no API changes.
Tests
New tests in
test/pycardano/test_cbor.pycover the warning, the self-test pass/fail paths, and the opt-out env var. README and FAQ are updated.