fix(crypto): match Node 26 parity - #6836
Conversation
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughKMAC key import, key generation, and output sizing now support empty or non-byte-aligned values. X.509 invalid-IP errors and legacy metadata access are aligned with expected behavior, with updated parity coverage and changelog documentation. ChangesCrypto parity
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-stdlib/src/webcrypto/jwk.rs`:
- Around line 268-269: The KMAC branches in key import must preserve whether the
JWK k value was explicitly present and valid instead of collapsing missing or
invalid decoding into an empty vector. Update the jwk_import_key_bytes handling
used by KeyAlgo::Kmac128 and KeyAlgo::Kmac256 to reject absent or malformed k
data, while still allowing an explicitly provided k: "" value.
In `@crates/perry-stdlib/src/webcrypto/keys.rs`:
- Line 660: The KMAC key-generation path around the key_bytes allocation must
preserve the requested bit length, not just the rounded byte size. Mask unused
padding bits after OsRng fills the buffer, pass the requested KMAC bit length
into CryptoKeyMaterial::new, and update the parity expectation if Node accepts
non-byte-aligned generated lengths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a4e3f95-29f8-44b1-8434-9b4f06dae3bb
📒 Files selected for processing (7)
crates/perry-stdlib/src/crypto/x509.rscrates/perry-stdlib/src/webcrypto/hmac.rscrates/perry-stdlib/src/webcrypto/jwk.rscrates/perry-stdlib/src/webcrypto/keys.rscrates/perry-stdlib/src/webcrypto/util.rstest-parity/node-suite/crypto/webcrypto/kmac.tstest-parity/node-suite/crypto/x509/signature-legacy-metadata.ts
💤 Files with no reviewable changes (1)
- crates/perry-stdlib/src/webcrypto/hmac.rs
| | KeyAlgo::Kmac128 | ||
| | KeyAlgo::Kmac256 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)Cargo\.toml$|(^|/)CLAUDE\.md$|crates/perry-stdlib/src/webcrypto/jwk\.rs|changelog\.d/' || true
echo "== jwk relevant sections =="
if [ -f crates/perry-stdlib/src/webcrypto/jwk.rs ]; then
wc -l crates/perry-stdlib/src/webcrypto/jwk.rs
sed -n '200,310p' crates/perry-stdlib/src/webcrypto/jwk.rs
echo "== k/import functions outline =="
ast-grep outline crates/perry-stdlib/src/webcrypto/jwk.rs --view compact | sed -n '1,220p' || true
fi
echo "== search jwk_import_key_bytes =="
rg -n "jwk_import_key_bytes|KeyAlgo::Kmac|Kmac|k:" crates/perry-stdlib/src/webcrypto || trueRepository: PerryTS/perry
Length of output: 15088
🌐 Web query:
WebCrypto modern algorithms JWK k field decoding invalid JWK fail wicg github.io webcrypto modernalgos
💡 Result:
The "k" field in a JSON Web Key (JWK) is used to carry the raw key material for symmetric algorithms (typically with the "oct" key type) [1][2][3]. Decoding issues related to this field are generally caused by improper handling of Base64URL encoding—a variant of Base64 required by the JOSE specifications (RFC 7517) which replaces '+' with '-' and '/' with '', and omits padding [1][2]. In the context of the Web Cryptography API and its extensions (such as the Modern Algorithms proposal), "invalid JWK" errors during the decoding of the "k" field typically arise from: 1. Improper Encoding/Decoding: The "k" value must be decoded from its Base64URL representation into a raw byte array (Uint8Array) before it can be processed as a key by the WebCrypto subtle.importKey operation [1][3]. If an implementation attempts to treat the string directly as a key or uses standard Base64 decoding (which fails on characters like '-' and ''), a DataError or SyntaxError is standard behavior [1][2]. 2. Validation Requirements: Specifications and browser implementations enforce strict checks on the "k" field. For example, if the key length implied by the decoded "k" parameter does not match the requirements of the algorithm specified in the JWK (or provided to importKey), the operation will fail [4][2]. 3. WebCrypto "Modern Algorithms": The WICG "Modern Algorithms" specification [2] explicitly governs the handling of JWK inputs for new algorithms (like ML-KEM). Similar to the base WebCrypto API, it mandates that the "k" (or "priv"/"pub") fields must be valid Base64URL encoded data [2]. If a JWK is malformed—such as having an incorrect key type ("kty") for the algorithm, missing required fields, or having invalid Base64URL characters—the API is required to throw a DataError [2]. If you are encountering this error, ensure that: - The Base64URL string is correctly padded if your decoder requires it (though RFC 7517 suggests padding should be omitted) or, conversely, ensure your decoder is RFC-compliant and handles unpadded Base64URL correctly [1][2]. - The "kty" (Key Type) and "alg" (Algorithm) fields in your JWK object align with the WebCrypto algorithm you are importing into [2][3]. - You are not passing the JWK object directly as a key material buffer, but rather passing the parsed JWK structure to the importKey function where the browser/runtime handles the internal decoding [3].
Citations:
- 1: https://explore.market.dev/ecosystems/typescript/projects/unjwt
- 2: https://wicg.github.io/webcrypto-modern-algos/
- 3: https://mintlify.wiki/nodejs/node/api/webcrypto
- 4: https://mailman.nginx.org/pipermail/nginx-devel/2023-January/53LUPELTUWYALH5RZFMBE56SMTK7XO2O.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== jwk.rs 720-820 =="
sed -n '720,820p' crates/perry-stdlib/src/webcrypto/jwk.rs
echo "== jwk.rs 820-940 for validate/import key bits =="
sed -n '820,940p' crates/perry-stdlib/src/webcrypto/jwk.rs
echo "== jwk.rs 428-470 and 508-535 for export KMAC =="
sed -n '428,470p' crates/perry-stdlib/src/webcrypto/jwk.rs
sed -n '508,535p' crates/perry-stdlib/src/webcrypto/jwk.rs
echo "== imports/top =="
sed -n '1,90p' crates/perry-stdlib/src/webcrypto/jwk.rsRepository: PerryTS/perry
Length of output: 14707
Do not allow malformed JWK k decoding as an empty KMAC key.
jwk_import_key_bytes(...).unwrap_or_else(|| Vec::new()) converts a missing or invalid decoded k value into the same zero-length byte vector as an explicitly present empty JWK. Since KMAC is now in the empty-key allowlist, malformed JWKs can be imported as zero-length keys instead of failing key-material validation. Preserve the decode result: allow k: "" only when explicit, and reject missing or invalid k data.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-stdlib/src/webcrypto/jwk.rs` around lines 268 - 269, The KMAC
branches in key import must preserve whether the JWK k value was explicitly
present and valid instead of collapsing missing or invalid decoding into an
empty vector. Update the jwk_import_key_bytes handling used by KeyAlgo::Kmac128
and KeyAlgo::Kmac256 to reject absent or malformed k data, while still allowing
an explicitly provided k: "" value.
2 participants
Summary
checkIPinvalid-name errors in Node 26X509Certificate.toLegacyObject()with its certificate receiver so the parity fixture runsValidation
./run_parity_tests.sh --suite node-suite --module crypto— 243 pass, 0 failuresInvalid IPfor X509 invalid strings and rejects the KMAC cases that Node 26 accepts. Node 26.5.0 remains the repository oracle.cargo fmt --check -p perry-stdlibgit diff --checkCloses #6783
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation