fix: correctly double escape when script runs a known .cmd file

Author: nlf

Diff for: README.md

@@ -157,4 +157,8 @@ escaped to ensure they are processed as literal strings. We then instruct
 the shell to execute the script file, and when the process exits we remove
 the temporary file.
 
+In Windows, when the shell is cmd, and when the initial command in the script
+is a known batch file (i.e. `something.cmd`) we double escape additional
+arguments so that the shim scripts npm installs work correctly.
+
 The actual implementation of the escaping is in `lib/escape.js`.

Diff for: lib/make-spawn-args.js

@@ -3,7 +3,7 @@ const isWindows = require('./is-windows.js')
 const setPATH = require('./set-path.js')
 const { chmodSync: chmod, unlinkSync: unlink, writeFileSync: writeFile } = require('fs')
 const { tmpdir } = require('os')
-const { resolve } = require('path')
+const { isAbsolute, resolve } = require('path')
 const which = require('which')
 const npm_config_node_gyp = require.resolve('node-gyp/bin/node-gyp.js')
 const escape = require('./escape.js')
@@ -20,35 +20,71 @@ const makeSpawnArgs = options => {
     stdioString = false,
   } = options
 
+  const spawnEnv = setPATH(path, {
+    // we need to at least save the PATH environment var
+    ...process.env,
+    ...env,
+    npm_package_json: resolve(path, 'package.json'),
+    npm_lifecycle_event: event,
+    npm_lifecycle_script: cmd,
+    npm_config_node_gyp,
+  })
+
   let scriptFile
   let script = ''
+
   const isCmd = /(?:^|\\)cmd(?:\.exe)?$/i.test(scriptShell)
   if (isCmd) {
+    let initialCmd = ''
+    let insideQuotes = false
+    for (let i = 0; i < cmd.length; ++i) {
+      const char = cmd.charAt(i)
+      if (char === ' ' && !insideQuotes) {
+        break
+      }
+
+      initialCmd += char
+      if (char === '"' || char === "'") {
+        insideQuotes = !insideQuotes
+      }
+    }
+
+    let pathToInitial
+    try {
+      pathToInitial = which.sync(initialCmd, { path: spawnEnv.path, pathext: spawnEnv.pathext })
+    } catch (err) {
+      pathToInitial = initialCmd
+    }
+
+    const doubleEscape = pathToInitial.toLowerCase().endsWith('.cmd')
+
     scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.cmd`)
     script += '@echo off\n'
-    script += `${cmd} ${args.map((arg) => escape.cmd(arg)).join(' ')}`
+    const escapedArgs = args.map((arg) => {
+      let result = escape.cmd(arg)
+      if (doubleEscape) {
+        result = escape.cmd(result)
+      }
+      return result
+    })
+    script += `${cmd} ${escapedArgs.join(' ')}`
   } else {
-    const shellPath = which.sync(scriptShell)
+    const shebang = isAbsolute(scriptShell)
+      ? `#!${scriptShell}`
+      : `#!/usr/bin/env ${scriptShell}`
     scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.sh`)
-    script += `#!${shellPath}\n`
+    script += `${shebang}\n`
     script += `${cmd} ${args.map((arg) => escape.sh(arg)).join(' ')}`
   }
+
   writeFile(scriptFile, script)
   if (!isCmd) {
     chmod(scriptFile, '0775')
   }
   const spawnArgs = isCmd ? ['/d', '/s', '/c', scriptFile] : ['-c', scriptFile]
 
   const spawnOpts = {
-    env: setPATH(path, {
-      // we need to at least save the PATH environment var
-      ...process.env,
-      ...env,
-      npm_package_json: resolve(path, 'package.json'),
-      npm_lifecycle_event: event,
-      npm_lifecycle_script: cmd,
-      npm_config_node_gyp,
-    }),
+    env: spawnEnv,
     stdioString,
     stdio,
     cwd: path,

Diff for: test/make-spawn-args.js

@@ -81,6 +81,9 @@ if (isWindows) {
         windowsVerbatimArguments: true,
       }, 'got expected options')
 
+      const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
+      // the contents will have a trailing space if no args are passed
+      t.equal(contents, `@echo off\nscript "quoted parameter"; second command `)
       t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
       cleanup()
       t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')
