From 2b68d7ea844557c0233d6c9d79fc8b378a228508 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Wed, 8 May 2024 14:51:37 -0300 Subject: [PATCH 1/2] src: fix external module env and kDisableNodeOptionsEnv --- doc/api/cli.md | 7 +++++++ src/node.cc | 9 +++++++++ test/embedding/embedtest.cc | 11 +++++++++-- test/embedding/test-embedding.js | 17 +++++++++++++++++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 10ade9da2bd448..6ea9bc6ad107f5 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -2844,6 +2844,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 index 21b1ef2b23c7a4..87488cf0e015fd 100644 --- 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 index 741ee6fcad44f9..ef074ac0d68df9 100644 --- a/test/embedding/embedtest.cc +++ b/test/embedding/embedtest.cc @@ -33,8 +33,15 @@ int main(int argc, char** argv) { std::shared_ptr 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 index 21d517b1ec47b1..29d91bc02e2512 100644 --- a/test/embedding/test-embedding.js +++ b/test/embedding/test-embedding.js @@ -151,3 +151,20 @@ for (const extraSnapshotArgs of [ [ '--', ...runEmbeddedArgs ], { cwd: tmpdir.path }); } + +// Guarantee NODE_REPL_EXTERNAL_MODULE won't bypass kDisableNodeOptionsEnv +{ + spawnSyncAndExit( + binary, + ['require("os")'], + { + env: { + 'NODE_REPL_EXTERNAL_MODULE': 'fs', + }, + }, + { + status: 9, + signal: null, + stderr: `${binary}: NODE_REPL_EXTERNAL_MODULE can't be used with kDisableNodeOptionsEnv\n`, + }); +} From 2928f9525f690cff305e09d0e256bc2c0439b741 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Wed, 22 May 2024 10:41:31 -0300 Subject: [PATCH 2/2] fixup! src: fix external module env and kDisableNodeOptionsEnv --- test/embedding/test-embedding.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/embedding/test-embedding.js b/test/embedding/test-embedding.js index 29d91bc02e2512..1a579a11edc8b6 100644 --- a/test/embedding/test-embedding.js +++ b/test/embedding/test-embedding.js @@ -159,6 +159,7 @@ for (const extraSnapshotArgs of [ ['require("os")'], { env: { + ...process.env, 'NODE_REPL_EXTERNAL_MODULE': 'fs', }, },