src: fix external module env and kDisableNodeOptionsEnv

PR-URL: nodejs#52905 Fixes: nodejs#51227 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
eliphazbouye · Jun 20, 2024 · 8691502 · 8691502
1 parent 9907349
commit 8691502
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 2 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -2870,6 +2870,13 @@ equivalent to using the `--redirect-warnings=file` command-line flag.
 added:
  - v13.0.0
  - v12.16.0
+changes:
+  - version:
+     - REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/52905
+    description:
+      Remove the possibility to use this env var with
+      kDisableNodeOptionsEnv for embedders.
 -->
 
 Path to a Node.js module which will be loaded in place of the built-in REPL.

diff --git a/src/node.cc b/src/node.cc
@@ -872,6 +872,15 @@ static ExitCode InitializeNodeWithArgsInternal(
           &env_argv, nullptr, errors, kAllowedInEnvvar);
       if (exit_code != ExitCode::kNoFailure) return exit_code;
     }
+  } else {
+    std::string node_repl_external_env = {};
+    if (credentials::SafeGetenv("NODE_REPL_EXTERNAL_MODULE",
+                                &node_repl_external_env) ||
+        !node_repl_external_env.empty()) {
+      errors->emplace_back("NODE_REPL_EXTERNAL_MODULE can't be used with "
+                           "kDisableNodeOptionsEnv");
+      return ExitCode::kInvalidCommandLineArgument;
+    }
   }
 #endif
 

diff --git a/test/embedding/embedtest.cc b/test/embedding/embedtest.cc
@@ -35,8 +35,15 @@ NODE_MAIN(int argc, node::argv_type raw_argv[]) {
   std::shared_ptr<node::InitializationResult> result =
       node::InitializeOncePerProcess(
           args,
-          {node::ProcessInitializationFlags::kNoInitializeV8,
-           node::ProcessInitializationFlags::kNoInitializeNodeV8Platform});
+          {
+              node::ProcessInitializationFlags::kNoInitializeV8,
+              node::ProcessInitializationFlags::kNoInitializeNodeV8Platform,
+              // This is used to test NODE_REPL_EXTERNAL_MODULE is disabled with
+              // kDisableNodeOptionsEnv. If other tests need NODE_OPTIONS
+              // support in the future, split this configuration out as a
+              // command line option.
+              node::ProcessInitializationFlags::kDisableNodeOptionsEnv,
+          });
 
   for (const std::string& error : result->errors())
     fprintf(stderr, "%s: %s\n", args[0].c_str(), error.c_str());

diff --git a/test/embedding/test-embedding.js b/test/embedding/test-embedding.js
@@ -151,3 +151,21 @@ for (const extraSnapshotArgs of [
     [ '--', ...runEmbeddedArgs ],
     { cwd: tmpdir.path });
 }
+
+// Guarantee NODE_REPL_EXTERNAL_MODULE won't bypass kDisableNodeOptionsEnv
+{
+  spawnSyncAndExit(
+    binary,
+    ['require("os")'],
+    {
+      env: {
+        ...process.env,
+        'NODE_REPL_EXTERNAL_MODULE': 'fs',
+      },
+    },
+    {
+      status: 9,
+      signal: null,
+      stderr: `${binary}: NODE_REPL_EXTERNAL_MODULE can't be used with kDisableNodeOptionsEnv\n`,
+    });
+}