Repository navigation
feat!: make the invalid Value shapes unrepresentable - #56
Merged
Merged
Conversation
#52 made the canonicalizer fail closed on a hand-built BigInt that is not a digit run and on a Map with a repeated key. This removes the possibility of building them at all, so the "closed data set" holds for the public Rust API and not only on the reader path. BREAKING (public Rust API; the README states nothing here is stable): - Value::BigInt now carries BigIntValue, a newtype with a private field. BigIntValue::try_new validates an optional sign followed by ASCII digits; canonical() (A3's normal form) is infallible. The reader builds it through try_new. - Map's storage is private. Map::new, Map::from_pairs (refuses a repeated key), get_mut, iter and IntoIterator replace direct access to the Vec; push is crate-private and debug-asserts the uniqueness its callers claim; insert still replaces. With the types carrying the invariant, the #52 guards in the canonicalizer become unreachable and are removed together with the expect() calls they replaced (canonical, conformance, collection). canonical_integer's logic lives once, in BigIntValue::canonical. The constructors raise E_MALFORMED_VALUE (R-VALUE-BIGINT-DIGITS, R-VALUE-UNIQUE-KEYS, stage read). compile_fail doctests pin that neither type can be built around its constructor; engine/tests/value_invariants.rs covers the constructors and the consumers through the public API. Mutating each guarantee (public field, constructor check off) fails a test or doctest. Closes #53 --- [signing-metadata] key = cic-my-sign-key signature = vault:v1:MEYCIQDHqqlrPL3VQOER1UxHQdaiMvBLZFcfq2bwBJaFfGn0rgIhAPokO42sm4TlNSMguvGaIv3KEkdqq6Wu8eEcBSwtFEpg hash-algorithm = sha256 digest = TPhCmwsBD0+kiaAKAa5rSkhbH85LlkfUREN4Qkrp4Yo= [certificate] -----BEGIN CERTIFICATE----- MIICBjCCAaygAwIBAgIUSnRMR6RPnEbg296XWPOqq/u5PCwwCgYIKoZIzj0EAwIw QzELMAkGA1UEBhMCSFUxGTAXBgNVBAoMEENlbnRyYWxJbmZyYUNvcmUxGTAXBgNV BAMMEENJQyBEZXZlbG9wZXIgQ0EwHhcNMjYwMzIwMTMyMjU5WhcNMjYxMjMxMTMy MjU5WjBFMQswCQYDVQQGEwJIVTEZMBcGA1UECgwQQ2VudHJhbEluZnJhQ29yZTEb MBkGA1UEAwwSR2Fib3IgWm9sdGFuIFNpbmtvMFkwEwYHKoZIzj0CAQYIKoZIzj0D AQcDQgAEIG2CVmTfmLB9pLLclj7YmP2eedAjklpy4LGrU2ijoiy6Xqpuybv7OgJe i+ez31s65NEV8+X/ByeX1cstR988z6N8MHowCQYDVR0TBAIwADAdBgNVHQ4EFgQU yZN6AIX/TNnIJ9GwAa/NRN3ujHAwHwYDVR0jBBgwFoAUXn6CHYzPUqU4JVP8g+OS WeDYjhcwDgYDVR0PAQH/BAQDAgeAMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEF BQcDBDAKBggqhkjOPQQDAgNIADBFAiEA+bFzXRoJ4PCQbhAAtpkcMjt0vNj5rEW0 lOMBGDNyaWkCIB1vmM7PcZzv/c9bIrxF5kqv6QXomouhByUfeNUTbpKW -----END CERTIFICATE-----
This was referenced Oct 17, 2026
…ebug Map::push relied on its caller having proven the key new and checked that with debug_assert!, so a release build would silently keep a repeated key. The one caller that handles arbitrary untrusted keys, the reader, was covered only by scan_input's event-level duplicate refusal. I probed that scan with 15 spellings of a repeated key (plain, quoted, escaped, hex-escaped, flow, explicit, tagged, nested, in a sequence, merge keys, multi-line keys) in a release build: every one is refused, so this was not exploitable. It is hardening so the type's guarantee does not rest on a scan plus a debug-only assertion. - reader: build the mapping through Map::from_pairs (linear, hash-set based) and report a repeat as R-READ-DUPLICATE-KEY like the scan does. It cannot fire today; it makes the uniqueness structural. - Map::push: assert! instead of debug_assert!, documented as crate-private with a linear check. Remaining callers add a handful of literal keys. - test: push_refuses_a_repeated_key_in_every_profile. It passes in debug even with the weak debug_assert! and fails only in release, so the gate needs a release test run (added with the property-test work). --- [signing-metadata] key = cic-my-sign-key signature = vault:v1:MEUCIQCrEZ8M/hZ0u5KPm97BYYgPHk5tLTLQSPM7ovfZGYbzWQIgEdSwl4oqSRzBsHj3JUlEbsYcsHWrCAuBZNuJ9cVkiqQ= hash-algorithm = sha256 digest = IK19+yc0O5DPj/mQ/So02UNQyiAAp0VIv0fdWIpfSnE= [certificate] -----BEGIN CERTIFICATE----- MIICBjCCAaygAwIBAgIUSnRMR6RPnEbg296XWPOqq/u5PCwwCgYIKoZIzj0EAwIw QzELMAkGA1UEBhMCSFUxGTAXBgNVBAoMEENlbnRyYWxJbmZyYUNvcmUxGTAXBgNV BAMMEENJQyBEZXZlbG9wZXIgQ0EwHhcNMjYwMzIwMTMyMjU5WhcNMjYxMjMxMTMy MjU5WjBFMQswCQYDVQQGEwJIVTEZMBcGA1UECgwQQ2VudHJhbEluZnJhQ29yZTEb MBkGA1UEAwwSR2Fib3IgWm9sdGFuIFNpbmtvMFkwEwYHKoZIzj0CAQYIKoZIzj0D AQcDQgAEIG2CVmTfmLB9pLLclj7YmP2eedAjklpy4LGrU2ijoiy6Xqpuybv7OgJe i+ez31s65NEV8+X/ByeX1cstR988z6N8MHowCQYDVR0TBAIwADAdBgNVHQ4EFgQU yZN6AIX/TNnIJ9GwAa/NRN3ujHAwHwYDVR0jBBgwFoAUXn6CHYzPUqU4JVP8g+OS WeDYjhcwDgYDVR0PAQH/BAQDAgeAMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEF BQcDBDAKBggqhkjOPQQDAgNIADBFAiEA+bFzXRoJ4PCQbhAAtpkcMjt0vNj5rEW0 lOMBGDNyaWkCIB1vmM7PcZzv/c9bIrxF5kqv6QXomouhByUfeNUTbpKW -----END CERTIFICATE-----
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.
Closes #53. Stacked on #55 (base is
fix/canonical-fail-closed; retarget tomainonce #55 is merged).#55 made the canonicalizer fail closed on a hand-built
BigIntthat is not a digit run and on aMapwith a repeated key. This makes those shapes impossible to construct through the public API.Breaking (public Rust API — README: nothing here is a stable API)
Value::BigInt(BigIntValue)— newtype, private field,BigIntValue::try_new(sign + ASCII digits), infalliblecanonical()/as_str().Mapstorage private:Map::new,Map::from_pairs(refuses a repeated key),get_mut,iter,IntoIterator;pushcrate-private with a debug assertion;insertunchanged (replaces).expect()s they replaced are removed (canonical, conformance, collection);canonical_integerlogic lives once inBigIntValue::canonical.E_MALFORMED_VALUE(R-VALUE-BIGINT-DIGITS,R-VALUE-UNIQUE-KEYS, stageread).Tests:
compile_faildoctests show neither type can be built around its constructor;engine/tests/value_invariants.rs(public API only) covers constructors, reader output and consumers (compare / classify / set element key). Verified: fmt, clippy -D warnings, cargo test --workspace (incl. 4 doctests). Mutations: field madepub(both types) → the compile_fail doctests fail;try_newcheck off → test fails;from_pairscheck off → test and doctest fail.Go is unaffected (its
Valueisinterface{};CanonicalInteger/ToCanonicalJSONalready check).🤖 Generated with Claude Code