Skip to content

Commit f5014bb

Browse files
bartlomiejuclaude
andauthored
fix(ext/node): child_process improvements (internalMessage, argv0, DEP0190) (#32885)
## Summary Three child_process Node.js compatibility fixes: ### 1. Emit `internalMessage` event for `NODE_` prefixed IPC messages In Node.js, IPC messages with a `cmd` property starting with `NODE_` are internal messages emitted as `"internalMessage"` instead of `"message"`. Deno was silently dropping these messages. ### 2. Support `argv0` option in `spawn`/`spawnSync` Pass `argv0` through a new internal `kArgv0` symbol down to the Rust op, which uses `std::os::unix::process::CommandExt::arg0()` to set `argv[0]` at the OS level via `execve`. This matches how Node.js handles it through libuv. ### 3. Emit DEP0190 warning for shell spawn with args Emit a `DeprecationWarning` (DEP0190) when `child_process.spawn()` is called with both args and `shell: true`, matching Node.js behavior. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fd73dff commit f5014bb

5 files changed

Lines changed: 44 additions & 4 deletions

File tree

ext/node/polyfills/internal/child_process.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ import { Pipe, socketType } from "ext:deno_node/internal_binding/pipe_wrap.ts";
7171
import { Server as NetServer, Socket } from "node:net";
7272
import { Socket as DgramSocket } from "node:dgram";
7373
import {
74+
kArgv0,
7475
kExtraStdio,
7576
kInputOption,
7677
kIpc,
@@ -131,6 +132,7 @@ export function stdioStringToArray(
131132
const kClosesNeeded = Symbol("_closesNeeded");
132133
const kClosesReceived = Symbol("_closesReceived");
133134
const kCanDisconnect = Symbol("_canDisconnect");
135+
let emittedShellDeprecation = false;
134136

135137
// We only want to emit a close event for the child process when all of
136138
// the writable streams have closed. The value of `child[kClosesNeeded]` should be 1 +
@@ -367,7 +369,9 @@ export class ChildProcess extends EventEmitter {
367369
] = normalizedStdio;
368370

369371
// buildCommand handles Node.js to Deno CLI arg translation when spawning Deno
370-
// Note: args[0] is argv0 (prepended by normalizeSpawnArguments), so we skip it
372+
// args[0] is argv0 (prepended by normalizeSpawnArguments). Capture it
373+
// before slicing so we can pass it via kArgv0 for OS-level argv[0].
374+
const argv0 = args.length > 0 ? args[0] : command;
371375
const [cmd, cmdArgs, includeNpmProcessState] = buildCommand(
372376
command,
373377
args.slice(1),
@@ -411,6 +415,7 @@ export class ChildProcess extends EventEmitter {
411415
[kExtraStdio]: extraStdioNormalized,
412416
[kNeedsNpmProcessState]: options[kNeedsNpmProcessState] ||
413417
includeNpmProcessState,
418+
[kArgv0]: argv0 !== cmd ? argv0 : undefined,
414419
}).spawn();
415420
this.pid = this.#process.pid;
416421

@@ -1126,6 +1131,15 @@ export function normalizeSpawnArguments(
11261131
// for shell interpretation, so don't escape.
11271132
let command;
11281133
if (args.length > 0) {
1134+
if (!emittedShellDeprecation) {
1135+
process.emitWarning(
1136+
"Passing args to a child process with shell option true can lead to security " +
1137+
"vulnerabilities, as the arguments are not escaped, only concatenated.",
1138+
"DeprecationWarning",
1139+
"DEP0190",
1140+
);
1141+
emittedShellDeprecation = true;
1142+
}
11291143
const escapedParts = [escapeShellArg(file), ...args.map(escapeShellArg)];
11301144
command = ArrayPrototypeJoin(escapedParts, " ");
11311145
} else {
@@ -1661,7 +1675,9 @@ export function spawnSync(
16611675
_channel, // TODO(kt3k): handle this correctly
16621676
] = normalizeStdioOption(stdio);
16631677
let includeNpmProcessState = false;
1664-
// Skip argv0 when calling buildCommand (same as #spawnInternal)
1678+
// args[0] is argv0 (prepended by normalizeSpawnArguments). Capture it
1679+
// before slicing so we can pass it via kArgv0 for OS-level argv[0].
1680+
const argv0 = args && args.length > 0 ? args[0] : command;
16651681
const argsToProcess = args && args.length > 0 ? args.slice(1) : [];
16661682
[command, args, includeNpmProcessState] = buildCommand(
16671683
command,
@@ -1676,6 +1692,7 @@ export function spawnSync(
16761692
args,
16771693
cwd,
16781694
env: mapValues(env, (value) => value.toString()),
1695+
[kArgv0]: argv0 !== command ? argv0 : undefined,
16791696
stdout: toDenoStdio(stdout_),
16801697
stderr: toDenoStdio(stderr_),
16811698
stdin: stdin_ == "inherit" ? "inherit" : "null",
@@ -1839,6 +1856,11 @@ export function setupChannel(
18391856
// TODO(nathanwhit): once we add support for sending
18401857
// handles, if we want to support deno-node IPC interop,
18411858
// we'll need to handle the NODE_HANDLE_* messages here.
1859+
// Emit as internalMessage for internal consumers.
1860+
if (serialization === "json") {
1861+
restorePrototype(msg);
1862+
}
1863+
nextTick(() => target.emit("internalMessage", msg));
18421864
continue;
18431865
}
18441866
}

ext/process/40_process.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ export const kExtraStdio = Symbol("extraStdio");
166166
export const kIpc = Symbol("ipc");
167167
export const kNeedsNpmProcessState = Symbol("needsNpmProcessState");
168168
export const kSerialization = Symbol("serialization");
169+
const kArgv0 = Symbol("argv0");
169170

170171
const illegalConstructorKey = Symbol("illegalConstructorKey");
171172

@@ -186,12 +187,14 @@ function spawnChildInner(command, apiName, {
186187
[kExtraStdio]: extraStdio = [],
187188
[kIpc]: ipc = -1,
188189
[kNeedsNpmProcessState]: needsNpmProcessState = false,
190+
[kArgv0]: argv0 = undefined,
189191
} = { __proto__: null }) {
190192
const child = op_spawn_child({
191193
cmd: pathFromURL(command),
192194
args: ArrayPrototypeMap(args, String),
193195
cwd: pathFromURL(cwd),
194196
clearEnv,
197+
argv0,
195198
env: ObjectEntries(env),
196199
uid,
197200
gid,
@@ -471,6 +474,7 @@ function spawnSyncInner(command, {
471474
windowsRawArguments = false,
472475
[kInputOption]: input,
473476
[kNeedsNpmProcessState]: needsNpmProcessState = false,
477+
[kArgv0]: argv0 = undefined,
474478
} = { __proto__: null }) {
475479
if (stdin === "piped") {
476480
throw new TypeError(
@@ -493,6 +497,7 @@ function spawnSyncInner(command, {
493497
detached: false,
494498
needsNpmProcessState,
495499
input,
500+
argv0,
496501
});
497502
return {
498503
success: result.status.success,
@@ -595,6 +600,7 @@ function spawnAndWaitSync(command, argsOrOptions, maybeOptions) {
595600
export {
596601
ChildProcess,
597602
Command,
603+
kArgv0,
598604
kill,
599605
kInputOption,
600606
Process,

ext/process/lib.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,8 @@ pub struct SpawnArgs {
250250
extra_stdio: Vec<Stdio>,
251251
detached: bool,
252252
needs_npm_process_state: bool,
253+
#[cfg(unix)]
254+
argv0: Option<String>,
253255
}
254256

255257
#[derive(Deserialize)]
@@ -486,7 +488,12 @@ fn create_command(
486488
}
487489

488490
#[cfg(not(windows))]
489-
command.args(args.args);
491+
{
492+
if let Some(ref argv0) = args.argv0 {
493+
command.arg0(argv0);
494+
}
495+
command.args(args.args);
496+
}
490497

491498
command.current_dir(run_env.cwd);
492499
command.env_clear();

tests/node_compat/config.jsonc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@
223223
"parallel/test-child-process-execsync-maxbuf.js": {},
224224
"parallel/test-child-process-exit-code.js": {},
225225
"parallel/test-child-process-flush-stdio.js": {},
226+
"parallel/test-child-process-internal.js": {},
226227
"parallel/test-child-process-fork.js": {},
227228
"parallel/test-child-process-fork-abort-signal.js": {},
228229
"parallel/test-child-process-fork-and-spawn.js": {},
@@ -245,9 +246,13 @@
245246
"parallel/test-child-process-send-type-error.js": {},
246247
"parallel/test-child-process-set-blocking.js": {},
247248
"parallel/test-child-process-silent.js": {},
249+
"parallel/test-child-process-spawn-argv0.js": {
250+
"windows": false
251+
},
248252
"parallel/test-child-process-spawn-controller.js": {},
249253
"parallel/test-child-process-spawn-error.js": {},
250254
"parallel/test-child-process-spawn-event.js": {},
255+
"parallel/test-child-process-spawn-shell.js": {},
251256
"parallel/test-child-process-spawn-timeout-kill-signal.js": {},
252257
"parallel/test-child-process-spawn-typeerror.js": {},
253258
"parallel/test-child-process-spawnsync-args.js": {},

tests/specs/node/child_process_shell_escape/main.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ PASS: Args with spaces work
1515
Test 8: Shell features work with string command
1616
PASS: Shell features work
1717
Test 9: Async spawn escapes args
18-
PASS: Async spawn injection blocked
18+
[WILDCARD]PASS: Async spawn injection blocked
1919
Test 10: Backtick + $VAR injection in args
2020
PASS: Backtick + $VAR injection blocked
2121
Test 11: $() + $VAR injection in args

0 commit comments

Comments
 (0)