fix: allow NODE_OPTIONS from none-sandboxed parent

shell/common/mac/codesign_util.cc

@@ -16,25 +16,40 @@
 
 namespace electron {
 
-std::optional<bool> IsUnsignedOrAdHocSigned(SecCodeRef code) {
+base::apple::ScopedCFTypeRef<CFDictionaryRef> GetSigningInfo(SecCodeRef code,
+                                                             OSStatus* status) {
   base::apple::ScopedCFTypeRef<SecStaticCodeRef> static_code;
-  OSStatus status = SecCodeCopyStaticCode(code, kSecCSDefaultFlags,
-                                          static_code.InitializeInto());
-  if (status == errSecCSUnsigned) {
-    return true;
+  *status = SecCodeCopyStaticCode(code, kSecCSDefaultFlags,
+                                  static_code.InitializeInto());
+  if (*status == errSecCSUnsigned) {
+    return base::apple::ScopedCFTypeRef<CFDictionaryRef>();
   }
-  if (status != errSecSuccess) {
-    OSSTATUS_LOG(ERROR, status) << "SecCodeCopyStaticCode";
-    return std::optional<bool>();
+  if (*status != errSecSuccess) {
+    OSSTATUS_LOG(ERROR, *status) << "SecCodeCopyStaticCode";
+    return base::apple::ScopedCFTypeRef<CFDictionaryRef>();
   }
   // Copy the signing info from the SecStaticCodeRef.
   base::apple::ScopedCFTypeRef<CFDictionaryRef> signing_info;
-  status =
+  *status =
       SecCodeCopySigningInformation(static_code.get(), kSecCSSigningInformation,
                                     signing_info.InitializeInto());
-  if (status != errSecSuccess) {
-    OSSTATUS_LOG(ERROR, status) << "SecCodeCopySigningInformation";
-    return std::optional<bool>();
+  if (*status != errSecSuccess) {
+    OSSTATUS_LOG(ERROR, *status) << "SecCodeCopySigningInformation";
+    return base::apple::ScopedCFTypeRef<CFDictionaryRef>();
+  }
+  return signing_info;
+}
+
+std::optional<bool> IsUnsignedOrAdHocSigned(SecCodeRef code) {
+  OSStatus status = errSecSuccess;
+  auto signing_info = GetSigningInfo(code, &status);
+  if (status == errSecCSUnsigned) {
+    // Code is unsigned.
+    return true;
+  }
+  if (!signing_info) {
+    // Error happened.
+    return std::nullopt;
   }
   // Look up the code signing flags. If the flags are absent treat this as
   // unsigned. This decision is consistent with the StaticCode source:
@@ -59,6 +74,30 @@ std::optional<bool> IsUnsignedOrAdHocSigned(SecCodeRef code) {
   return false;
 }
 
+std::optional<bool> IsSandboxed(SecCodeRef code) {
+  OSStatus status = errSecSuccess;
+  auto signing_info = GetSigningInfo(code, &status);
+  if (status == errSecCSUnsigned) {
+    // Code is unsigned.
+    return false;
+  }
+  if (!signing_info) {
+    // Error happened, treat as sandboxed to go the strict checking route.
+    return true;
+  }
+  CFDictionaryRef entitlements =
+      base::apple::GetValueFromDictionary<CFDictionaryRef>(
+          signing_info.get(), kSecCodeInfoEntitlementsDict);
+  if (!entitlements) {
+    // No entitlements means un-sandboxed.
+    return false;
+  }
+  // Check the sandbox entry.
+  CFBooleanRef app_sandbox = base::apple::GetValueFromDictionary<CFBooleanRef>(
+      entitlements, CFSTR("com.apple.security.app-sandbox"));
+  return app_sandbox && CFBooleanGetValue(app_sandbox);
+}
+
 bool ProcessSignatureIsSameWithCurrentApp(pid_t pid) {
   // Get and check the code signature of current app.
   base::apple::ScopedCFTypeRef<SecCodeRef> self_code;
@@ -93,6 +132,12 @@ bool ProcessSignatureIsSameWithCurrentApp(pid_t pid) {
     OSSTATUS_LOG(ERROR, status) << "SecCodeCopyGuestWithAttributes";
     return false;
   }
+  if (!IsSandboxed(process_code.get())) {
+    // Parent app is not sandboxed, skip the check since it already has full
+    // permissions. This usually happens when user launches the 'code' command
+    // of VS Code from Terminal.
+    return true;
+  }
   // Get the requirement of current app's code signature.
   base::apple::ScopedCFTypeRef<SecRequirementRef> self_requirement;
   status = SecCodeCopyDesignatedRequirement(self_code.get(), kSecCSDefaultFlags,

shell/common/mac/codesign_util.h

@@ -12,11 +12,11 @@ namespace electron {
 
 // Given a pid, return true if the process has the same code signature with
 // with current app.
-// This API returns true if current app is not signed or ad-hoc signed, because
-// checking code signature is meaningless in this case, and failing the
-// signature check would break some features with unsigned binary (for example,
-// process.send stops working in processes created by child_process.fork, due
-// to the NODE_CHANNEL_ID env getting removed).
+// This API returns true if current or parent app is not signed or ad-hoc
+// signed, because checking code signature is meaningless in this case, and
+// failing the signature check would break some features with unsigned binary
+// (for example, process.send stops working in processes created by
+// child_process.fork, due to the NODE_CHANNEL_ID env getting removed).
 bool ProcessSignatureIsSameWithCurrentApp(pid_t pid);
 
 }  // namespace electron

spec/node-spec.ts

@@ -667,6 +667,15 @@ describe('node feature', () => {
   });
 
   ifdescribe(shouldRunCodesignTests)('NODE_OPTIONS in signed app', function () {
+    // Find system node.
+    const nodePath = process.env.PATH?.split(path.delimiter).find(dir => fs.existsSync(path.join(dir, 'node')));
+    if (!nodePath) {
+      // The tests rely on existence of system node to run.
+      return;
+    }
+    // The tests assume the system node is signed.
+    childProcess.spawnSync('codesign', ['--verify', path.join(nodePath, 'node')]);
+
     let identity = '';
 
     beforeEach(function () {
@@ -694,16 +703,11 @@ describe('node feature', () => {
       });
     });
 
-    it('is disabled when invoked by alien binary in app bundle in ELECTRON_RUN_AS_NODE mode', async function () {
+    it('is disabled when invoked by alien binary in app bundle in ELECTRON_RUN_AS_NODE mode', async () => {
       await withTempDirectory(async (dir) => {
         const appPath = await copyApp(dir);
         await signApp(appPath, identity);
-        // Find system node and copy it to app bundle.
-        const nodePath = process.env.PATH?.split(path.delimiter).find(dir => fs.existsSync(path.join(dir, 'node')));
-        if (!nodePath) {
-          this.skip();
-          return;
-        }
+        // Copy system node to app bundle.
         const alienBinary = path.join(appPath, 'Contents/MacOS/node');
         await fs.copy(path.join(nodePath, 'node'), alienBinary);
         // Try to execute electron app from the alien node in app bundle.
@@ -724,6 +728,22 @@ describe('node feature', () => {
         expect(out).to.include('NODE_OPTIONS passed to child');
       });
     });
+
+    it('is respected when parent app is not signed', async () => {
+      await withTempDirectory(async (dir) => {
+        const appPath = await copyApp(dir);
+        await signApp(appPath, identity);
+        // The system node is expected to be signed, remove its signature.
+        const newNode = path.join(dir, 'node');
+        await fs.copy(path.join(nodePath, 'node'), newNode);
+        childProcess.spawnSync('codesign', ['--remove-signature', newNode]);
+        // Invoke with the unsigned node binary.
+        const { code, out } = await spawn(newNode, [script, path.join(appPath, 'Contents/MacOS/Electron')]);
+        expect(out).to.not.include(nodeOptionsWarning);
+        expect(out).to.include('NODE_OPTIONS passed to child');
+        expect(code).to.equal(1);
+      });
+    });
   });
 
   describe('Node.js cli flags', () => {