Browse Source

fix: disable `nodeIntegrationInWorker` for certain Worker types (#36009)

fix: disable nodeIntegrationInWorker for certain Worker types

Co-authored-by: Shelley Vohr <[email protected]>

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
trop[bot] 2 years ago
parent
commit
a86510b30c

+ 2 - 0
docs/tutorial/multithreading.md

@@ -20,6 +20,8 @@ const win = new BrowserWindow({
 The `nodeIntegrationInWorker` can be used independent of `nodeIntegration`, but
 `sandbox` must not be set to `true`.
 
+**Note:** This option is not available in [`SharedWorker`s](https://developer.mozilla.org/en-US/docs/Web/API/SharedWorker) or [`Service Worker`s](https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorker) owing to incompatibilities in sandboxing policies.
+
 ## Available APIs
 
 All built-in modules of Node.js are supported in Web Workers, and `asar`

+ 14 - 2
shell/renderer/electron_renderer_client.cc

@@ -21,6 +21,7 @@
 #include "third_party/blink/public/common/web_preferences/web_preferences.h"
 #include "third_party/blink/public/web/web_document.h"
 #include "third_party/blink/public/web/web_local_frame.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context.h"  // nogncheck
 
 namespace electron {
 
@@ -156,8 +157,15 @@ void ElectronRendererClient::WillReleaseScriptContext(
 
 void ElectronRendererClient::WorkerScriptReadyForEvaluationOnWorkerThread(
     v8::Local<v8::Context> context) {
-  // TODO(loc): Note that this will not be correct for in-process child windows
-  // with webPreferences that have a different value for nodeIntegrationInWorker
+  // We do not create a Node.js environment in service or shared workers
+  // owing to an inability to customize sandbox policies in these workers
+  // given that they're run out-of-process.
+  auto* ec = blink::ExecutionContext::From(context);
+  if (ec->IsServiceWorkerGlobalScope() || ec->IsSharedWorkerGlobalScope())
+    return;
+
+  // This won't be correct for in-process child windows with webPreferences
+  // that have a different value for nodeIntegrationInWorker
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kNodeIntegrationInWorker)) {
     WebWorkerObserver::GetCurrent()->WorkerScriptReadyForEvaluation(context);
@@ -166,6 +174,10 @@ void ElectronRendererClient::WorkerScriptReadyForEvaluationOnWorkerThread(
 
 void ElectronRendererClient::WillDestroyWorkerContextOnWorkerThread(
     v8::Local<v8::Context> context) {
+  auto* ec = blink::ExecutionContext::From(context);
+  if (ec->IsServiceWorkerGlobalScope() || ec->IsSharedWorkerGlobalScope())
+    return;
+
   // TODO(loc): Note that this will not be correct for in-process child windows
   // with webPreferences that have a different value for nodeIntegrationInWorker
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(

+ 29 - 20
spec-main/chromium-spec.ts

@@ -652,7 +652,7 @@ describe('chromium features', () => {
       w.loadFile(path.join(fixturesPath, 'pages', 'service-worker', 'custom-scheme-index.html'));
     });
 
-    it('should not crash when nodeIntegration is enabled', (done) => {
+    it('should not allow nodeIntegrationInWorker', async () => {
       const w = new BrowserWindow({
         show: false,
         webPreferences: {
@@ -663,21 +663,19 @@ describe('chromium features', () => {
         }
       });
 
-      w.webContents.on('ipc-message', (event, channel, message) => {
-        if (channel === 'reload') {
-          w.webContents.reload();
-        } else if (channel === 'error') {
-          done(`unexpected error : ${message}`);
-        } else if (channel === 'response') {
-          expect(message).to.equal('Hello from serviceWorker!');
-          session.fromPartition('sw-file-scheme-worker-spec').clearStorageData({
-            storages: ['serviceworkers']
-          }).then(() => done());
-        }
-      });
+      await w.loadURL(`file://${fixturesPath}/pages/service-worker/empty.html`);
 
-      w.webContents.on('crashed', () => done(new Error('WebContents crashed.')));
-      w.loadFile(path.join(fixturesPath, 'pages', 'service-worker', 'index.html'));
+      const data = await w.webContents.executeJavaScript(`
+        navigator.serviceWorker.register('worker-no-node.js', {
+          scope: './'
+        }).then(() => navigator.serviceWorker.ready)
+
+        new Promise((resolve) => {
+          navigator.serviceWorker.onmessage = event => resolve(event.data);
+        });
+      `);
+
+      expect(data).to.equal('undefined undefined undefined undefined');
     });
   });
 
@@ -789,11 +787,22 @@ describe('chromium features', () => {
         expect(data).to.equal('undefined undefined undefined undefined');
       });
 
-      it('has node integration with nodeIntegrationInWorker', async () => {
-        const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true, nodeIntegrationInWorker: true, contextIsolation: false } });
-        w.loadURL(`file://${fixturesPath}/pages/shared_worker.html`);
-        const [, data] = await emittedOnce(ipcMain, 'worker-result');
-        expect(data).to.equal('object function object function');
+      it('does not have node integration with nodeIntegrationInWorker', async () => {
+        const w = new BrowserWindow({
+          show: false,
+          webPreferences: {
+            nodeIntegration: true,
+            nodeIntegrationInWorker: true,
+            contextIsolation: false
+          }
+        });
+
+        await w.loadURL(`file://${fixturesPath}/pages/blank.html`);
+        const data = await w.webContents.executeJavaScript(`
+          const worker = new SharedWorker('../workers/shared_worker_node.js');
+          new Promise((resolve) => { worker.port.onmessage = e => resolve(e.data); })
+        `);
+        expect(data).to.equal('undefined undefined undefined undefined');
       });
     });
   });

+ 0 - 0
spec/fixtures/pages/service-worker/empty.html


+ 6 - 0
spec/fixtures/pages/service-worker/worker-no-node.js

@@ -0,0 +1,6 @@
+self.clients.matchAll({ includeUncontrolled: true }).then((clients) => {
+  if (!clients?.length) return;
+
+  const msg = [typeof process, typeof setImmediate, typeof global, typeof Buffer].join(' ');
+  clients[0].postMessage(msg);
+});

+ 0 - 12
spec/fixtures/pages/shared_worker.html

@@ -1,12 +0,0 @@
-<html>
-<body>
-<script type="text/javascript" charset="utf-8">
-  const {ipcRenderer} = require('electron')
-  // Pass a random parameter to create independent worker.
-  let worker = new SharedWorker(`../workers/shared_worker_node.js?a={Math.random()}`)
-  worker.port.onmessage = function (event) {
-    ipcRenderer.send('worker-result', event.data)
-  }
-</script>
-</body>
-</html>