fix: run subframe preload bundles in isolated context (#17165)

* fix: run subframe preload bundles in isolated context

* test subframe contextIsolation when disabled

atom/renderer/atom_render_frame_observer.cc

@@ -12,6 +12,8 @@
 #include "atom/common/heap_snapshot.h"
 #include "atom/common/native_mate_converters/value_converter.h"
 #include "atom/common/node_includes.h"
+#include "atom/common/options_switches.h"
+#include "base/command_line.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/trace_event/trace_event.h"
@@ -96,9 +98,18 @@ void AtomRenderFrameObserver::DidCreateScriptContext(
   if (ShouldNotifyClient(world_id))
     renderer_client_->DidCreateScriptContext(context, render_frame_);
 
-  if (renderer_client_->isolated_world() && IsMainWorld(world_id) &&
+  bool use_context_isolation = renderer_client_->isolated_world();
-      // Only the top window's main frame has isolated world.
+  bool is_main_world = IsMainWorld(world_id);
-      render_frame_->IsMainFrame() && !render_frame_->GetWebFrame()->Opener()) {
+  bool is_main_frame = render_frame_->IsMainFrame();
+  bool is_not_opened = !render_frame_->GetWebFrame()->Opener();
+  bool allow_node_in_sub_frames =
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kNodeIntegrationInSubFrames);
+  bool should_create_isolated_context =
+      use_context_isolation && is_main_world &&
+      (is_main_frame || allow_node_in_sub_frames) && is_not_opened;
+
+  if (should_create_isolated_context) {
     CreateIsolatedWorldContext();
     renderer_client_->SetupMainWorldOverrides(context, render_frame_);
   }
@@ -155,7 +166,11 @@ bool AtomRenderFrameObserver::IsIsolatedWorld(int world_id) {
 }
 
 bool AtomRenderFrameObserver::ShouldNotifyClient(int world_id) {
-  if (renderer_client_->isolated_world() && render_frame_->IsMainFrame())
+  bool allow_node_in_sub_frames =
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kNodeIntegrationInSubFrames);
+  if (renderer_client_->isolated_world() &&
+      (render_frame_->IsMainFrame() || allow_node_in_sub_frames))
     return IsIsolatedWorld(world_id);
   else
     return IsMainWorld(world_id);

spec/api-subframe-spec.js

@@ -8,8 +8,8 @@ const { closeWindow } = require('./window-helpers')
 const { BrowserWindow } = remote
 
 describe('renderer nodeIntegrationInSubFrames', () => {
-  const generateTests = (sandboxEnabled) => {
+  const generateTests = (description, webPreferences) => {
-    describe(`with sandbox ${sandboxEnabled ? 'enabled' : 'disabled'}`, () => {
+    describe(description, () => {
       let w
 
       beforeEach(async () => {
@@ -19,15 +19,17 @@ describe('renderer nodeIntegrationInSubFrames', () => {
           width: 400,
           height: 400,
           webPreferences: {
-            sandbox: sandboxEnabled,
             preload: path.resolve(__dirname, 'fixtures/sub-frames/preload.js'),
-            nodeIntegrationInSubFrames: true
+            nodeIntegrationInSubFrames: true,
+            ...webPreferences
           }
         })
       })
 
       afterEach(() => {
-        return closeWindow(w).then(() => { w = null })
+        return closeWindow(w).then(() => {
+          w = null
+        })
       })
 
       it('should load preload scripts in top level iframes', async () => {
@@ -80,9 +82,34 @@ describe('renderer nodeIntegrationInSubFrames', () => {
         const details = await pongPromise
         expect(details[1]).to.equal(event3[0].frameId)
       })
+
+      it('should not expose globals in main world', async () => {
+        const detailsPromise = emittedNTimes(remote.ipcMain, 'preload-ran', 2)
+        w.loadFile(path.resolve(__dirname, 'fixtures/sub-frames/frame-container.html'))
+        const details = await detailsPromise
+        const senders = details.map(event => event[0].sender)
+        await new Promise((resolve) => {
+          let resultCount = 0
+          senders.forEach(sender => {
+            sender.webContents.executeJavaScript('window.isolatedGlobal', result => {
+              if (webPreferences.contextIsolation) {
+                expect(result).to.be.null()
+              } else {
+                expect(result).to.equal(true)
+              }
+              resultCount++
+              if (resultCount === senders.length) {
+                resolve()
+              }
+            })
+          })
+        })
+      })
     })
   }
 
-  generateTests(false)
+  generateTests('without sandbox', {})
-  generateTests(true)
+  generateTests('with sandbox', { sandbox: true })
+  generateTests('with contextIsolation', { contextIsolation: true })
+  generateTests('with contextIsolation + sandbox', { contextIsolation: true, sandbox: true })
 })

spec/fixtures/sub-frames/preload.js

@@ -1,5 +1,7 @@
 const { ipcRenderer, webFrame } = require('electron')
 
+window.isolatedGlobal = true
+
 ipcRenderer.send('preload-ran', window.location.href, webFrame.routingId)
 
 ipcRenderer.on('preload-ping', () => {