Skip to content

Commit 9f187df

Browse files
nlfnlf
committed
fix: correctly double escape when script runs a known .cmd file
1 parent 2520f32 commit 9f187df

File tree

3 files changed

+200
-13
lines changed

3 files changed

+200
-13
lines changed

‎README.md

+4
Original file line numberDiff line numberDiff line change
@@ -157,4 +157,8 @@ escaped to ensure they are processed as literal strings. We then instruct
157157
the shell to execute the script file, and when the process exits we remove
158158
the temporary file.
159159

160+
In Windows, when the shell is cmd, and when the initial command in the script
161+
is a known batch file (i.e. `something.cmd`) we double escape additional
162+
arguments so that the shim scripts npm installs work correctly.
163+
160164
The actual implementation of the escaping is in `lib/escape.js`.

‎lib/make-spawn-args.js

+52-13
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const isWindows = require('./is-windows.js')
33
const setPATH = require('./set-path.js')
44
const { chmodSync: chmod, unlinkSync: unlink, writeFileSync: writeFile } = require('fs')
55
const { tmpdir } = require('os')
6-
const { resolve } = require('path')
6+
const { isAbsolute, resolve } = require('path')
77
const which = require('which')
88
const npm_config_node_gyp = require.resolve('node-gyp/bin/node-gyp.js')
99
const escape = require('./escape.js')
@@ -20,35 +20,74 @@ const makeSpawnArgs = options => {
2020
stdioString = false,
2121
} = options
2222

23+
const spawnEnv = setPATH(path, {
24+
// we need to at least save the PATH environment var
25+
...process.env,
26+
...env,
27+
npm_package_json: resolve(path, 'package.json'),
28+
npm_lifecycle_event: event,
29+
npm_lifecycle_script: cmd,
30+
npm_config_node_gyp,
31+
})
32+
2333
let scriptFile
2434
let script = ''
35+
2536
const isCmd = /(?:^|\\)cmd(?:\.exe)?$/i.test(scriptShell)
2637
if (isCmd) {
38+
let initialCmd = ''
39+
let insideQuotes = false
40+
for (let i = 0; i < cmd.length; ++i) {
41+
const char = cmd.charAt(i)
42+
if (char === ' ' && !insideQuotes) {
43+
break
44+
}
45+
46+
initialCmd += char
47+
if (char === '"' || char === "'") {
48+
insideQuotes = !insideQuotes
49+
}
50+
}
51+
52+
let pathToInitial
53+
try {
54+
pathToInitial = which.sync(initialCmd, {
55+
path: spawnEnv.path,
56+
pathext: spawnEnv.pathext,
57+
}).toLowerCase()
58+
} catch (err) {
59+
pathToInitial = initialCmd.toLowerCase()
60+
}
61+
62+
const doubleEscape = pathToInitial.endsWith('.cmd') || pathToInitial.endsWith('.bat')
63+
2764
scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.cmd`)
2865
script += '@echo off\n'
29-
script += `${cmd} ${args.map((arg) => escape.cmd(arg)).join(' ')}`
66+
const escapedArgs = args.map((arg) => {
67+
let result = escape.cmd(arg)
68+
if (doubleEscape) {
69+
result = escape.cmd(result)
70+
}
71+
return result
72+
})
73+
script += `${cmd} ${escapedArgs.join(' ')}`
3074
} else {
31-
const shellPath = which.sync(scriptShell)
75+
const shebang = isAbsolute(scriptShell)
76+
? `#!${scriptShell}`
77+
: `#!/usr/bin/env ${scriptShell}`
3278
scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.sh`)
33-
script += `#!${shellPath}\n`
79+
script += `${shebang}\n`
3480
script += `${cmd} ${args.map((arg) => escape.sh(arg)).join(' ')}`
3581
}
82+
3683
writeFile(scriptFile, script)
3784
if (!isCmd) {
3885
chmod(scriptFile, '0775')
3986
}
4087
const spawnArgs = isCmd ? ['/d', '/s', '/c', scriptFile] : ['-c', scriptFile]
4188

4289
const spawnOpts = {
43-
env: setPATH(path, {
44-
// we need to at least save the PATH environment var
45-
...process.env,
46-
...env,
47-
npm_package_json: resolve(path, 'package.json'),
48-
npm_lifecycle_event: event,
49-
npm_lifecycle_script: cmd,
50-
npm_config_node_gyp,
51-
}),
90+
env: spawnEnv,
5291
stdioString,
5392
stdio,
5493
cwd: path,

‎test/make-spawn-args.js

+144
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ if (isWindows) {
8181
windowsVerbatimArguments: true,
8282
}, 'got expected options')
8383

