src: allow CLI args in env with NODE_OPTIONS

Not all CLI options are supported, those that are problematic from a
security or implementation point of view are disallowed, as are ones
that are inappropriate (for example, -e, -p, --i), or that only make
sense when changed with code changes (such as options that change the
javascript syntax or add new APIs).

PR-URL: #12028
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>

Author: sam-github

configure

@@ -432,6 +432,11 @@ parser.add_option('--without-ssl',
     dest='without_ssl',
     help='build without SSL')
 
+parser.add_option('--without-node-options',
+    action='store_true',
+    dest='without_node_options',
+    help='build without NODE_OPTIONS support')
+
 parser.add_option('--xcode',
     action='store_true',
     dest='use_xcode',
@@ -965,6 +970,9 @@ def configure_openssl(o):
   o['variables']['openssl_no_asm'] = 1 if options.openssl_no_asm else 0
   if options.use_openssl_ca_store:
     o['defines'] += ['NODE_OPENSSL_CERT_STORE']
+  o['variables']['node_without_node_options'] = b(options.without_node_options)
+  if options.without_node_options:
+      o['defines'] += ['NODE_WITHOUT_NODE_OPTIONS']
   if options.openssl_fips:
     o['variables']['openssl_fips'] = options.openssl_fips
     fips_dir = os.path.join(root_dir, 'deps', 'openssl', 'fips')

doc/api/cli.md

@@ -365,6 +365,40 @@ added: v7.5.0
 
 When set to `1`, process warnings are silenced.
 
+### `NODE_OPTIONS=options...`
+<!-- YAML
+added: REPLACEME
+-->
+
+`options...` are interpreted as if they had been specified on the command line
+before the actual command line (so they can be overriden).  Node will exit with
+an error if an option that is not allowed in the environment is used, such as
+`-p` or a script file.
+
+Node options that are allowed are:
+- `--enable-fips`
+- `--force-fips`
+- `--icu-data-dir`
+- `--no-deprecation`
+- `--no-warnings`
+- `--openssl-config`
+- `--prof-process`
+- `--redirect-warnings`
+- `--require`, `-r`
+- `--throw-deprecation`
+- `--trace-deprecation`
+- `--trace-events-enabled`
+- `--trace-sync-io`
+- `--trace-warnings`
+- `--track-heap-objects`
+- `--use-bundled-ca`
+- `--use-openssl-ca`
+- `--v8-pool-size`
+- `--zero-fill-buffers`
+
+V8 options that are allowed are:
+- `--max_old_space_size`
+
 ### `NODE_PRESERVE_SYMLINKS=1`
 <!-- YAML
 added: v7.1.0

doc/node.1

@@ -237,6 +237,13 @@ with small\-icu support.
 .BR NODE_NO_WARNINGS =\fI1\fR
 When set to \fI1\fR, process warnings are silenced.
 
+.TP
+.BR NODE_OPTIONS =\fIoptions...\fR
+\fBoptions...\fR are interpreted as if they had been specified on the command
+line before the actual command line (so they can be overriden).  Node will exit
+with an error if an option that is not allowed in the environment is used, such
+as \fB-p\fR or a script file.
+
 .TP
 .BR NODE_PATH =\fIpath\fR[:\fI...\fR]
 \':\'\-separated list of directories prefixed to the module search path.

src/node.cc

@@ -3548,6 +3548,9 @@ static void PrintHelp() {
 #endif
 #endif
          "NODE_NO_WARNINGS             set to 1 to silence process warnings\n"
+#if !defined(NODE_WITHOUT_NODE_OPTIONS)
+         "NODE_OPTIONS                 set CLI options in the environment\n"
+#endif
 #ifdef _WIN32
          "NODE_PATH                    ';'-separated list of directories\n"
 #else
@@ -3562,6 +3565,51 @@ static void PrintHelp() {
 }
 
 
+static void CheckIfAllowedInEnv(const char* exe, bool is_env,
+                                const char* arg) {
+  if (!is_env)
+    return;
+
+  // Find the arg prefix when its --some_arg=val
+  const char* eq = strchr(arg, '=');
+  size_t arglen = eq ? eq - arg : strlen(arg);
+
+  static const char* whitelist[] = {
+    // Node options
+    "-r", "--require",
+    "--no-deprecation",
+    "--no-warnings",
+    "--trace-warnings",
+    "--redirect-warnings",
+    "--trace-deprecation",
+    "--trace-sync-io",
+    "--trace-events-enabled",
+    "--track-heap-objects",
+    "--throw-deprecation",
+    "--zero-fill-buffers",
+    "--v8-pool-size",
+    "--use-openssl-ca",
+    "--use-bundled-ca",
+    "--enable-fips",
+    "--force-fips",
+    "--openssl-config",
+    "--icu-data-dir",
+
+    // V8 options
+    "--max_old_space_size",
+  };
+
+  for (unsigned i = 0; i < arraysize(whitelist); i++) {
+    const char* allowed = whitelist[i];
+    if (strlen(allowed) == arglen && strncmp(allowed, arg, arglen) == 0)
+      return;
+  }
+
+  fprintf(stderr, "%s: %s is not allowed in NODE_OPTIONS\n", exe, arg);
+  exit(9);
+}
+
+
 // Parse command line arguments.
 //
 // argv is modified in place. exec_argv and v8_argv are out arguments that
@@ -3578,7 +3626,8 @@ static void ParseArgs(int* argc,
                       int* exec_argc,
                       const char*** exec_argv,
                       int* v8_argc,
-                      const char*** v8_argv) {
+                      const char*** v8_argv,
+                      bool is_env) {
   const unsigned int nargs = static_cast<unsigned int>(*argc);
   const char** new_exec_argv = new const char*[nargs];
   const char** new_v8_argv = new const char*[nargs];
@@ -3605,6 +3654,8 @@ static void ParseArgs(int* argc,
     const char* const arg = argv[index];
     unsigned int args_consumed = 1;
 
+    CheckIfAllowedInEnv(argv[0], is_env, arg);
+
     if (debug_options.ParseOption(arg)) {
       // Done, consumed by DebugOptions::ParseOption().
     } else if (strcmp(arg, "--version") == 0 || strcmp(arg, "-v") == 0) {
@@ -3733,6 +3784,13 @@ static void ParseArgs(int* argc,
 
   // Copy remaining arguments.
   const unsigned int args_left = nargs - index;
+
+  if (is_env && args_left) {
+    fprintf(stderr, "%s: %s is not supported in NODE_OPTIONS\n",
+            argv[0], argv[index]);
+    exit(9);
+  }
+
   memcpy(new_argv + new_argc, argv + index, args_left * sizeof(*argv));
   new_argc += args_left;
 
@@ -4166,6 +4224,54 @@ inline void PlatformInit() {
 }
 
 
+void ProcessArgv(int* argc,
+                 const char** argv,
+                 int* exec_argc,
+                 const char*** exec_argv,
+                 bool is_env = false) {
+  // Parse a few arguments which are specific to Node.
+  int v8_argc;
+  const char** v8_argv;
+  ParseArgs(argc, argv, exec_argc, exec_argv, &v8_argc, &v8_argv, is_env);
+
+  // TODO(bnoordhuis) Intercept --prof arguments and start the CPU profiler
+  // manually?  That would give us a little more control over its runtime
+  // behavior but it could also interfere with the user's intentions in ways
+  // we fail to anticipate.  Dillema.
+  for (int i = 1; i < v8_argc; ++i) {
+    if (strncmp(v8_argv[i], "--prof", sizeof("--prof") - 1) == 0) {
+      v8_is_profiling = true;
+      break;
+    }
+  }
+
+#ifdef __POSIX__
+  // Block SIGPROF signals when sleeping in epoll_wait/kevent/etc.  Avoids the
+  // performance penalty of frequent EINTR wakeups when the profiler is running.
+  // Only do this for v8.log profiling, as it breaks v8::CpuProfiler users.
+  if (v8_is_profiling) {
+    uv_loop_configure(uv_default_loop(), UV_LOOP_BLOCK_SIGNAL, SIGPROF);
+  }
+#endif
+
+  // The const_cast doesn't violate conceptual const-ness.  V8 doesn't modify
+  // the argv array or the elements it points to.
+  if (v8_argc > 1)
+    V8::SetFlagsFromCommandLine(&v8_argc, const_cast<char**>(v8_argv), true);
+
+  // Anything that's still in v8_argv is not a V8 or a node option.
+  for (int i = 1; i < v8_argc; i++) {
+    fprintf(stderr, "%s: bad option: %s\n", argv[0], v8_argv[i]);
+  }
+  delete[] v8_argv;
+  v8_argv = nullptr;
+
+  if (v8_argc > 1) {
+    exit(9);
+  }
+}
+
+
 void Init(int* argc,
           const char** argv,
           int* exec_argc,
@@ -4208,31 +4314,36 @@ void Init(int* argc,
   if (openssl_config.empty())
     SafeGetenv("OPENSSL_CONF", &openssl_config);
 
-  // Parse a few arguments which are specific to Node.
-  int v8_argc;
-  const char** v8_argv;
-  ParseArgs(argc, argv, exec_argc, exec_argv, &v8_argc, &v8_argv);
-
-  // TODO(bnoordhuis) Intercept --prof arguments and start the CPU profiler
-  // manually?  That would give us a little more control over its runtime
-  // behavior but it could also interfere with the user's intentions in ways
-  // we fail to anticipate.  Dillema.
-  for (int i = 1; i < v8_argc; ++i) {
-    if (strncmp(v8_argv[i], "--prof", sizeof("--prof") - 1) == 0) {
-      v8_is_profiling = true;
-      break;
+#if !defined(NODE_WITHOUT_NODE_OPTIONS)
+  std::string node_options;
+  if (SafeGetenv("NODE_OPTIONS", &node_options)) {
+    // Smallest tokens are 2-chars (a not space and a space), plus 2 extra
+    // pointers, for the prepended executable name, and appended NULL pointer.
+    size_t max_len = 2 + (node_options.length() + 1) / 2;
+    const char** argv_from_env = new const char*[max_len];
+    int argc_from_env = 0;
+    // [0] is expected to be the program name, fill it in from the real argv.
+    argv_from_env[argc_from_env++] = argv[0];
+
+    char* cstr = strdup(node_options.c_str());
+    char* initptr = cstr;
+    char* token;
+    while ((token = strtok(initptr, " "))) {  // NOLINT(runtime/threadsafe_fn)
+      initptr = nullptr;
+      argv_from_env[argc_from_env++] = token;
     }
-  }
-
-#ifdef __POSIX__
-  // Block SIGPROF signals when sleeping in epoll_wait/kevent/etc.  Avoids the
-  // performance penalty of frequent EINTR wakeups when the profiler is running.
-  // Only do this for v8.log profiling, as it breaks v8::CpuProfiler users.
-  if (v8_is_profiling) {
-    uv_loop_configure(uv_default_loop(), UV_LOOP_BLOCK_SIGNAL, SIGPROF);
+    argv_from_env[argc_from_env] = nullptr;
+    int exec_argc_;
+    const char** exec_argv_ = nullptr;
+    ProcessArgv(&argc_from_env, argv_from_env, &exec_argc_, &exec_argv_, true);
+    delete[] exec_argv_;
+    delete[] argv_from_env;
+    free(cstr);
   }
 #endif
 
+  ProcessArgv(argc, argv, exec_argc, exec_argv);
+
 #if defined(NODE_HAVE_I18N_SUPPORT)
   // If the parameter isn't given, use the env variable.
   if (icu_data_dir.empty())
@@ -4244,21 +4355,6 @@ void Init(int* argc,
                      "(check NODE_ICU_DATA or --icu-data-dir parameters)");
   }
 #endif
-  // The const_cast doesn't violate conceptual const-ness.  V8 doesn't modify
-  // the argv array or the elements it points to.
-  if (v8_argc > 1)
-    V8::SetFlagsFromCommandLine(&v8_argc, const_cast<char**>(v8_argv), true);
-
-  // Anything that's still in v8_argv is not a V8 or a node option.
-  for (int i = 1; i < v8_argc; i++) {
-    fprintf(stderr, "%s: bad option: %s\n", argv[0], v8_argv[i]);
-  }
-  delete[] v8_argv;
-  v8_argv = nullptr;
-
-  if (v8_argc > 1) {
-    exit(9);
-  }
 
   // Unconditionally force typed arrays to allocate outside the v8 heap. This
   // is to prevent memory pointers from being moved around that are returned by

test/parallel/test-cli-node-options.js

@@ -0,0 +1,75 @@
+'use strict';
+const common = require('../common');
+if (process.config.variables.node_without_node_options)
+  return common.skip('missing NODE_OPTIONS support');
+
+// Test options specified by env variable.
+
+const assert = require('assert');
+const exec = require('child_process').execFile;
+
+disallow('--version');
+disallow('-v');
+disallow('--help');
+disallow('-h');
+disallow('--eval');
+disallow('-e');
+disallow('--print');
+disallow('-p');
+disallow('-pe');
+disallow('--check');
+disallow('-c');
+disallow('--interactive');
+disallow('-i');
+disallow('--v8-options');
+disallow('--');
+
+function disallow(opt) {
+  const options = {env: {NODE_OPTIONS: opt}};
+  exec(process.execPath, options, common.mustCall(function(err) {
+    const message = err.message.split(/\r?\n/)[1];
+    const expect = process.execPath + ': ' + opt +
+                   ' is not allowed in NODE_OPTIONS';
+
+    assert.strictEqual(err.code, 9);
+    assert.strictEqual(message, expect);
+  }));
+}
+
+const printA = require.resolve('../fixtures/printA.js');
+
+expect('-r ' + printA, 'A\nB\n');
+expect('--no-deprecation', 'B\n');
+expect('--no-warnings', 'B\n');
+expect('--trace-warnings', 'B\n');
+expect('--trace-deprecation', 'B\n');
+expect('--trace-sync-io', 'B\n');
+expect('--trace-events-enabled', 'B\n');
+expect('--track-heap-objects', 'B\n');
+expect('--throw-deprecation', 'B\n');
+expect('--zero-fill-buffers', 'B\n');
+expect('--v8-pool-size=10', 'B\n');
+expect('--use-openssl-ca', 'B\n');
+expect('--use-bundled-ca', 'B\n');
+expect('--openssl-config=_ossl_cfg', 'B\n');
+expect('--icu-data-dir=_d', 'B\n');
+
+    // V8 options
+expect('--max_old_space_size=0', 'B\n');
+
+function expect(opt, want) {
+  const printB = require.resolve('../fixtures/printB.js');
+  const argv = [printB];
+  const opts = {
+    env: {NODE_OPTIONS: opt},
+    maxBuffer: 1000000000,
+  };
+  exec(process.execPath, argv, opts, common.mustCall(function(err, stdout) {
+    assert.ifError(err);
+    if (!RegExp(want).test(stdout)) {
+      console.error('For %j, failed to find %j in: <\n%s\n>',
+                    opt, expect, stdout);
+      assert(false, 'Expected ' + expect);
+    }
+  }));
+}