Skip to content

Commit 2354d06

Browse files
authoredJun 27, 2022
fix: remove invalid characters from filename (#86)
* chore: remove coverage map * fix: remove invalid characters from filename
1 parent eec7472 commit 2354d06

File tree

4 files changed

+74
-6
lines changed

4 files changed

+74
-6
lines changed
 

Diff for: ‎lib/escape.js

+6
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,13 @@ const sh = (input) => {
6565
return result
6666
}
6767

68+
// disabling the no-control-regex rule for this line as we very specifically _do_ want to
69+
// replace those characters if they somehow exist at this point, which is highly unlikely
70+
// eslint-disable-next-line no-control-regex
71+
const filename = (input) => input.replace(/[<>:"/\\|?*\x00-\x31]/g, '')
72+
6873
module.exports = {
6974
cmd,
7075
sh,
76+
filename,
7177
}

Diff for: ‎lib/make-spawn-args.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ const makeSpawnArgs = options => {
3030
npm_config_node_gyp,
3131
})
3232

33+
const fileName = escape.filename(`${event}-${Date.now()}`)
3334
let scriptFile
3435
let script = ''
3536

@@ -61,7 +62,7 @@ const makeSpawnArgs = options => {
6162

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

64-
scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.cmd`)
65+
scriptFile = resolve(tmpdir(), `${fileName}.cmd`)
6566
script += '@echo off\n'
6667
script += cmd
6768
if (args.length) {
@@ -71,7 +72,7 @@ const makeSpawnArgs = options => {
7172
const shebang = isAbsolute(scriptShell)
7273
? `#!${scriptShell}`
7374
: `#!/usr/bin/env ${scriptShell}`
74-
scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.sh`)
75+
scriptFile = resolve(tmpdir(), `${fileName}.sh`)
7576
script += `${shebang}\n`
7677
script += cmd
7778
if (args.length) {

Diff for: ‎package.json

-4
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@
1717
"posttest": "npm run lint",
1818
"template-oss-apply": "template-oss-apply --force"
1919
},
20-
"tap": {
21-
"check-coverage": true,
22-
"coverage-map": "map.js"
23-
},
2420
"devDependencies": {
2521
"@npmcli/eslint-config": "^3.0.1",
2622
"@npmcli/template-oss": "3.5.0",

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

+65
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,39 @@ if (isWindows) {
102102
t.end()
103103
})
104104

105+
t.test('event with invalid characters runs', (t) => {
106+
const [shell, args, opts, cleanup] = makeSpawnArgs({
107+
event: 'event<:>\x03', // everything after the word "event" is invalid
108+
path: 'path',
109+
cmd: 'script "quoted parameter"; second command',
110+
})
111+
t.equal(shell, 'cmd', 'default shell applies')
112+
// disabling no-control-regex because we are testing specifically if the control
113+
// character gets removed
114+
// eslint-disable-next-line no-control-regex
115+
t.match(args, ['/d', '/s', '/c', /(?:\\|\/)[^<:>\x03]+.cmd\^"$/], 'got expected args')
116+
t.match(opts, {
117+
env: {
118+
npm_package_json: /package\.json$/,
119+
npm_lifecycle_event: 'event',
120+
npm_lifecycle_script: 'script',
121+
npm_config_node_gyp: require.resolve('node-gyp/bin/node-gyp.js'),
122+
},
123+
stdio: undefined,
124+
cwd: 'path',
125+
windowsVerbatimArguments: true,
126+
}, 'got expected options')
127+
128+
const filename = unescapeCmd(args[args.length - 1])
129+
const contents = fs.readFileSync(filename, { encoding: 'utf8' })
130+
t.equal(contents, `@echo off\nscript "quoted parameter"; second command`)
131+
t.ok(fs.existsSync(filename), 'script file was written')
132+
cleanup()
133+
t.not(fs.existsSync(filename), 'cleanup removes script file')
134+
135+
t.end()
136+
})
137+
105138
t.test('with a funky ComSpec', (t) => {
106139
process.env.ComSpec = 'blrorp'
107140
whichPaths.set('blrorp', '/bin/blrorp')
@@ -314,6 +347,38 @@ if (isWindows) {
314347
t.end()
315348
})
316349

350+
t.test('event with invalid characters runs', (t) => {
351+
const [shell, args, opts, cleanup] = makeSpawnArgs({
352+
event: 'event<:>/\x04',
353+
path: 'path',
354+
cmd: 'script',
355+
args: ['"quoted parameter";', 'second command'],
356+
})
357+
t.equal(shell, 'sh', 'defaults to sh')
358+
// no-control-regex disabled because we're specifically testing control chars
359+
// eslint-disable-next-line no-control-regex
360+
t.match(args, ['-c', /(?:\\|\/)[^<:>/\x04]+\.sh'$/], 'got expected args')
361+
t.match(opts, {
362+
env: {
363+
npm_package_json: /package\.json$/,
364+
npm_lifecycle_event: 'event',
365+
npm_lifecycle_script: 'script',
366+
},
367+
stdio: undefined,
368+
cwd: 'path',
369+
windowsVerbatimArguments: undefined,
370+
}, 'got expected options')
371+
372+
const filename = unescapeSh(args[args.length - 1])
373+
const contents = fs.readFileSync(filename, { encoding: 'utf8' })
374+
t.equal(contents, `#!/usr/bin/env sh\nscript '"quoted parameter";' 'second command'`)
375+
t.ok(fs.existsSync(filename), 'script file was written')
376+
cleanup()
377+
t.not(fs.existsSync(filename), 'cleanup removes script file')
378+
379+
t.end()
380+
})
381+
317382
t.test('skips /usr/bin/env if scriptShell is absolute', (t) => {
318383
const [shell, args, opts, cleanup] = makeSpawnArgs({
319384
event: 'event',

0 commit comments

Comments
 (0)
Please sign in to comment.