From 5e7024ff454b6049f8ce41695600e065e58363f1 Mon Sep 17 00:00:00 2001
From: Christopher Hiller <boneskull@boneskull.com>
Date: Sun, 24 Feb 2019 21:07:39 -0800
Subject: [PATCH] fix handling of bareword args matching node flags; closes
 #3761

---
 lib/cli/node-flags.js                       | 32 +++++++++++++++------
 lib/cli/options.js                          |  5 ++--
 test/integration/options/node-flags.spec.js | 19 ++++++++++++
 test/node-unit/cli/node-flags.spec.js       | 12 ++++++++
 4 files changed, 58 insertions(+), 10 deletions(-)
 create mode 100644 test/integration/options/node-flags.spec.js

diff --git a/lib/cli/node-flags.js b/lib/cli/node-flags.js
index c2c77c54b0..33658e44f5 100644
--- a/lib/cli/node-flags.js
+++ b/lib/cli/node-flags.js
@@ -29,16 +29,32 @@ const debugFlags = new Set(['debug', 'debug-brk', 'inspect', 'inspect-brk']);
  *   - `--v8-*` (but *not* `--v8-options`)
  * @summary Whether or not to pass a flag along to the `node` executable.
  * @param {string} flag - Flag to test
- * @returns {boolean}
+ * @param {boolean} [bareword=true] - If `false`, we expect `flag` to have one or two leading dashes.
+ * @returns {boolean} If the flag is considered a "Node" flag.
  * @private
  */
-exports.isNodeFlag = flag =>
-  !/^(?:require|r)$/.test(flag) &&
-  (nodeFlags.has(flag) ||
-    debugFlags.has(flag) ||
-    /(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc(?:[_-]global)?$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test(
-      flag
-    ));
+exports.isNodeFlag = (flag, bareword = true) => {
+  if (!bareword) {
+    // check if the flag begins with dashes; if not, not a node flag.
+    if (!/^--?/.test(flag)) {
+      return false;
+    }
+    // strip the leading dashes to match against subsequent regexes
+    flag = flag.replace(/^--?/, '');
+  }
+  return (
+    // treat --require/-r as Mocha flag even though it's also a node flag
+    !/^r(?:equire)?$/.test(flag) &&
+    // check actual node flags from `process.allowedNodeEnvironmentFlags`,
+    // then historical support for various V8 and non-`NODE_OPTIONS` flags
+    // and also any V8 flags with `--v8-` prefix
+    (nodeFlags.has(flag) ||
+      debugFlags.has(flag) ||
+      /(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc(?:[_-]global)?$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test(
+        flag
+      ))
+  );
+};
 
 /**
  * Returns `true` if the flag is a "debug-like" flag.  These require timeouts
diff --git a/lib/cli/options.js b/lib/cli/options.js
index e332c58c89..1e72dee034 100644
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -85,8 +85,9 @@ const parse = (args = [], ...configObjects) => {
   const nodeArgs = (Array.isArray(args) ? args : args.split(' ')).reduce(
     (acc, arg) => {
       const pair = arg.split('=');
-      const flag = pair[0].replace(/^--?/, '');
-      if (isNodeFlag(flag)) {
+      let flag = pair[0];
+      if (isNodeFlag(flag, false)) {
+        flag = flag.replace(/^--?/, '');
         return arg.includes('=')
           ? acc.concat([[flag, pair[1]]])
           : acc.concat([[flag, true]]);
diff --git a/test/integration/options/node-flags.spec.js b/test/integration/options/node-flags.spec.js
new file mode 100644
index 0000000000..b884482527
--- /dev/null
+++ b/test/integration/options/node-flags.spec.js
@@ -0,0 +1,19 @@
+'use strict';
+
+var invokeMocha = require('../helpers').invokeMocha;
+
+describe('node flags', function() {
+  it('should not consider argument values to be node flags', function(done) {
+    invokeMocha(
+      ['--require', 'trace-dependency'],
+      function(err, res) {
+        if (err) {
+          return done(err);
+        }
+        expect(res, 'not to have failed with output', /bad option/i);
+        done();
+      },
+      'pipe'
+    );
+  });
+});
diff --git a/test/node-unit/cli/node-flags.spec.js b/test/node-unit/cli/node-flags.spec.js
index 22868fe584..b118d5d5e6 100644
--- a/test/node-unit/cli/node-flags.spec.js
+++ b/test/node-unit/cli/node-flags.spec.js
@@ -15,6 +15,18 @@ describe('node-flags', function() {
       });
     });
 
+    describe('when expecting leading dashes', function() {
+      it('should require leading dashes', function() {
+        expect(isNodeFlag('throw-deprecation', false), 'to be false');
+        expect(isNodeFlag('--throw-deprecation', false), 'to be true');
+      });
+
+      it('should return false for --require/-r', function() {
+        expect(isNodeFlag('--require', false), 'to be false');
+        expect(isNodeFlag('-r', false), 'to be false');
+      });
+    });
+
     describe('special cases', function() {
       it('should return true for flags starting with "preserve-symlinks"', function() {
         expect(isNodeFlag('preserve-symlinks'), 'to be true');