feat: add fuses for NODE_OPTIONS and --inspect (#30190)

* feat: add fuses for NODE_OPTIONS and --inspect

* chore: add node patch to ensure NODE_OPTIONS are never parsed when fuse is disabledd

* chore: fix lint

* chore: flip boolean logic

* chore: update patches

* chore: add trailing _ to static member

* Update add_should_read_node_options_from_env_option_to_disable_node_options.patch

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

build/fuses/fuses.json5

@@ -3,5 +3,7 @@
   "_schema": "0 == off, 1 == on, r == removed fuse",
   "_version": 1,
   "run_as_node": "1",
-  "cookie_encryption": "0"
+  "cookie_encryption": "0",
+  "node_options": "1",
+  "node_cli_inspect": "1"
 }

patches/node/.patches

@@ -25,3 +25,4 @@ src_allow_embedders_to_provide_a_custom_pageallocator_to.patch
 fix_crypto_tests_to_run_with_bssl.patch
 fix_account_for_debugger_agent_race_condition.patch
 fix_use_new_v8_error_message_property_access_format.patch
+add_should_read_node_options_from_env_option_to_disable_node_options.patch

patches/node/add_should_read_node_options_from_env_option_to_disable_node_options.patch

@@ -0,0 +1,68 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Samuel Attard <samuel.r.attard@gmail.com>
+Date: Wed, 21 Jul 2021 13:40:59 -0700
+Subject: add should_read_node_options_from_env option to disable NODE_OPTIONS
+ parsing at runtime
+
+We can remove the NODE_OPTIONS environment variable but it in theory could be injected / re-inserted at runtime and be used for workers.  In order to ensure the fuse is respected we need a hard runtime toggle for NODE_OPTION support.
+
+diff --git a/src/env.cc b/src/env.cc
+index 1cc7da1ce15f43905ce607adcc1a23ed9d92948a..16af6aec3791df1363682f1ed024c52208b9a8f6 100644
+--- a/src/env.cc
++++ b/src/env.cc
+@@ -329,6 +329,9 @@ std::string GetExecPath(const std::vector<std::string>& argv) {
+   return exec_path;
+ }
+ 
++/* static */
++bool Environment::should_read_node_options_from_env_ = true;
++
+ Environment::Environment(IsolateData* isolate_data,
+                          Isolate* isolate,
+                          const std::vector<std::string>& args,
+diff --git a/src/env.h b/src/env.h
+index 45210f074a0ca4d57f9fdc5019e8e82540b28b72..c0da6c53028bc9459065caf25c1221f556b22d68 100644
+--- a/src/env.h
++++ b/src/env.h
+@@ -1143,6 +1143,8 @@ class Environment : public MemoryRetainer {
+   inline double trigger_async_id();
+   inline double get_default_trigger_async_id();
+ 
++  static bool should_read_node_options_from_env_;
++
+   // List of id's that have been destroyed and need the destroy() cb called.
+   inline std::vector<double>* destroy_async_id_list();
+ 
+diff --git a/src/node.cc b/src/node.cc
+index 6302bb925339d709a54151a8fc8b58f1a2253881..48a9eedfaf6350fc05fb104a1f44e85b75878f9a 100644
+--- a/src/node.cc
++++ b/src/node.cc
+@@ -882,7 +882,7 @@ int InitializeNodeWithArgs(std::vector<std::string>* argv,
+ #if !defined(NODE_WITHOUT_NODE_OPTIONS)
+   std::string node_options;
+ 
+-  if (credentials::SafeGetenv("NODE_OPTIONS", &node_options)) {
++  if (Environment::should_read_node_options_from_env_ && credentials::SafeGetenv("NODE_OPTIONS", &node_options)) {
+     std::vector<std::string> env_argv =
+         ParseNodeOptionsEnvVar(node_options, errors);
+ 
+diff --git a/src/node_worker.cc b/src/node_worker.cc
+index 43a3862cc69dc39b95b329f713691b19de3b8d7e..7782e8488f797a8cf83b1e02f012797ef256afe7 100644
+--- a/src/node_worker.cc
++++ b/src/node_worker.cc
+@@ -457,6 +457,7 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
+     });
+ 
+ #ifndef NODE_WITHOUT_NODE_OPTIONS
++    if (Environment::should_read_node_options_from_env_) {
+     MaybeLocal<String> maybe_node_opts =
+         env_vars->Get(isolate, OneByteString(isolate, "NODE_OPTIONS"));
+     Local<String> node_opts;
+@@ -487,6 +488,7 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
+         return;
+       }
+     }
++    }
+ #endif
+   }
+ 

