fix: <webview> not working with contextIsolation + sandbox

electron · Jan 20, 2019 · 67044a4 · 67044a4
1 parent 4038053
commit 67044a4
Show file tree

Hide file tree

Showing 7 changed files with 108 additions and 49 deletions.
diff --git a/atom/renderer/atom_sandboxed_renderer_client.cc b/atom/renderer/atom_sandboxed_renderer_client.cc
@@ -202,6 +202,30 @@ void AtomSandboxedRendererClient::DidCreateScriptContext(
       &preload_bundle_params, &preload_bundle_args, nullptr);
 }
 
+void AtomSandboxedRendererClient::SetupMainWorldOverrides(
+    v8::Handle<v8::Context> context,
+    content::RenderFrame* render_frame) {
+  // Setup window overrides in the main world context
+  // Wrap the bundle into a function that receives the isolatedWorld as
+  // an argument.
+  auto* isolate = context->GetIsolate();
+
+  mate::Dictionary process = mate::Dictionary::CreateEmpty(isolate);
+  process.SetMethod("binding", GetBinding);
+
+  std::vector<v8::Local<v8::String>> isolated_bundle_params = {
+      node::FIXED_ONE_BYTE_STRING(isolate, "nodeProcess"),
+      node::FIXED_ONE_BYTE_STRING(isolate, "isolatedWorld")};
+
+  std::vector<v8::Local<v8::Value>> isolated_bundle_args = {
+      process.GetHandle(),
+      GetContext(render_frame->GetWebFrame(), isolate)->Global()};
+
+  node::per_process::native_module_loader.CompileAndCall(
+      context, "electron/js2c/isolated_bundle", &isolated_bundle_params,
+      &isolated_bundle_args, nullptr);
+}
+
 void AtomSandboxedRendererClient::WillReleaseScriptContext(
     v8::Handle<v8::Context> context,
     content::RenderFrame* render_frame) {

diff --git a/atom/renderer/atom_sandboxed_renderer_client.h b/atom/renderer/atom_sandboxed_renderer_client.h
@@ -29,7 +29,7 @@ class AtomSandboxedRendererClient : public RendererClientBase {
   void WillReleaseScriptContext(v8::Handle<v8::Context> context,
                                 content::RenderFrame* render_frame) override;
   void SetupMainWorldOverrides(v8::Handle<v8::Context> context,
-                               content::RenderFrame* render_frame) override {}
+                               content::RenderFrame* render_frame) override;
   // content::ContentRendererClient:
   void RenderFrameCreated(content::RenderFrame*) override;
   void RenderViewCreated(content::RenderView*) override;

diff --git a/lib/isolated_renderer/init.js b/lib/isolated_renderer/init.js
@@ -3,15 +3,21 @@
 /* global nodeProcess, isolatedWorld */
 
 // Note: Don't use "process", as it will be replaced by browserify's one.
-const v8Util = nodeProcess.atomBinding('v8_util')
+process.atomBinding = require('@electron/internal/common/atom-binding-setup')(nodeProcess.binding, 'renderer')
 
-const isolatedWorldArgs = v8Util.getHiddenValue(isolatedWorld, 'isolated-world-args')
-const { webViewImpl, ipcRenderer, guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen } = isolatedWorldArgs
+const v8Util = process.atomBinding('v8_util')
+
+const webViewImpl = v8Util.getHiddenValue(isolatedWorld, 'web-view-impl')
 
 if (webViewImpl) {
   // Must setup the WebView element in main world.
   const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
   setupWebView(v8Util, webViewImpl)
 }
 
-require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
+const isolatedWorldArgs = v8Util.getHiddenValue(isolatedWorld, 'isolated-world-args')
+
+if (isolatedWorldArgs) {
+  const { ipcRenderer, guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen } = isolatedWorldArgs
+  require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
+}
diff --git a/lib/renderer/init.js b/lib/renderer/init.js
@@ -58,33 +58,29 @@ if (preloadScript) {
   preloadScripts.push(preloadScript)
 }
 
-if (window.location.protocol === 'chrome-devtools:') {
-  // Override some inspector APIs.
-  require('@electron/internal/renderer/inspector')
-} else if (window.location.protocol === 'chrome-extension:') {
-  // Add implementations of chrome API.
-  require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
-} else if (window.location.protocol === 'chrome:') {
-  // Disable node integration for chrome UI scheme.
-} else {
-  // Override default web functions.
-  require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
-
-  // Inject content scripts.
-  require('@electron/internal/renderer/content-scripts-injector')
+switch (window.location.protocol) {
+  case 'chrome-devtools:': {
+    // Override some inspector APIs.
+    require('@electron/internal/renderer/inspector')
+    break
+  }
+  case 'chrome-extension:': {
+    // Inject the chrome.* APIs that chrome extensions require
+    require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
+    break
+  }
+  default: {
+    // Override default web functions.
+    require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
 
-  // Load webview tag implementation.
-  if (webviewTag && guestInstanceId == null) {
-    const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl')
-    if (contextIsolation) {
-      Object.assign(isolatedWorldArgs, { webViewImpl })
-    } else {
-      const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
-      setupWebView(v8Util, webViewImpl)
-    }
+    // Inject content scripts.
+    require('@electron/internal/renderer/content-scripts-injector')
   }
 }
 
+// Load webview tag implementation.
+require('@electron/internal/renderer/web-view/web-view-init')(contextIsolation, webviewTag, guestInstanceId)
+
 // Pass the arguments to isolatedWorld.
 if (contextIsolation) {
   v8Util.setHiddenValue(global, 'isolated-world-args', isolatedWorldArgs)
@@ -163,9 +159,3 @@ for (const preloadScript of preloadScripts) {
 
 // Warn about security issues
 require('@electron/internal/renderer/security-warnings')(nodeIntegration)
-
-if (guestInstanceId) {
-  // Report focus/blur events of webview to browser.
-  const { handleFocusBlur } = require('@electron/internal/renderer/web-view/web-view-init')
-  handleFocusBlur(guestInstanceId)
-}
diff --git a/lib/renderer/web-view/web-view-init.js b/lib/renderer/web-view/web-view-init.js
@@ -1,8 +1,9 @@
 'use strict'
 
-exports.handleFocusBlur = function (guestInstanceId) {
-  const ipcRenderer = require('@electron/internal/renderer/ipc-renderer-internal')
+const ipcRenderer = require('@electron/internal/renderer/ipc-renderer-internal')
+const v8Util = process.atomBinding('v8_util')
 
+function handleFocusBlur (guestInstanceId) {
   // Note that while Chromium content APIs have observer for focus/blur, they
   // unfortunately do not work for webview.
 
@@ -14,3 +15,21 @@ exports.handleFocusBlur = function (guestInstanceId) {
     ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', false, guestInstanceId)
   })
 }
+
+module.exports = function (contextIsolation, webviewTag, guestInstanceId) {
+  // Load webview tag implementation.
+  if (webviewTag && guestInstanceId == null) {
+    const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl')
+    if (contextIsolation) {
+      v8Util.setHiddenValue(window, 'web-view-impl', webViewImpl)
+    } else {
+      const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
+      setupWebView(v8Util, webViewImpl)
+    }
+  }
+
+  if (guestInstanceId) {
+    // Report focus/blur events of webview to browser.
+    handleFocusBlur(guestInstanceId)
+  }
+}
diff --git a/lib/sandboxed_renderer/init.js b/lib/sandboxed_renderer/init.js
@@ -105,8 +105,12 @@ function preloadRequire (module) {
   throw new Error('module not found')
 }
 
+// Process command line arguments.
 const { hasSwitch } = process.atomBinding('command_line')
 
+const isBackgroundPage = hasSwitch('background-page')
+const contextIsolation = hasSwitch('context-isolation')
+
 switch (window.location.protocol) {
   case 'chrome-devtools:': {
     // Override some inspector APIs.
@@ -115,20 +119,15 @@ switch (window.location.protocol) {
   }
   case 'chrome-extension:': {
     // Inject the chrome.* APIs that chrome extensions require
-    const isBackgroundPage = hasSwitch('background-page')
     require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
     break
   }
 }
 
 const guestInstanceId = binding.guestInstanceId && parseInt(binding.guestInstanceId)
 
-// Don't allow recursive `<webview>`.
-if (!guestInstanceId && isWebViewTagEnabled) {
-  const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl')
-  const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
-  setupWebView(v8Util, webViewImpl)
-}
+// Load webview tag implementation.
+require('@electron/internal/renderer/web-view/web-view-init')(contextIsolation, isWebViewTagEnabled, guestInstanceId)
 
 const errorUtils = require('@electron/internal/common/error-utils')
 
@@ -178,9 +177,3 @@ try {
 
 // Warn about security issues
 require('@electron/internal/renderer/security-warnings')()
-
-if (guestInstanceId) {
-  // Report focus/blur events of webview to browser.
-  const { handleFocusBlur } = require('@electron/internal/renderer/web-view/web-view-init')
-  handleFocusBlur(guestInstanceId)
-}
diff --git a/spec/webview-spec.js b/spec/webview-spec.js
@@ -76,6 +76,19 @@ describe('<webview> tag', function () {
     await emittedOnce(ipcMain, 'pong')
   })
 
+  it('works with sandbox', async () => {
+    const w = await openTheWindow({
+      show: false,
+      webPreferences: {
+        webviewTag: true,
+        nodeIntegration: true,
+        sandbox: true
+      }
+    })
+    w.loadFile(path.join(fixtures, 'pages', 'webview-isolated.html'))
+    await emittedOnce(ipcMain, 'pong')
+  })
+
   it('works with contextIsolation', async () => {
     const w = await openTheWindow({
       show: false,
@@ -89,6 +102,20 @@ describe('<webview> tag', function () {
     await emittedOnce(ipcMain, 'pong')
   })
 
+  it('works with contextIsolation + sandbox', async () => {
+    const w = await openTheWindow({
+      show: false,
+      webPreferences: {
+        webviewTag: true,
+        nodeIntegration: true,
+        contextIsolation: true,
+        sandbox: true
+      }
+    })
+    w.loadFile(path.join(fixtures, 'pages', 'webview-isolated.html'))
+    await emittedOnce(ipcMain, 'pong')
+  })
+
   it('is disabled by default', async () => {
     const w = await openTheWindow({
       show: false,