Escape path-related npm options on Windows

index.js

@@ -9,6 +9,8 @@ const parseArgs = require('./parse-args.js')
 const path = require('path')
 const which = promisify(require('which'))
 
+const isWindows = process.platform === 'win32'
+
 module.exports = npx
 module.exports.parseArgs = parseArgs
 function npx (argv) {
@@ -207,10 +209,30 @@ function getNpmCache (opts) {
 
 module.exports._buildArgs = buildArgs
 function buildArgs (specs, prefix, opts) {
+  // NOTE: Why the weird escape condition? Because the `child.spawn` npm always
+  // runs in shell mode when on Windows (it's the default, to prevent weird
+  // behavior). That means that arguments to npm need to be escaped (but as
+  // strings, not as paths).
+  //
+  // The regular unix invocation doesn't need escaping, because it doesn't run
+  // in shell mode.
   const args = ['install'].concat(specs)
-  args.push('--global', '--prefix', prefix)
-  if (opts.cache) args.push('--cache', opts.cache)
-  if (opts.userconfig) args.push('--userconfig', opts.userconfig)
+  args.push(
+    '--global',
+    '--prefix', isWindows ? child.escapeArg(prefix) : prefix
+  )
+  if (opts.cache) {
+    args.push(
+      '--cache',
+      isWindows ? child.escapeArg(opts.cache) : opts.cache
+    )
+  }
+  if (opts.userconfig) {
+    args.push(
+      '--userconfig',
+      isWindows ? child.escapeArg(opts.userconfig) : opts.userconfig
+    )
+  }
   args.push('--loglevel', 'error', '--json')
 
   return args
@@ -222,7 +244,7 @@ function installPackages (specs, prefix, opts) {
   return findNodeScript(opts.npm, {isLocal: true}).then(npmPath => {
     if (npmPath) {
       args.unshift(
-        process.platform === 'win32'
+        isWindows
           ? child.escapeArg(opts.npm)
           : opts.npm
       )
@@ -231,7 +253,7 @@ function installPackages (specs, prefix, opts) {
       return opts.npm
     }
   }).then(npmPath => {
-    return process.platform === 'win32' ? child.escapeArg(npmPath, true) : npmPath
+    return isWindows ? child.escapeArg(npmPath, true) : npmPath
   }).then(npmPath => {
     return child.spawn(npmPath, args, {
       stdio: opts.installerStdio
@@ -273,8 +295,9 @@ function execCommand (_existing, argv) {
       let cmdOpts = argvCmdOpts
       if (existing) {
         cmd = process.argv[0]
-        if (process.platform === 'win32') {
+        if (isWindows || argv.shell) {
           cmd = child.escapeArg(cmd, true)
+          existing = child.escapeArg(existing, true)
         }
         // If we know we're running a run script and we got a --node-arg,
         // we need to fudge things a bit to get them working right.

test/index.js

@@ -14,7 +14,7 @@ const isWindows = process.platform === 'win32'
 const NPX_PATH = path.resolve(__dirname, 'util', 'npx-bin.js')
 const NPM_PATH = path.resolve(__dirname, '..', 'node_modules', 'npm', 'bin', 'npm-cli.js')
 
-const NPX_ESC = isWindows ? child.escapeArg(NPX_PATH) : NPX_PATH
+const NPX_ESC = isWindows ? child.escapeArg(NPX_PATH, true) : NPX_PATH
 
 test('npx --always-spawn', t => {
   return child.spawn('node', [
@@ -141,24 +141,17 @@ test('installPackages unit', t => {
           })
         }
       },
-      escapeArg (arg) {
-        if (arg === '/f@ke_/path to/node') {
-          return '\'/f@ke_/path to/node\''
-        } else if (arg === 'C:\\f@ke_\\path to\\node') {
-          return '"C:\\f@ke_\\path to\\node"'
-        }
-        return arg
-      }
+      escapeArg: child.escapeArg
     }
   })._installPackages
-  return installPkgs(['installme@latest', 'file:foo'], 'myprefix', {
+  return installPkgs(['installme@latest', 'file:foo'], 'my prefix', {
     npm: NPM_PATH
   }).then(deets => {
     t.deepEqual(deets[1], [
-      NPM_PATH,
+      isWindows ? `"${NPM_PATH}"` : NPM_PATH,
       'install', 'installme@latest', 'file:foo',
       '--global',
-      '--prefix', 'myprefix',
+      '--prefix', isWindows ? '"my prefix"' : 'my prefix',
       '--loglevel', 'error',
       '--json'
     ], 'args to spawn were correct for installing requested package')
@@ -191,7 +184,7 @@ test('installPackages unit', t => {
     }).then((npmPath) => {
       process.argv[0] = nodePath
       if (isWindows) {
-        t.equal(npmPath, '"C:\\f@ke_\\path to\\node"', 'incorrectly escaped path win32')
+        t.equal(npmPath, 'C:\\f@ke_\\"path to"\\node', 'incorrectly escaped path win32')
       } else {
         t.equal(npmPath, '/f@ke_/path to/node', 'incorrectly escaped path *nix')
       }