Skip to content

Commit

Permalink
fix: correctly double escape when script runs a known .cmd file (#80)
Browse files Browse the repository at this point in the history
* fix: correctly double escape when script runs a known .cmd file

* chore(tests): add some integration tests
  • Loading branch information
nlf authored Jun 22, 2022
1 parent 2520f32 commit 0f613cd
Show file tree
Hide file tree
Showing 5 changed files with 308 additions and 58 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
8 changes: 6 additions & 2 deletions lib/escape.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

// 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) => {
const cmd = (input, doubleEscape) => {
if (!input.length) {
return '""'
}
Expand Down Expand Up @@ -37,7 +37,11 @@ const cmd = (input) => {

// and finally, prefix shell meta chars with a ^
result = result.replace(/[!^&()<>|"]/g, '^$&')
// except for % which is escaped with another %
if (doubleEscape) {
result = result.replace(/[!^&()<>|"]/g, '^$&')
}

// except for % which is escaped with another %, and only once
result = result.replace(/%/g, '%%')

return result
Expand Down
58 changes: 45 additions & 13 deletions lib/make-spawn-args.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -20,35 +20,67 @@ 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,
}).toLowerCase()
} catch (err) {
pathToInitial = initialCmd.toLowerCase()
}

const doubleEscape = pathToInitial.endsWith('.cmd') || pathToInitial.endsWith('.bat')

scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.cmd`)
script += '@echo off\n'
script += `${cmd} ${args.map((arg) => escape.cmd(arg)).join(' ')}`
script += `${cmd} ${args.map((arg) => escape.cmd(arg, doubleEscape)).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,
Expand Down
152 changes: 109 additions & 43 deletions test/escape.js
Original file line number Diff line number Diff line change
@@ -1,63 +1,129 @@
'use strict'

const { writeFileSync: writeFile, unlinkSync: unlink, chmodSync: chmod } = require('fs')
const { join } = require('path')
const t = require('tap')
const promiseSpawn = require('@npmcli/promise-spawn')

const escape = require('../lib/escape.js')
const isWindows = process.platform === 'win32'

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')
})
const expectations = [
['', `''`],
['test', 'test'],
['test words', `'test words'`],
['$1', `'$1'`],
['"$1"', `'"$1"'`],
[`'$1'`, `\\''$1'\\'`],
['\\$1', `'\\$1'`],
['--arg="$1"', `'--arg="$1"'`],
['--arg=npm exec -c "$1"', `'--arg=npm exec -c "$1"'`],
[`--arg=npm exec -c '$1'`, `'--arg=npm exec -c '\\''$1'\\'`],
[`'--arg=npm exec -c "$1"'`, `\\''--arg=npm exec -c "$1"'\\'`],
]

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')
})
for (const [input, expectation] of expectations) {
t.equal(escape.sh(input), expectation,
`expected to escape \`${input}\` to \`${expectation}\``)
}

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.test('integration', { skip: isWindows && 'posix only' }, async (t) => {
const dir = t.testdir()

for (const [input] of expectations) {
const filename = join(dir, 'posix.sh')
const script = `#!/usr/bin/env sh\nnode -p process.argv[1] -- ${escape.sh(input)}`
writeFile(filename, script)
chmod(filename, '0755')
const p = await promiseSpawn('sh', ['-c', filename], { stdioString: true })
const stdout = p.stdout.trim()
t.equal(input, stdout, 'actual output matches input')
unlink(filename)
}

t.end()
})

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')
})
const expectations = [
['', '""'],
['test', 'test'],
['%PATH%', '%%PATH%%'],
['%PATH%', '%%PATH%%', true],
['"%PATH%"', '^"\\^"%%PATH%%\\^"^"'],
['"%PATH%"', '^^^"\\^^^"%%PATH%%\\^^^"^^^"', true],
[`'%PATH%'`, `'%%PATH%%'`],
[`'%PATH%'`, `'%%PATH%%'`, true],
['\\%PATH%', '\\%%PATH%%'],
['\\%PATH%', '\\%%PATH%%', true],
['--arg="%PATH%"', '^"--arg=\\^"%%PATH%%\\^"^"'],
['--arg="%PATH%"', '^^^"--arg=\\^^^"%%PATH%%\\^^^"^^^"', true],
['--arg=npm exec -c "%PATH%"', '^"--arg=npm exec -c \\^"%%PATH%%\\^"^"'],
['--arg=npm exec -c "%PATH%"', '^^^"--arg=npm exec -c \\^^^"%%PATH%%\\^^^"^^^"', true],
[`--arg=npm exec -c '%PATH%'`, `^"--arg=npm exec -c '%%PATH%%'^"`],
[`--arg=npm exec -c '%PATH%'`, `^^^"--arg=npm exec -c '%%PATH%%'^^^"`, true],
[`'--arg=npm exec -c "%PATH%"'`, `^"'--arg=npm exec -c \\^"%%PATH%%\\^"'^"`],
[`'--arg=npm exec -c "%PATH%"'`, `^^^"'--arg=npm exec -c \\^^^"%%PATH%%\\^^^"'^^^"`, true],
['"C:\\Program Files\\test.bat"', '^"\\^"C:\\Program Files\\test.bat\\^"^"'],
['"C:\\Program Files\\test.bat"', '^^^"\\^^^"C:\\Program Files\\test.bat\\^^^"^^^"', true],
['"C:\\Program Files\\test%.bat"', '^"\\^"C:\\Program Files\\test%%.bat\\^"^"'],
['"C:\\Program Files\\test%.bat"', '^^^"\\^^^"C:\\Program Files\\test%%.bat\\^^^"^^^"', true],
['% % %', '^"%% %% %%^"'],
['% % %', '^^^"%% %% %%^^^"', true],
['hello^^^^^^', 'hello^^^^^^^^^^^^'],
['hello^^^^^^', 'hello^^^^^^^^^^^^^^^^^^^^^^^^', true],
['hello world', '^"hello world^"'],
['hello world', '^^^"hello world^^^"', true],
['hello"world', '^"hello\\^"world^"'],
['hello"world', '^^^"hello\\^^^"world^^^"', true],
['hello""world', '^"hello\\^"\\^"world^"'],
['hello""world', '^^^"hello\\^^^"\\^^^"world^^^"', true],
['hello\\world', 'hello\\world'],
['hello\\world', 'hello\\world', true],
['hello\\\\world', 'hello\\\\world'],
['hello\\\\world', 'hello\\\\world', true],
['hello\\"world', '^"hello\\\\\\^"world^"'],
['hello\\"world', '^^^"hello\\\\\\^^^"world^^^"', true],
['hello\\\\"world', '^"hello\\\\\\\\\\^"world^"'],
['hello\\\\"world', '^^^"hello\\\\\\\\\\^^^"world^^^"', true],
['hello world\\', '^"hello world\\\\^"'],
['hello world\\', '^^^"hello world\\\\^^^"', true],
['hello %PATH%', '^"hello %%PATH%%^"'],
['hello %PATH%', '^^^"hello %%PATH%%^^^"', true],
]

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')
})
for (const [input, expectation, double] of expectations) {
const msg = `expected to${double ? ' double' : ''} escape \`${input}\` to \`${expectation}\``
t.equal(escape.cmd(input, double), expectation, msg)
}

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('integration', { skip: !isWindows && 'Windows only' }, async (t) => {
const dir = t.testdir()

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')
})
for (const [input,, double] of expectations) {
const filename = join(dir, 'win.cmd')
if (double) {
const shimFile = join(dir, 'shim.cmd')
const shim = `@echo off\nnode -p process.argv[1] -- %*`
writeFile(shimFile, shim)
const script = `@echo off\n"${shimFile}" ${escape.cmd(input, double)}`
writeFile(filename, script)
} else {
const script = `@echo off\nnode -p process.argv[1] -- ${escape.cmd(input)}`
writeFile(filename, script)
}
const p = await promiseSpawn('cmd', ['/d', '/s', '/c', filename], { stdioString: true })
const stdout = p.stdout.trim()
t.equal(input, stdout, 'actual output matches input')
unlink(filename)
}

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.end()
})

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()
})
Loading

0 comments on commit 0f613cd

Please sign in to comment.