src: fix external module env and kDisableNodeOptionsEnv

RafaelGSS · targos · commit 7aa9519ad4fb · 2024-06-01T10:54:59.000+02:00
PR-URL: #52905 Fixes: #51227 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -2921,6 +2921,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
@@ -921,6 +921,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`,
+    });
+}
