diff --git a/README.md b/README.md index ff8f5d3..7ac7e2d 100644 --- a/README.md +++ b/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`. diff --git a/lib/escape.js b/lib/escape.js new file mode 100644 index 0000000..29d24a8 --- /dev/null +++ b/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, +} diff --git a/lib/make-spawn-args.js b/lib/make-spawn-args.js index 9cfc84b..6f3aa4c 100644 --- a/lib/make-spawn-args.js +++ b/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 diff --git a/lib/run-script-pkg.js b/lib/run-script-pkg.js index a6fa4d2..84c5e2b 100644 --- a/lib/run-script-pkg.js +++ b/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 diff --git a/test/escape.js b/test/escape.js new file mode 100644 index 0000000..35a419c --- /dev/null +++ b/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 --git a/test/make-spawn-args.js b/test/make-spawn-args.js index ae92c72..aacd79e 100644 --- a/test/make-spawn-args.js +++ b/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,22 +11,65 @@ 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') +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) + }, + }, + which, + os: { + ...require('os'), + tmpdir: () => tmpdir, + }, }) if (isWindows) { t.test('windows', t => { // with no ComSpec delete process.env.ComSpec - t.match(makeSpawnArgs({ - event: 'event', - path: 'path', - cmd: 'script "quoted parameter"; second command', - }), [ - 'cmd', - ['/d', '/s', '/c', `script "quoted parameter"; second command`], - { + whichPaths.set('cmd', 'C:\\Windows\\System32\\cmd.exe') + t.teardown(() => { + whichPaths.delete('cmd') + }) + + t.test('simple script', (t) => { + const [shell, args, opts, cleanup] = makeSpawnArgs({ + event: 'event', + path: 'path', + cmd: 'script "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', @@ -35,19 +79,29 @@ if (isWindows) { stdio: undefined, cwd: 'path', windowsVerbatimArguments: true, - }, - ]) - - // with a funky ComSpec - process.env.ComSpec = 'blrorp' - t.match(makeSpawnArgs({ - event: 'event', - path: 'path', - cmd: 'script "quoted parameter"; second command', - }), [ - 'blrorp', - ['-c', `script "quoted parameter"; second command`], - { + }, 'got expected options') + + 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('with a funky ComSpec', (t) => { + process.env.ComSpec = 'blrorp' + whichPaths.set('blrorp', '/bin/blrorp') + t.teardown(() => { + whichPaths.delete('blrorp') + }) + const [shell, args, opts, cleanup] = makeSpawnArgs({ + event: 'event', + path: 'path', + cmd: 'script "quoted parameter"; second command', + }) + t.equal(shell, 'blrorp', 'used ComSpec as default shell') + t.match(args, ['-c', /\.sh$/], 'got expected args') + t.match(opts, { env: { npm_package_json: /package\.json$/, npm_lifecycle_event: 'event', @@ -56,18 +110,26 @@ if (isWindows) { stdio: undefined, cwd: 'path', windowsVerbatimArguments: undefined, - }, - ]) - - t.match(makeSpawnArgs({ - event: 'event', - path: 'path', - cmd: 'script "quoted parameter"; second command', - scriptShell: 'cmd.exe', - }), [ - 'cmd.exe', - ['/d', '/s', '/c', `script "quoted parameter"; second command`], - { + }, 'got expected options') + + 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('with cmd.exe as scriptShell', (t) => { + const [shell, args, opts, cleanup] = makeSpawnArgs({ + event: 'event', + path: 'path', + cmd: 'script', + args: ['"quoted parameter";', 'second command'], + scriptShell: 'cmd.exe', + }) + t.equal(shell, 'cmd.exe', 'kept cmd.exe') + t.match(args, ['/d', '/s', '/c', /\.cmd$/], 'got expected args') + t.match(opts, { env: { npm_package_json: /package\.json$/, npm_lifecycle_event: 'event', @@ -76,21 +138,34 @@ if (isWindows) { stdio: undefined, cwd: 'path', windowsVerbatimArguments: true, - }, - ]) + }, 'got expected options') + + 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 { t.test('posix', t => { - t.match(makeSpawnArgs({ - event: 'event', - path: 'path', - cmd: 'script "quoted parameter"; second command', - }), [ - 'sh', - ['-c', `script "quoted parameter"; second command`], - { + whichPaths.set('sh', '/bin/sh') + t.teardown(() => { + whichPaths.delete('sh') + }) + + t.test('simple script', (t) => { + const [shell, args, opts, cleanup] = makeSpawnArgs({ + event: 'event', + path: 'path', + cmd: 'script', + args: ['"quoted parameter";', 'second command'], + }) + t.equal(shell, 'sh', 'defaults to sh') + t.match(args, ['-c', /\.sh$/], 'got expected args') + t.match(opts, { env: { npm_package_json: /package\.json$/, npm_lifecycle_event: 'event', @@ -99,20 +174,27 @@ if (isWindows) { stdio: undefined, cwd: 'path', windowsVerbatimArguments: undefined, - }, - ]) - - // test that we can explicitly run in cmd.exe, even on posix systems - // relevant for running under WSL - t.match(makeSpawnArgs({ - event: 'event', - path: 'path', - cmd: 'script "quoted parameter"; second command', - scriptShell: 'cmd.exe', - }), [ - 'cmd.exe', - ['/d', '/s', '/c', `script "quoted parameter"; second command`], - { + }, 'got expected options') + + 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('can use cmd.exe', (t) => { + // test that we can explicitly run in cmd.exe, even on posix systems + // relevant for running under WSL + const [shell, args, opts, cleanup] = makeSpawnArgs({ + event: 'event', + path: 'path', + cmd: 'script "quoted parameter"; second command', + scriptShell: 'cmd.exe', + }) + t.equal(shell, 'cmd.exe', 'kept cmd.exe') + t.match(args, ['/d', '/s', '/c', /\.cmd$/], 'got expected args') + t.match(opts, { env: { npm_package_json: /package\.json$/, npm_lifecycle_event: 'event', @@ -121,8 +203,14 @@ if (isWindows) { stdio: undefined, cwd: 'path', windowsVerbatimArguments: true, - }, - ]) + }, 'got expected options') + + 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() }) diff --git a/test/run-script-pkg.js b/test/run-script-pkg.js index 8c27070..466ca0b 100644 --- a/test/run-script-pkg.js +++ b/test/run-script-pkg.js @@ -59,6 +59,7 @@ t.test('pkg has server.js, start not specified', async t => { event: 'start', path, scriptShell: 'sh', + args: [], env: { environ: 'value', }, @@ -88,7 +89,7 @@ t.test('pkg has server.js, start not specified, with args', async t => { scripts: {}, }, }) - t.strictSame(res, ['sh', ['-c', 'node server.js "a" "b" "c"'], { + t.strictSame(res, ['sh', ['-c', 'node server.js'], { stdioString: false, event: 'start', path, @@ -97,10 +98,11 @@ t.test('pkg has server.js, start not specified, with args', async t => { environ: 'value', }, stdio: 'pipe', - cmd: 'node server.js "a" "b" "c"', + cmd: 'node server.js', + args: ['a', 'b', 'c'], }, { event: 'start', - script: 'node server.js "a" "b" "c"', + script: 'node server.js', pkgid: 'foo@1.2.3', path, }]) @@ -129,6 +131,7 @@ t.test('pkg has no foo script, but custom cmd provided', t => runScriptPkg({ event: 'foo', path: 'path', scriptShell: 'sh', + args: [], env: { environ: 'value', }, @@ -164,6 +167,7 @@ t.test('do the banner when stdio is inherited, handle line breaks', t => { event: 'foo', path: 'path', scriptShell: 'sh', + args: [], env: { environ: 'value', }, @@ -201,6 +205,7 @@ t.test('do not show banner when stdio is inherited, if suppressed', t => { event: 'foo', path: 'path', scriptShell: 'sh', + args: [], env: { environ: 'value', }, @@ -236,6 +241,7 @@ t.test('do the banner with no pkgid', t => { event: 'foo', path: 'path', scriptShell: 'sh', + args: [], env: { environ: 'value', }, @@ -268,6 +274,7 @@ t.test('pkg has foo script', t => runScriptPkg({ event: 'foo', path: 'path', scriptShell: 'sh', + args: [], env: { environ: 'value', }, @@ -295,19 +302,20 @@ t.test('pkg has foo script, with args', t => runScriptPkg({ }, }, args: ['a', 'b', 'c'], -}).then(res => t.strictSame(res, ['sh', ['-c', 'bar "a" "b" "c"'], { +}).then(res => t.strictSame(res, ['sh', ['-c', 'bar'], { stdioString: false, event: 'foo', path: 'path', scriptShell: 'sh', + args: ['a', 'b', 'c'], env: { environ: 'value', }, stdio: 'pipe', - cmd: 'bar "a" "b" "c"', + cmd: 'bar', }, { event: 'foo', - script: 'bar "a" "b" "c"', + script: 'bar', pkgid: 'foo@1.2.3', path: 'path', }]))) @@ -337,6 +345,7 @@ t.test('pkg has no install or preinstall script, but node-gyp files are present' event: 'install', path: 'path', scriptShell: 'sh', + args: [], env: { environ: 'value' }, stdio: 'pipe', cmd: 'node-gyp rebuild',