From 60eb325575d22170e71db16b35638b3462a441bb Mon Sep 17 00:00:00 2001
From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com>
Date: Wed, 24 Jan 2024 15:53:00 +0000
Subject: [PATCH] fix: only remove hijackable envs from foreign parent

Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
---
 filenames.gni                 |  1 -
 shell/app/node_main.cc        | 21 +++++++++++++++++++--
 shell/common/node_util.h      |  6 ------
 shell/common/node_util_mac.mm | 23 -----------------------
 4 files changed, 19 insertions(+), 32 deletions(-)
 delete mode 100644 shell/common/node_util_mac.mm

diff --git a/filenames.gni b/filenames.gni
index 2bfaf17f30fd0..9b81d07d6ef78 100644
--- a/filenames.gni
+++ b/filenames.gni
@@ -660,7 +660,6 @@ filenames = {
     "shell/common/node_includes.h",
     "shell/common/node_util.cc",
     "shell/common/node_util.h",
-    "shell/common/node_util_mac.mm",
     "shell/common/options_switches.cc",
     "shell/common/options_switches.h",
     "shell/common/platform_util.cc",
diff --git a/shell/app/node_main.cc b/shell/app/node_main.cc
index 1d6af29d98cfc..e375cb6cb12d6 100644
--- a/shell/app/node_main.cc
+++ b/shell/app/node_main.cc
@@ -83,6 +83,23 @@ void ExitIfContainsDisallowedFlags(const std::vector<std::string>& argv) {
   }
 }
 
+#if BUILDFLAG(IS_MAC)
+// A list of node envs that may be used to inject scripts.
+const char* kHijackableEnvs[] = {"NODE_OPTIONS", "NODE_REPL_EXTERNAL_MODULE"};
+
+// Return true if there is any env in kHijackableEnvs.
+bool UnsetHijackableEnvs(base::Environment* env) {
+  bool has = false;
+  for (const char* name : kHijackableEnvs) {
+    if (env->HasVar(name)) {
+      env->UnSetVar(name);
+      has = true;
+    }
+  }
+  return has;
+}
+#endif
+
 #if IS_MAS_BUILD()
 void SetCrashKeyStub(const std::string& key, const std::string& value) {}
 void ClearCrashKeyStub(const std::string& key) {}
@@ -124,8 +141,8 @@ int NodeMain(int argc, char* argv[]) {
     //                               NODE_OPTIONS: "--require 'bad.js'"}})
     // To prevent Electron apps from being used to work around macOS security
     // restrictions, when the parent process is not part of the app bundle, all
-    // environment variables starting with NODE_ will be removed.
-    if (util::UnsetAllNodeEnvs()) {
+    // environment variables that may be used to inject scripts are removed.
+    if (UnsetHijackableEnvs(os_env.get())) {
       LOG(ERROR) << "Node.js environment variables are disabled because this "
                     "process is invoked by other apps.";
     }
diff --git a/shell/common/node_util.h b/shell/common/node_util.h
index d4aa75a0db45b..7044abcac2db1 100644
--- a/shell/common/node_util.h
+++ b/shell/common/node_util.h
@@ -27,12 +27,6 @@ v8::MaybeLocal<v8::Value> CompileAndCall(
     std::vector<v8::Local<v8::String>>* parameters,
     std::vector<v8::Local<v8::Value>>* arguments);
 
-#if BUILDFLAG(IS_MAC)
-// Unset all environment variables that start with NODE_. Return false if there
-// is no node env at all.
-bool UnsetAllNodeEnvs();
-#endif
-
 }  // namespace electron::util
 
 #endif  // ELECTRON_SHELL_COMMON_NODE_UTIL_H_
diff --git a/shell/common/node_util_mac.mm b/shell/common/node_util_mac.mm
deleted file mode 100644
index 85c0725098673..0000000000000
--- a/shell/common/node_util_mac.mm
+++ /dev/null
@@ -1,23 +0,0 @@
-// Copyright (c) 2023 Microsoft, Inc.
-// Use of this source code is governed by the MIT license that can be
-// found in the LICENSE file.
-
-#include "shell/common/node_util.h"
-
-#include <Foundation/Foundation.h>
-
-namespace electron::util {
-
-bool UnsetAllNodeEnvs() {
-  bool has_unset = false;
-  for (NSString* env in NSProcessInfo.processInfo.environment) {
-    if (![env hasPrefix:@"NODE_"])
-      continue;
-    const char* name = [[env componentsSeparatedByString:@"="][0] UTF8String];
-    unsetenv(name);
-    has_unset = true;
-  }
-  return has_unset;
-}
-
-}  // namespace electron::util