Skip to content

Commit a72c413

Browse files
authored
fix(ext/node): forward permissions when spawning deno with subcommand args (#35443)
When an npm tool running under Deno spawns a child process that is itself Deno, the node-compat layer translates the Node.js argv to Deno argv and grants the child full permissions (-A) so it behaves like a permissionless Node.js process. That injection was skipped whenever the spawned argv already started with a Deno subcommand: a tool that detected it was running under Deno and spawned something like "deno run <script>" had its args passed through verbatim, dropping the implicit -A. The child then ran with no permissions and tripped a permission prompt on the first env read (TERM), which hung because the parent CLI holds stdin in raw mode and the prompt could never be answered. This restores Node.js semantics in that pass-through path: when the subcommand executes user code (run/test/bench/serve) and no explicit permission flag is present, -A is injected. Callers that pass their own permission flags (for example vitest workers that spawn "run -A ...") are left untouched, and detection covers long, short, bundled and =value forms to mirror the CLI parser. Fixes #35441
1 parent dbcd1a9 commit a72c413

5 files changed

Lines changed: 183 additions & 3 deletions

File tree

libs/node_shim/lib.rs

Lines changed: 146 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3260,6 +3260,68 @@ pub fn is_deno_subcommand(arg: &str) -> bool {
32603260
DENO_SUBCOMMANDS.contains(&arg)
32613261
}
32623262

3263+
/// Subcommands that execute user code and accept permission flags. A spawned
3264+
/// Node.js process has no permission model, so when one of these is produced by
3265+
/// the node-compat translation without explicit permission flags, we grant `-A`
3266+
/// to mirror Node.js behavior (and to match the non-pass-through script path).
3267+
fn subcommand_runs_user_code(subcommand: &str) -> bool {
3268+
matches!(subcommand, "run" | "test" | "bench" | "serve")
3269+
}
3270+
3271+
/// Long-form permission flag names (without the leading `--`): allow/deny
3272+
/// grants plus `--permission-set`. Mirrors the permission flags defined in the
3273+
/// CLI parser (see `permission_args` in cli/args/flags.rs).
3274+
const PERMISSION_LONG_FLAGS: &[&str] = &[
3275+
"allow-all",
3276+
"allow-read",
3277+
"allow-write",
3278+
"allow-net",
3279+
"allow-env",
3280+
"allow-run",
3281+
"allow-sys",
3282+
"allow-ffi",
3283+
"allow-import",
3284+
"deny-read",
3285+
"deny-write",
3286+
"deny-net",
3287+
"deny-env",
3288+
"deny-run",
3289+
"deny-sys",
3290+
"deny-ffi",
3291+
"deny-import",
3292+
"permission-set",
3293+
];
3294+
3295+
/// Short-form permission flag characters. Mirrors the `.short(...)` aliases in
3296+
/// the CLI parser: -A/-R/-W/-N/-E/-S/-I plus -P for `--permission-set`.
3297+
const PERMISSION_SHORT_FLAGS: &[char] =
3298+
&['A', 'R', 'W', 'N', 'E', 'S', 'I', 'P'];
3299+
3300+
/// Whether `arg` is an explicit permission flag: long (`--allow-net`) or short
3301+
/// (`-A`, `-RW`), with or without an `=value` (permission flags require `=` for
3302+
/// values, so the value can be split off first).
3303+
fn is_permission_flag(arg: &str) -> bool {
3304+
let flag = arg.split('=').next().unwrap_or(arg);
3305+
if let Some(long) = flag.strip_prefix("--") {
3306+
return PERMISSION_LONG_FLAGS.contains(&long);
3307+
}
3308+
if let Some(shorts) = flag.strip_prefix('-') {
3309+
// Short flags may be bundled (e.g. `-RW`); treat the arg as a permission
3310+
// flag if every character is an alphabetic short flag and at least one is a
3311+
// permission flag.
3312+
return !shorts.is_empty()
3313+
&& shorts.chars().all(|c| c.is_ascii_alphabetic())
3314+
&& shorts.chars().any(|c| PERMISSION_SHORT_FLAGS.contains(&c));
3315+
}
3316+
false
3317+
}
3318+
3319+
/// Whether the args already carry an explicit permission flag, in which case we
3320+
/// must respect the caller's intent and not inject `-A`.
3321+
fn has_permission_flag(args: &[String]) -> bool {
3322+
args.iter().any(|a| is_permission_flag(a))
3323+
}
3324+
32633325
/// Resolve a Node-style entrypoint to a specifier that Deno's `run` subcommand
32643326
/// understands.
32653327
///
@@ -3366,12 +3428,23 @@ pub fn translate_to_deno_args(
33663428
let node_options = &mut result.node_options;
33673429

33683430
// Check if the args already look like Deno args (e.g., from vitest workers)
3369-
// If the first remaining arg is a Deno subcommand, pass through unchanged
3431+
// If the first remaining arg is a Deno subcommand, pass through unchanged.
33703432
if let Some(first_arg) = parsed_args.remaining_args.first()
33713433
&& is_deno_subcommand(first_arg)
33723434
{
3373-
// Already Deno-style args, return unchanged
3374-
result.deno_args = parsed_args.remaining_args;
3435+
let mut deno_args = parsed_args.remaining_args;
3436+
// A spawned Node.js process has no permission model. When a tool detects it
3437+
// is running under Deno and spawns an execution subcommand (e.g.
3438+
// `deno run <script>`) without any permission flags, grant `-A` so the
3439+
// child behaves like Node.js instead of tripping a permission prompt. Tools
3440+
// that pass explicit permission flags (e.g. vitest workers spawn
3441+
// `run -A ...`) are left untouched. See #35441.
3442+
if subcommand_runs_user_code(&deno_args[0])
3443+
&& !has_permission_flag(&deno_args)
3444+
{
3445+
deno_args.insert(1, "-A".to_string());
3446+
}
3447+
result.deno_args = deno_args;
33753448
return result;
33763449
}
33773450

@@ -5223,4 +5296,74 @@ mod tests {
52235296
"foo.js"
52245297
]
52255298
);
5299+
5300+
/// Helper for the deno-subcommand pass-through path (used when a tool spawns
5301+
/// Deno with deno-style args). Translation is mode-independent here, so use
5302+
/// the child_process options.
5303+
fn translate_passthrough(input: Vec<String>) -> Vec<String> {
5304+
let parsed_args = parse_args(input).unwrap();
5305+
let options = TranslateOptions::for_child_process();
5306+
translate_to_deno_args(parsed_args, &options).deno_args
5307+
}
5308+
5309+
#[test]
5310+
fn passthrough_run_without_perms_gets_allow_all() {
5311+
// Regression test for #35441: `deno run <script>` spawned without any
5312+
// permission flags must receive `-A` so the Node-compat child has full
5313+
// access instead of prompting.
5314+
assert_eq!(
5315+
translate_passthrough(svec!["run", "child.mjs"]),
5316+
svec!["run", "-A", "child.mjs"]
5317+
);
5318+
assert_eq!(
5319+
translate_passthrough(svec!["run", "npm:storybook", "init"]),
5320+
svec!["run", "-A", "npm:storybook", "init"]
5321+
);
5322+
}
5323+
5324+
#[test]
5325+
fn passthrough_run_with_explicit_perms_is_unchanged() {
5326+
// vitest workers and similar pass `-A` already; respect explicit flags,
5327+
// including long, short, bundled and `=value` forms.
5328+
for flags in [
5329+
svec!["-A"],
5330+
svec!["--allow-all"],
5331+
svec!["--allow-read"],
5332+
svec!["--allow-read=/etc"],
5333+
svec!["-R"],
5334+
svec!["-R=/etc"],
5335+
svec!["-RW"],
5336+
svec!["-N=localhost"],
5337+
svec!["--deny-net"],
5338+
svec!["-P=prod"],
5339+
] {
5340+
let mut input = svec!["run"];
5341+
input.extend(flags.iter().cloned());
5342+
input.push("worker.mjs".to_string());
5343+
assert_eq!(
5344+
translate_passthrough(input.clone()),
5345+
input,
5346+
"expected {input:?} to pass through unchanged"
5347+
);
5348+
}
5349+
}
5350+
5351+
#[test]
5352+
fn passthrough_run_with_non_permission_short_flag_gets_allow_all() {
5353+
// `-q` (quiet) is not a permission flag, so `-A` is still injected.
5354+
assert_eq!(
5355+
translate_passthrough(svec!["run", "-q", "worker.mjs"]),
5356+
svec!["run", "-A", "-q", "worker.mjs"]
5357+
);
5358+
}
5359+
5360+
#[test]
5361+
fn passthrough_non_execution_subcommand_is_unchanged() {
5362+
// Non-execution subcommands don't take `-A`; leave them alone.
5363+
assert_eq!(translate_passthrough(svec!["install"]), svec!["install"]);
5364+
assert_eq!(
5365+
translate_passthrough(svec!["add", "npm:foo"]),
5366+
svec!["add", "npm:foo"]
5367+
);
5368+
}
52265369
}

