From 2bf46ae45ed49073369627f0e60aa720354ce0d3 Mon Sep 17 00:00:00 2001
From: Christopher Hiller <boneskull@boneskull.com>
Date: Tue, 13 Mar 2018 15:53:39 -0700
Subject: [PATCH] process: add allowedNodeEnvironmentFlags property

`process.allowedNodeEnvironmentFlags` provides an API to validate and
list flags as specified in `NODE_OPTIONS` from user code.

Refs: https://github.com/nodejs/node/issues/17740
Signed-off-by: Christopher Hiller <boneskull@boneskull.com>

PR-URL: https://github.com/nodejs/node/pull/19335
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Sam Ruby <rubys@intertwingly.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
---
 doc/api/process.md                            | 51 +++++++++++
 lib/internal/bootstrap/node.js                | 89 +++++++++++++++++++
 src/node.cc                                   | 62 +++++++++++++
 src/node_config.cc                            | 17 ++++
 src/node_internals.h                          |  5 ++
 .../test-process-env-allowed-flags.js         | 74 +++++++++++++++
 tools/doc/type-parser.js                      |  2 +-
 7 files changed, 299 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-process-env-allowed-flags.js

diff --git a/doc/api/process.md b/doc/api/process.md
index 0c35007103c253..5aec229e9e8ed9 100644
--- a/doc/api/process.md
+++ b/doc/api/process.md
@@ -416,6 +416,55 @@ generate a core file.
 
 This feature is not available in [`Worker`][] threads.
 
+## process.allowedNodeEnvironmentFlags
+<!-- YAML
+added: REPLACEME
+-->
+
+* {Set}
+
+The `process.allowedNodeEnvironmentFlags` property is a special,
+read-only `Set` of flags allowable within the [`NODE_OPTIONS`][]
+environment variable.
+
+`process.allowedNodeEnvironmentFlags` extends `Set`, but overrides
+`Set.prototype.has` to recognize several different possible flag
+representations.  `process.allowedNodeEnvironmentFlags.has()` will
+return `true` in the following cases:
+
+- Flags may omit leading single (`-`) or double (`--`) dashes; e.g.,
+  `inspect-brk` for `--inspect-brk`, or `r` for `-r`.
+- Flags passed through to V8 (as listed in `--v8-options`) may replace
+  one or more *non-leading* dashes for an underscore, or vice-versa;
+  e.g., `--perf_basic_prof`, `--perf-basic-prof`, `--perf_basic-prof`,
+  etc.
+- Flags may contain one or more equals (`=`) characters; all
+  characters after and including the first equals will be ignored;
+  e.g., `--stack-trace-limit=100`.
+- Flags *must* be allowable within [`NODE_OPTIONS`][].
+
+When iterating over `process.allowedNodeEnvironmentFlags`, flags will
+appear only *once*; each will begin with one or more dashes. Flags
+passed through to V8 will contain underscores instead of non-leading
+dashes:
+
+```js
+process.allowedNodeEnvironmentFlags.forEach((flag) => {
+  // -r
+  // --inspect-brk
+  // --abort_on_uncaught_exception
+  // ...
+});
+```
+
+The methods `add()`, `clear()`, and `delete()` of
+`process.allowedNodeEnvironmentFlags` do nothing, and will fail
+silently.
+
+If Node.js was compiled *without* [`NODE_OPTIONS`][] support (shown in
+[`process.config`][]), `process.allowedNodeEnvironmentFlags` will
+contain what *would have* been allowable.
+
 ## process.arch
 <!-- YAML
 added: v0.5.0
@@ -2061,8 +2110,10 @@ cases:
 [`end()`]: stream.html#stream_writable_end_chunk_encoding_callback
 [`net.Server`]: net.html#net_class_net_server
 [`net.Socket`]: net.html#net_class_net_socket
