From 54122862cf77bcb510a0dd1562073ec88fa70916 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Fri, 11 Feb 2022 10:32:09 -0800 Subject: [PATCH 01/14] feat: warn that preloads will be sandboxed by default in v20 --- lib/browser/api/web-contents.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/browser/api/web-contents.ts b/lib/browser/api/web-contents.ts index 5705cec695a07..5f028a80cebbe 100644 --- a/lib/browser/api/web-contents.ts +++ b/lib/browser/api/web-contents.ts @@ -1,4 +1,4 @@ -import { app, ipcMain, session, webFrameMain } from 'electron/main'; +import { app, ipcMain, session, webFrameMain, deprecate } from 'electron/main'; import type { BrowserWindowConstructorOptions, LoadURLOptions } from 'electron/main'; import * as url from 'url'; @@ -560,6 +560,10 @@ const loggingEnabled = () => { // Add JavaScript wrappers for WebContents class. WebContents.prototype._init = function () { + const prefs = this.getLastWebPreferences(); + if (!prefs.nodeIntegration && (prefs.preload != null || prefs.preloadURL != null) && prefs.sandbox == null) { + deprecate.log('The default sandbox option for windows without nodeIntegration is changing. Presently, by default, when a window has a preload script, it defaults to being unsandboxed. In Electron 20, this default will be changing, and all windows that have nodeIntegration: false (which is the default) will be sandboxed by default. If your preload script doesn\'t use Node, no action is needed. If your preload script does use Node, either refactor it to move Node usage to the main process, or specify sandbox: false in your WebPreferences.'); + } // Read off the ID at construction time, so that it's accessible even after // the underlying C++ WebContents is destroyed. const id = this.id; From 345a572af2c8f64c3253d595867695519e22d96f Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Fri, 11 Feb 2022 10:39:04 -0800 Subject: [PATCH 02/14] add a note to breaking changes --- docs/breaking-changes.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index e603146d0372e..6d561f87bd75e 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -12,6 +12,25 @@ This document uses the following convention to categorize breaking changes: * **Deprecated:** An API was marked as deprecated. The API will continue to function, but will emit a deprecation warning, and will be removed in a future release. * **Removed:** An API or feature was removed, and is no longer supported by Electron. +## Planned Breaking API Changes (20.0) + +### Default Changed: renderers without `nodeIntegration: true` are sandboxed by default + +Previously, renderers that specified a preload script defaulted to being +unsandboxed. This meant that by default, preload scripts had access to Node.js. +In Electron 20, this default has changed. Beginning in Electron 20, renderers +will be sandboxed by default, unless `nodeIntegration: true` or `sandbox: false` +is specified. + +If your preload scripts do not depend on Node, no action is needed. If your +preload scripts _do_ depend on Node, either refactor them to remove Node usage +from the renderer, or explicitly specify `sandbox: false` for the relevant +renderers. + +## Planned Breaking API Changes (19.0) + +*None (yet)* + ## Planned Breaking API Changes (18.0) ### Removed: `nativeWindowOpen` From 6899bc923c0041eb9641a8472873a3dbccfc0dfc Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Fri, 11 Feb 2022 10:42:38 -0800 Subject: [PATCH 03/14] feat: sandbox preloads by default --- lib/browser/api/web-contents.ts | 6 +----- shell/browser/web_contents_preferences.cc | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/browser/api/web-contents.ts b/lib/browser/api/web-contents.ts index 5f028a80cebbe..5705cec695a07 100644 --- a/lib/browser/api/web-contents.ts +++ b/lib/browser/api/web-contents.ts @@ -1,4 +1,4 @@ -import { app, ipcMain, session, webFrameMain, deprecate } from 'electron/main'; +import { app, ipcMain, session, webFrameMain } from 'electron/main'; import type { BrowserWindowConstructorOptions, LoadURLOptions } from 'electron/main'; import * as url from 'url'; @@ -560,10 +560,6 @@ const loggingEnabled = () => { // Add JavaScript wrappers for WebContents class. WebContents.prototype._init = function () { - const prefs = this.getLastWebPreferences(); - if (!prefs.nodeIntegration && (prefs.preload != null || prefs.preloadURL != null) && prefs.sandbox == null) { - deprecate.log('The default sandbox option for windows without nodeIntegration is changing. Presently, by default, when a window has a preload script, it defaults to being unsandboxed. In Electron 20, this default will be changing, and all windows that have nodeIntegration: false (which is the default) will be sandboxed by default. If your preload script doesn\'t use Node, no action is needed. If your preload script does use Node, either refactor it to move Node usage to the main process, or specify sandbox: false in your WebPreferences.'); - } // Read off the ID at construction time, so that it's accessible even after // the underlying C++ WebContents is destroyed. const id = this.id; diff --git a/shell/browser/web_contents_preferences.cc b/shell/browser/web_contents_preferences.cc index e54772a0bfce2..cadab65bce8a9 100644 --- a/shell/browser/web_contents_preferences.cc +++ b/shell/browser/web_contents_preferences.cc @@ -325,9 +325,7 @@ bool WebContentsPreferences::IsSandboxed() const { if (sandbox_) return *sandbox_; bool sandbox_disabled_by_default = - node_integration_ || node_integration_in_worker_ || preload_path_ || - !SessionPreferences::GetValidPreloads(web_contents_->GetBrowserContext()) - .empty(); + node_integration_ || node_integration_in_worker_; return !sandbox_disabled_by_default; } From cd3268ddec017f11a57470ddc981bb9e3468b18b Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Wed, 30 Mar 2022 13:31:12 -0700 Subject: [PATCH 04/14] nodeIntegrationInSubFrames ==> !sandbox --- shell/browser/web_contents_preferences.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shell/browser/web_contents_preferences.cc b/shell/browser/web_contents_preferences.cc index e99947dd4031a..020a7752a1a93 100644 --- a/shell/browser/web_contents_preferences.cc +++ b/shell/browser/web_contents_preferences.cc @@ -315,8 +315,9 @@ bool WebContentsPreferences::GetPreloadPath(base::FilePath* path) const { bool WebContentsPreferences::IsSandboxed() const { if (sandbox_) return *sandbox_; - bool sandbox_disabled_by_default = - node_integration_ || node_integration_in_worker_; + bool sandbox_disabled_by_default = node_integration_ || + node_integration_in_worker_ || + node_integration_in_sub_frames_; return !sandbox_disabled_by_default; } From 0a2d18e2e331cba1ed4a565f079955e39c1fce55 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Wed, 30 Mar 2022 14:43:45 -0700 Subject: [PATCH 05/14] explicitly turn off sandbox in libuv-hang test --- spec-main/fixtures/apps/libuv-hang/main.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec-main/fixtures/apps/libuv-hang/main.js b/spec-main/fixtures/apps/libuv-hang/main.js index 4ca4ef15d9024..3c9b9d0b6f26a 100644 --- a/spec-main/fixtures/apps/libuv-hang/main.js +++ b/spec-main/fixtures/apps/libuv-hang/main.js @@ -5,7 +5,8 @@ async function createWindow () { const mainWindow = new BrowserWindow({ show: false, webPreferences: { - preload: path.join(__dirname, 'preload.js') + preload: path.join(__dirname, 'preload.js'), + sandbox: false } }); From f1089a40aa6fdc3103c8fd796d0d1f86e5b631fe Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Wed, 30 Mar 2022 16:32:06 -0700 Subject: [PATCH 06/14] fix some webview specs --- spec/webview-spec.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/webview-spec.js b/spec/webview-spec.js index 0b649d1bd77ad..58666941040fc 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -243,6 +243,7 @@ describe(' tag', function () { it('preload script can require modules that still use "process" and "Buffer" when nodeintegration is off', async () => { const message = await startLoadingWebViewAndWaitForMessage(webview, { preload: `${fixtures}/module/preload-node-off-wrapper.js`, + nodeIntegration: true, src: `file://${fixtures}/api/blank.html` }); @@ -288,6 +289,7 @@ describe(' tag', function () { it('works without script tag in page', async () => { const message = await startLoadingWebViewAndWaitForMessage(webview, { preload: `${fixtures}/module/preload.js`, + nodeIntegration: true, src: `file://${fixtures}pages/base-page.html` }); @@ -303,6 +305,7 @@ describe(' tag', function () { it('resolves relative URLs', async () => { const message = await startLoadingWebViewAndWaitForMessage(webview, { preload: '../fixtures/module/preload.js', + nodeIntegration: true, src: `file://${fixtures}/pages/e.html` }); @@ -390,6 +393,7 @@ describe(' tag', function () { const message = await startLoadingWebViewAndWaitForMessage(webview, { disablewebsecurity: '', preload: `${fixtures}/module/preload.js`, + nodeIntegration: true, src: `file://${fixtures}/pages/e.html` }); From 36a5886476e4b2d2c80fe9021ab8976a6f763fed Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Wed, 8 Jun 2022 12:30:07 -0700 Subject: [PATCH 07/14] don't disable sandbox for node_integration_in_sub_frames --- shell/browser/web_contents_preferences.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shell/browser/web_contents_preferences.cc b/shell/browser/web_contents_preferences.cc index 020a7752a1a93..154471ef6ebae 100644 --- a/shell/browser/web_contents_preferences.cc +++ b/shell/browser/web_contents_preferences.cc @@ -316,8 +316,7 @@ bool WebContentsPreferences::IsSandboxed() const { if (sandbox_) return *sandbox_; bool sandbox_disabled_by_default = node_integration_ || - node_integration_in_worker_ || - node_integration_in_sub_frames_; + node_integration_in_worker_; return !sandbox_disabled_by_default; } From c1cbac18379061d88ba6093dfdaebaf312b0f79e Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Wed, 8 Jun 2022 13:10:15 -0700 Subject: [PATCH 08/14] lint --- shell/browser/web_contents_preferences.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/browser/web_contents_preferences.cc b/shell/browser/web_contents_preferences.cc index 154471ef6ebae..e99947dd4031a 100644 --- a/shell/browser/web_contents_preferences.cc +++ b/shell/browser/web_contents_preferences.cc @@ -315,8 +315,8 @@ bool WebContentsPreferences::GetPreloadPath(base::FilePath* path) const { bool WebContentsPreferences::IsSandboxed() const { if (sandbox_) return *sandbox_; - bool sandbox_disabled_by_default = node_integration_ || - node_integration_in_worker_; + bool sandbox_disabled_by_default = + node_integration_ || node_integration_in_worker_; return !sandbox_disabled_by_default; } From 77cf96a57adf4434f91ae64157001df338a8f5a4 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Mon, 13 Jun 2022 13:48:58 -0700 Subject: [PATCH 09/14] fix: make preload calculation identical between sandbox & non-sandboxed --- lib/renderer/init.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/renderer/init.ts b/lib/renderer/init.ts index 9807348568976..3b7fa74039b67 100644 --- a/lib/renderer/init.ts +++ b/lib/renderer/init.ts @@ -2,6 +2,7 @@ import * as path from 'path'; import { IPC_MESSAGES } from '@electron/internal/common/ipc-messages'; import type * as ipcRendererInternalModule from '@electron/internal/renderer/ipc-renderer-internal'; +import type * as ipcRendererUtilsModule from '@electron/internal/renderer/ipc-renderer-internal-utils'; const Module = require('module'); @@ -38,6 +39,7 @@ require('../common/reset-search-paths'); require('@electron/internal/common/init'); const { ipcRendererInternal } = require('@electron/internal/renderer/ipc-renderer-internal') as typeof ipcRendererInternalModule; +const ipcRendererUtils = require('@electron/internal/renderer/ipc-renderer-internal-utils') as typeof ipcRendererUtilsModule; process.getProcessMemoryInfo = () => { return ipcRendererInternal.invoke(IPC_MESSAGES.BROWSER_GET_PROCESS_MEMORY_INFO); @@ -48,15 +50,8 @@ const { hasSwitch, getSwitchValue } = process._linkedBinding('electron_common_co const { mainFrame } = process._linkedBinding('electron_renderer_web_frame'); const nodeIntegration = mainFrame.getWebPreference('nodeIntegration'); -const preloadScript = mainFrame.getWebPreference('preload'); -const preloadScripts = mainFrame.getWebPreference('preloadScripts'); const appPath = hasSwitch('app-path') ? getSwitchValue('app-path') : null; -// The webContents preload script is loaded after the session preload scripts. -if (preloadScript) { - preloadScripts.push(preloadScript); -} - // Common renderer initialization require('@electron/internal/renderer/common-init'); @@ -127,8 +122,9 @@ if (nodeIntegration) { } } +const { preloadScripts } = ipcRendererUtils.invokeSync(IPC_MESSAGES.BROWSER_SANDBOX_LOAD); // Load the preload scripts. -for (const preloadScript of preloadScripts) { +for (const { preloadPath: preloadScript } of preloadScripts) { try { Module._load(preloadScript); } catch (error) { From f617ad045b09e1c8360b43282e4235ebdbcb7db7 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Mon, 13 Jun 2022 14:02:55 -0700 Subject: [PATCH 10/14] fix test --- spec-main/api-browser-window-spec.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index e68f47434582b..3edd3becbe933 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -3263,7 +3263,15 @@ describe('BrowserWindow module', () => { }); w.webContents.setWindowOpenHandler(() => ({ action: 'allow', - overrideBrowserWindowOptions: { show: false, webPreferences: { contextIsolation: false, webviewTag: true, nodeIntegrationInSubFrames: true } } + overrideBrowserWindowOptions: { + show: false, + webPreferences: { + contextIsolation: false, + webviewTag: true, + nodeIntegrationInSubFrames: true, + preload + } + } })); const webviewLoaded = emittedOnce(ipcMain, 'webview-loaded'); From 78c8c303bce138322a3ee1e85d873c6ea201e09e Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Tue, 14 Jun 2022 13:06:17 -0700 Subject: [PATCH 11/14] split IPCs --- lib/browser/rpc-server.ts | 4 ++++ lib/renderer/init.ts | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/browser/rpc-server.ts b/lib/browser/rpc-server.ts index d8a80802060b7..44f8b2563a923 100644 --- a/lib/browser/rpc-server.ts +++ b/lib/browser/rpc-server.ts @@ -68,6 +68,10 @@ ipcMainUtils.handleSync(IPC_MESSAGES.BROWSER_SANDBOX_LOAD, async function (event }; }); +ipcMainUtils.handleSync(IPC_MESSAGES.BROWSER_NONSANDBOX_LOAD, async function (event) { + return { preloadPaths: event.sender._getPreloadPaths() }; +}); + ipcMainInternal.on(IPC_MESSAGES.BROWSER_PRELOAD_ERROR, function (event, preloadPath: string, error: Error) { event.sender.emit('preload-error', event, preloadPath, error); }); diff --git a/lib/renderer/init.ts b/lib/renderer/init.ts index 3b7fa74039b67..7047c4ffea927 100644 --- a/lib/renderer/init.ts +++ b/lib/renderer/init.ts @@ -122,9 +122,9 @@ if (nodeIntegration) { } } -const { preloadScripts } = ipcRendererUtils.invokeSync(IPC_MESSAGES.BROWSER_SANDBOX_LOAD); +const { preloadPaths } = ipcRendererUtils.invokeSync(IPC_MESSAGES.BROWSER_NONSANDBOX_LOAD); // Load the preload scripts. -for (const { preloadPath: preloadScript } of preloadScripts) { +for (const preloadScript of preloadPaths) { try { Module._load(preloadScript); } catch (error) { From e2f39bbe592801c60f8cde56507285f4d379228d Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Tue, 14 Jun 2022 16:34:54 -0700 Subject: [PATCH 12/14] nodeIntegration: true --> sandbox: false --- spec/webview-spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/webview-spec.js b/spec/webview-spec.js index 4c37dae16b302..710ea358911d9 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -243,7 +243,7 @@ describe(' tag', function () { it('preload script can require modules that still use "process" and "Buffer" when nodeintegration is off', async () => { const message = await startLoadingWebViewAndWaitForMessage(webview, { preload: `${fixtures}/module/preload-node-off-wrapper.js`, - nodeIntegration: true, + sandbox: false, src: `file://${fixtures}/api/blank.html` }); @@ -289,7 +289,7 @@ describe(' tag', function () { it('works without script tag in page', async () => { const message = await startLoadingWebViewAndWaitForMessage(webview, { preload: `${fixtures}/module/preload.js`, - nodeIntegration: true, + sandbox: false, src: `file://${fixtures}pages/base-page.html` }); @@ -305,7 +305,7 @@ describe(' tag', function () { it('resolves relative URLs', async () => { const message = await startLoadingWebViewAndWaitForMessage(webview, { preload: '../fixtures/module/preload.js', - nodeIntegration: true, + sandbox: false, src: `file://${fixtures}/pages/e.html` }); @@ -393,7 +393,7 @@ describe(' tag', function () { const message = await startLoadingWebViewAndWaitForMessage(webview, { disablewebsecurity: '', preload: `${fixtures}/module/preload.js`, - nodeIntegration: true, + sandbox: false, src: `file://${fixtures}/pages/e.html` }); From 87cb9aa125c75e64042fe3bff709a0a7ce80ceb8 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Tue, 14 Jun 2022 16:36:29 -0700 Subject: [PATCH 13/14] fix ipc constant --- lib/common/ipc-messages.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/common/ipc-messages.ts b/lib/common/ipc-messages.ts index b920c79c48c59..c6a881c3c0a23 100644 --- a/lib/common/ipc-messages.ts +++ b/lib/common/ipc-messages.ts @@ -3,6 +3,7 @@ export const enum IPC_MESSAGES { BROWSER_GET_LAST_WEB_PREFERENCES = 'BROWSER_GET_LAST_WEB_PREFERENCES', BROWSER_PRELOAD_ERROR = 'BROWSER_PRELOAD_ERROR', BROWSER_SANDBOX_LOAD = 'BROWSER_SANDBOX_LOAD', + BROWSER_NONSANDBOX_LOAD = 'BROWSER_NONSANDBOX_LOAD', BROWSER_WINDOW_CLOSE = 'BROWSER_WINDOW_CLOSE', BROWSER_GET_PROCESS_MEMORY_INFO = 'BROWSER_GET_PROCESS_MEMORY_INFO', From 9f75a067643d9711eaa19b9705178cfc5bbd0074 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Thu, 16 Jun 2022 16:16:21 -0700 Subject: [PATCH 14/14] use webpreferences to set sandbox=no --- spec/webview-spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/webview-spec.js b/spec/webview-spec.js index 157445d2d8416..b3d0c7ec4f249 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -243,7 +243,7 @@ describe(' tag', function () { it('preload script can require modules that still use "process" and "Buffer" when nodeintegration is off', async () => { const message = await startLoadingWebViewAndWaitForMessage(webview, { preload: `${fixtures}/module/preload-node-off-wrapper.js`, - sandbox: false, + webpreferences: 'sandbox=no', src: `file://${fixtures}/api/blank.html` }); @@ -289,7 +289,7 @@ describe(' tag', function () { it('works without script tag in page', async () => { const message = await startLoadingWebViewAndWaitForMessage(webview, { preload: `${fixtures}/module/preload.js`, - sandbox: false, + webpreferences: 'sandbox=no', src: `file://${fixtures}pages/base-page.html` }); @@ -305,7 +305,7 @@ describe(' tag', function () { it('resolves relative URLs', async () => { const message = await startLoadingWebViewAndWaitForMessage(webview, { preload: '../fixtures/module/preload.js', - sandbox: false, + webpreferences: 'sandbox=no', src: `file://${fixtures}/pages/e.html` }); @@ -393,7 +393,7 @@ describe(' tag', function () { const message = await startLoadingWebViewAndWaitForMessage(webview, { disablewebsecurity: '', preload: `${fixtures}/module/preload.js`, - sandbox: false, + webpreferences: 'sandbox=no', src: `file://${fixtures}/pages/e.html` });