From 4ca3f674aaa8f9e8225d0e91805efd9c4483edd5 Mon Sep 17 00:00:00 2001 From: Renegade334 Date: Sat, 31 May 2025 14:01:41 +0100 Subject: [PATCH] child_process: validate exec's `options.shell` as string --- lib/child_process.js | 20 ++++++++++++------- ...t-child-process-exec-any-shells-windows.js | 2 +- .../parallel/test-child-process-exec-shell.js | 15 ++++++++++++++ 3 files changed, 29 insertions(+), 8 deletions(-) create mode 100644 test/parallel/test-child-process-exec-shell.js diff --git a/lib/child_process.js b/lib/child_process.js index 17c6b69c118a75..99a9610fb7bc2b 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -199,7 +199,13 @@ function normalizeExecArgs(command, options, callback) { // Make a shallow copy so we don't clobber the user's options object. options = { __proto__: null, ...options }; - options.shell = typeof options.shell === 'string' ? options.shell : true; + + // Validate the shell, if present, otherwise request the default shell. + if (options.shell != null && options.shell !== true) { + validateString(options.shell, 'options.shell'); + } else { + options.shell = true; + } return { file: command, @@ -613,11 +619,12 @@ function normalizeSpawnArguments(file, args, options) { } // Validate the shell, if present. - if (options.shell != null && - typeof options.shell !== 'boolean' && - typeof options.shell !== 'string') { - throw new ERR_INVALID_ARG_TYPE('options.shell', - ['boolean', 'string'], options.shell); + if (options.shell != null) { + if (typeof options.shell !== 'boolean' && typeof options.shell !== 'string') { + throw new ERR_INVALID_ARG_TYPE('options.shell', + ['boolean', 'string'], options.shell); + } + validateArgumentNullCheck(options.shell, 'options.shell'); } // Validate argv0, if present. @@ -639,7 +646,6 @@ function normalizeSpawnArguments(file, args, options) { } if (options.shell) { - validateArgumentNullCheck(options.shell, 'options.shell'); if (args.length > 0 && !emittedDEP0190Already) { process.emitWarning( 'Passing args to a child process with shell option true can lead to security ' + diff --git a/test/parallel/test-child-process-exec-any-shells-windows.js b/test/parallel/test-child-process-exec-any-shells-windows.js index 5c34bc77308cc3..3db563c96f1c27 100644 --- a/test/parallel/test-child-process-exec-any-shells-windows.js +++ b/test/parallel/test-child-process-exec-any-shells-windows.js @@ -33,7 +33,7 @@ const testCopy = (shellName, shellPath) => { const system32 = `${process.env.SystemRoot}\\System32`; // Test CMD -test(true); +test(null); test('cmd'); testCopy('cmd.exe', `${system32}\\cmd.exe`); test('cmd.exe'); diff --git a/test/parallel/test-child-process-exec-shell.js b/test/parallel/test-child-process-exec-shell.js new file mode 100644 index 00000000000000..ea3a2263123b2f --- /dev/null +++ b/test/parallel/test-child-process-exec-shell.js @@ -0,0 +1,15 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const { exec, execSync } = require('child_process'); + +const invalidArgTypeError = { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' +}; + +for (const fn of [exec, execSync]) { + assert.throws(() => fn('should-throw-on-boolean-shell-option', { shell: false }), invalidArgTypeError); +} + +// TODO: add DEP0196 tests following runtime deprecation