Browse Source

fix: NODE_OPTIONS parsing for child processes on macOS (#46243)

trop[bot] 3 weeks ago
parent
commit
7ee88bbdcb

+ 18 - 0
shell/browser/api/electron_api_app.cc

@@ -90,7 +90,10 @@
 
 #if BUILDFLAG(IS_MAC)
 #include <CoreFoundation/CoreFoundation.h>
+#include "base/no_destructor.h"
+#include "content/browser/mac_helpers.h"
 #include "shell/browser/ui/cocoa/electron_bundle_mover.h"
+#include "shell/common/process_util.h"
 #endif
 
 #if BUILDFLAG(IS_LINUX)
@@ -901,6 +904,21 @@ bool App::IsPackaged() {
 
 #if BUILDFLAG(IS_WIN)
   return base_name != FILE_PATH_LITERAL("electron.exe");
+#elif BUILDFLAG(IS_MAC)
+  static const base::NoDestructor<std::string> default_helper(
+      "electron helper" +
+      base::ToLowerASCII(content::kMacHelperSuffix_default));
+  static const base::NoDestructor<std::string> renderer_helper(
+      "electron helper" +
+      base::ToLowerASCII(content::kMacHelperSuffix_renderer));
+  static const base::NoDestructor<std::string> plugin_helper(
+      "electron helper" + base::ToLowerASCII(content::kMacHelperSuffix_plugin));
+  if (IsRendererProcess()) {
+    return base_name != *renderer_helper;
+  } else if (IsUtilityProcess()) {
+    return base_name != *default_helper && base_name != *plugin_helper;
+  }
+  return base_name != FILE_PATH_LITERAL("electron");
 #else
   return base_name != FILE_PATH_LITERAL("electron");
 #endif

+ 1 - 0
spec/fixtures/apps/node-options-utility-process/fail.js

@@ -0,0 +1 @@
+process.exit(1);

+ 15 - 0
spec/fixtures/apps/node-options-utility-process/main.js

@@ -0,0 +1,15 @@
+const { app, utilityProcess } = require('electron');
+
+const path = require('node:path');
+
+app.whenReady().then(() => {
+  const child = utilityProcess.fork(path.join(__dirname, 'noop.js'), [], {
+    stdio: 'inherit',
+    env: {
+      ...process.env,
+      NODE_OPTIONS: `--require ${path.join(__dirname, 'fail.js')}`
+    }
+  });
+
+  child.once('exit', (code) => app.exit(code));
+});

+ 1 - 0
spec/fixtures/apps/node-options-utility-process/noop.js

@@ -0,0 +1 @@
+process.exit(0);

+ 4 - 0
spec/fixtures/apps/node-options-utility-process/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "electron-test-node-options-utility-process",
+  "main": "main.js"
+}

+ 23 - 0
spec/node-spec.ts

@@ -697,6 +697,29 @@ describe('node feature', () => {
       expect(code).to.equal(1);
     });
 
+    it('does allow --require in utility process of non-packaged apps', async () => {
+      const appPath = path.join(fixtures, 'apps', 'node-options-utility-process');
+      // App should exit with code 1.
+      const child = childProcess.spawn(process.execPath, [appPath]);
+      const [code] = await once(child, 'exit');
+      expect(code).to.equal(1);
+    });
+
+    // TODO(deepak1556): will be enabled in follow-up PR
+    // https://github.com/electron/electron/pull/46210
+    it.skip('does not allow --require in utility process of packaged apps', async () => {
+      const appPath = path.join(fixtures, 'apps', 'node-options-utility-process');
+      // App should exit with code 1.
+      const child = childProcess.spawn(process.execPath, [appPath], {
+        env: {
+          ...process.env,
+          ELECTRON_FORCE_IS_PACKAGED: 'true'
+        }
+      });
+      const [code] = await once(child, 'exit');
+      expect(code).to.equal(0);
+    });
+
     it('does not allow --require in packaged apps', async () => {
       const appPath = path.join(fixtures, 'module', 'noop.js');
       const env = {