Skip to content

fix(crypto): #1076 createHmac/createHash silent empty on non-literal alg - #1080

Merged
proggeramlug merged 1 commit into
mainfrom
worktree-agent-aeaeaa6a9190f9cd2
May 19, 2026
Merged

fix(crypto): #1076 createHmac/createHash silent empty on non-literal alg#1080
proggeramlug merged 1 commit into
mainfrom
worktree-agent-aeaeaa6a9190f9cd2

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Closes #1076.

Summary

  • crypto.createHmac(alg, key).update(data).digest(enc) silently returned "" whenever alg wasn't an inline Expr::String literal — const-bound names, for-of bindings, ternaries, etc. HMAC webhook verification, TOTP, and any code that picked the algorithm dynamically silently produced wrong answers instead of failing.
  • Replace the chain-collapse catch-all empty-string return with a runtime handle path: allocate an Hmac/Hash handle and invoke .update(data).digest(enc) via js_native_call_method. Covers all non-literal alg cases plus the algorithms the fast path doesn't have direct FFI helpers for (sha1, sha512, md5 for HMAC; sha1, sha512 for hash).
  • Add HmacHandle + js_crypto_create_hmac + dispatch_hmac in perry-stdlib mirroring the existing HashHandle shape. Supports sha1/sha256/sha512/md5 + hex/base64/base64url/binary digest encodings.
  • Add a standalone codegen arm for crypto.createHmac(alg, key) mirroring the existing standalone createHash arm — covers const h = createHmac(...); h.update().digest() split across statements.

Approach

Option C (per the task brief): runtime dispatch fallback for non-literal alg, plus a standalone handle-based codegen path for the bound-then-used pattern. The literal-"sha256" fast path (createHash sha256/md5, createHmac sha256) stays unchanged for the common case.

Test plan

  • cargo test --release -p perry-codegen --test manifest_consistency — 4/4 pass
  • cargo build --release -p perry-runtime -p perry-stdlib -p perry
  • test-files/test_crypto_hmac_dynamic_alg.ts covers inline-literal, for-of, const-ref, ternary, bound-then-used, and createHash const-ref. Byte-for-byte match with node --experimental-strip-types.
  • test-files/test_gap_node_crypto_buffer.ts still passes (literal-alg fast path unchanged).

Sibling APIs

  • createHmac (algorithms: sha1, sha256, sha512, md5) — fixed via handle path.
  • createHash (algorithms: sha1, sha256, sha512, md5) — non-literal alg now flows through the same handle path that the standalone arm already used; previously the chain-collapse caught it and returned "".
  • createCipheriv, createDecipheriv, createSign, createVerify — out of scope for this PR. The const-binding aspect described in the issue would affect them similarly, but they're tracked elsewhere (crypto.createCipheriv not wired in codegen (AES-GCM blocker for envelope encryption) #1075 is the active issue for createCipheriv AES-GCM wiring; signed APIs would need fresh handle types and FFI). Worth a follow-up sweep but not bundled here to keep this PR focused on the reported failure.

No version bump or changelog change (per worktree rules — maintainer applies at merge time).

…for-of alg

`crypto.createHmac(alg, key).update(data).digest(enc)` silently returned
"" whenever `alg` wasn't an inline `Expr::String` literal — const-bound
names, for-of bindings, ternaries, function-returned algs all matched the
chain-collapse fast path's `_ =>` catch-all (which returned an empty
string). HMAC webhook verification, TOTP, and any code that picks the
algorithm dynamically silently produced wrong answers instead of failing.

Fix:
- Replace the catch-all empty-string return with a runtime handle path
  that allocates an Hmac/Hash handle (`js_crypto_create_hmac` /
  `js_crypto_create_hash`) and invokes `.update(data).digest(enc)` via
  `js_native_call_method`. This covers all non-literal alg cases AND the
  algorithms the fast path doesn't have direct FFI helpers for (sha1,
  sha512, md5 for HMAC; sha1, sha512 for hash).
- Add `HmacHandle` + `js_crypto_create_hmac` + `dispatch_hmac` in
  perry-stdlib mirroring the existing `HashHandle` + `js_crypto_create_hash`
  + `dispatch_hash` shape. Supports sha1/sha256/sha512/md5 + hex/base64/
  base64url/binary digest encodings.
- Register `HmacHandle` in `js_handle_method_dispatch` for `update` /
  `digest`.
- Add a standalone `Expr::Call` arm for `crypto.createHmac(alg, key)`
  mirroring the existing standalone `createHash` arm — covers
  `const h = createHmac(...); h.update().digest()` where the chain is
  split across statements.

Verified byte-for-byte against `node --experimental-strip-types` for
inline-literal, for-of, const-ref, ternary, and bound-then-used patterns
in `test-files/test_crypto_hmac_dynamic_alg.ts`. Existing
`test_gap_node_crypto_buffer.ts` still passes (literal-alg fast path
unchanged).
@proggeramlug
proggeramlug merged commit c956692 into main May 19, 2026
16 of 17 checks passed
@proggeramlug
proggeramlug deleted the worktree-agent-aeaeaa6a9190f9cd2 branch May 19, 2026 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

crypto.createHmac(alg, key) returns "" when alg is a const reference (string-literal-only dispatch — same pattern as #1074)

1 participant