diff --git a/ng-dev/utils/child-process.ts b/ng-dev/utils/child-process.ts index 9052e93de..9cb8915a2 100644 --- a/ng-dev/utils/child-process.ts +++ b/ng-dev/utils/child-process.ts @@ -85,6 +85,11 @@ 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. + // 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); @@ -95,7 +100,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 +121,18 @@ 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. + // 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); return processAsyncCmd( commandText, options, - _spawn(command, args, {...options, env, shell: true, stdio: 'pipe'}), + _spawn(command, args, {...options, env, shell: false, stdio: 'pipe'}), ); } @@ -133,8 +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 ` + + `(command: ${command}). Prefer ChildProcess.spawn with an array of arguments.`, + ); const env = getEnvironmentForNonInteractiveCommand(options.env); return processAsyncCmd(command, options, _exec(command, {...options, env})); }