+[`NODE_OPTIONS`]: cli.html#cli_node_options_options
 [`os.constants.dlopen`]: os.html#os_dlopen_constants
 [`process.argv`]: #process_process_argv
+[`process.config`]: #process_process_config
 [`process.execPath`]: #process_process_execpath
 [`process.exit()`]: #process_process_exit_code
 [`process.exitCode`]: #process_process_exitcode
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
index 016c0c5e2304bc..3e51f6ca69b52f 100644
--- a/lib/internal/bootstrap/node.js
+++ b/lib/internal/bootstrap/node.js
@@ -194,6 +194,8 @@
 
     perf.markMilestone(NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE);
 
+    setupAllowedFlags();
+
     // There are various modes that Node can run in. The most common two
     // are running from a script and running the REPL - but there are a few
     // others like the debugger or running --eval arguments. Here we decide
@@ -593,5 +595,92 @@
     new vm.Script(source, { displayErrors: true, filename });
   }
 
+  function setupAllowedFlags() {
+    // This builds process.allowedNodeEnvironmentFlags
+    // from data in the config binding
+
+    const replaceDashesRegex = /-/g;
+    const leadingDashesRegex = /^--?/;
+    const trailingValuesRegex = /=.*$/;
+
+    // Save references so user code does not interfere
+    const replace = Function.call.bind(String.prototype.replace);
+    const has = Function.call.bind(Set.prototype.has);
+    const test = Function.call.bind(RegExp.prototype.test);
+
+    const {
+      allowedV8EnvironmentFlags,
+      allowedNodeEnvironmentFlags
+    } = process.binding('config');
+
+    const trimLeadingDashes = (flag) => replace(flag, leadingDashesRegex, '');
+
+    // Save these for comparison against flags provided to
+    // process.allowedNodeEnvironmentFlags.has() which lack leading dashes.
+    // Avoid interference w/ user code by flattening `Set.prototype` into
+    // each object.
+    const [nodeFlags, v8Flags] = [
+      allowedNodeEnvironmentFlags, allowedV8EnvironmentFlags
+    ].map((flags) => Object.defineProperties(
+      new Set(flags.map(trimLeadingDashes)),
+      Object.getOwnPropertyDescriptors(Set.prototype))
+    );
+
+    class NodeEnvironmentFlagsSet extends Set {
+      constructor(...args) {
+        super(...args);
+
+        // the super constructor consumes `add`, but
+        // disallow any future adds.
+        this.add = () => this;
+      }
+
+      delete() {
+        // noop, `Set` API compatible
+        return false;
+      }
+
+      clear() {
+        // noop
+      }
+
+      has(key) {
+        // This will return `true` based on various possible
+        // permutations of a flag, including present/missing leading
+        // dash(es) and/or underscores-for-dashes in the case of V8-specific
+        // flags.  Strips any values after `=`, inclusive.
+        if (typeof key === 'string') {
+          key = replace(key, trailingValuesRegex, '');
+          if (test(leadingDashesRegex, key)) {
+            return has(this, key) ||
+                  has(v8Flags,
+                      replace(
+                        replace(
+                          key,
+                          leadingDashesRegex,
+                          ''
+                        ),
+                        replaceDashesRegex,
+                        '_'
+                      )
+                  );
+          }
+          return has(nodeFlags, key) ||
+                has(v8Flags, replace(key, replaceDashesRegex, '_'));
+        }
+        return false;
+      }
+    }
+
+    Object.freeze(NodeEnvironmentFlagsSet.prototype.constructor);
+    Object.freeze(NodeEnvironmentFlagsSet.prototype);
+
+    process.allowedNodeEnvironmentFlags = Object.freeze(
+      new NodeEnvironmentFlagsSet(
+        allowedNodeEnvironmentFlags.concat(allowedV8EnvironmentFlags)
+      )
+    );
+  }
+
   startup();
 });
