fix(crypto): decode inline SSO string args instead of dereferencing them (#11481)

[proggeramlug]

Summary

crypto read string/bytes arguments by masking the NaN-box to 48 bits and passing the result to bytes_from_ptr. A runtime-built string of 5 bytes or fewer is an inline SSO value (SHORT_STRING_TAG). Its low 48 bits hold the length and payload, not an address, so bytes_from_ptr read those bytes as a StringHeader* and segfaulted. The issue repro (createHash("sha1").update("x" + (i & 7))) is one case of this. The same mask appears in two layers, and on main both of them crash.

Changes

• stdlib, f64 args. Adds bytes_from_value(f64) in crypto/util.rs. It decodes SSO bytes directly and otherwise uses the existing masked-pointer path (Buffer, heap string or raw pointer), so only SSO values behave differently. These now go through it:
  • arg_bytes/arg_string, decode_hash_update_value, decode_crypto_value and decode_ecdh_input (which now takes the f64)
  • the direct masks in hash/hmac digest(enc), cipher update/setAuthTag/setAAD, sign/verify update, ECDH convertKey, crypto_value_bytes, and PEM key inputs
  • arg_ptr had no remaining callers, so it is removed.
• codegen, i64 FFI args. crypto_{hash,kdf,keys,misc}.rs now unbox string/bytes args with unbox_str_handle (js_get_string_pointer_unified, which materializes SSO into a heap header) instead of unbox_to_i64. The fast paths in crypto_hash.rs already did this. It covers:
  • createHash/createHmac algorithm and key
  • pbkdf2, scrypt, hkdf and argon2 inputs
  • createCipheriv, createSign/Verify, createECDH, createSecretKey, generateKey*
  • one-shot sign/verify and RSA publicEncrypt-family data
  • The generateKeyPairSync options object keeps the plain mask.
• No new side tables and no version bump.

Related issue

Fixes #11481

Test plan

• New #[test]s in crypto/hash_handles.rs (sso_arg_tests) drive dispatch_hash/dispatch_hmac with SSO update data, input encoding and digest encoding. Each fixture asserts it really is is_short_string(), so a pass can't come from the heap-string path. Result: RUST_TEST_THREADS=1 cargo test -p perry-stdlib --lib crypto:: gives 30 passed.
• New gap test test-files/test_gap_11481_crypto_sso_string_args.ts matches Node byte for byte.
• A/B on a perry-dev build with PERRY_NO_AUTO_OPTIMIZE=1 and pinned PERRY_RUNTIME_DIR, with the static .as rebuilt each time:
  • On main the issue repro and the gap test both segfault (exit 139). So does each of these 9 shapes run on its own: SSO update data, digest encoding, algorithm, input encoding, HMAC key, HMAC algorithm, pbkdf2 password, pbkdf2 digest, scrypt password. The codegen half is a real crash, not only the stdlib half.
  • On this branch all 9 shapes and the gap test match Node.
• Every existing crypto/hmac/pbkdf/scrypt/hkdf/cipher test under test-files/ gives the same output as Node, except test_crypto.ts (a non-module script that Node itself rejects) and test_parity_crypto (already in known_failures.json for inventory lengths). Both differ on main too.
• cargo test -p perry-codegen --lib crypto passes. check_file_size.sh, addr_class_inventory.py and unrooted_local_shape.py --check pass. rustfmt is clean.
• Added a test under test-files/ and a #[test] in the affected crate

Checklist

• I have NOT bumped the workspace version or edited CLAUDE.md / CHANGELOG.md
• Commit follows the fix: prefix convention

🤖 Generated with Claude Code

https://claude.ai/code/session_01LdEUAyNTC8iuUa5RDkLyny

---

Generated by Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Fixed crypto operations handling short strings built at runtime, including hashing, HMAC, key derivation, ciphers, and key-related operations.
  • Improved handling of string and byte inputs across crypto operations, including encoded data and signatures.
• Tests
  • Added regression coverage for runtime-built short strings used in crypto operations.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 446594b6-5ba9-41b1-b051-02aa542261f7

📥 Commits

Reviewing files that changed from the base of the PR and between d57f513 and bdb0c62.

📒 Files selected for processing (12)

• changelog.d/11513-crypto-sso-string-args.md
• crates/perry-codegen/src/expr/calls/crypto_hash.rs
• crates/perry-codegen/src/expr/calls/crypto_kdf.rs
• crates/perry-codegen/src/expr/calls/crypto_keys.rs
• crates/perry-codegen/src/expr/calls/crypto_misc.rs
• crates/perry-stdlib/src/crypto/cipher.rs
• crates/perry-stdlib/src/crypto/ecdh.rs
• crates/perry-stdlib/src/crypto/hash_handles.rs
• crates/perry-stdlib/src/crypto/random.rs
• crates/perry-stdlib/src/crypto/util.rs
• crates/perry-stdlib/src/crypto/x509.rs
• test-files/test_gap_11481_crypto_sso_string_args.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

Crypto code generation now unboxes string-valued arguments with unbox_str_handle. Crypto byte readers use bytes_from_value to handle inline short strings and other values. The changes include regression tests using runtime-built short strings in crypto calls.

Changes

Crypto SSO string handling

Layer / File(s)	Summary
Crypto string argument unboxing crates/perry-codegen/src/expr/calls/crypto_hash.rs, crates/perry-codegen/src/expr/calls/crypto_kdf.rs, crates/perry-codegen/src/expr/calls/crypto_keys.rs, crates/perry-codegen/src/expr/calls/crypto_misc.rs	Crypto code generation uses unbox_str_handle for string-valued arguments passed to existing crypto runtime calls.
NaN-boxed value byte decoding crates/perry-stdlib/src/crypto/util.rs	The new bytes_from_value helper decodes inline short-string bytes and uses masked-pointer extraction for other values. Existing byte-conversion helpers and PEM input conversions use it.
Crypto runtime decoding and regression coverage crates/perry-stdlib/src/crypto/cipher.rs, crates/perry-stdlib/src/crypto/ecdh.rs, crates/perry-stdlib/src/crypto/hash_handles.rs, crates/perry-stdlib/src/crypto/random.rs, crates/perry-stdlib/src/crypto/x509.rs, test-files/test_gap_11481_crypto_sso_string_args.ts, changelog.d/11513-crypto-sso-string-args.md	Crypto runtime readers pass NaN-boxed values to bytes_from_value. Tests exercise runtime-built short strings in hash, HMAC, KDF, and cipher calls. The changelog describes the changes and coverage.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to bdb0c

The changed crypto argument paths have no identified merge-blocking issue. Merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to bdb0c

Short string inputs can now reach cryptographic operations without being mistaken for pointers. The reviewed paths retain their key validation and authentication checks. No new security weakness was established, but execution was not verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The representation change is reachable through multiple crypto operations, including HMAC, cipher creation, key derivation, and signing. Its scope is callers of these operations, not a newly introduced service or privilege boundary.

Trust Boundaries and Controls

• observed — The new runtime branch requires the short-string tag; non-short-string inputs follow the previous pointer-backed reader rather than acquiring a new conversion route.
• observed — HMAC call lowering validates the algorithm and key before unboxing them; cipher creation validates algorithm, key, and IV properties before registering a handle.

Resilience and Maintainability Implications

• inferred — Reviewed hash and cipher state transitions continue to use their existing ownership and finalization paths when supplied with short-string bytes; no new route around those controls was established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.07% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 56 functions across 11 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary crypto fix: decoding inline SSO string arguments instead of treating them as pointers.
Description check	✅ Passed	The description includes the required Summary, Changes, Related issue, Test plan, and Checklist sections. It explains the failure, implementation, affected paths, regression coverage, and test results…
Linked Issues check	✅ Passed	Issue #11481 is closed and supplies historical context only. No active directly linked issue remains, so this pull request has no linked-issue coding requirements under the assessment rules.
Out of Scope Changes check	✅ Passed	The changed files address the historical #11481 failure mode. They decode SSO values in crypto stdlib paths, unbox string arguments in crypto codegen paths, and add regression coverage. The changelog …

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.07% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 56 functions across 11 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

⚔️ Resolve merge conflicts 💡

• Resolve merge conflict in branch claude/vigilant-maxwell-0ggyai

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

Closing as superseded: main already fixes #11481 via #11486 (dabeb0a / 6acf012 — crypto native calls materialize an SSO string argument). This PR now conflicts with that fix in the same 8 crypto files; #11481 is closed.