src: fix external module env and kDisableNodeOptionsEnv

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>
nodejs · Jun 1, 2024 · 7aa9519 · 7aa9519
1 parent c61f909
commit 7aa9519
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
@@ -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`,
+ });
+}