From eb852a355aaa760b50119e5a433df6119c8992ca Mon Sep 17 00:00:00 2001 From: Brandon Cheng Date: Mon, 15 Apr 2024 12:00:16 -0400 Subject: [PATCH 1/2] Revert "fix: spawn EINVAL on Windows with script-shell configured" This reverts commit 326dba8dbcdef5ed6cb653ece71f754a6cd67b15. --- index.js | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/index.js b/index.js index 7404f3d..931c917 100644 --- a/index.js +++ b/index.js @@ -241,20 +241,6 @@ function runCmd_ (cmd, pkg, env, wd, opts, stage, unsafe, uid, gid, cb_) { conf.windowsVerbatimArguments = true } - // Spawning .bat and .cmd files on Windows requires the "shell" option to - // spawn to be set. Otherwise spawn will throw with EINVAL. - // - // https://nodejs.org/api/child_process.html#spawning-bat-and-cmd-files-on-windows - // https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2 - // - // The shell option is security sensitive. It should make sense for this - // usecase since scripts in package.json intentionally run on the shell. - // Avoiding setting the shell option in all cases to preserve existing - // behavior on non-Windows platforms. - if (process.platform === 'win32' && customShell && (customShell.endsWith('.bat') || customShell.endsWith('.cmd'))) { - conf.shell = true - } - opts.log.verbose('lifecycle', logid(pkg, stage), 'PATH:', env[PATH]) opts.log.verbose('lifecycle', logid(pkg, stage), 'CWD:', wd) opts.log.silly('lifecycle', logid(pkg, stage), 'Args:', [shFlag, cmd]) From a283761ddb0cb09b319a013852e7894261bc43a5 Mon Sep 17 00:00:00 2001 From: Brandon Cheng Date: Mon, 15 Apr 2024 12:52:58 -0400 Subject: [PATCH 2/2] fix: use spawn's shell option to follow npm and fix batch script shells --- index.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 931c917..1f53a0a 100644 --- a/index.js +++ b/index.js @@ -233,9 +233,7 @@ function runCmd_ (cmd, pkg, env, wd, opts, stage, unsafe, uid, gid, cb_) { const customShell = opts.scriptShell - if (customShell) { - sh = customShell - } else if (process.platform === 'win32') { + if (!customShell && process.platform === 'win32') { sh = process.env.comspec || 'cmd' shFlag = '/d /s /c' conf.windowsVerbatimArguments = true @@ -273,7 +271,21 @@ function runCmd_ (cmd, pkg, env, wd, opts, stage, unsafe, uid, gid, cb_) { return } - const proc = spawn(sh, [shFlag, cmd], conf, opts.log) + // The @npm/run-script library uses spawn's shell option when the script-shell + // .npmrc option is set. Do the same to match the behavior of npm. + // + // Note that this is required if script-shell is set to a .bat or .cmd file. + // On Windows, this will throw "spawn EINVAL" if spawn's shell option is + // unset. + // + // https://nodejs.org/api/child_process.html#spawning-bat-and-cmd-files-on-windows + // https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2 + // + // The shell option is security sensitive, but it should make sense for this + // usecase since scripts in package.json intentionally run on the shell. + const proc = customShell + ? spawn(cmd, [], { ...conf, shell: customShell }, opts.log) + : spawn(sh, [shFlag, cmd], conf, opts.log) proc.on('error', procError) proc.on('close', (code, signal) => {