From 2354d064e6ef833d9797bf70c333455f075d1b3b Mon Sep 17 00:00:00 2001
From: nlf <quitlahok@gmail.com>
Date: Mon, 27 Jun 2022 10:08:36 -0700
Subject: [PATCH] fix: remove invalid characters from filename (#86)

* chore: remove coverage map

* fix: remove invalid characters from filename
---
 lib/escape.js           |  6 ++++
 lib/make-spawn-args.js  |  5 ++--
 package.json            |  4 ---
 test/make-spawn-args.js | 65 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/lib/escape.js b/lib/escape.js
index 5254be2..3c57437 100644
--- a/lib/escape.js
+++ b/lib/escape.js
@@ -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,
 }
diff --git a/lib/make-spawn-args.js b/lib/make-spawn-args.js
index 660588e..47f7346 100644
--- a/lib/make-spawn-args.js
+++ b/lib/make-spawn-args.js
@@ -30,6 +30,7 @@ const makeSpawnArgs = options => {
     npm_config_node_gyp,
   })
 
+  const fileName = escape.filename(`${event}-${Date.now()}`)
   let scriptFile
   let script = ''
 
@@ -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) {
@@ -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) {
diff --git a/package.json b/package.json
index ef8b43f..1f8ca6a 100644
--- a/package.json
+++ b/package.json
@@ -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",
diff --git a/test/make-spawn-args.js b/test/make-spawn-args.js
index 560702f..dd441f5 100644
--- a/test/make-spawn-args.js
+++ b/test/make-spawn-args.js
@@ -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')
@@ -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',