feat: write script and args to temp file and run that

README.md

@@ -147,3 +147,14 @@ Functionally, this means:
   hook scripts, then they can override the default package script with an
   explicit `cmd` option pointing to the `node_modules/.hook/${event}`
   script.
+
+## Escaping
+
+In order to ensure that arguments are handled consistently, this module
+writes a temporary script file containing the command as it exists in
+the package.json, followed by the user supplied arguments having been
+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.
+
+The actual implementation of the escaping is in `lib/escape.js`.

lib/escape.js

@@ -0,0 +1,67 @@
+'use strict'
+
+// eslint-disable-next-line max-len
+// this code adapted from: https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/
+const cmd = (input) => {
+  if (!input.length) {
+    return '""'
+  }
+
+  let result
+  if (!/[ \t\n\v"]/.test(input)) {
+    result = input
+  } else {
+    result = '"'
+    for (let i = 0; i <= input.length; ++i) {
+      let slashCount = 0
+      while (input[i] === '\\') {
+        ++i
+        ++slashCount
+      }
+
+      if (i === input.length) {
+        result += '\\'.repeat(slashCount * 2)
+        break
+      }
+
+      if (input[i] === '"') {
+        result += '\\'.repeat(slashCount * 2 + 1)
+        result += input[i]
+      } else {
+        result += '\\'.repeat(slashCount)
+        result += input[i]
+      }
+    }
+    result += '"'
+  }
+
+  // and finally, prefix shell meta chars with a ^
+  result = result.replace(/[!^&()<>|"]/g, '^$&')
+  // except for % which is escaped with another %
+  result = result.replace(/%/g, '%%')
+
+  return result
+}
+
+const sh = (input) => {
+  if (!input.length) {
+    return `''`
+  }
+
+  if (!/[\t\n\r "#$&'()*;<>?\\`|~]/.test(input)) {
+    return input
+  }
+
+  // replace single quotes with '\'' and wrap the whole result in a fresh set of quotes
+  const result = `'${input.replace(/'/g, `'\\''`)}'`
+    // if the input string already had single quotes around it, clean those up
+    .replace(/^(?:'')+(?!$)/, '')
+    .replace(/\\'''/g, `\\'`)
+
+  return result
+}
+
+module.exports = {
+  cmd,
+  sh,
+}

lib/make-spawn-args.js

@@ -1,8 +1,12 @@
 /* eslint camelcase: "off" */
 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 which = require('which')
 const npm_config_node_gyp = require.resolve('node-gyp/bin/node-gyp.js')
+const escape = require('./escape.js')
 
 const makeSpawnArgs = options => {
   const {
@@ -12,11 +16,28 @@ const makeSpawnArgs = options => {
     env = {},
     stdio,
     cmd,
+    args = [],
     stdioString = false,
   } = options
 
+  let scriptFile
+  let script = ''
   const isCmd = /(?:^|\\)cmd(?:\.exe)?$/i.test(scriptShell)
-  const args = isCmd ? ['/d', '/s', '/c', cmd] : ['-c', cmd]
+  if (isCmd) {
+    scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.cmd`)
+    script += '@echo off\n'
+    script += `${cmd} ${args.map((arg) => escape.cmd(arg)).join(' ')}`
+  } else {
+    const shellPath = which.sync(scriptShell)
+    scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.sh`)
+    script += `#!${shellPath}\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, {
@@ -34,7 +55,14 @@ const makeSpawnArgs = options => {
     ...(isCmd ? { windowsVerbatimArguments: true } : {}),
   }
 
-  return [scriptShell, args, spawnOpts]
+  const cleanup = () => {
+    // delete the script, this is just a best effort
+    try {
+      unlink(scriptFile)
+    } catch (err) {}
+  }
+
+  return [scriptShell, spawnArgs, spawnOpts, cleanup]
 }
 
 module.exports = makeSpawnArgs

lib/run-script-pkg.js

@@ -31,7 +31,7 @@ const runScriptPkg = async options => {
   if (options.cmd) {
     cmd = options.cmd
   } else if (pkg.scripts && pkg.scripts[event]) {
-    cmd = pkg.scripts[event] + args.map(a => ` ${JSON.stringify(a)}`).join('')
+    cmd = pkg.scripts[event]
   } else if (
     // If there is no preinstall or install script, default to rebuilding node-gyp packages.
     event === 'install' &&
@@ -42,7 +42,7 @@ const runScriptPkg = async options => {
   ) {
     cmd = defaultGypInstallScript
   } else if (event === 'start' && await isServerPackage(path)) {
-    cmd = 'node server.js' + args.map(a => ` ${JSON.stringify(a)}`).join('')
+    cmd = 'node server.js'
   }
 
   if (!cmd) {
@@ -54,15 +54,18 @@ const runScriptPkg = async options => {
     console.log(bruce(pkg._id, event, cmd))
   }
 
-  const p = promiseSpawn(...makeSpawnArgs({
+  const [spawnShell, spawnArgs, spawnOpts, cleanup] = makeSpawnArgs({
     event,
     path,
     scriptShell,
     env: packageEnvs(env, pkg),
     stdio,
     cmd,
+    args,
     stdioString,
-  }), {
+  })
+
+  const p = promiseSpawn(spawnShell, spawnArgs, spawnOpts, {
     event,
     script: cmd,
     pkgid: pkg._id,
@@ -88,7 +91,7 @@ const runScriptPkg = async options => {
     } else {
       throw er
     }
-  })
+  }).finally(cleanup)
 }
 
 module.exports = runScriptPkg

test/escape.js

@@ -0,0 +1,63 @@
+const t = require('tap')
+
+const escape = require('../lib/escape.js')
+
+t.test('sh', (t) => {
+  t.test('returns empty quotes when input is empty', async (t) => {
+    const input = ''
+    const output = escape.sh(input)
+    t.equal(output, `''`, 'returned empty single quotes')
+  })
+
+  t.test('returns plain string if quotes are not necessary', async (t) => {
+    const input = 'test'
+    const output = escape.sh(input)
+    t.equal(output, input, 'returned plain string')
+  })
+
+  t.test('wraps in single quotes if special character is present', async (t) => {
+    const input = 'test words'
+    const output = escape.sh(input)
+    t.equal(output, `'test words'`, 'wrapped in single quotes')
+  })
+  t.end()
+})
+
+t.test('cmd', (t) => {
+  t.test('returns empty quotes when input is empty', async (t) => {
+    const input = ''
+    const output = escape.cmd(input)
+    t.equal(output, '""', 'returned empty double quotes')
+  })
+
+  t.test('returns plain string if quotes are not necessary', async (t) => {
+    const input = 'test'
+    const output = escape.cmd(input)
+    t.equal(output, input, 'returned plain string')
+  })
+
+  t.test('wraps in double quotes when necessary', async (t) => {
+    const input = 'test words'
+    const output = escape.cmd(input)
+    t.equal(output, '^"test words^"', 'wrapped in double quotes')
+  })
+
+  t.test('doubles up backslashes at end of input', async (t) => {
+    const input = 'one \\ two \\'
+    const output = escape.cmd(input)
+    t.equal(output, '^"one \\ two \\\\^"', 'doubles backslash at end of string')
+  })
+
+  t.test('doubles up backslashes immediately before a double quote', async (t) => {
+    const input = 'one \\"'
+    const output = escape.cmd(input)
+    t.equal(output, '^"one \\\\\\^"^"', 'doubles backslash before double quote')
+  })
+
+  t.test('backslash escapes double quotes', async (t) => {
+    const input = '"test"'
+    const output = escape.cmd(input)
+    t.equal(output, '^"\\^"test\\^"^"', 'escaped double quotes')
+  })
+  t.end()
+})