Skip to content

child_process: validate options.shell and correctly enforce shell invocation in exec/execSync

The intention of exec and execSync is that a shell is always invoked to run the provided command, as opposed to execFile, which by default executes the target directly and does not invoke a shell. Unlike its counterparts elsewhere in child_process, exec's options.shell is documented as not accepting a boolean, as it's not intended to be possible to disable shell invocation and switch to execFile-like behaviour.

exec's options.shell is currently handled the following way:

  • passed to normalizeExecArgs()
    • if the shell parameter is not a string, it is coerced to boolean true
  • passed to normalizeSpawnArguments()
    • if non-nullish, the parameter is validated as being of type string|boolean
    • if the parameter is truthy at this point, then spawn() runs a shell; if not, it executes the target directly

The intention is clearly to force shell invocation with exec/execSync in all cases, but the current implementation has the following problems:

  • The parameter is not validated at all; instead, if it's not a string, it's silently ignored and coerced to true, which passes downstream validation in normalizeSpawnArguments(). In other words, passing an invalid value to options.shell will never raise a validation error.
  • The logic in the normalize functions combines to yield an edge case, which is when exec/execSync is called with options.shell set to the empty string:
    • the parameter is passed through unchanged by normalizeExecArgs()
    • the parameter passes validation in normalizeSpawnArguments()
    • the parameter is not truthy, so normalizeSpawnArguments() fails to set up the shell
    • the child process is spawned without a shell, with the file target set to the entire command string:
      child_process.exec('./script some-parameter', { shell: '' })
      // should invoke a shell to run "./script" and pass an argument
      // instead, attempts to execute a file called "script some-parameter"

This patch makes two simple changes. Firstly, it validates the shell option sent to exec/execSync as a string. Secondly, it coerces all falsy values to boolean true, so will never bypass shell setup. (This makes { shell: '' } equivalent to { shell: undefined }, which matches how it behaves in the other child_process functions.)

Merge request reports

Loading