Repository navigation
feat(prf): derive canonical equality terms from Value - #380
Merged
coderdan merged 37 commits intoOct 9, 2026
Merged
Conversation
Contributor
✅ No CRAP threshold violations758 function(s) analyzed · threshold 30 |
Contributor
🧬 Mutation testing (cargo-mutants,
|
| caught | missed | unviable | timeout |
|---|---|---|---|
| 287 | 0 | 148 | 0 |
✅ Every mutant in the changed lines was caught by a test.
Map JS BigInt through 128 bits and Go integers to exact widths. Add calendar dates, precise timestamps and finite decimal SDK values, with shared Rust/Node/Go vectors and an updated WASM guest. BREAKING CHANGE: JS integer kinds now always decode as BigInt. Go int8, int16, uint8 and uint16 retain their widths instead of widening to 32 bits. Closes #375
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The cryptographic protocol still requires the stated human sign-off and depends on an unreleased, blocked Git-pinned dependency.
0 open findings
What changed in this PR
Adds canonical, domain-separated equality-term derivation for scalar Value variants while preserving existing primitive PRF encodings.
Changes:
- Adds canonical numeric, timestamp, decimal, byte, and Unicode NFC encodings.
- Implements
PrfValuefor borrowedValueinstances with typed errors. - Adds domain vectors, property tests, documentation, dependencies, and CI coverage.
| File | Description |
|---|---|
packages/prf/src/value.rs |
Maps canonical values to versioned PRF domains. |
packages/prf/src/lib.rs |
Exposes value support and canonical errors. |
packages/prf/src/error.rs |
Adds canonicalization failures. |
packages/prf/README.md |
Documents equality-term semantics and gates. |
packages/prf/Cargo.toml |
Adds value dependency and scalar features. |
packages/hmac/tests/value_equality.rs |
Adds HMAC known-answer vectors. |
packages/hmac/Cargo.toml |
Adds vector-test dependencies. |
packages/aead-value/src/lib.rs |
Exposes the optional canonical module. |
packages/aead-value/src/canonical.rs |
Implements canonical scalar encodings. |
packages/aead-value/src/canonical_tests.rs |
Tests normalization and ordering properties. |
packages/aead-value/Cargo.toml |
Adds canonicalization dependencies and features. |
Cargo.lock |
Locks new dependencies and the temporary Git revision. |
.github/workflows/test.yml |
Exercises canonical and optional-feature configurations. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
coderdan
added this pull request to stack #379
October 8, 2026 22:37
coderdan
marked this pull request as ready for review
October 8, 2026 22:38
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1f34179c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tobyhede
reviewed
Oct 8, 2026
tobyhede
approved these changes
Oct 9, 2026
vitaminc-prf (and the facade's prf/hmac features) no longer pull in vitaminc-aead-value, its Unicode tables and orderable-bytes unless the new `value` feature is enabled; `chrono` and `rust_decimal` imply it. This also breaks the aead (dev) -> prf -> aead-value -> aead dependency cycle. CI now tests prf alone without the feature, since the workspace run unifies hmac's dev-dependency features, and no longer runs the aead-value all-features suite twice. BREAKING CHANGE: PrfError is now #[non_exhaustive], and its Canonical variant exists only with the `value` feature. Matches on PrfError need a wildcard arm. Refs #380
A JavaScript BigInt takes the smallest integer kind that fits, while Go and Rust usually write int64, so per-width domains meant the same number never matched across languages. Every integer kind now shares one domain, vitaminc/prf/value/integer-orderable/v1, and a 17-byte order-preserving encoding: a sign byte, then the value as a big-endian 128-bit word. Ciphertext keeps the original width. The canonical layer now returns each term's domain with its bytes from the exhaustive Value match, and equality_domain matches ValueKind exhaustively in its own crate, so a new kind cannot compile without a domain and prf no longer canonicalizes (including NFC) before a fallible domain lookup. NFC output is measured before allocating, so the protected buffer never reallocates and frees an unwiped partial copy when normalization expands text. The normalization iterator's own buffers are documented as unwiped. unicode-normalization moves to a caret requirement; only the Unicode 16 assignment table stays exactly pinned. Refs #380
Merge the ValueKind::ALL slice change, the opt-in prf value feature and the shared integer equality domain. The Node kind inventory now samples every extended kind and expects BigInt for every integer width.
Timestamps with years outside 0000-9999 decrypt in ISO 8601 expanded form
(+12000-..., -0005-...), which the RFC 3339 parser refused, so a decrypted
{ timestamp } could not be encrypted again. Accept the expanded form and
pin both directions with corpus vectors.
A wrapper whose payload is not a string ({ date: new Date() },
{ decimal: 1n }, { timestamp: 1700000000000 }) now throws the wrapper's
ERR_INVALID_* code instead of a generic N-API error. Parsing goes through
one match on the key, without the per-parser fn casts.
Drop the JS Date range check on decryption: chrono's range lies inside
JS Date's, so it could never fail. Document that the wrapper keys are
reserved, which makes decrypt-then-encrypt lossy for one-key objects
named date, timestamp or decimal written by other languages.
Refs #381
decodeScalar built a tag-to-width map on every call. A switch keeps the decode path allocation-free. Refs #381
Go can write float32 while JavaScript numbers are always float64, so per-width domains meant 1.5 from Go never matched 1.5 from JS. Both float kinds now share vitaminc/prf/value/float-orderable/v1: a float32 widens exactly to float64, then folds signed zero and NaN as before. Numbers that differ between widths, such as 0.1, still give different terms. The widening uses integer operations only, so subnormal inputs cannot take a slower hardware path. It is checked against the hardware conversion at every exponent and subnormal bit, by property test, and (on demand) for every f32 bit pattern. Refs #380
A JS number is always Float64, and float and integer equality domains are distinct, so integer columns written by other languages only match BigInt. Refs #381
wrapper_text tripped the CRAP gate (CC 9, no coverage): it takes a live napi Object, so Rust coverage cannot reach it, and the Node test runs in a separately built addon. Move the error choice into the pure scalar::non_string_payload, unit-tested, and exempt the remaining glue by name like the other live N-API helpers. The Node conformance test still exercises it end to end, including under mutation testing. Refs #381
Rust and Go only checked that decoding then encoding a corpus vector gave the same bytes, so a bug that decodes and encodes symmetrically wrong (a date read and written a day off, say) passed. Only Node compared the decoded value with the vector's `value` field. Rust and Go now build the expected host value from `kind` and `value` with their standard parsers, not the transport decoder, and check both directions: decoding gives that value, and encoding it from scratch gives the bytes. Node now also encodes its expected value, not only the decoded one. An unknown kind fails until each helper learns to build it. Refs #381
The reflection path read float32 values through rv.Float(), which widens to float64; the hardware widening sets the quiet bit of a signaling NaN, so Go re-encoded 0xff833030 as 0xffc33030 where Rust keeps the bits. Decrypting and re-encrypting such a value in Go changed it. Plain float32 values are now read directly. A named float32 type has no bit-exact reflect path without unsafe and still widens. Found by the stricter fuzz property; the minimized inputs are kept as regression seeds. Refs #381
The fuzz targets only required accepted input to re-encode without error, and their seeds predated the extended scalar tags. They now require the same bytes back (Undefined becoming Null is the one allowed difference), and a fixed point on a second pass, and seed from every shared corpus vector, bare and nested in arrays, passthrough and ciphertext. PR CI still runs the seeds only. A new nightly workflow fuzzes each target for five minutes and uploads any failing input. Refs #381
The codec had only hand-written cases. Add quickcheck properties over random value trees covering every variant (raw float bits, so NaN payloads and signaling NaNs; leap seconds; every decimal scale): - every value round-trips through encode and decode; - any bytes the decoder accepts re-encode to exactly those bytes, both for random bytes and for valid encodings corrupted by one or two edits, of which about a fifth are still accepted and so checked. Refs #381
widen_f32 assembles an f64 by OR-ing bit fields that never share a set bit, so swapping | for ^ cannot change any result and no test can catch it. Exclude exactly that swap in that function, and move the one | whose operands do overlap (mantissa | 1) into highest_set_bit so it stays gated.
A scheduled run has no PR to fail, so a crash it found would only be visible to someone browsing the Actions tab. Record failures on a single open issue labelled fuzz, which notifies repository watchers and stays visible until closed.
Each decoder's own fuzz targets only prove it agrees with itself. These targets feed the same bytes to vcffi and to the Rust decoder in the wasm guest, and fail when one accepts what the other rejects, or when both accept but re-encode differently. No guest export is added. Calling vc_encrypt or vc_decrypt with a never-issued handle reports Rust's accept or reject verdict, because both decode before the handle lookup. Rust's re-encoding comes from decrypting a ciphertext that is a single passthrough node, which avoids AES and the cipher's sealing rules. Refs #383
The committed wasm guest is rebuilt by hand and can lag its source, so the nightly job rebuilds it before fuzzing, comparing Go with the Rust on the same commit. Document the approach, and mark the guest's decode-before-handle-lookup order as relied on. Closes #383
A named float32 type still went through rv.Float(), whose widening to float64 sets the quiet bit. Converting to the built-in float32 copies the bits instead, since reflect special-cases float32 to float32 (Go issue 36400), so named types now round-trip like plain float32. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
…tags The fuzz targets accepted a 0x01 to 0x00 change at any offset, so a corrupted payload byte, such as one inside Bytes, passed as the documented Undefined projection. Walk the accepted input to find its Undefined value-node tags, nested ones included, and require the re-encoding to change exactly those bytes. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
…ned tags The differential comparison accepted a 0x01 to 0x00 change at any offset, so Rust Int32(1) against Go Int32(0) passed as the Undefined projection. Walk Rust's re-encoding to find its Undefined value-node tags and require Go's to differ at exactly those bytes. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
coderdan
removed this pull request from stack #379
October 9, 2026 09:19
The live Node suite only covered scalar conversions. Add a suite for which keys are read (own, enumerable, string only, prototype pollution ignored), how output properties are written (own data properties, no inherited setter runs), NUL and numeric keys, and the ciphertext node projection, which the test addon now exposes as a round trip. The addon itself no longer needs unsafe. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
All 39 unsafe blocks were Node-API calls, which Miri cannot run. Most had safe napi-rs equivalents: build JS values with into_unknown, read keys with get_all_property_names, read properties with get_property_unchecked, and define them with define_properties. Two unsafe blocks remain, inside the FromNapiValue impls napi-rs requires, and the crate now denies unsafe_op_in_unsafe_fn and undocumented unsafe blocks so each states why it is sound. Behaviour is unchanged: the object-safety suite passes before and after. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
The conversion functions were exempt from the mutants gate because no Rust test could reach them. tests/node.rs now drives them through Node, rebuilding the test addon from the mutated source, so drop the exemption. Of 77 mutants, 14 survived at first; new tests cover the nesting limits in both directions and for ciphertext trees, own property attributes, a millisecond-aligned leap second, and passthrough values. The property attributes are built with union rather than |, so no equivalent | to ^ mutant arises. All mutants are now caught or unviable. The CRAP exemption stays: llvm-cov cannot see into the addon. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
…dary Review of #386 found boundary bugs, reproduced in Node: - Decrypted array elements, and a ciphertext node's t, v and sequence elements, were written with [[Set]], so a polluted setter on Array.prototype or Object.prototype received the plaintext or ciphertext and the result lost the property. Every property this crate builds is now an own-property define, one define call per object. - A key holding an unpaired surrogate was read as U+FFFD, so the lookup missed and the value sealed as undefined. Such keys are now refused, as string values are, and each property is read through its original JS key handle. - A primitive was accepted as a ciphertext node: napi-rs's Object conversion does not check the type, and N-API reads a primitive's properties through its prototype. Nodes are now checked to be objects, and the SAFETY comment that claimed otherwise is corrected. - Building JS from a Rust tree had no depth limit, so a deep tree built in Rust could overflow the stack. Both directions now stop at 128 levels, counted as the transport decoder counts them. Also fixes a test that could not fail, restores define_own_properties' documentation, and drops the redundant attribute constant: napi-rs's default attributes are the ones assignment gives. The Node suite gains a test for each bug, and every mutant in the conversion code is caught. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
…h into payloads A node object missing its own t or v took it from a possibly polluted prototype, and a hole in a sequence took an element from Array.prototype. Both now count as malformed ciphertext; an own v: undefined is still accepted. A passthrough payload restarted the depth count at zero, so a tree could nest 128 ciphertext levels and then 128 payload levels, unlike the transport encoding, where the payload continues the count. JsCipherText's payload now converts through a new NapiPassthrough trait, implemented for NapiValue and (), whose methods take the payload's depth. The () payload now also checks that it receives undefined, which napi-rs's own conversion does not. Also names define_own_properties instead of the removed define_own_property in the README and the CRAP allow list, and exempts wrapper_to_js, which sat exactly on the CRAP threshold. BREAKING CHANGE: JsCipherText's payload type P must implement NapiPassthrough instead of napi-rs's ToNapiValue and FromNapiValue. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
The own-key list and the own t/v checks were snapshots taken before the reads. A getter that ran in between could delete a later property, and the read then fell through to a possibly polluted prototype. Each read of an object property, a ciphertext map entry, or a node's t or v now checks ownership immediately before it, where no JS can run in between for an ordinary object. Also restores reading a passthrough node whose v was dropped by JSON.stringify (a passthrough of undefined) as undefined; requiring an own v had broken it. The prototype is still never read. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
The depth limit returned an error but still dropped the unconverted remainder of the tree through Value's recursive Drop, so a tree deep enough could overflow the stack and abort Node anyway. A tree built in Rust is now measured iteratively before conversion, and one past the limit is dropped iteratively before the error is returned. The conversion itself no longer needs a depth argument. NapiPassthrough gains exceeds_depth and drop_flat so ciphertext payloads take part, and a () payload now counts against the limit like any other payload, in both directions. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
refactor(aead-napi): move conversions onto napi-rs's safe API
feat(bindings): support extended scalar value kinds
coderdan
added this pull request to stack #388
October 9, 2026 22:58
Brings in the napi 3.14.2 lockfile bump that reached this stack when feat/371-value-model was rebased onto main. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
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.
Stacked on #378 (
feat/372-extended-value-kinds).Derive scalar equality terms directly from
&Value(behind prf's new opt-invaluefeature) under versioned domains. A shared canonical layer inside the value crate exhaustively dispatches variants, returns each term's domain with its bytes, preserves protected byte custody, and feeds orderable bytes to the PRF. Every integer kind shares one domain and a 17-byte encoding, and both float kinds share one domain (float32 widened exactly to float64), so equal numbers match across widths and languages; other scalars use their natural width. It folds float signed zero and NaNs, truncates timestamp fractional seconds to microseconds, normalizes decimal scale, and applies Unicode 16 NFC without case or accent folding. Unsupported values and unassigned code points produce typed errors. Existing primitive PRF encodings remain unchanged.The HMAC tests independently pin canonical payloads, domain labels and SHA-256 outputs for every supported scalar kind. Conformance checks explicitly exclude booleans and containers; property tests cover normalization idempotence and float ordering. CI exercises optional scalar features.
Merge/release gates:
orderable-bytesis temporarily pinned to feat(orderable-bytes)!: variable-length encodings for strings and byte strings ore.rs#97, revisionb3b7800b9c0cd8f7c53c5382f8e018a4e8bcbbd5. Replace the Git pin with the upstream registry release before publication.Validation: value/PRF/HMAC suites and domain vectors pass; chrono and decimal feature checks pass; Clippy, formatting and rustdoc pass with warnings denied. Local coverage-based CRAP checks pass without threshold or exclusion changes (maximum 26).
Closes #373
Breaking:
PrfErroris now#[non_exhaustive]; itsCanonicalvariant exists only with thevaluefeature.