From ef5ad2688c8a6ff151915ff7d18cc7d43fb2c70b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 18 Jun 2026 08:32:34 +0200 Subject: [PATCH 1/4] fix(hir): don't fold user/library .sort(list) into array intrinsic A method named `sort` on a class instance or an imported module-exports object (semver re-exports `sort = (list) => list.sort(cmp)`, called as `semver.sort(list)`) was unconditionally folded into `Expr::ArraySort`, mis-routing the single `list` argument into the comparator slot. The runtime comparator validator then saw the array (not a function) and threw "The comparison function must be either a function or undefined: ...". Gate the `sort` fold on the receiver being a known array, mirroring the existing map/filter/join guards: - array_only_methods.rs: require !recv_is_class for the `sort` arm, and bail for imported-binding receivers (treated like namespaces). - imported_array_methods.rs: only fold when the imported binding's return_type is statically Array (mirrors the #420 `join` fix). - local_array_methods.rs: skip the fold for known class-instance receivers. --- .../src/lower/expr_call/array_only_methods.rs | 23 +++++++++- .../lower/expr_call/imported_array_methods.rs | 21 ++++++++- .../lower/expr_call/local_array_methods.rs | 45 ++++++++++++++++++- tests/test_user_method_named_sort.sh | 44 ++++++++++++++++++ 4 files changed, 130 insertions(+), 3 deletions(-) create mode 100755 tests/test_user_method_named_sort.sh diff --git a/crates/perry-hir/src/lower/expr_call/array_only_methods.rs b/crates/perry-hir/src/lower/expr_call/array_only_methods.rs index 1283ddcb09..0cf1437ea6 100644 --- a/crates/perry-hir/src/lower/expr_call/array_only_methods.rs +++ b/crates/perry-hir/src/lower/expr_call/array_only_methods.rs @@ -303,6 +303,20 @@ pub(super) fn try_array_only_methods( if ctx.namespace_import_locals.contains(&n) { return Ok(Err(args)); } + // A default/CJS-module import binding used as a method + // receiver (`import semver from "semver"; semver.sort(x)`) + // is a module-exports object, never a local array. It + // lowers to an `ExternFuncRef` receiver, so the array- + // method fold would mis-route `semver.sort(list)` into + // `Expr::ArraySort { array: semver, comparator: list }` + // (the single `list` arg landing in the comparator slot → + // "comparison function must be either a function or + // undefined"). Treat imported bindings like namespaces and + // bail to the namespace/object method dispatch. + if ctx.lookup_local(&n).is_none() && ctx.lookup_imported_func(&n).is_some() + { + return Ok(Err(args)); + } let ty = ctx.lookup_local_type(&n); let class_typed = ty .as_ref() @@ -722,7 +736,14 @@ pub(super) fn try_array_only_methods( let array_expr = lower_expr(ctx, &member.obj)?; return Ok(Ok(Expr::ArrayValues(Box::new(array_expr)))); } - "sort" if !args.is_empty() => { + // `!recv_is_class` gate: semver re-exports + // `sort = (list) => list.sort(cmp)` and the driver calls + // `semver.sort(list)`. Without the guard the namespace/class + // receiver was folded to `Expr::ArraySort`, mis-routing the + // single `list` argument into the comparator slot ("comparison + // function must be either a function or undefined"). Mirrors + // the sibling `map`/`filter`/`reduce` arms. + "sort" if !args.is_empty() && !recv_is_class => { let array_expr = lower_expr(ctx, &member.obj)?; return Ok(Ok(Expr::ArraySort { array: Box::new(array_expr), diff --git a/crates/perry-hir/src/lower/expr_call/imported_array_methods.rs b/crates/perry-hir/src/lower/expr_call/imported_array_methods.rs index 0e2d0451c2..00d340a4d3 100644 --- a/crates/perry-hir/src/lower/expr_call/imported_array_methods.rs +++ b/crates/perry-hir/src/lower/expr_call/imported_array_methods.rs @@ -119,7 +119,26 @@ pub(super) fn try_imported_array_methods( } } "sort" => { - if !args.is_empty() { + // Like `join` above: only fold when the + // imported binding is statically Array-typed. + // semver re-exports `sort = (list) => + // list.sort(cmp)` and the driver calls + // `semver.sort(list)`; `semver` is an imported + // module-exports object (return_type Any), so + // folding to `Expr::ArraySort { array: semver, + // comparator: list }` mis-routed the single + // `list` arg into the comparator slot → + // "comparison function must be either a + // function or undefined". Fall through to the + // generic call path, which invokes the imported + // `sort` function correctly. + if !args.is_empty() + && matches!( + extern_ref, + Expr::ExternFuncRef { ref return_type, .. } + if matches!(return_type, Type::Array(_)) + ) + { return Ok(Ok(Expr::ArraySort { array: Box::new(extern_ref), comparator: Box::new(args.into_iter().next().unwrap()), diff --git a/crates/perry-hir/src/lower/expr_call/local_array_methods.rs b/crates/perry-hir/src/lower/expr_call/local_array_methods.rs index 40e574e377..f5088dd9fa 100644 --- a/crates/perry-hir/src/lower/expr_call/local_array_methods.rs +++ b/crates/perry-hir/src/lower/expr_call/local_array_methods.rs @@ -15,6 +15,37 @@ use super::super::{ resolve_typed_parse_ty, LoweringContext, }; +/// True when `recv_ty` is a statically-known class/namespace instance type +/// (a `Named` or `Generic` type that is not itself an array). Used to gate +/// array-method folds (`.sort`/`.map`/…) so a user/library method that merely +/// shares a builtin Array name (e.g. semver's `semver.sort(list)`) is not +/// rewritten into the corresponding `Expr::Array*` fast path. TypedArray +/// `Named` types are deliberately treated as arrays (not class instances). +fn receiver_is_class_instance(recv_ty: Option<&Type>) -> bool { + let is_typed_array = |n: &str| { + matches!( + n, + "Int8Array" + | "Int16Array" + | "Int32Array" + | "Uint8Array" + | "Uint8ClampedArray" + | "Uint16Array" + | "Uint32Array" + | "Float16Array" + | "Float32Array" + | "Float64Array" + | "BigInt64Array" + | "BigUint64Array" + ) + }; + match recv_ty { + Some(Type::Named(n)) => !is_typed_array(n), + Some(Type::Generic { base, .. }) => base != "Array", + _ => false, + } +} + pub(super) fn try_local_array_methods( ctx: &mut LoweringContext, call: &ast::CallExpr, @@ -533,7 +564,19 @@ pub(super) fn try_local_array_methods( } } "sort" => { - if !args.is_empty() { + // semver `module.exports.sort = (list) => …` is + // re-exported as a plain function and called as + // `semver.sort(list)`. The receiver there is a + // class/namespace instance, NOT an array, so + // folding to `Expr::ArraySort` mis-routed the + // single `list` argument into the comparator slot + // → "comparison function must be either a function + // or undefined". Only fold when the receiver is + // not a statically-known class instance (mirrors + // the `map`/`filter`/`with` guards). + if !args.is_empty() + && !receiver_is_class_instance(ctx.lookup_local_type(&arr_name)) + { return Ok(Ok(Expr::ArraySort { array: Box::new(Expr::LocalGet(array_id)), comparator: Box::new(args.into_iter().next().unwrap()), diff --git a/tests/test_user_method_named_sort.sh b/tests/test_user_method_named_sort.sh new file mode 100755 index 0000000000..aab847410a --- /dev/null +++ b/tests/test_user_method_named_sort.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash +set -euo pipefail + +# A user-defined method named `sort` on a class instance — or a function +# re-exported as `sort` from an imported module (semver: +# `sort = (list) => list.sort(cmp)` called as `semver.sort(list)`) — must NOT +# be folded into the `Expr::ArraySort` array intrinsic. The fold mis-routed the +# single `list` argument into the comparator slot, so the runtime comparator +# validator saw the array (not a function) and threw "The comparison function +# must be either a function or undefined: ...". The HIR sort arms in +# `array_only_methods.rs` / `imported_array_methods.rs` now require the +# receiver to be a known array (not a class instance / imported binding) before +# folding, mirroring the existing `map`/`filter`/`join` guards. + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +PERRY="${PERRY_BIN:-${PERRY:-$REPO_ROOT/target/release/perry}}" +if [[ ! -x "$PERRY" ]]; then PERRY="$REPO_ROOT/target/debug/perry"; fi +if [[ ! -x "$PERRY" ]]; then + echo "SKIP: perry binary not found (build with cargo build -p perry)" + exit 0 +fi + +TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TMPDIR"' EXIT + +cat >"$TMPDIR/f.ts" <<'TS' +// A wrapper class whose `sort(list)` method internally calls Array.sort with +// a comparator — exactly semver's re-exported `sort`. +class Sorter { + sort(list: any[]) { + return list.sort((a: any, b: any) => (a < b ? -1 : a > b ? 1 : 0)); + } +} +const s = new Sorter(); +console.log(JSON.stringify(s.sort(["1.2.0", "1.0.1", "1.0.0", "2.0.0"]))); +TS + +OUT="$("$PERRY" run "$TMPDIR/f.ts" 2>&1)" || { echo "FAIL: perry run errored"; echo "$OUT"; exit 1; } +EXPECT='["1.0.0","1.0.1","1.2.0","2.0.0"]' +if ! grep -qF "$EXPECT" <<<"$OUT"; then + echo "FAIL: expected $EXPECT, got:"; echo "$OUT"; exit 1 +fi +echo "PASS: user-method-named-sort is not folded to the array intrinsic" From 0b94d76a171b47ffb676ec732aa588ff74024796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 18 Jun 2026 08:44:57 +0200 Subject: [PATCH 2/4] fix(runtime): expose inherited Object.prototype methods on the Object constructor `Object` is a function whose member reads resolve up the prototype chain, so `Object.hasOwnProperty` IS `Object.prototype.hasOwnProperty` (a callable). immer's `isPlainObject` does `O.hasOwnProperty.call(proto, CONSTRUCTOR)` with `const O = Object`; the reified Object constructor value had only its static methods installed, so reading `hasOwnProperty` returned `undefined` and `.call` threw "Function.prototype.call was called on a value that is not a function". Install the Object.prototype methods reachable on the constructor by inheritance (hasOwnProperty / isPrototypeOf / propertyIsEnumerable / toString / toLocaleString / valueOf) as statics on the reified ctor, reusing the existing `object_prototype_*_thunk` impls (which read the receiver from IMPLICIT_THIS, so `.call(receiver, ...)` binds correctly). --- .../perry-runtime/src/object/global_this.rs | 51 +++++++++++++++++++ ...est_object_ctor_inherited_proto_methods.sh | 38 ++++++++++++++ 2 files changed, 89 insertions(+) create mode 100755 tests/test_object_ctor_inherited_proto_methods.sh diff --git a/crates/perry-runtime/src/object/global_this.rs b/crates/perry-runtime/src/object/global_this.rs index 69b398c988..3c35cc4453 100644 --- a/crates/perry-runtime/src/object/global_this.rs +++ b/crates/perry-runtime/src/object/global_this.rs @@ -6387,6 +6387,57 @@ fn install_builtin_constructor_statics(name: &str, ctor: *mut crate::closure::Cl true, ); install_constructor_static(ctor, "hasOwn", object_hasown_thunk as *const u8, 2, false); + // `Object` is a function, so reading a non-static member resolves up + // its prototype chain (Function.prototype → Object.prototype). In + // particular `Object.hasOwnProperty` IS `Object.prototype.hasOwnProperty` + // — a callable. immer's `O.hasOwnProperty.call(proto, "constructor")` + // (with `const O = Object`) relied on this; without the inherited + // methods installed on the reified ctor value the read returned + // `undefined` and `.call` threw "Function.prototype.call on a value + // that is not a function". Install the Object.prototype methods that + // are reachable on the constructor by inheritance. + install_constructor_static( + ctor, + "hasOwnProperty", + object_prototype_has_own_property_thunk as *const u8, + 1, + false, + ); + install_constructor_static( + ctor, + "isPrototypeOf", + object_prototype_is_prototype_of_thunk as *const u8, + 1, + false, + ); + install_constructor_static( + ctor, + "propertyIsEnumerable", + object_prototype_property_is_enumerable_thunk as *const u8, + 1, + false, + ); + install_constructor_static( + ctor, + "toString", + object_prototype_to_string_thunk as *const u8, + 0, + false, + ); + install_constructor_static( + ctor, + "toLocaleString", + object_prototype_to_locale_string_thunk as *const u8, + 0, + false, + ); + install_constructor_static( + ctor, + "valueOf", + object_prototype_value_of_thunk as *const u8, + 0, + false, + ); } "Array" => { install_constructor_static( diff --git a/tests/test_object_ctor_inherited_proto_methods.sh b/tests/test_object_ctor_inherited_proto_methods.sh new file mode 100755 index 0000000000..b6597969a0 --- /dev/null +++ b/tests/test_object_ctor_inherited_proto_methods.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +set -euo pipefail + +# `Object` is a function, so reading a non-static member resolves up its +# prototype chain — `Object.hasOwnProperty` IS `Object.prototype.hasOwnProperty`, +# a callable. immer's `O.hasOwnProperty.call(proto, "constructor")` (with +# `const O = Object`) depends on this. Before the fix, reading `hasOwnProperty` +# (and `isPrototypeOf` / `propertyIsEnumerable` / `toString` / `valueOf` / +# `toLocaleString`) off the reified `Object` constructor value returned +# `undefined`, so `.call(...)` threw "Function.prototype.call was called on a +# value that is not a function". + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +PERRY="${PERRY_BIN:-${PERRY:-$REPO_ROOT/target/release/perry}}" +if [[ ! -x "$PERRY" ]]; then PERRY="$REPO_ROOT/target/debug/perry"; fi +if [[ ! -x "$PERRY" ]]; then + echo "SKIP: perry binary not found (build with cargo build -p perry)" + exit 0 +fi + +TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TMPDIR"' EXIT + +cat >"$TMPDIR/f.ts" <<'TS' +const O: any = Object; +if (typeof O.hasOwnProperty !== "function") throw new Error("hasOwnProperty not a function"); +if (typeof O.isPrototypeOf !== "function") throw new Error("isPrototypeOf not a function"); +if (typeof O.propertyIsEnumerable !== "function") throw new Error("propertyIsEnumerable not a function"); +const proto = O.getPrototypeOf({ a: 1 }); +if (O.hasOwnProperty.call(proto, "constructor") !== true) throw new Error("call(constructor) wrong"); +if (O.hasOwnProperty.call(proto, "nope") !== false) throw new Error("call(nope) wrong"); +console.log("OK"); +TS + +OUT="$("$PERRY" run "$TMPDIR/f.ts" 2>&1)" || { echo "FAIL: perry run errored"; echo "$OUT"; exit 1; } +if ! grep -q "^OK$" <<<"$OUT"; then echo "FAIL: expected OK, got:"; echo "$OUT"; exit 1; fi +echo "PASS: Object constructor exposes inherited Object.prototype methods" From 438a31ac52b8ba67f42435bbe6aef8c9d87dfc4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 18 Jun 2026 09:04:53 +0200 Subject: [PATCH 3/4] fix(codegen): honor constructor return-override on the standalone-symbol new path ECMAScript: a constructor that explicitly returns an object (or function) makes `new C()` evaluate to that value, not the freshly-allocated `this`. The default codegen path calls a shared standalone `_constructor` symbol (the inline path is opt-in via PERRY_INLINE_CTOR=1); that path discarded the ctor's return value and always returned `obj_box`, so a constructor like chalk's `class Chalk { constructor(o){ return chalkFactory(o); } }` (no-constructor-return) produced the empty default instance instead of the returned factory function. call_local_constructor_symbol now returns the standalone ctor's call result; the new site feeds it through js_ctor_return_override (the same helper the inline path already used). The standalone ctor returns `undefined` for an ordinary body, and js_ctor_return_override maps `undefined`/primitive back to `obj_box`, so ordinary constructors are unaffected (verified: import-smoke 59/59, functional OK count unchanged). --- crates/perry-codegen/src/lower_call/new.rs | 46 +++++++++++++++---- tests/test_ctor_return_override_standalone.sh | 46 +++++++++++++++++++ 2 files changed, 84 insertions(+), 8 deletions(-) create mode 100755 tests/test_ctor_return_override_standalone.sh diff --git a/crates/perry-codegen/src/lower_call/new.rs b/crates/perry-codegen/src/lower_call/new.rs index 7614462e38..f385fcc577 100644 --- a/crates/perry-codegen/src/lower_call/new.rs +++ b/crates/perry-codegen/src/lower_call/new.rs @@ -172,20 +172,24 @@ fn local_constructor_symbol_exists(ctx: &FnCtx<'_>, class: &perry_hir::Class) -> .contains_key(&(class.name.clone(), ctor_method_name)) } +/// Emit a call to the shared standalone `_constructor` symbol and +/// return the raw value it produced. The standalone ctor function returns +/// `undefined` for an ordinary constructor (implicit `return this`) or the +/// explicitly-returned value for a `return ` body — the caller applies +/// `js_ctor_return_override` to that raw value to honor ECMAScript's +/// constructor-return-override rule (a returned object/function replaces the +/// freshly-allocated `this`). Returns `None` when no standalone symbol exists. fn call_local_constructor_symbol( ctx: &mut FnCtx<'_>, class: &perry_hir::Class, obj_box: &str, lowered_args: &[String], -) { +) -> Option { let ctor_method_name = format!("{}_constructor", class.name); - let Some(ctor_name) = ctx + let ctor_name = ctx .methods .get(&(class.name.clone(), ctor_method_name)) - .cloned() - else { - return; - }; + .cloned()?; // The standalone `_constructor` symbol's signature is the class's // OWN ctor params, OR — when the class has no own ctor — the closest // ancestor-with-a-ctor's params (codegen/artifacts.rs synthesizes the @@ -243,7 +247,7 @@ fn call_local_constructor_symbol( for arg in &ctor_values { ctor_args.push((DOUBLE, arg.as_str())); } - let _ = ctx.block().call(DOUBLE, &ctor_name, &ctor_args); + Some(ctx.block().call(DOUBLE, &ctor_name, &ctor_args)) } /// Lower `new ClassName(args…)` — Phase C.1. @@ -871,7 +875,33 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) -> || ctor_alias_collision || force_ctor_call { - call_local_constructor_symbol(ctx, class, &obj_box, &lowered_args); + // Apply ECMAScript constructor return-override semantics on the + // standalone-symbol path too. The shared `_constructor` symbol + // returns `undefined` for an ordinary ctor (implicit `return this`) or + // the explicitly-returned value for a `return ` body. Pre-fix this + // path discarded that value and always yielded `obj_box`, so a ctor like + // chalk's `class Chalk { constructor(o){ return chalkFactory(o); } }` + // produced the empty default instance instead of the returned factory + // function ("value is not a function" on `new Chalk(...).red(...)`). + // `js_ctor_return_override` returns `obj_box` for an `undefined`/ + // primitive (base) return, so ordinary ctors are unaffected. + if let Some(ctor_ret) = call_local_constructor_symbol(ctx, class, &obj_box, &lowered_args) { + let is_derived = class.extends.is_some() + || class.extends_name.is_some() + || class.native_extends.is_some() + || class.extends_expr.is_some(); + let is_derived_lit = if is_derived { "1" } else { "0" }; + let final_box = ctx.block().call( + DOUBLE, + "js_ctor_return_override", + &[ + (DOUBLE, &obj_box), + (DOUBLE, &ctor_ret), + (crate::types::I32, is_derived_lit), + ], + ); + return Ok(final_box); + } return Ok(obj_box); } diff --git a/tests/test_ctor_return_override_standalone.sh b/tests/test_ctor_return_override_standalone.sh new file mode 100755 index 0000000000..d34944a072 --- /dev/null +++ b/tests/test_ctor_return_override_standalone.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +set -euo pipefail + +# ECMAScript constructor return-override: a constructor that explicitly returns +# an object (or function) makes `new C()` evaluate to that object, not the +# freshly-allocated `this`. The default codegen path calls a shared standalone +# `_constructor` symbol (opt out with PERRY_INLINE_CTOR=1); that path +# discarded the ctor's return value and always yielded the empty default +# instance. chalk's `class Chalk { constructor(o){ return chalkFactory(o); } }` +# (no-constructor-return) depends on the function being returned. + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +PERRY="${PERRY_BIN:-${PERRY:-$REPO_ROOT/target/release/perry}}" +if [[ ! -x "$PERRY" ]]; then PERRY="$REPO_ROOT/target/debug/perry"; fi +if [[ ! -x "$PERRY" ]]; then + echo "SKIP: perry binary not found (build with cargo build -p perry)" + exit 0 +fi + +TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TMPDIR"' EXIT + +cat >"$TMPDIR/f.ts" <<'TS' +// Constructor returns a plain object. +class W1 { constructor() { return { kind: "obj" }; } } +const a: any = new W1(); +if (typeof a !== "object" || a.kind !== "obj") throw new Error("W1: " + typeof a + " " + a.kind); + +// Constructor returns a function (chalk's no-constructor-return shape). +function makeFn() { const f: any = (x: string) => "F:" + x; f.tag = "T"; return f; } +class W2 { constructor() { return makeFn(); } } +const b: any = new W2(); +if (typeof b !== "function" || b.tag !== "T" || b("x") !== "F:x") throw new Error("W2: " + typeof b); + +// Ordinary constructor (implicit return this) is unaffected. +class P { x: number; constructor(v: number) { this.x = v; } } +const p = new P(5); +if (p.x !== 5) throw new Error("P: " + p.x); + +console.log("OK"); +TS + +OUT="$("$PERRY" run "$TMPDIR/f.ts" 2>&1)" || { echo "FAIL: perry run errored"; echo "$OUT"; exit 1; } +if ! grep -q "^OK$" <<<"$OUT"; then echo "FAIL: expected OK, got:"; echo "$OUT"; exit 1; fi +echo "PASS: constructor return-override honored on the standalone-symbol path" From deec9faa2a9cd0c16b361ff1a304bd4e7f5da0e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 18 Jun 2026 09:18:08 +0200 Subject: [PATCH 4/4] fix(crypto): createPrivateKey/createPublicKey throw on invalid key material MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Node throws when createPrivateKey/createPublicKey receive input that is not valid key material (e.g. a plain non-PEM string) instead of producing a KeyObject with `type === undefined`. perry returned the raw string as a key, so its `.type` was undefined. jsonwebtoken depends on the throw: sign()/verify() do `try { createPrivateKey(secret) } catch { createSecretKey(...) }`, so a string HMAC secret must make createPrivateKey/createPublicKey throw to reach the symmetric-key fallback — otherwise HS256 signing reported "secretOrPrivateKey must be a symmetric key" and verify reported "invalid algorithm". js_crypto_create_private_key_value / js_crypto_create_public_key_value now throw (ERR_OSSL_PEM_NO_START_LINE) unless the input classifies as a real key surrogate or carries a PEM `-----BEGIN ... KEY-----` header. The legitimate createSecretKey(Buffer) path is unaffected (still type 'secret'). No functional/import-smoke regressions. --- crates/perry-stdlib/src/crypto/sign.rs | 47 +++++++++++++++++-- .../test_crypto_create_key_invalid_throws.sh | 45 ++++++++++++++++++ 2 files changed, 88 insertions(+), 4 deletions(-) create mode 100755 tests/test_crypto_create_key_invalid_throws.sh diff --git a/crates/perry-stdlib/src/crypto/sign.rs b/crates/perry-stdlib/src/crypto/sign.rs index f748e79ae1..a4928ace6b 100644 --- a/crates/perry-stdlib/src/crypto/sign.rs +++ b/crates/perry-stdlib/src/crypto/sign.rs @@ -354,10 +354,30 @@ pub unsafe extern "C" fn js_crypto_create_public_key( pub unsafe extern "C" fn js_crypto_create_private_key_value(key_bits: f64) -> *mut StringHeader { let pem = match crypto_key_input_to_private_pem(key_bits.to_bits()) { Some(pem) => pem, - None => return std::ptr::null_mut(), + None => { + perry_runtime::fs::validate::throw_type_error_with_code( + "Invalid key object type, expected private key material", + "ERR_INVALID_ARG_TYPE", + ); + } }; + let asym_type = classify_private_key_surrogate(&pem); + // Node validates key material: `createPrivateKey("not a pem")` THROWS, it + // does NOT produce a key with `type === undefined`. jsonwebtoken's sign() + // relies on this — it `try { createPrivateKey(secret) } catch { createSecretKey(...) }`, + // so a string HMAC secret must make createPrivateKey throw to reach the + // createSecretKey fallback. Accept only inputs we can classify as a real + // private key, or that carry a PEM `-----BEGIN ... PRIVATE KEY-----` header + // (covers encrypted / DER-in-PEM forms the surrogate parsers may not cover); + // reject everything else. + if asym_type.is_none() && !pem.contains("PRIVATE KEY") { + perry_runtime::fs::validate::throw_type_error_with_code( + "Invalid PEM formatted message.", + "ERR_OSSL_PEM_NO_START_LINE", + ); + } let ptr = js_string_from_bytes(pem.as_ptr(), pem.len() as u32); - if let Some(asym_type) = classify_private_key_surrogate(&pem) { + if let Some(asym_type) = asym_type { mark_keyobject_string(ptr, KeyKind::Private, asym_type); } ptr @@ -367,10 +387,29 @@ pub unsafe extern "C" fn js_crypto_create_private_key_value(key_bits: f64) -> *m pub unsafe extern "C" fn js_crypto_create_public_key_value(key_bits: f64) -> *mut StringHeader { let pem = match crypto_key_input_to_public_pem(key_bits.to_bits()) { Some(pem) => pem, - None => return std::ptr::null_mut(), + None => { + perry_runtime::fs::validate::throw_type_error_with_code( + "Invalid key object type, expected public key material", + "ERR_INVALID_ARG_TYPE", + ); + } }; + let asym_type = classify_public_key_surrogate(&pem); + // Mirror `createPrivateKey`: Node throws on non-key material rather than + // producing a `type === undefined` key. jsonwebtoken's verify() does + // `try { createPublicKey(secret) } catch { createSecretKey(...) }`, so a + // string HMAC secret must make createPublicKey throw to reach the + // createSecretKey fallback (otherwise verify picks PUB_KEY_ALGS and rejects + // an HS256 token with "invalid algorithm"). Accept only classifiable public + // keys or PEM-headed input; reject everything else. + if asym_type.is_none() && !pem.contains("PUBLIC KEY") && !pem.contains("PRIVATE KEY") { + perry_runtime::fs::validate::throw_type_error_with_code( + "Invalid PEM formatted message.", + "ERR_OSSL_PEM_NO_START_LINE", + ); + } let ptr = js_string_from_bytes(pem.as_ptr(), pem.len() as u32); - if let Some(asym_type) = classify_public_key_surrogate(&pem) { + if let Some(asym_type) = asym_type { mark_keyobject_string(ptr, KeyKind::Public, asym_type); } ptr diff --git a/tests/test_crypto_create_key_invalid_throws.sh b/tests/test_crypto_create_key_invalid_throws.sh new file mode 100755 index 0000000000..6033ace66c --- /dev/null +++ b/tests/test_crypto_create_key_invalid_throws.sh @@ -0,0 +1,45 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Node's `crypto.createPrivateKey` / `createPublicKey` THROW on input that is +# not valid key material (e.g. a plain non-PEM string), rather than producing a +# KeyObject with `type === undefined`. jsonwebtoken's sign()/verify() rely on +# this: they `try { createPrivateKey(secret) } catch { createSecretKey(...) }`, +# so a string HMAC secret must make createPrivateKey/createPublicKey throw to +# reach the createSecretKey fallback. Pre-fix perry returned the string as a +# bogus key (type undefined), so HS256 signing reported "secretOrPrivateKey +# must be a symmetric key" and verify reported "invalid algorithm". + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +PERRY="${PERRY_BIN:-${PERRY:-$REPO_ROOT/target/release/perry}}" +if [[ ! -x "$PERRY" ]]; then PERRY="$REPO_ROOT/target/debug/perry"; fi +if [[ ! -x "$PERRY" ]]; then + echo "SKIP: perry binary not found (build with cargo build -p perry)" + exit 0 +fi + +TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TMPDIR"' EXIT + +cat >"$TMPDIR/f.ts" <<'TS' +import { createPrivateKey, createPublicKey, createSecretKey } from "crypto"; + +let priv = false; +try { createPrivateKey("test-secret-key"); } catch (_) { priv = true; } +if (!priv) throw new Error("createPrivateKey should throw on a non-PEM string"); + +let pub = false; +try { createPublicKey("test-secret-key"); } catch (_) { pub = true; } +if (!pub) throw new Error("createPublicKey should throw on a non-PEM string"); + +// The legitimate symmetric-key fallback still works and is type 'secret'. +const sk: any = createSecretKey(Buffer.from("test-secret-key")); +if (sk.type !== "secret") throw new Error("createSecretKey type: " + sk.type); + +console.log("OK"); +TS + +OUT="$("$PERRY" run "$TMPDIR/f.ts" 2>&1)" || { echo "FAIL: perry run errored"; echo "$OUT"; exit 1; } +if ! grep -q "^OK$" <<<"$OUT"; then echo "FAIL: expected OK, got:"; echo "$OUT"; exit 1; fi +echo "PASS: createPrivateKey/createPublicKey throw on invalid key material"