feat: write scripts to a file and run that instead of passing scripts as a single string

Author: nlf

Diff for: lib/escape.js

@@ -0,0 +1,65 @@
+'use strict'
+
+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,
+}

Diff for: 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, 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,7 @@ const makeSpawnArgs = options => {
     ...(isCmd ? { windowsVerbatimArguments: true } : {}),
   }
 
-  return [scriptShell, args, spawnOpts]
+  return [scriptShell, spawnArgs, spawnOpts]
 }
 
 module.exports = makeSpawnArgs

Diff for: 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) {
@@ -61,6 +61,7 @@ const runScriptPkg = async options => {
     env: packageEnvs(env, pkg),
     stdio,
     cmd,
+    args,
     stdioString,
   }), {
     event,

Diff for: 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()
+})

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

@@ -1,4 +1,5 @@
 const t = require('tap')
+const fs = require('fs')
 const requireInject = require('require-inject')
 const isWindows = require('../lib/is-windows.js')
 
@@ -10,21 +11,63 @@ if (!process.env.__FAKE_TESTING_PLATFORM__) {
   } })
 }
 
+const whichPaths = new Map()
+const which = {
+  sync: (req) => {
+    if (whichPaths.has(req)) {
+      return whichPaths.get(req)
+    }
+
+    throw new Error('not found')
+  },
+}
+
+const path = require('path')[isWindows ? 'win32' : 'posix']
+const tmpdir = path.resolve(t.testdir())
+
 const makeSpawnArgs = requireInject('../lib/make-spawn-args.js', {
-  path: require('path')[isWindows ? 'win32' : 'posix'],
+  fs: {
+    ...fs,
+    chmodSync (_path, mode) {
+      if (process.platform === 'win32') {
+        _path = _path.replace(/\//g, '\\')
+      } else {
+        _path = _path.replace(/\\/g, '/')
+      }
+      return fs.chmodSync(_path, mode)
+    },
+    writeFileSync (_path, content) {
+      if (process.platform === 'win32') {
+        _path = _path.replace(/\//g, '\\')
+      } else {
+        _path = _path.replace(/\\/g, '/')
+      }
+      return fs.writeFileSync(_path, content)
+    },
+  },
+  path,
+  which,
+  os: {
+    ...require('os'),
+    tmpdir: () => tmpdir,
+  },
 })
 
 if (isWindows) {
   t.test('windows', t => {
     // with no ComSpec
     delete process.env.ComSpec
+    whichPaths.set('cmd', 'C:\\Windows\\System32\\cmd.exe')
+    t.teardown(() => {
+      whichPaths.delete('cmd')
+    })
     t.match(makeSpawnArgs({
       event: 'event',
       path: 'path',
       cmd: 'script "quoted parameter"; second command',
     }), [
       'cmd',
-      ['/d', '/s', '/c', `script "quoted parameter"; second command`],
+      ['/d', '/s', '/c', /\.cmd$/],
       {
         env: {
           npm_package_json: /package\.json$/,
@@ -40,13 +83,17 @@ if (isWindows) {
 
     // with a funky ComSpec
     process.env.ComSpec = 'blrorp'
+    whichPaths.set('blrorp', '/bin/blrorp')
+    t.teardown(() => {
+      whichPaths.delete('blrorp')
+    })
     t.match(makeSpawnArgs({
       event: 'event',
       path: 'path',
       cmd: 'script "quoted parameter"; second command',
     }), [
       'blrorp',
-      ['-c', `script "quoted parameter"; second command`],
+      ['-c', /\.sh$/],
       {
         env: {
           npm_package_json: /package\.json$/,
@@ -62,11 +109,12 @@ if (isWindows) {
     t.match(makeSpawnArgs({
       event: 'event',
       path: 'path',
-      cmd: 'script "quoted parameter"; second command',
+      cmd: 'script',
+      args: ['"quoted parameter";', 'second command'],
       scriptShell: 'cmd.exe',
     }), [
       'cmd.exe',
-      ['/d', '/s', '/c', `script "quoted parameter"; second command`],
+      ['/d', '/s', '/c', /\.cmd$/],
       {
         env: {
           npm_package_json: /package\.json$/,
@@ -83,13 +131,18 @@ if (isWindows) {
   })
 } else {
   t.test('posix', t => {
+    whichPaths.set('sh', '/bin/sh')
+    t.teardown(() => {
+      whichPaths.delete('sh')
+    })
     t.match(makeSpawnArgs({
       event: 'event',
       path: 'path',
-      cmd: 'script "quoted parameter"; second command',
+      cmd: 'script',
+      args: ['"quoted parameter";', 'second command'],
     }), [
       'sh',
-      ['-c', `script "quoted parameter"; second command`],
+      ['-c', /\.sh$/],
       {
         env: {
           npm_package_json: /package\.json$/,
@@ -111,7 +164,7 @@ if (isWindows) {
       scriptShell: 'cmd.exe',
     }), [
       'cmd.exe',
-      ['/d', '/s', '/c', `script "quoted parameter"; second command`],
+      ['/d', '/s', '/c', /\.cmd$/],
       {
         env: {
           npm_package_json: /package\.json$/,