diff --git a/src/node.cc b/src/node.cc
index 949e7909d3bedb..97102700f65800 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -587,6 +587,68 @@ const char* signo_string(int signo) {
   }
 }
 
+// These are all flags available for use with NODE_OPTIONS.
+//
+// Disallowed flags:
+//  These flags cause Node to do things other than run scripts:
+//    --version / -v
+//    --eval / -e
+//    --print / -p
+//    --check / -c
+//    --interactive / -i
+//    --prof-process
+//    --v8-options
+//  These flags are disallowed because security:
+//    --preserve-symlinks
+const char* const environment_flags[] = {
+  // Node options, sorted in `node --help` order for ease of comparison.
+  "--enable-fips",
+  "--experimental-modules",
+  "--experimenatl-repl-await",
+  "--experimental-vm-modules",
+  "--experimental-worker",
+  "--force-fips",
+  "--icu-data-dir",
+  "--inspect",
+  "--inspect-brk",
+  "--inspect-port",
+  "--loader",
+  "--napi-modules",
+  "--no-deprecation",
+  "--no-force-async-hooks-checks",
+  "--no-warnings",
+  "--openssl-config",
+  "--pending-deprecation",
+  "--redirect-warnings",
+  "--require",
+  "--throw-deprecation",
+  "--tls-cipher-list",
+  "--trace-deprecation",
+  "--trace-event-categories",
+  "--trace-event-file-pattern",
+  "--trace-events-enabled",
+  "--trace-sync-io",
+  "--trace-warnings",
+  "--track-heap-objects",
+  "--use-bundled-ca",
+  "--use-openssl-ca",
+  "--v8-pool-size",
+  "--zero-fill-buffers",
+  "-r"
+};
+
+  // V8 options (define with '_', which allows '-' or '_')
+const char* const v8_environment_flags[] = {
+  "--abort_on_uncaught_exception",
+  "--max_old_space_size",
+  "--perf_basic_prof",
+  "--perf_prof",
+  "--stack_trace_limit",
+};
+
+int v8_environment_flags_count = arraysize(v8_environment_flags);
+int environment_flags_count = arraysize(environment_flags);
+
 // Look up environment variable unless running as setuid root.
 bool SafeGetenv(const char* key, std::string* text) {
 #if !defined(__CloudABI__) && !defined(_WIN32)
diff --git a/src/node_config.cc b/src/node_config.cc
index d34269912e4713..c6e6211da288e4 100644
--- a/src/node_config.cc
+++ b/src/node_config.cc
@@ -5,6 +5,7 @@
 
 namespace node {
 
+using v8::Array;
 using v8::Boolean;
 using v8::Context;
 using v8::Integer;
@@ -132,6 +133,22 @@ static void Initialize(Local<Object> target,
   READONLY_PROPERTY(debug_options_obj,
                     "inspectorEnabled",
                     Boolean::New(isolate, debug_options->inspector_enabled));
+
+  Local<Array> environmentFlags = Array::New(env->isolate(),
+                                  environment_flags_count);
+  READONLY_PROPERTY(target, "allowedNodeEnvironmentFlags", environmentFlags);
+  for (int i = 0; i < environment_flags_count; ++i) {
+    environmentFlags->Set(i, OneByteString(env->isolate(),
+        environment_flags[i]));
+  }
+
+  Local<Array> v8EnvironmentFlags = Array::New(env->isolate(),
+                                    v8_environment_flags_count);
+  READONLY_PROPERTY(target, "allowedV8EnvironmentFlags", v8EnvironmentFlags);
+  for (int i = 0; i < v8_environment_flags_count; ++i) {
+    v8EnvironmentFlags->Set(i, OneByteString(env->isolate(),
+        v8_environment_flags[i]));
+  }
 }  // InitConfig
 
 }  // namespace node
diff --git a/src/node_internals.h b/src/node_internals.h
index c3150edda45fb6..b8ab492c44a675 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -178,6 +178,11 @@ extern bool v8_initialized;
 
 extern std::shared_ptr<PerProcessOptions> per_process_opts;
 
+extern const char* const environment_flags[];
+extern int environment_flags_count;
+extern const char* const v8_environment_flags[];
+extern int v8_environment_flags_count;
+
 // Forward declaration
 class Environment;
 
diff --git a/test/parallel/test-process-env-allowed-flags.js b/test/parallel/test-process-env-allowed-flags.js
new file mode 100644
index 00000000000000..25907c3b0e8f3c
--- /dev/null
+++ b/test/parallel/test-process-env-allowed-flags.js
@@ -0,0 +1,74 @@
+'use strict';
+
+const assert = require('assert');
+require('../common');
+
+// assert legit flags are allowed, and bogus flags are disallowed
+{
+  const goodFlags = [
+    '--inspect-brk',
+    'inspect-brk',
+    '--perf_basic_prof',
+    '--perf-basic-prof',
+    'perf-basic-prof',
+    '--perf_basic-prof',
+    'perf_basic-prof',
+    'perf_basic_prof',
+    '-r',
+    'r',
+    '--stack-trace-limit=100',
+    '--stack-trace-limit=-=xX_nodejs_Xx=-'
+  ];
+
+  const badFlags = [
+    '--inspect_brk',
+    'INSPECT-BRK',
+    '--INSPECT-BRK',
+    '--r',
+    '-R',
+    '---inspect-brk',
+    '--cheeseburgers'
+  ];
+
+  goodFlags.forEach((flag) => {
+    assert.strictEqual(
+      process.allowedNodeEnvironmentFlags.has(flag),
+      true,
+      `flag should be in set: ${flag}`
+    );
+  });
+
+  badFlags.forEach((flag) => {
+    assert.strictEqual(
+      process.allowedNodeEnvironmentFlags.has(flag),
+      false,
+      `flag should not be in set: ${flag}`
+    );
+  });
+}
+
+// assert all "canonical" flags begin with dash(es)
+{
+  process.allowedNodeEnvironmentFlags.forEach((flag) => {
+    assert.strictEqual(/^--?[a-z8_-]+$/.test(flag), true);
+  });
+}
+
+// assert immutability of process.allowedNodeEnvironmentFlags
+{
+  assert.strictEqual(Object.isFrozen(process.allowedNodeEnvironmentFlags),
+                     true);
+
+  process.allowedNodeEnvironmentFlags.add('foo');
+  assert.strictEqual(process.allowedNodeEnvironmentFlags.has('foo'), false);
+  process.allowedNodeEnvironmentFlags.forEach((flag) => {
+    assert.strictEqual(flag === 'foo', false);
+  });
+
+  process.allowedNodeEnvironmentFlags.clear();
+  assert.strictEqual(process.allowedNodeEnvironmentFlags.size > 0, true);
+
+  const size = process.allowedNodeEnvironmentFlags.size;
+  process.allowedNodeEnvironmentFlags.delete('-r');
+  assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
+}
diff --git a/tools/doc/type-parser.js b/tools/doc/type-parser.js
index 2a63f97c798075..b57dc6957a9ae0 100644
--- a/tools/doc/type-parser.js
+++ b/tools/doc/type-parser.js
@@ -16,7 +16,7 @@ const jsPrimitives = {
 const jsGlobalObjectsUrl = `${jsDocPrefix}Reference/Global_Objects/`;
 const jsGlobalTypes = [
   'Array', 'ArrayBuffer', 'DataView', 'Date', 'Error', 'EvalError', 'Function',
-  'Object', 'Promise', 'RangeError', 'ReferenceError', 'RegExp',
+  'Object', 'Promise', 'RangeError', 'ReferenceError', 'RegExp', 'Set',
   'SharedArrayBuffer', 'SyntaxError', 'TypeError', 'TypedArray', 'URIError',
   'Uint8Array',
 ];