From dee91a82b26d97ee0f4270b67dbeb3b39ed3c4a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 16 Jun 2026 14:02:07 +0200 Subject: [PATCH 1/2] fix(security): allow dynamic stdlib member access by default; gate U006 under --lockdown (#5263) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #503 dynamic-stdlib-dispatch refusal (`error[U006]`) was always-on and over-broad: it blocked legitimate packages that do dynamic member access over the linked fs namespace — graceful-fs stores its retry queue on `fs[Symbol.for('graceful-fs.queue')]`, fs-extra wraps the known `fs[method]` functions. Dynamic `fs[name]` over a *linked* namespace can only select among the methods Perry already statically linked (dynamic selection of a known set, not arbitrary code), so it is safe to allow by default. Part 1 — gate the refusal under lockdown instead of always-on: - Default `refuse_dynamic_stdlib_dispatch` is now false (allow). The thread-local default in perry-hir matches. - `--lockdown` (the supply-chain gate, #496) re-arms the refusal; the install into the HIR thread-locals now runs after the lockdown ladder is fully resolved. - An explicit `perry.allowDynamicStdlibDispatch: false` / `PERRY_ALLOW_DYNAMIC_STDLIB=0` still re-arms just this check (last wins). Part 2 — make dynamic access actually work at runtime: - String-keyed reads via the codegen NativeModuleRef fast-path (`js_native_module_property_by_name`) now consult the module-keyed override side-table, so `fs[name] = v` round-trips on reads (it already stored, but the read path skipped the table). - The top-level `fs` namespace object is now cached so a user-set symbol property (`fs[Symbol.for(...)] = queue`, keyed by object pointer in SYMBOL_PROPERTIES) round-trips — otherwise each NativeModuleRef minted a fresh object and the write was lost. Docs: dynamic-dispatch.md + lockdown.md updated to the new default + rationale. Tests: new default_allows_dynamic_stdlib_member_access HIR test; the existing refuses_* tests pin the armed gate and still pass. --- crates/perry-hir/src/ir/constants.rs | 7 +- .../tests/dynamic_stdlib_dispatch.rs | 30 +++++++++ .../perry-runtime/src/object/native_module.rs | 22 +++++++ .../perry/src/commands/compile/host_config.rs | 29 ++++++-- crates/perry/src/commands/compile/types.rs | 27 ++++++-- docs/src/cli/dynamic-dispatch.md | 66 ++++++++++++------- docs/src/cli/lockdown.md | 18 +++-- 7 files changed, 155 insertions(+), 44 deletions(-) diff --git a/crates/perry-hir/src/ir/constants.rs b/crates/perry-hir/src/ir/constants.rs index e1d4ac92fc..954f4dbd13 100644 --- a/crates/perry-hir/src/ir/constants.rs +++ b/crates/perry-hir/src/ir/constants.rs @@ -156,7 +156,12 @@ thread_local! { /// list lives in `lower/expr_member.rs::STDLIB_NAMESPACE_NAMES`. Set /// to false by `perry.allowDynamicStdlibDispatch: true` or /// `PERRY_ALLOW_DYNAMIC_STDLIB=1`. - static REFUSE_DYNAMIC_STDLIB_DISPATCH: std::cell::Cell = const { std::cell::Cell::new(true) }; + /// + /// #5263: default is now **false** (allow). Dynamic access over the linked + /// namespace can only select among already-linked members, so it is safe by + /// default; the compile driver re-arms it (`set_refuse_dynamic_stdlib_dispatch(true)`) + /// under `--lockdown` or an explicit opt-in. + static REFUSE_DYNAMIC_STDLIB_DISPATCH: std::cell::Cell = const { std::cell::Cell::new(false) }; /// #503: per-thread set of npm package names that opted out of the /// dynamic-stdlib-dispatch refusal (`perry.allowDynamicStdlibDispatch: diff --git a/crates/perry-hir/tests/dynamic_stdlib_dispatch.rs b/crates/perry-hir/tests/dynamic_stdlib_dispatch.rs index 6af0697711..7a13c3c2a7 100644 --- a/crates/perry-hir/tests/dynamic_stdlib_dispatch.rs +++ b/crates/perry-hir/tests/dynamic_stdlib_dispatch.rs @@ -255,6 +255,36 @@ fn global_disable_opts_out() { outcome.expect("disabling refusal must allow the call"); } +/// #5263 — dynamic stdlib member access is allow-by-default (refusal off +/// unless lockdown / explicit opt-in). graceful-fs (`fs[symbolKey]`) and +/// fs-extra (`fs[method]`) shapes must lower cleanly with the default config. +/// Mirrors the compile driver, which leaves `refuse_dynamic_stdlib_dispatch` +/// false by default. The armed-gate behavior is covered by the `refuses_*` +/// tests above, which explicitly `set_refuse_dynamic_stdlib_dispatch(true)`. +#[test] +fn default_allows_dynamic_stdlib_member_access() { + let src = r#" + import * as fs from "node:fs"; + const k: string = "stat" + "Sync"; + // fs-extra-style dynamic method selection + const m = (fs as any)[k]; + // graceful-fs-style symbol-keyed queue write + read + const key = Symbol.for("graceful-fs.queue"); + (fs as any)[key] = []; + const q = (fs as any)[key]; + "#; + let mut cache = SourceCache::new(); + let parsed = parse_typescript_with_cache(src, "test.ts", &mut cache).unwrap(); + // Default driver state: refusal OFF (#5263). + set_refuse_dynamic_stdlib_dispatch(false); + set_current_module_source(src.to_string()); + let outcome = lower_module(&parsed.module, "test", "/tmp/host.ts"); + clear_current_module_source(); + // Re-arm so we don't poison sibling tests on this thread. + set_refuse_dynamic_stdlib_dispatch(true); + outcome.expect("#5263: dynamic stdlib member access must lower by default"); +} + #[test] fn site_annotation_ignored_inside_node_modules() { // #996 — a malicious dependency must not be able to grant itself diff --git a/crates/perry-runtime/src/object/native_module.rs b/crates/perry-runtime/src/object/native_module.rs index 718ea16bb8..bfa855e4bb 100644 --- a/crates/perry-runtime/src/object/native_module.rs +++ b/crates/perry-runtime/src/object/native_module.rs @@ -3134,6 +3134,18 @@ fn should_cache_native_module_namespace(module_name: &str) -> bool { | "async_hooks.default" | "constants" | "constants.default" + // #5263: cache the top-level namespace objects whose dynamic + // member access is now allowed by default. A stable (cached) + // namespace object means a user-set symbol property + // (`fs[Symbol.for('graceful-fs.queue')] = queue`, keyed by object + // pointer in `SYMBOL_PROPERTIES`) round-trips on reads — otherwise + // each `NativeModuleRef` mints a fresh object and the write is lost. + // String-keyed writes already persist via the module-keyed + // `NATIVE_NAMESPACE_PROP_OVERRIDES` side-table. These are pure + // tag+name holders (all real dispatch keys off the module name, not + // object state), so caching only affects object identity. + | "fs" + | "fs.default" | "dns.default" | "dns/promises.default" | "child_process.default" @@ -3231,6 +3243,16 @@ pub unsafe extern "C" fn js_native_module_property_by_name( property_name_len, )) .unwrap_or(""); + // #5263 / monkey-patch parity: a user-stored override of a namespace + // property (`fs[k] = v`, `require('node:timers').setImmediate = fn`) wins + // all built-in resolution below — CJS exports are mutable in Node, and + // dynamic stdlib member writes are allowed by default. This mirrors + // `vt_get_own_field`, which the generic object-by-name read path uses; the + // codegen `NativeModuleRef` fast-path landed here without consulting the + // side-table, so writes via `PutValueSet` didn't round-trip on reads. + if let Some(value) = native_namespace_prop_override_get(module_name, property_name) { + return value; + } if module_name == "process.namespace" && property_name == "default" { return cjs_default_export_value("process") .unwrap_or_else(|| js_create_native_module_namespace(b"process".as_ptr(), 7)); diff --git a/crates/perry/src/commands/compile/host_config.rs b/crates/perry/src/commands/compile/host_config.rs index 10d778874b..b72638fe7e 100644 --- a/crates/perry/src/commands/compile/host_config.rs +++ b/crates/perry/src/commands/compile/host_config.rs @@ -451,13 +451,10 @@ pub(super) fn apply_pkg_and_toml_config( } _ => {} } - // #503: install the resolved configuration into the HIR thread-locals - // before any module lowering begins. Re-installed per-thread by - // `collect_modules.rs` (rayon workers don't inherit thread-locals), - // but this set covers the driver thread's own lowering work and - // serves as documentation of the source of truth. - perry_hir::set_refuse_dynamic_stdlib_dispatch(ctx.refuse_dynamic_stdlib_dispatch); - perry_hir::set_allow_dynamic_stdlib_packages(ctx.allow_dynamic_stdlib_packages.clone()); + // #503/#5263: the resolved configuration is installed into the HIR + // thread-locals further below — AFTER lockdown is fully resolved (env + + // CLI), because lockdown re-arms the dynamic-dispatch refusal that #5263 + // turned off by default. See the install just after the lockdown ladder. // #497: `PERRY_ALLOW_PERRY_FEATURES=1` opts every name into both // host allowlists at once — emergency escape hatch for builds @@ -516,6 +513,24 @@ pub(super) fn apply_pkg_and_toml_config( ctx.lockdown = true; } + // #5263: lockdown is the supply-chain gate that re-arms the + // dynamic-stdlib-dispatch refusal (#503), which is allow-by-default since + // #5263. An explicit `perry.allowDynamicStdlibDispatch: false` / + // `PERRY_ALLOW_DYNAMIC_STDLIB=0` already set `refuse_dynamic_stdlib_dispatch` + // true above; lockdown forces it on regardless. Computed here, after the + // lockdown ladder is fully resolved (package.json → env → CLI). + if ctx.lockdown { + ctx.refuse_dynamic_stdlib_dispatch = true; + } + + // #503/#5263: install the resolved configuration into the HIR + // thread-locals before any module lowering begins. Re-installed + // per-thread by `collect_modules.rs` (rayon workers don't inherit + // thread-locals), but this set covers the driver thread's own lowering + // work and serves as documentation of the source of truth. + perry_hir::set_refuse_dynamic_stdlib_dispatch(ctx.refuse_dynamic_stdlib_dispatch); + perry_hir::set_allow_dynamic_stdlib_packages(ctx.allow_dynamic_stdlib_packages.clone()); + // #5206 — strict-eval precedence (last wins): package.json // `perry.eval`/`perry.strict` (read above) → perry.toml `[perry] eval` / // `[perry] strict` → env `PERRY_ALLOW_EVAL=1` (forces OFF, back-compat) → diff --git a/crates/perry/src/commands/compile/types.rs b/crates/perry/src/commands/compile/types.rs index ac6a1d0036..b9a522875e 100644 --- a/crates/perry/src/commands/compile/types.rs +++ b/crates/perry/src/commands/compile/types.rs @@ -678,13 +678,18 @@ pub struct CompilationContext { /// rebuild without the `bundled-streams` feature. This set lets /// the registry drain inject the missing feature directly. pub extra_stdlib_features: BTreeSet<&'static str>, - /// #503: when true, HIR lowering refuses dynamic-dispatch on known + /// #503/#5263: when true, HIR lowering refuses dynamic-dispatch on known /// stdlib namespaces (`process[runtimeVar]()` and similar). Default - /// true. Sources, last wins: `perry.allowDynamicStdlibDispatch: true` - /// in package.json → env `PERRY_ALLOW_DYNAMIC_STDLIB=1` flips this - /// off. An array value (`["@scope/pkg", ...]`) keeps refusal on but - /// allows the listed packages — captured in - /// `allow_dynamic_stdlib_packages`. + /// **false** (#5263 — allow): dynamic member access over a *linked* + /// namespace can only reach methods perry already statically linked, so it + /// is dynamic selection among a known set, not arbitrary code. The refusal + /// is re-armed by `--lockdown` (the supply-chain gate, #496), or by an + /// explicit opt-in: `perry.allowDynamicStdlibDispatch: false` in + /// package.json / env `PERRY_ALLOW_DYNAMIC_STDLIB=0`. `…: true` / `=1` + /// keeps the default-allow even under those explicit knobs (last wins). An + /// array value (`["@scope/pkg", ...]`) is captured in + /// `allow_dynamic_stdlib_packages` and is meaningful only while refusal is + /// active (i.e. under lockdown / explicit re-enable). pub refuse_dynamic_stdlib_dispatch: bool, /// #5206: strict-eval mode. When true, a runtime-unknown `eval(...)` / /// `new Function()` site is a hard compile-time refusal @@ -914,7 +919,15 @@ impl CompilationContext { windows_subsystem: "auto".to_string(), entry_canonical: None, extra_stdlib_features: BTreeSet::new(), - refuse_dynamic_stdlib_dispatch: true, + // #5263: default-allow dynamic stdlib member access. Dynamic + // `fs[name]` over the *linked* namespace can only select among + // methods perry already statically linked (dynamic selection of a + // known set, not arbitrary code), so it is safe by default and is + // needed by legitimate packages (graceful-fs's symbol-keyed retry + // queue, fs-extra's `fs[method]` wrapping). The refusal is re-armed + // under `--lockdown` (the supply-chain gate) or by an explicit + // `perry.allowDynamicStdlibDispatch: false` / `PERRY_ALLOW_DYNAMIC_STDLIB=0`. + refuse_dynamic_stdlib_dispatch: false, strict_eval: false, strict_dynamic_import: false, strict_unimplemented: false, diff --git a/docs/src/cli/dynamic-dispatch.md b/docs/src/cli/dynamic-dispatch.md index 7a75a8331f..d334f20518 100644 --- a/docs/src/cli/dynamic-dispatch.md +++ b/docs/src/cli/dynamic-dispatch.md @@ -1,25 +1,39 @@ -# Dynamic Stdlib Dispatch (`@perry-allow-dynamic`) +# Dynamic Stdlib Dispatch (`--lockdown`) -Perry refuses compile-time *dynamic dispatch* on Node-core stdlib namespaces. -A call site like +Perry can refuse compile-time *dynamic dispatch* on Node-core stdlib +namespaces. A call site like ```typescript,no-test const m = "exit"; (process as any)[m](0); ``` -fails to compile. The check exists to catch the standard string-based -obfuscation pattern used by malicious npm packages: -`process["bind" + "ing"]("dns")`, `globalThis[atob("ZXZhbA==")]()`, +is the standard string-based obfuscation pattern used by malicious npm +packages: `process["bind" + "ing"]("dns")`, `globalThis[atob("ZXZhbA==")]()`, `fs[methodName]()` where `methodName` is computed at runtime. -The pass is purely compile-time — **zero runtime cost** — and is on by -default. Issue [#503](https://github.com/PerryTS/perry/issues/503) tracks the -design. - -## What's checked - -Dynamic dispatch is refused when **all** of the following hold: +**Default: allowed.** Since [#5263](https://github.com/PerryTS/perry/issues/5263) +the refusal is *off* by default. Dynamic `fs[name]` over a namespace Perry has +already statically linked can only *select among the methods that were linked* +— it is dynamic selection of a known set, not a way to reach arbitrary code — +so it is safe to allow, and legitimate packages depend on it (graceful-fs +stores its retry queue on `fs[Symbol.for('graceful-fs.queue')]`; fs-extra wraps +the known `fs[method]` functions). Dynamic reads resolve the linked member by +name; symbol/string writes the program performs are stored on the namespace +(graceful-fs's `fs[symbol] = queue` then reads it back). + +**The refusal is re-armed by [`--lockdown`](./lockdown.md)** — the +supply-chain gate — and by an explicit opt-out (below). Under those, the +site below fails to compile with `error[U006]`. The pass is purely +compile-time (**zero runtime cost**). Issue +[#503](https://github.com/PerryTS/perry/issues/503) tracks the original design. + +## What's checked (when the refusal is armed) + +The refusal is armed under `--lockdown` (or `perry.lockdown: true` / +`PERRY_LOCKDOWN=1`), or by an explicit `perry.allowDynamicStdlibDispatch: false` +/ `PERRY_ALLOW_DYNAMIC_STDLIB=0`. When armed, dynamic dispatch is refused when +**all** of the following hold: 1. The receiver resolves to a known Node-core stdlib namespace: `process`, `fs`, `crypto`, `child_process`, `net`, `os`, `path`, `http`, @@ -38,9 +52,10 @@ const k = "greet"; me[k]("world"); // ✓ user object, not a stdlib namespace ``` -## Opt-outs +## Opt-outs (when armed) -The error message lists the available opt-outs in priority order: +When the refusal is armed and a site is refused, the error message lists the +available opt-outs in priority order: ### 1. Replace with a static call @@ -109,14 +124,19 @@ PERRY_ALLOW_DYNAMIC_STDLIB=1 perry build src/main.ts CI can enforce the check by setting `PERRY_ALLOW_DYNAMIC_STDLIB=0`, which beats any package.json opt-out. -## Why on by default - -The check is the cheapest possible defense against the -dispatch-by-string class of supply-chain evasion. The cost to legitimate -code is essentially zero — static calls and literal-keyed access compile -unchanged. Code that genuinely needs the indirection has four ways to -say so explicitly, and the failure mode is a build error rather than a -silent miss in detection. +## Why allowed by default (and gated under lockdown) + +Dynamic member access over a *linked* stdlib namespace can only reach the +methods Perry already linked statically — it is dynamic *selection* among a +known set, not a way to construct or reach arbitrary code. The +dispatch-by-string obfuscation it could otherwise hide is only meaningful when +paired with the arbitrary-code surfaces that `--lockdown` already forbids +(`eval`/`Function`, `child_process`, native archives). So the refusal belongs +with the rest of the lockdown gate, not always-on: default builds allow it (so +graceful-fs, fs-extra, and similar legitimate patterns compile), while +security-sensitive builds opt into `--lockdown` and get the refusal back. An +explicit `perry.allowDynamicStdlibDispatch: false` / `PERRY_ALLOW_DYNAMIC_STDLIB=0` +re-arms just this check without the rest of lockdown. See [`#503`](https://github.com/PerryTS/perry/issues/503) for design discussion and the broader supply-chain hardening series ([`#495`–`#506`] diff --git a/docs/src/cli/lockdown.md b/docs/src/cli/lockdown.md index 90cdbe2ae0..5c2d04f0ef 100644 --- a/docs/src/cli/lockdown.md +++ b/docs/src/cli/lockdown.md @@ -19,10 +19,12 @@ SwiftUI / JS) inherits the protection from one choke point. | `perry-jsruntime` (QuickJS) in graph | `ctx.needs_js_runtime` flipped during collection. | | `perry.nativeLibrary` archive reference | `ctx.native_libraries` non-empty after resolution. | | `child_process.*` call sites | HIR walker covers every `ChildProcess*` variant + the general-shape `NativeMethodCall { module: "child_process", … }` fallback. | +| Dynamic stdlib dispatch (`fs[runtimeVar]`) | HIR lowering re-arms the `#503` refusal (`error[U006]`). Allowed by default since [#5263](https://github.com/PerryTS/perry/issues/5263); lockdown turns it back on. | -All three checks run together; the failure lists every offending -surface in one combined diagnostic so the reviewer can address the -whole surface at once. +The `child_process`/jsruntime/nativeLibrary checks run together as a +combined post-collect diagnostic; the dynamic-dispatch refusal is enforced +during HIR lowering (it re-arms the always-existing `#503` pass). The failure +lists every offending surface so the reviewer can address it at once. ## Enabling lockdown (priority order) @@ -63,9 +65,13 @@ are summarised as `... and N more`. Lockdown is the umbrella mode for the wider supply-chain hardening series ([`#495`–`#506`](https://github.com/PerryTS/perry/issues?q=is%3Aissue+label%3Aenhancement+security)): -- [`#503`](https://github.com/PerryTS/perry/issues/503) — refuses - dynamic stdlib dispatch (`obj[runtimeVar]()`). On by default - regardless of lockdown. +- [`#503`](https://github.com/PerryTS/perry/issues/503) / + [`#5263`](https://github.com/PerryTS/perry/issues/5263) — refuses + dynamic stdlib dispatch (`obj[runtimeVar]()`). **Allowed by default** + (dynamic selection over a linked namespace can only reach already-linked + members); lockdown re-arms the refusal. An explicit + `perry.allowDynamicStdlibDispatch: false` / `PERRY_ALLOW_DYNAMIC_STDLIB=0` + re-arms just this check without the rest of lockdown. - [`#499`](https://github.com/PerryTS/perry/issues/499) — gates `perry-jsruntime` behind explicit host opt-in. Lockdown forces the gate to its strict default. From 5ce788b2a873e3f047303c0cd35b4700716c131a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 16 Jun 2026 14:39:03 +0200 Subject: [PATCH 2/2] address CodeRabbit (#5265): fix stale 'on by default' comment, drop dead fs.default cache entry, clarify string-key storage docs - expr_member.rs: refusal pass is OFF by default since #5263 (re-armed under lockdown/opt-out) - native_module.rs: remove dead `fs.default` namespace-cache arm (referenced nowhere else; no cjs_default_* mappings emit it; the `fs` arm backs symbol-key persistence). fs[symbol] + fs[name] verified still working; dynamic_stdlib tests 24/24. - docs/dynamic-dispatch.md: clarify string keys persist via module-keyed override table, symbol keys on cached namespace object --- crates/perry-hir/src/lower/expr_member.rs | 6 ++++-- crates/perry-runtime/src/object/native_module.rs | 1 - docs/src/cli/dynamic-dispatch.md | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/perry-hir/src/lower/expr_member.rs b/crates/perry-hir/src/lower/expr_member.rs index 793f8e2472..b932d096e2 100644 --- a/crates/perry-hir/src/lower/expr_member.rs +++ b/crates/perry-hir/src/lower/expr_member.rs @@ -2369,8 +2369,10 @@ fn lower_member_inner(ctx: &mut LoweringContext, member: &ast::MemberExpr) -> Re // - the index is NOT a string literal at the source level // (literal keys are caught by the fold below, and never // constitute string-obfuscation), - // - the refusal pass is enabled (`PERRY_ALLOW_DYNAMIC_STDLIB=0` / - // `perry.allowDynamicStdlibDispatch: false`; on by default), + // - the refusal pass is enabled — OFF by default since #5263, + // re-armed under `--lockdown` / `perry.lockdown` or the explicit + // opt-out `PERRY_ALLOW_DYNAMIC_STDLIB=0` / + // `perry.allowDynamicStdlibDispatch: false`, // - the currently-lowering source file does NOT belong to a // package on the per-package allow-list, and // - there is no `// @perry-allow-dynamic` line annotation on diff --git a/crates/perry-runtime/src/object/native_module.rs b/crates/perry-runtime/src/object/native_module.rs index bfa855e4bb..d3ede29229 100644 --- a/crates/perry-runtime/src/object/native_module.rs +++ b/crates/perry-runtime/src/object/native_module.rs @@ -3145,7 +3145,6 @@ fn should_cache_native_module_namespace(module_name: &str) -> bool { // tag+name holders (all real dispatch keys off the module name, not // object state), so caching only affects object identity. | "fs" - | "fs.default" | "dns.default" | "dns/promises.default" | "child_process.default" diff --git a/docs/src/cli/dynamic-dispatch.md b/docs/src/cli/dynamic-dispatch.md index d334f20518..d0525ad7eb 100644 --- a/docs/src/cli/dynamic-dispatch.md +++ b/docs/src/cli/dynamic-dispatch.md @@ -19,8 +19,9 @@ already statically linked can only *select among the methods that were linked* so it is safe to allow, and legitimate packages depend on it (graceful-fs stores its retry queue on `fs[Symbol.for('graceful-fs.queue')]`; fs-extra wraps the known `fs[method]` functions). Dynamic reads resolve the linked member by -name; symbol/string writes the program performs are stored on the namespace -(graceful-fs's `fs[symbol] = queue` then reads it back). +name; writes the program performs persist and read back — **string** keys via a +module-keyed override side-table, **symbol** keys on the cached namespace object +(so graceful-fs's `fs[Symbol.for('graceful-fs.queue')] = queue` round-trips). **The refusal is re-armed by [`--lockdown`](./lockdown.md)** — the supply-chain gate — and by an explicit opt-out (below). Under those, the