shell/common/node_bindings.cc

@@ -25,6 +25,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_paths.h"
 #include "electron/buildflags/buildflags.h"
+#include "electron/fuses.h"
 #include "shell/browser/api/electron_api_app.h"
 #include "shell/common/api/electron_bindings.h"
 #include "shell/common/electron_command_line.h"
@@ -188,16 +189,26 @@ void ErrorMessageListener(v8::Local<v8::Message> message,
   }
 }
 
-// Initialize Node.js cli options to pass to Node.js
-// See https://nodejs.org/api/cli.html#cli_options
-void SetNodeCliFlags() {
+const std::unordered_set<base::StringPiece, base::StringPieceHash>
+GetAllowedDebugOptions() {
+  if (electron::fuses::IsNodeCliInspectEnabled()) {
     // Only allow DebugOptions in non-ELECTRON_RUN_AS_NODE mode
-  const std::unordered_set<base::StringPiece, base::StringPieceHash> allowed = {
+    return {
         "--inspect",          "--inspect-brk",
         "--inspect-port",     "--debug",
         "--debug-brk",        "--debug-port",
         "--inspect-brk-node", "--inspect-publish-uid",
     };
+  }
+  // If node CLI inspect support is disabled, allow no debug options.
+  return {};
+}
+
+// Initialize Node.js cli options to pass to Node.js
+// See https://nodejs.org/api/cli.html#cli_options
+void SetNodeCliFlags() {
+  const std::unordered_set<base::StringPiece, base::StringPieceHash> allowed =
+      GetAllowedDebugOptions();
 
   const auto argv = base::CommandLine::ForCurrentProcess()->argv();
   std::vector<std::string> args;
@@ -231,7 +242,7 @@ void SetNodeCliFlags() {
   } else if (!errors.empty()) {
     LOG(ERROR) << err_str << base::JoinString(errors, " ");
   }
-}  // namespace
+}
 
 // Initialize NODE_OPTIONS to pass to Node.js
 // See https://nodejs.org/api/cli.html#cli_node_options_options
@@ -246,6 +257,7 @@ void SetNodeOptions(base::Environment* env) {
                                                      "--http-parser"};
 
   if (env->HasVar("NODE_OPTIONS")) {
+    if (electron::fuses::IsNodeOptionsEnabled()) {
       std::string options;
       env->GetVar("NODE_OPTIONS", &options);
       std::vector<std::string> parts = base::SplitString(
@@ -274,6 +286,10 @@ void SetNodeOptions(base::Environment* env) {
 
       // overwrite new NODE_OPTIONS without unsupported variables
       env->SetVar("NODE_OPTIONS", options);
+    } else {
+      LOG(ERROR) << "NODE_OPTIONS have been disabled in this app";
+      env->SetVar("NODE_OPTIONS", "");
+    }
   }
 }
 
@@ -364,6 +380,8 @@ void NodeBindings::Initialize() {
 
   auto env = base::Environment::Create();
   SetNodeOptions(env.get());
+  node::Environment::should_read_node_options_from_env_ =
+      fuses::IsNodeOptionsEnabled();
 
   std::vector<std::string> argv = {"electron"};
   std::vector<std::string> exec_argv;