fix: resolve the shell from a trusted PATH

README.md

@@ -57,7 +57,15 @@ spawned process.
   the system default shell is used. If a String, that specific shell is used.
   When a shell is used, the given command runs from within that shell by
   concatenating the command and its escaped arguments and running the result.
-  This option is _not_ passed through to `child_process.spawn`.
+  An explicit shell path is used as provided. A bare shell name is resolved from
+  the ambient process `PATH` (`process.env.PATH`) rather than any `PATH` supplied
+  via `opts.env`, so an untrusted `opts.env.PATH` cannot substitute the shell
+  interpreter. Relative ambient `PATH` entries are resolved against
+  `process.cwd()` before `opts.cwd` is applied. On Windows, the current directory
+  is not searched unless the ambient `PATH` explicitly contains `.` or a relative
+  directory. If a bare shell name cannot be resolved, the returned promise
+  rejects with `ENOENT`. This option is _not_ passed through to
+  `child_process.spawn`.
 - Any other options for `child_process.spawn` can be passed as well.
 
 ### `promiseSpawn.open(arg, opts, extra)` -> `Promise`

lib/index.js

@@ -1,7 +1,9 @@
 'use strict'
 
 const { spawn } = require('child_process')
+const { EventEmitter } = require('events')
 const os = require('os')
+const path = require('path')
 const which = require('which')
 
 const escape = require('./escape.js')
@@ -63,6 +65,79 @@ const promiseSpawn = (cmd, args, opts = {}, extra = {}) => {
   return promise
 }
 
+const hasPath = (command) => process.platform === 'win32'
+  ? /[\\/:]/.test(command)
+  : command.includes('/')
+
+const unquotePath = (pathPart) =>
+  /^".*"$/.test(pathPart) ? pathPart.slice(1, -1) : pathPart
+
+const resolveShell = (command) => {
+  if (hasPath(command)) {
+    return command
+  }
+
+  const pathExt = process.env.PATHEXT
+  if (process.platform !== 'win32') {
+    const resolved = which.sync(command, {
+      path: process.env.PATH,
+      pathExt,
+      nothrow: true,
+    })
+    return !resolved || path.posix.isAbsolute(resolved)
+      ? resolved
+      : path.posix.resolve(process.cwd(), resolved)
+  }
+
+  if (!process.env.PATH) {
+    return null
+  }
+
+  for (const pathPart of process.env.PATH.split(path.win32.delimiter)) {
+    if (!pathPart) {
+      continue
+    }
+
+    const directory = unquotePath(pathPart)
+    if (!directory) {
+      continue
+    }
+
+    const candidate = path.win32.resolve(directory, command)
+    const resolved = which.sync(candidate, {
+      pathExt,
+      nothrow: true,
+    })
+    if (resolved) {
+      return resolved
+    }
+  }
+
+  return null
+}
+
+const getNotFoundError = (command) =>
+  Object.assign(new Error(`not found: ${command}`), { code: 'ENOENT' })
+
+const rejectNotFound = (command, cmd, args, opts, extra) => {
+  const proc = new EventEmitter()
+  proc.stdin = null
+  proc.kill = () => false
+
+  const error = Object.assign(getNotFoundError(command), {
+    cmd,
+    args,
+    ...stdioResult([], [], opts),
+    ...extra,
+  })
+  const promise = Promise.reject(error)
+  promise.stdin = proc.stdin
+  promise.process = proc
+
+  process.nextTick(() => proc.emit('exit', null, null))
+  return promise
+}
+
 const spawnWithShell = (cmd, args, opts, extra) => {
   let command = opts.shell
   // if shell is set to true, we use a platform default. we can't let the core
@@ -73,6 +148,12 @@ const spawnWithShell = (cmd, args, opts, extra) => {
     command = process.platform === 'win32' ? (process.env.ComSpec || 'cmd.exe') : 'sh'
   }
 
+  const resolved = resolveShell(command)
+  if (!resolved) {
+    return rejectNotFound(command, cmd, args, opts, extra)
+  }
+  command = resolved
+
   const options = { ...opts, shell: false }
   const realArgs = []
   let script = cmd
@@ -109,7 +190,7 @@ const spawnWithShell = (cmd, args, opts, extra) => {
     try {
       pathToInitial = which.sync(initialCmd, {
         path: (options.env && findInObject(options.env, 'PATH')) || process.env.PATH,
-        pathext: (options.env && findInObject(options.env, 'PATHEXT')) || process.env.PATHEXT,
+        pathExt: (options.env && findInObject(options.env, 'PATHEXT')) || process.env.PATHEXT,
       }).toLowerCase()
     } catch (err) {
       pathToInitial = initialCmd.toLowerCase()

test/open.js

@@ -3,8 +3,22 @@
 const spawk = require('spawk')
 const t = require('tap')
 const os = require('node:os')
-
-const promiseSpawn = require('../lib/index.js')
+const path = require('node:path')
+
+const pathMock = {
+  ...path,
+  posix: {
+    ...path.posix,
+    isAbsolute: () => true,
+  },
+}
+
+// Stub shell resolution so the spawn assertions remain stable; trusted-PATH
+// resolution is tested in test/shell.js.
+const promiseSpawn = t.mock('../lib/index.js', {
+  path: pathMock,
+  which: { sync: (cmd) => cmd },
+})
 
 spawk.preventUnmatched()
 t.afterEach(() => {
@@ -164,6 +178,8 @@ t.test('process.platform === linux', (t) => {
       os: {
         release: () => 'Microsoft',
       },
+      path: pathMock,
+      which: { sync: (cmd) => cmd },
     })
     const browser = process.env.BROWSER
     process.env.BROWSER = '/mnt/c/Program Files (x86)/Google/Chrome/Application/chrome.exe'
@@ -188,6 +204,8 @@ t.test('process.platform === linux', (t) => {
       os: {
         release: () => 'microsoft',
       },
+      path: pathMock,
+      which: { sync: (cmd) => cmd },
     })
     const browser = process.env.BROWSER
     process.env.BROWSER = '/mnt/c/Program Files (x86)/Google/Chrome/Application/chrome.exe'