@@ -93,6 +96,7 @@ if (isWindows) {
       whichPaths.set('blrorp', '/bin/blrorp')
       t.teardown(() => {
         whichPaths.delete('blrorp')
+        delete process.env.ComSpec
       })
       const [shell, args, opts, cleanup] = makeSpawnArgs({
         event: 'event',
@@ -147,6 +151,114 @@ if (isWindows) {
       t.end()
     })
 
+    t.test('single escapes when initial command is not a batch file', (t) => {
+      whichPaths.set('script', '/path/script.exe')
+      t.teardown(() => whichPaths.delete('script'))
+
+      const [shell, args, opts, cleanup] = makeSpawnArgs({
+        event: 'event',
+        path: 'path',
+        cmd: 'script',
+        args: ['"quoted parameter";', 'second command'],
+      })
+      t.equal(shell, 'cmd', 'default shell applies')
+      t.match(args, ['/d', '/s', '/c', /\.cmd$/], 'got expected args')
+      t.match(opts, {
+        env: {
+          npm_package_json: /package\.json$/,
+          npm_lifecycle_event: 'event',
+          npm_lifecycle_script: 'script',
+          npm_config_node_gyp: require.resolve('node-gyp/bin/node-gyp.js'),
+        },
+        stdio: undefined,
+        cwd: 'path',
+        windowsVerbatimArguments: true,
+      }, 'got expected options')
+
+      const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
+      t.equal(contents, `@echo off\nscript ^"\\^"quoted parameter\\^";^" ^"second command^"`)
+      t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
+      cleanup()
+      t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')
+
+      t.end()
+    })
+
+    t.test('double escapes when initial command is a batch file', (t) => {
+      whichPaths.set('script', '/path/script.cmd')
+      t.teardown(() => whichPaths.delete('script'))
+
+      const [shell, args, opts, cleanup] = makeSpawnArgs({
+        event: 'event',
+        path: 'path',
+        cmd: 'script',
+        args: ['"quoted parameter";', 'second command'],
+      })
+      t.equal(shell, 'cmd', 'default shell applies')
+      t.match(args, ['/d', '/s', '/c', /\.cmd$/], 'got expected args')
+      t.match(opts, {
+        env: {
+          npm_package_json: /package\.json$/,
+          npm_lifecycle_event: 'event',
+          npm_lifecycle_script: 'script',
+          npm_config_node_gyp: require.resolve('node-gyp/bin/node-gyp.js'),
+        },
+        stdio: undefined,
+        cwd: 'path',
+        windowsVerbatimArguments: true,
+      }, 'got expected options')
+
+      const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
+      t.equal(contents, [
+        '@echo off',
+        `script ^"^^\\^"\\^^\\^"quoted parameter\\^^\\^";^^\\^"^" ^"^^\\^"second command^^\\^"^"`,
+      ].join('\n'))
+      t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
+      cleanup()
+      t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')
+
+      t.end()
+    })
+
+    t.test('correctly identifies initial cmd with spaces', (t) => {
+      // we do blind lookups in our test fixture here, however node-which
+      // will remove surrounding quotes
+      whichPaths.set('"my script"', '/path/script.cmd')
+      t.teardown(() => whichPaths.delete('my script'))
+
+      const [shell, args, opts, cleanup] = makeSpawnArgs({
+        event: 'event',
+        path: 'path',
+        cmd: '"my script"',
+        args: ['"quoted parameter";', 'second command'],
+      })
+      t.equal(shell, 'cmd', 'default shell applies')
+      t.match(args, ['/d', '/s', '/c', /\.cmd$/], 'got expected args')
+      t.match(opts, {
+        env: {
+          npm_package_json: /package\.json$/,
+          npm_lifecycle_event: 'event',
+          npm_lifecycle_script: 'script',
+          npm_config_node_gyp: require.resolve('node-gyp/bin/node-gyp.js'),
+        },
+        stdio: undefined,
+        cwd: 'path',
+        windowsVerbatimArguments: true,
+      }, 'got expected options')
+
+      const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
+      t.equal(contents, [
+        '@echo off',
+        // eslint-disable-next-line max-len
+        `"my script" ^"^^\\^"\\^^\\^"quoted parameter\\^^\\^";^^\\^"^" ^"^^\\^"second command^^\\^"^"`,
+      ].join('\n'))
+      t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
+      cleanup()
+      t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')
+
+      t.end()
+    })
+
     t.end()
   })
 } else {
@@ -176,6 +288,38 @@ if (isWindows) {
         windowsVerbatimArguments: undefined,
       }, 'got expected options')
 
+      const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
+      t.equal(contents, `#!/usr/bin/env sh\nscript '"quoted parameter";' 'second command'`)
+      t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
+      cleanup()
+      t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')
+
+      t.end()
+    })
+
+    t.test('skips /usr/bin/env if scriptShell is absolute', (t) => {
+      const [shell, args, opts, cleanup] = makeSpawnArgs({
+        event: 'event',
+        path: 'path',
+        cmd: 'script',
+        args: ['"quoted parameter";', 'second command'],
+        scriptShell: '/bin/sh',
+      })
+      t.equal(shell, '/bin/sh', 'kept provided setting')
+      t.match(args, ['-c', /\.sh$/], 'got expected args')
+      t.match(opts, {
+        env: {
+          npm_package_json: /package\.json$/,
+          npm_lifecycle_event: 'event',
+          npm_lifecycle_script: 'script',
+        },
+        stdio: undefined,
+        cwd: 'path',
+        windowsVerbatimArguments: undefined,
+      }, 'got expected options')
+
+      const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
+      t.equal(contents, `#!/bin/sh\nscript '"quoted parameter";' 'second command'`)
       t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
       cleanup()
       t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')