feat: support chrome extensions in sandboxed renderer (#16218)

* Add content script injector to sandboxed renderer

* Fix 'getRenderProcessPreferences' binding to the wrong object

* Pass getRenderProcessPreferences to content-scripts-injector

* Emit document-start and document-end  events in sandboxed renderer

* Use GetContext from RendererClientBase

* Prevent script context crash caused by lazily initialization

* Remove frame filtering logic for onExit callback

Since we're keeping track of which frames we've injected the bundle into, this logic is redundant.

* Add initial content script tests

* Add contextIsolation variants to content script tests

* Add set include

* Fix already loaded extension error

* Add tests for content scripts 'run_at' options

* Catch script injection eval error when CSP forbids it

This can occur in a rendered sandbox when a CSP is enabled. We'll need to switch to using isolated worlds to fix this.

* Fix content script tests not properly cleaning up extensions

* Fix lint and type errors

atom/renderer/atom_sandboxed_renderer_client.cc

@@ -176,6 +176,38 @@ void AtomSandboxedRendererClient::RenderViewCreated(
   RendererClientBase::RenderViewCreated(render_view);
 }
 
+void AtomSandboxedRendererClient::RunScriptsAtDocumentStart(
+    content::RenderFrame* render_frame) {
+  if (injected_frames_.find(render_frame) == injected_frames_.end())
+    return;
+
+  auto* isolate = blink::MainThreadIsolate();
+  v8::HandleScope handle_scope(isolate);
+
+  v8::Local<v8::Context> context =
+      GetContext(render_frame->GetWebFrame(), isolate);
+  v8::Context::Scope context_scope(context);
+
+  InvokeIpcCallback(context, "onDocumentStart",
+                    std::vector<v8::Local<v8::Value>>());
+}
+
+void AtomSandboxedRendererClient::RunScriptsAtDocumentEnd(
+    content::RenderFrame* render_frame) {
+  if (injected_frames_.find(render_frame) == injected_frames_.end())
+    return;
+
+  auto* isolate = blink::MainThreadIsolate();
+  v8::HandleScope handle_scope(isolate);
+
+  v8::Local<v8::Context> context =
+      GetContext(render_frame->GetWebFrame(), isolate);
+  v8::Context::Scope context_scope(context);
+
+  InvokeIpcCallback(context, "onDocumentEnd",
+                    std::vector<v8::Local<v8::Value>>());
+}
+
 void AtomSandboxedRendererClient::DidCreateScriptContext(
     v8::Handle<v8::Context> context,
     content::RenderFrame* render_frame) {
@@ -195,6 +227,8 @@ void AtomSandboxedRendererClient::DidCreateScriptContext(
   if (!should_load_preload)
     return;
 
+  injected_frames_.insert(render_frame);
+
   // Wrap the bundle into a function that receives the binding object as
   // argument.
   auto* isolate = context->GetIsolate();
@@ -239,12 +273,9 @@ void AtomSandboxedRendererClient::SetupMainWorldOverrides(
 void AtomSandboxedRendererClient::WillReleaseScriptContext(
     v8::Handle<v8::Context> context,
     content::RenderFrame* render_frame) {
-  // Only allow preload for the main frame
+  if (injected_frames_.find(render_frame) == injected_frames_.end())
-  // Or for sub frames when explicitly enabled
-  if (!render_frame->IsMainFrame() &&
-      !base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kNodeIntegrationInSubFrames))
     return;
+  injected_frames_.erase(render_frame);
 
   auto* isolate = context->GetIsolate();
   v8::HandleScope handle_scope(isolate);

atom/renderer/atom_sandboxed_renderer_client.h

@@ -5,6 +5,7 @@
 #define ATOM_RENDERER_ATOM_SANDBOXED_RENDERER_CLIENT_H_
 
 #include <memory>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -34,10 +35,17 @@ class AtomSandboxedRendererClient : public RendererClientBase {
   // content::ContentRendererClient:
   void RenderFrameCreated(content::RenderFrame*) override;
   void RenderViewCreated(content::RenderView*) override;
+  void RunScriptsAtDocumentStart(content::RenderFrame* render_frame) override;
+  void RunScriptsAtDocumentEnd(content::RenderFrame* render_frame) override;
 
  private:
   std::unique_ptr<base::ProcessMetrics> metrics_;
 
+  // Getting main script context from web frame would lazily initializes
+  // its script context. Doing so in a web page without scripts would trigger
+  // assertion, so we have to keep a book of injected web frames.
+  std::set<content::RenderFrame*> injected_frames_;
+
   DISALLOW_COPY_AND_ASSIGN(AtomSandboxedRendererClient);
 };
 

lib/renderer/content-scripts-injector.ts

@@ -15,12 +15,18 @@ const runContentScript = function (this: any, extensionId: string, url: string,
   const context: { chrome?: any } = {}
   require('@electron/internal/renderer/chrome-api').injectTo(extensionId, false, context)
   const wrapper = `((chrome) => {\n  ${code}\n  })`
-  const compiledWrapper = runInThisContext(wrapper, {
+  try {
-    filename: url,
+    const compiledWrapper = runInThisContext(wrapper, {
-    lineOffset: 1,
+      filename: url,
-    displayErrors: true
+      lineOffset: 1,
-  })
+      displayErrors: true
-  return compiledWrapper.call(this, context.chrome)
+    })
+    return compiledWrapper.call(this, context.chrome)
+  } catch (error) {
+    // TODO(samuelmaddock): Run scripts in isolated world, see chromium script_injection.cc
+    console.error(`Error running content script JavaScript for '${extensionId}'`)
+    console.error(error)
+  }
 }
 
 const runAllContentScript = function (scripts: Array<Electron.InjectionBase>, extensionId: string) {
@@ -39,13 +45,19 @@ const runStylesheet = function (this: any, url: string, code: string) {
     document.addEventListener('DOMContentLoaded', init);
   })`
 
-  const compiledWrapper = runInThisContext(wrapper, {
+  try {
-    filename: url,
+    const compiledWrapper = runInThisContext(wrapper, {
-    lineOffset: 1,
+      filename: url,
-    displayErrors: true
+      lineOffset: 1,
-  })
+      displayErrors: true
+    })
 
-  return compiledWrapper.call(this, code)
+    return compiledWrapper.call(this, code)
+  } catch (error) {
+    // TODO(samuelmaddock): Insert stylesheet directly into document, see chromium script_injection.cc
+    console.error(`Error inserting content script stylesheet ${url}`)
+    console.error(error)
+  }
 }
 
 const runAllStylesheet = function (css: Array<Electron.InjectionBase>) {
@@ -84,7 +96,7 @@ const injectContentScript = function (extensionId: string, script: Electron.Cont
 
 // Handle the request of chrome.tabs.executeJavaScript.
 ipcRendererInternal.on('CHROME_TABS_EXECUTESCRIPT', function (
-  event,
+  event: Electron.Event,
   senderWebContentsId: number,
   requestId: number,
   extensionId: string,
@@ -95,13 +107,15 @@ ipcRendererInternal.on('CHROME_TABS_EXECUTESCRIPT', function (
   ipcRendererInternal.sendToAll(senderWebContentsId, `CHROME_TABS_EXECUTESCRIPT_RESULT_${requestId}`, result)
 })
 
-// Read the renderer process preferences.
+module.exports = (getRenderProcessPreferences: typeof process.getRenderProcessPreferences) => {
-const preferences = process.getRenderProcessPreferences()
+  // Read the renderer process preferences.
-if (preferences) {
+  const preferences = getRenderProcessPreferences()
-  for (const pref of preferences) {
+  if (preferences) {
-    if (pref.contentScripts) {
+    for (const pref of preferences) {
-      for (const script of pref.contentScripts) {
+      if (pref.contentScripts) {
-        injectContentScript(pref.extensionId, script)
+        for (const script of pref.contentScripts) {
+          injectContentScript(pref.extensionId, script)
+        }
       }
     }
   }

lib/renderer/init.ts

@@ -86,7 +86,7 @@ switch (window.location.protocol) {
 
     // Inject content scripts.
     if (process.isMainFrame) {
-      require('@electron/internal/renderer/content-scripts-injector')
+      require('@electron/internal/renderer/content-scripts-injector')(process.getRenderProcessPreferences)
     }
   }
 }

lib/sandboxed_renderer/init.js

@@ -61,6 +61,14 @@ ipcNative.onExit = function () {
   process.emit('exit')
 }
 
+ipcNative.onDocumentStart = function () {
+  process.emit('document-start')
+}
+
+ipcNative.onDocumentEnd = function () {
+  process.emit('document-end')
+}
+
 const { webFrameInit } = require('@electron/internal/renderer/web-frame-init')
 webFrameInit()
 
@@ -110,6 +118,13 @@ switch (window.location.protocol) {
     require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
     break
   }
+  case 'chrome': {
+    break
+  }
+  default: {
+    // Inject content scripts.
+    require('@electron/internal/renderer/content-scripts-injector')(binding.getRenderProcessPreferences)
+  }
 }
 
 const guestInstanceId = binding.guestInstanceId && parseInt(binding.guestInstanceId)

spec/content-script-spec.js

@@ -0,0 +1,76 @@
+const { expect } = require('chai')
+const { remote } = require('electron')
+const path = require('path')
+
+const { closeWindow } = require('./window-helpers')
+
+const { BrowserWindow } = remote
+
+describe('chrome content scripts', () => {
+  const generateTests = (sandboxEnabled, contextIsolationEnabled) => {
+    describe(`with sandbox ${sandboxEnabled ? 'enabled' : 'disabled'} and context isolation ${contextIsolationEnabled ? 'enabled' : 'disabled'}`, () => {
+      let w
+
+      beforeEach(async () => {
+        await closeWindow(w)
+        w = new BrowserWindow({
+          show: false,
+          width: 400,
+          height: 400,
+          webPreferences: {
+            contextIsolation: contextIsolationEnabled,
+            sandbox: sandboxEnabled
+          }
+        })
+      })
+
+      afterEach(() => {
+        Object.keys(BrowserWindow.getExtensions()).map(extName => {
+          BrowserWindow.removeExtension(extName)
+        })
+        return closeWindow(w).then(() => { w = null })
+      })
+
+      const addExtension = (name) => {
+        const extensionPath = path.join(__dirname, 'fixtures', 'extensions', name)
+        BrowserWindow.addExtension(extensionPath)
+      }
+
+      it('should run content script at document_start', (done) => {
+        addExtension('content-script-document-start')
+        w.webContents.once('dom-ready', () => {
+          w.webContents.executeJavaScript('document.documentElement.style.backgroundColor', (result) => {
+            expect(result).to.equal('red')
+            done()
+          })
+        })
+        w.loadURL('about:blank')
+      })
+
+      it('should run content script at document_idle', (done) => {
+        addExtension('content-script-document-idle')
+        w.loadURL('about:blank')
+        w.webContents.executeJavaScript('document.body.style.backgroundColor', (result) => {
+          expect(result).to.equal('red')
+          done()
+        })
+      })
+
+      it('should run content script at document_end', (done) => {
+        addExtension('content-script-document-end')
+        w.webContents.once('did-finish-load', () => {
+          w.webContents.executeJavaScript('document.documentElement.style.backgroundColor', (result) => {
+            expect(result).to.equal('red')
+            done()
+          })
+        })
+        w.loadURL('about:blank')
+      })
+    })
+  }
+
+  generateTests(false, false)
+  generateTests(false, true)
+  generateTests(true, false)
+  generateTests(true, true)
+})

spec/fixtures/extensions/content-script-document-end/end.js

@@ -0,0 +1 @@
+document.documentElement.style.backgroundColor = 'red'

spec/fixtures/extensions/content-script-document-end/manifest.json

@@ -0,0 +1,14 @@
+{
+    "name": "document-end",
+    "version": "1.0",
+    "description": "",
+    "content_scripts": [
+      {
+        "matches": ["<all_urls>"],
+        "js": ["end.js"],
+        "run_at": "document_end"
+      }
+    ],
+    "manifest_version": 2
+  }
+  

spec/fixtures/extensions/content-script-document-idle/idle.js

@@ -0,0 +1 @@
+document.body.style.backgroundColor = 'red'

spec/fixtures/extensions/content-script-document-idle/manifest.json

@@ -0,0 +1,14 @@
+{
+    "name": "document-idle",
+    "version": "1.0",
+    "description": "",
+    "content_scripts": [
+      {
+        "matches": ["<all_urls>"],
+        "js": ["idle.js"],
+        "run_at": "document_idle"
+      }
+    ],
+    "manifest_version": 2
+  }
+  

spec/fixtures/extensions/content-script-document-start/manifest.json

@@ -0,0 +1,14 @@
+{
+    "name": "document-start",
+    "version": "1.0",
+    "description": "",
+    "content_scripts": [
+      {
+        "matches": ["<all_urls>"],
+        "js": ["start.js"],
+        "run_at": "document_start"
+      }
+    ],
+    "manifest_version": 2
+  }
+  

spec/fixtures/extensions/content-script-document-start/start.js

@@ -0,0 +1 @@
+document.documentElement.style.backgroundColor = 'red'