tests/specs/node/child_process_node_cli_args/__test__.jsonc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@
3535
"deno_subcommand_passthrough": {
3636
"args": "run -A --unstable-detect-cjs main.js deno_subcommand",
3737
"output": "deno_subcommand.out"
38+
},
39+
"permission_passthrough": {
40+
"args": "run -A --unstable-detect-cjs main.js permission_passthrough",
41+
"output": "permission_passthrough.out"
3842
}
3943
}
4044
}

tests/specs/node/child_process_node_cli_args/main.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,22 @@ async function testDenoSubcommandPassthrough() {
152152
console.log("passthrough:", result.stdout);
153153
}
154154

155+
async function testPermissionPassthrough() {
156+
// Regression test for #35441: spawning `deno run <script>` (deno-style args)
157+
// without permission flags must still grant the Node-compat child full
158+
// permissions, instead of dropping to no permissions and prompting/erroring.
159+
const path = require("path");
160+
const script = path.join(__dirname, "perm_child.js");
161+
162+
// No permission flags -> child should be granted -A.
163+
const result = await runChild(["run", script]);
164+
console.log("run without perms:", result.stdout);
165+
166+
// Explicit permission flag -> respected, child still has env access.
167+
const result2 = await runChild(["run", "--allow-env", script]);
168+
console.log("run with --allow-env:", result2.stdout);
169+
}
170+
155171
async function main() {
156172
switch (testName) {
157173
case "eval":
@@ -181,6 +197,9 @@ async function main() {
181197
case "deno_subcommand":
182198
await testDenoSubcommandPassthrough();
183199
break;
200+
case "permission_passthrough":
201+
await testPermissionPassthrough();
202+
break;
184203
default:
185204
console.error("Unknown test:", testName);
186205
process.exit(1);
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Child used by the #35441 regression test. Reading an env var requires
2+
// --allow-env; when spawned as `deno run <script>` (deno-style args) without
3+
// permission flags, the Node-compat translation must still grant full
4+
// permissions, so this should print "env-ok" instead of throwing.
5+
let result;
6+
try {
7+
void process.env.PATH;
8+
result = "env-ok";
9+
} catch (e) {
10+
result = "env-denied: " + e.name;
11+
}
12+
console.log(result);
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
run without perms: env-ok
2+
run with --allow-env: env-ok

0 commit comments

Comments
 (0)