84+
const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
85+
// the contents will have a trailing space if no args are passed
86+
t.equal(contents, `@echo off\nscript "quoted parameter"; second command `)
8487
t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
8588
cleanup()
8689
t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')
@@ -93,6 +96,7 @@ if (isWindows) {
9396
whichPaths.set('blrorp', '/bin/blrorp')
9497
t.teardown(() => {
9598
whichPaths.delete('blrorp')
99+
delete process.env.ComSpec
96100
})
97101
const [shell, args, opts, cleanup] = makeSpawnArgs({
98102
event: 'event',
@@ -147,6 +151,114 @@ if (isWindows) {
147151
t.end()
148152
})
149153

154+
t.test('single escapes when initial command is not a batch file', (t) => {
155+
whichPaths.set('script', '/path/script.exe')
156+
t.teardown(() => whichPaths.delete('script'))
157+
158+
const [shell, args, opts, cleanup] = makeSpawnArgs({
159+
event: 'event',
160+
path: 'path',
161+
cmd: 'script',
162+
args: ['"quoted parameter";', 'second command'],
163+
})
164+
t.equal(shell, 'cmd', 'default shell applies')
165+
t.match(args, ['/d', '/s', '/c', /\.cmd$/], 'got expected args')
166+
t.match(opts, {
167+
env: {
168+
npm_package_json: /package\.json$/,
169+
npm_lifecycle_event: 'event',
170+
npm_lifecycle_script: 'script',
171+
npm_config_node_gyp: require.resolve('node-gyp/bin/node-gyp.js'),
172+
},
173+
stdio: undefined,
174+
cwd: 'path',
175+
windowsVerbatimArguments: true,
176+
}, 'got expected options')
177+
178+
const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
179+
t.equal(contents, `@echo off\nscript ^"\\^"quoted parameter\\^";^" ^"second command^"`)
180+
t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
181+
cleanup()
182+
t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')
183+
184+
t.end()
185+
})
186+
187+
t.test('double escapes when initial command is a batch file', (t) => {
188+
whichPaths.set('script', '/path/script.cmd')
189+
t.teardown(() => whichPaths.delete('script'))
190+
191+
const [shell, args, opts, cleanup] = makeSpawnArgs({
192+
event: 'event',
193+
path: 'path',
194+
cmd: 'script',
195+
args: ['"quoted parameter";', 'second command'],
196+
})
197+
t.equal(shell, 'cmd', 'default shell applies')
198+
t.match(args, ['/d', '/s', '/c', /\.cmd$/], 'got expected args')
199+
t.match(opts, {
200+
env: {
201+
npm_package_json: /package\.json$/,
202+
npm_lifecycle_event: 'event',
203+
npm_lifecycle_script: 'script',
204+
npm_config_node_gyp: require.resolve('node-gyp/bin/node-gyp.js'),
205+
},
206+
stdio: undefined,
207+
cwd: 'path',
208+
windowsVerbatimArguments: true,
209+
}, 'got expected options')
210+
211+
const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
212+
t.equal(contents, [
213+
'@echo off',
214+
`script ^"^^\\^"\\^^\\^"quoted parameter\\^^\\^";^^\\^"^" ^"^^\\^"second command^^\\^"^"`,
215+
].join('\n'))
216+
t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
217+
cleanup()
218+
t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')
219+
220+
t.end()
221+
})
222+
223+
t.test('correctly identifies initial cmd with spaces', (t) => {
224+
// we do blind lookups in our test fixture here, however node-which
225+
// will remove surrounding quotes
226+
whichPaths.set('"my script"', '/path/script.cmd')
227+
t.teardown(() => whichPaths.delete('my script'))
228+
229+
const [shell, args, opts, cleanup] = makeSpawnArgs({
230+
event: 'event',
231+
path: 'path',
232+
cmd: '"my script"',
233+
args: ['"quoted parameter";', 'second command'],
234+
})
235+
t.equal(shell, 'cmd', 'default shell applies')
236+
t.match(args, ['/d', '/s', '/c', /\.cmd$/], 'got expected args')
237+
t.match(opts, {
238+
env: {
239+
npm_package_json: /package\.json$/,
240+
npm_lifecycle_event: 'event',
241+
npm_lifecycle_script: 'script',
242+
npm_config_node_gyp: require.resolve('node-gyp/bin/node-gyp.js'),
243+
},
244+
stdio: undefined,
245+
cwd: 'path',
246+
windowsVerbatimArguments: true,
247+
}, 'got expected options')
248+
249+
const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
250+
t.equal(contents, [
251+
'@echo off',
252+
// eslint-disable-next-line max-len
253+
`"my script" ^"^^\\^"\\^^\\^"quoted parameter\\^^\\^";^^\\^"^" ^"^^\\^"second command^^\\^"^"`,
254+
].join('\n'))
255+
t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
256+
cleanup()
257+
t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')
258+
259+
t.end()
260+
})
261+
150262
t.end()
151263
})
152264
} else {
@@ -176,6 +288,38 @@ if (isWindows) {
176288
windowsVerbatimArguments: undefined,
177289
}, 'got expected options')
178290

291+
const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
292+
t.equal(contents, `#!/usr/bin/env sh\nscript '"quoted parameter";' 'second command'`)
293+
t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
294+
cleanup()
295+
t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')
296+
297+
t.end()
298+
})
299+
300+
t.test('skips /usr/bin/env if scriptShell is absolute', (t) => {
301+
const [shell, args, opts, cleanup] = makeSpawnArgs({
302+
event: 'event',
303+
path: 'path',
304+
cmd: 'script',
305+
args: ['"quoted parameter";', 'second command'],
306+
scriptShell: '/bin/sh',
307+
})
308+
t.equal(shell, '/bin/sh', 'kept provided setting')
309+
t.match(args, ['-c', /\.sh$/], 'got expected args')
310+
t.match(opts, {
311+
env: {
312+
npm_package_json: /package\.json$/,
313+
npm_lifecycle_event: 'event',
314+
npm_lifecycle_script: 'script',
315+
},
316+
stdio: undefined,
317+
cwd: 'path',
318+
windowsVerbatimArguments: undefined,
319+
}, 'got expected options')
320+
321+
const contents = fs.readFileSync(args[args.length - 1], { encoding: 'utf8' })
322+
t.equal(contents, `#!/bin/sh\nscript '"quoted parameter";' 'second command'`)
179323
t.ok(fs.existsSync(args[args.length - 1]), 'script file was written')
180324
cleanup()
181325
t.not(fs.existsSync(args[args.length - 1]), 'cleanup removes script file')

0 commit comments

Comments
 (0)