Skip to content

Commit

Permalink
fix: remove invalid characters from filename (#86)
Browse files Browse the repository at this point in the history
* chore: remove coverage map

* fix: remove invalid characters from filename
  • Loading branch information
nlf committed Jun 27, 2022
1 parent eec7472 commit 2354d06
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 6 deletions.
6 changes: 6 additions & 0 deletions lib/escape.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,13 @@ const sh = (input) => {
return result
}

// disabling the no-control-regex rule for this line as we very specifically _do_ want to
// replace those characters if they somehow exist at this point, which is highly unlikely
// eslint-disable-next-line no-control-regex
const filename = (input) => input.replace(/[<>:"/\\|?*\x00-\x31]/g, '')

module.exports = {
cmd,
sh,
filename,
}
5 changes: 3 additions & 2 deletions lib/make-spawn-args.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const makeSpawnArgs = options => {
npm_config_node_gyp,
})

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

Expand Down Expand Up @@ -61,7 +62,7 @@ const makeSpawnArgs = options => {

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

scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.cmd`)
scriptFile = resolve(tmpdir(), `${fileName}.cmd`)
script += '@echo off\n'
script += cmd
if (args.length) {
Expand All @@ -71,7 +72,7 @@ const makeSpawnArgs = options => {
const shebang = isAbsolute(scriptShell)
? `#!${scriptShell}`
: `#!/usr/bin/env ${scriptShell}`
scriptFile = resolve(tmpdir(), `${event}-${Date.now()}.sh`)
scriptFile = resolve(tmpdir(), `${fileName}.sh`)
script += `${shebang}\n`
script += cmd
if (args.length) {
Expand Down
4 changes: 0 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@
"posttest": "npm run lint",
"template-oss-apply": "template-oss-apply --force"
},
"tap": {
"check-coverage": true,
"coverage-map": "map.js"
},
"devDependencies": {
"@npmcli/eslint-config": "^3.0.1",
"@npmcli/template-oss": "3.5.0",
Expand Down
65 changes: 65 additions & 0 deletions test/make-spawn-args.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,39 @@ if (isWindows) {
t.end()
})

t.test('event with invalid characters runs', (t) => {
const [shell, args, opts, cleanup] = makeSpawnArgs({
event: 'event<:>\x03', // everything after the word "event" is invalid
path: 'path',
cmd: 'script "quoted parameter"; second command',
})
t.equal(shell, 'cmd', 'default shell applies')
// disabling no-control-regex because we are testing specifically if the control
// character gets removed
// eslint-disable-next-line no-control-regex
t.match(args, ['/d', '/s', '/c', /(?:\\|\/)[^<:>\x03]+.cmd\^"$/], 'got expected args')
t.match(opts, {
env: {
npm_package_json: /package\.json$/,
npm_lifecycle_event: 'event',
npm_lifecycle_script: 'script',
npm_config_node_gyp: require.resolve('node-gyp/bin/node-gyp.js'),
},
stdio: undefined,
cwd: 'path',
windowsVerbatimArguments: true,
}, 'got expected options')

const filename = unescapeCmd(args[args.length - 1])
const contents = fs.readFileSync(filename, { encoding: 'utf8' })
t.equal(contents, `@echo off\nscript "quoted parameter"; second command`)
t.ok(fs.existsSync(filename), 'script file was written')
cleanup()
t.not(fs.existsSync(filename), 'cleanup removes script file')

t.end()
})

t.test('with a funky ComSpec', (t) => {
process.env.ComSpec = 'blrorp'
whichPaths.set('blrorp', '/bin/blrorp')
Expand Down Expand Up @@ -314,6 +347,38 @@ if (isWindows) {
t.end()
})

t.test('event with invalid characters runs', (t) => {
const [shell, args, opts, cleanup] = makeSpawnArgs({
event: 'event<:>/\x04',
path: 'path',
cmd: 'script',
args: ['"quoted parameter";', 'second command'],
})
t.equal(shell, 'sh', 'defaults to sh')
// no-control-regex disabled because we're specifically testing control chars
// eslint-disable-next-line no-control-regex
t.match(args, ['-c', /(?:\\|\/)[^<:>/\x04]+\.sh'$/], 'got expected args')
t.match(opts, {
env: {
npm_package_json: /package\.json$/,
npm_lifecycle_event: 'event',
npm_lifecycle_script: 'script',
},
stdio: undefined,
cwd: 'path',
windowsVerbatimArguments: undefined,
}, 'got expected options')

const filename = unescapeSh(args[args.length - 1])
const contents = fs.readFileSync(filename, { encoding: 'utf8' })
t.equal(contents, `#!/usr/bin/env sh\nscript '"quoted parameter";' 'second command'`)
t.ok(fs.existsSync(filename), 'script file was written')
cleanup()
t.not(fs.existsSync(filename), 'cleanup removes script file')

t.end()
})

t.test('skips /usr/bin/env if scriptShell is absolute', (t) => {
const [shell, args, opts, cleanup] = makeSpawnArgs({
event: 'event',
Expand Down

0 comments on commit 2354d06

Please sign in to comment.