From b5239754bafa9a3eb20cd1003ff11f33f7a1d9b5 Mon Sep 17 00:00:00 2001 From: Hari Juturu Date: Wed, 31 May 2017 23:01:14 -0700 Subject: [PATCH 01/23] Enable sandbox on webview --- atom/app/atom_main_delegate.cc | 3 --- atom/browser/web_contents_preferences.cc | 14 +++++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/atom/app/atom_main_delegate.cc b/atom/app/atom_main_delegate.cc index 1490b890134..7d29034be87 100644 --- a/atom/app/atom_main_delegate.cc +++ b/atom/app/atom_main_delegate.cc @@ -130,9 +130,6 @@ void AtomMainDelegate::PreSandboxStartup() { // Disable setuid sandbox since it is not longer required on linux(namespace // sandbox is available on most distros). command_line->AppendSwitch(::switches::kDisableSetuidSandbox); - } else { - // Disable renderer sandbox for most of node's functions. - command_line->AppendSwitch(::switches::kNoSandbox); } // Allow file:// URIs to read other file:// URIs by default. diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 6c706199fb9..ba3be9592d7 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -108,11 +108,15 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( command_line->AppendSwitchASCII(switches::kWebviewTag, webview_tag ? "true" : "false"); - // If the `sandbox` option was passed to the BrowserWindow's webPreferences, - // pass `--enable-sandbox` to the renderer so it won't have any node.js - // integration. - if (IsSandboxed(web_contents)) - command_line->AppendSwitch(switches::kEnableSandbox); + if (IsSandboxed(web_contents)) { + // pass `--enable-sandbox` to the renderer so it won't have any node.js + // integration. + command_line->AppendSwitch(switches::kEnableSandbox); + } + else { + // Disable renderer sandbox for most of node's functions. + command_line->AppendSwitch(::switches::kNoSandbox); + } if (web_preferences.GetBoolean("nativeWindowOpen", &b) && b) command_line->AppendSwitch(switches::kNativeWindowOpen); From 3ba0e288f72bfe429ec6d6e96d1ba725812b5ed9 Mon Sep 17 00:00:00 2001 From: Hari Juturu Date: Wed, 31 May 2017 23:20:13 -0700 Subject: [PATCH 02/23] fix lint error --- atom/browser/web_contents_preferences.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index ba3be9592d7..26ff3d70338 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -111,12 +111,12 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( if (IsSandboxed(web_contents)) { // pass `--enable-sandbox` to the renderer so it won't have any node.js // integration. - command_line->AppendSwitch(switches::kEnableSandbox); - } - else { + command_line->AppendSwitch(switches::kEnableSandbox); + } else { // Disable renderer sandbox for most of node's functions. command_line->AppendSwitch(::switches::kNoSandbox); } + if (web_preferences.GetBoolean("nativeWindowOpen", &b) && b) command_line->AppendSwitch(switches::kNativeWindowOpen); From 07f550a7488924464a4d5c1fa74acf55c8622bcb Mon Sep 17 00:00:00 2001 From: Hari Juturu Date: Thu, 8 Jun 2017 23:29:35 -0700 Subject: [PATCH 03/23] enable-mixed-sandbox option --- atom/app/atom_main_delegate.cc | 15 ++++++++++----- atom/browser/web_contents_preferences.cc | 11 ++++------- atom/common/options_switches.cc | 3 +++ atom/common/options_switches.h | 1 + 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/atom/app/atom_main_delegate.cc b/atom/app/atom_main_delegate.cc index 7d29034be87..9516a20bfe4 100644 --- a/atom/app/atom_main_delegate.cc +++ b/atom/app/atom_main_delegate.cc @@ -125,11 +125,16 @@ void AtomMainDelegate::PreSandboxStartup() { // Only append arguments for browser process. if (!IsBrowserProcess(command_line)) return; - - if (command_line->HasSwitch(switches::kEnableSandbox)) { - // Disable setuid sandbox since it is not longer required on linux(namespace - // sandbox is available on most distros). - command_line->AppendSwitch(::switches::kDisableSetuidSandbox); + + if (!command_line->HasSwitch(switches::kEnableMixedSandbox)) { + if (command_line->HasSwitch(switches::kEnableSandbox)) { + // Disable setuid sandbox since it is not longer required on linux(namespace + // sandbox is available on most distros). + command_line->AppendSwitch(::switches::kDisableSetuidSandbox); + } else { + // Disable renderer sandbox for most of node's functions. + command_line->AppendSwitch(::switches::kNoSandbox); + } } // Allow file:// URIs to read other file:// URIs by default. diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 26ff3d70338..b651ee85147 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -108,14 +108,11 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( command_line->AppendSwitchASCII(switches::kWebviewTag, webview_tag ? "true" : "false"); - if (IsSandboxed(web_contents)) { - // pass `--enable-sandbox` to the renderer so it won't have any node.js - // integration. + // If the `sandbox` option was passed to the BrowserWindow's webPreferences, + // pass `--enable-sandbox` to the renderer so it won't have any node.js + // integration. + if (IsSandboxed(web_contents)) command_line->AppendSwitch(switches::kEnableSandbox); - } else { - // Disable renderer sandbox for most of node's functions. - command_line->AppendSwitch(::switches::kNoSandbox); - } if (web_preferences.GetBoolean("nativeWindowOpen", &b) && b) command_line->AppendSwitch(switches::kNativeWindowOpen); diff --git a/atom/common/options_switches.cc b/atom/common/options_switches.cc index ce63fc716a3..f63c1d95d0b 100644 --- a/atom/common/options_switches.cc +++ b/atom/common/options_switches.cc @@ -138,6 +138,9 @@ namespace switches { // Enable chromium sandbox. const char kEnableSandbox[] = "enable-sandbox"; +// Enable sandbox in only remote content windows. +const char kEnableMixedSandbox[] = "enable-mixed-sandbox"; + // Enable plugins. const char kEnablePlugins[] = "enable-plugins"; diff --git a/atom/common/options_switches.h b/atom/common/options_switches.h index 6fda408ee5c..171583ec2a8 100644 --- a/atom/common/options_switches.h +++ b/atom/common/options_switches.h @@ -74,6 +74,7 @@ extern const char kWebviewTag[]; namespace switches { extern const char kEnableSandbox[]; +extern const char kEnableMixedSandbox[]; extern const char kEnablePlugins[]; extern const char kPpapiFlashPath[]; extern const char kPpapiFlashVersion[]; From cbbd4a4a3b7b4dfd02dfb729cd18c4f106d6b730 Mon Sep 17 00:00:00 2001 From: Hari Juturu Date: Thu, 8 Jun 2017 23:31:58 -0700 Subject: [PATCH 04/23] Indentation --- atom/browser/web_contents_preferences.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index b651ee85147..6c706199fb9 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -111,9 +111,8 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( // If the `sandbox` option was passed to the BrowserWindow's webPreferences, // pass `--enable-sandbox` to the renderer so it won't have any node.js // integration. - if (IsSandboxed(web_contents)) + if (IsSandboxed(web_contents)) command_line->AppendSwitch(switches::kEnableSandbox); - if (web_preferences.GetBoolean("nativeWindowOpen", &b) && b) command_line->AppendSwitch(switches::kNativeWindowOpen); From 7bdd259d2e731abdc60d6987ea2734e78fac0459 Mon Sep 17 00:00:00 2001 From: Hari Juturu Date: Thu, 8 Jun 2017 23:39:53 -0700 Subject: [PATCH 05/23] Lint error --- atom/app/atom_main_delegate.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/atom/app/atom_main_delegate.cc b/atom/app/atom_main_delegate.cc index 9516a20bfe4..58762eddd71 100644 --- a/atom/app/atom_main_delegate.cc +++ b/atom/app/atom_main_delegate.cc @@ -125,11 +125,11 @@ void AtomMainDelegate::PreSandboxStartup() { // Only append arguments for browser process. if (!IsBrowserProcess(command_line)) return; - + if (!command_line->HasSwitch(switches::kEnableMixedSandbox)) { if (command_line->HasSwitch(switches::kEnableSandbox)) { - // Disable setuid sandbox since it is not longer required on linux(namespace - // sandbox is available on most distros). + // Disable setuid sandbox since it is not longer required on + // linux(namespace sandbox is available on most distros). command_line->AppendSwitch(::switches::kDisableSetuidSandbox); } else { // Disable renderer sandbox for most of node's functions. From 3ee55d9cd35584cd174533c7d51fdf8d9a1f8e1c Mon Sep 17 00:00:00 2001 From: Hari Juturu Date: Thu, 8 Jun 2017 23:46:37 -0700 Subject: [PATCH 06/23] lint error --- atom/app/atom_main_delegate.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/app/atom_main_delegate.cc b/atom/app/atom_main_delegate.cc index 58762eddd71..0507d5dafa5 100644 --- a/atom/app/atom_main_delegate.cc +++ b/atom/app/atom_main_delegate.cc @@ -128,7 +128,7 @@ void AtomMainDelegate::PreSandboxStartup() { if (!command_line->HasSwitch(switches::kEnableMixedSandbox)) { if (command_line->HasSwitch(switches::kEnableSandbox)) { - // Disable setuid sandbox since it is not longer required on + // Disable setuid sandbox since it is not longer required on // linux(namespace sandbox is available on most distros). command_line->AppendSwitch(::switches::kDisableSetuidSandbox); } else { From 421bf71b98ebdcc87acc85321435f8de1b8e306e Mon Sep 17 00:00:00 2001 From: Hari Juturu Date: Tue, 13 Jun 2017 18:45:24 -0700 Subject: [PATCH 07/23] Adding no-sandbox to non sandbox webcontents --- atom/browser/web_contents_preferences.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 6c706199fb9..e602ddcf88d 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -111,8 +111,11 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( // If the `sandbox` option was passed to the BrowserWindow's webPreferences, // pass `--enable-sandbox` to the renderer so it won't have any node.js // integration. - if (IsSandboxed(web_contents)) + if (IsSandboxed(web_contents)) { command_line->AppendSwitch(switches::kEnableSandbox); + } else if (!command_line->HasSwitch(switches::kEnableSandbox)) { + command_line->AppendSwitch(::switches::kNoSandbox); + } if (web_preferences.GetBoolean("nativeWindowOpen", &b) && b) command_line->AppendSwitch(switches::kNativeWindowOpen); From 0b7e7458c94db2f0d7a4a8a35e61db2d8e990d6a Mon Sep 17 00:00:00 2001 From: Hari Krishna Reddy Juturu Date: Fri, 16 Jun 2017 15:34:11 -0700 Subject: [PATCH 08/23] WIP: Adding UT --- spec/api-browser-window-spec.js | 52 +++++++++++++++++++ .../electron-app-mixed-sandbox-preload.js | 3 ++ spec/fixtures/api/mixed-sandbox-app/main.js | 52 +++++++++++++++++++ .../api/mixed-sandbox-app/package.json | 5 ++ 4 files changed, 112 insertions(+) create mode 100644 spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js create mode 100644 spec/fixtures/api/mixed-sandbox-app/main.js create mode 100644 spec/fixtures/api/mixed-sandbox-app/package.json diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 5c084cb178f..322f25ca32e 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1,11 +1,13 @@ 'use strict' const assert = require('assert') +const ChildProcess = require('child_process') const fs = require('fs') const path = require('path') const os = require('os') const qs = require('querystring') const http = require('http') +const net = require('net') const {closeWindow} = require('./window-helpers') const {ipcRenderer, remote, screen} = require('electron') @@ -1259,6 +1261,56 @@ describe('BrowserWindow module', function () { }) }) + describe('mixed sandbox option', function () { + // TOOD (juturu): This test needs to be fixed + let sandboxServer = null + const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-app-mixed-sandbox' : '/tmp/electron-app-mixed-sandbox' + + beforeEach(function (done) { + fs.unlink(socketPath, () => { + sandboxServer = net.createServer() + sandboxServer.listen(socketPath) + done() + }) + }) + + afterEach(function (done) { + sandboxServer.close(() => { + if (process.platform === 'win32') { + done() + } else { + fs.unlink(socketPath, () => { + done() + }) + } + }) + }) + + it('enable-mixed-sandbox', (done) => { + this.timeout(120000) + + let state = 'none' + sandboxServer.once('error', (error) => { + done(error) + }) + sandboxServer.on('connection', (client) => { + client.once('data', function (data) { + console.log('jhreddy -' + data) + if (String(data) === 'false' && state === 'none') { + state = 'first-launch' + } else if (String(data) === 'true' && state === 'first-launch') { + done() + } else { + done(`Unexpected state: ${state}`) + } + }) + }) + + const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app') + ChildProcess.spawn(remote.process.execPath, [appPath]) + }) + }) + describe('nativeWindowOpen option', () => { beforeEach(() => { w.destroy() diff --git a/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js b/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js new file mode 100644 index 00000000000..5a781c9b70e --- /dev/null +++ b/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js @@ -0,0 +1,3 @@ +process.once('loaded', function () { + window.argv = process.argv +}) \ No newline at end of file diff --git a/spec/fixtures/api/mixed-sandbox-app/main.js b/spec/fixtures/api/mixed-sandbox-app/main.js new file mode 100644 index 00000000000..179c0813f32 --- /dev/null +++ b/spec/fixtures/api/mixed-sandbox-app/main.js @@ -0,0 +1,52 @@ +const { app, BrowserWindow, dialog } = require('electron') +const net = require('net') +const path = require('path') + +const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-app-mixed-sandbox' : '/tmp/electron-app-mixed-sandbox' + +process.on('uncaughtException', () => { + app.exit(1) +}) + +app.once('ready', () => { + + let lastArg = process.argv[process.argv.length - 1] + const client = net.connect(socketPath) + client.once('connect', () => { + if (lastArg === '--enable-mixed-sandbox') { + dialog.showErrorBox('connected', app.getAppPath()) + let window = new BrowserWindow({ + show: true, + webPreferences: { + preload: path.join(app.getAppPath(), 'electron-app-mixed-sandbox-preload.js'), + sandbox: false + } + }) + + window.loadURL('data:,window') + let argv = 'test' + window.webContents.once('did-finish-load', () => { + dialog.showErrorBox('finished-load', 'finished') + window.webContents.executeJavaScript('window.argv', false, (result) => { + dialog.showErrorBox('hello', result) + client.end(String(result)) + }) + }) + // window.webContents.openDevTools() + // window.webContents.executeJavaScript('window.argv', true).then((result) => { + // dialog.showErrorBox('hello', result) + // client.end(String(argv)) + // }) + + } else { + client.end(String(false)) + } + }) + client.once('end', () => { + app.exit(0) + }) + + if (lastArg !== '--enable-mixed-sandbox') { + app.relaunch({ args: process.argv.slice(1).concat('--enable-mixed-sandbox') }) + } +}) diff --git a/spec/fixtures/api/mixed-sandbox-app/package.json b/spec/fixtures/api/mixed-sandbox-app/package.json new file mode 100644 index 00000000000..c5b70811e60 --- /dev/null +++ b/spec/fixtures/api/mixed-sandbox-app/package.json @@ -0,0 +1,5 @@ +{ + "name": "electron-app-mixed-sandbox", + "main": "main.js" +} + From e546820a4a0a1a83ae7c8ff44b7666cb2537fe26 Mon Sep 17 00:00:00 2001 From: Hari Krishna Reddy Juturu Date: Mon, 19 Jun 2017 07:46:14 -0700 Subject: [PATCH 09/23] Completing UTs --- spec/api-browser-window-spec.js | 3 +-- .../electron-app-mixed-sandbox-preload.js | 11 ++++++--- spec/fixtures/api/mixed-sandbox-app/main.js | 24 +++++-------------- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 322f25ca32e..92736315050 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1295,13 +1295,12 @@ describe('BrowserWindow module', function () { }) sandboxServer.on('connection', (client) => { client.once('data', function (data) { - console.log('jhreddy -' + data) if (String(data) === 'false' && state === 'none') { state = 'first-launch' } else if (String(data) === 'true' && state === 'first-launch') { done() } else { - done(`Unexpected state: ${state}`) + done(`Unexpected state: ${data}`) } }) }) diff --git a/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js b/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js index 5a781c9b70e..aee6c9f24bf 100644 --- a/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js +++ b/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js @@ -1,3 +1,8 @@ -process.once('loaded', function () { - window.argv = process.argv -}) \ No newline at end of file +const { ipcRenderer } = require('electron') +if (process) { + process.once('loaded', function () { + ipcRenderer.send('processArgs', process.argv) + }) +} else { + ipcRenderer.send('processArgs', 'unable to get process args') +} \ No newline at end of file diff --git a/spec/fixtures/api/mixed-sandbox-app/main.js b/spec/fixtures/api/mixed-sandbox-app/main.js index 179c0813f32..4caeafb9a77 100644 --- a/spec/fixtures/api/mixed-sandbox-app/main.js +++ b/spec/fixtures/api/mixed-sandbox-app/main.js @@ -1,10 +1,10 @@ -const { app, BrowserWindow, dialog } = require('electron') +const { app, BrowserWindow, ipcMain } = require('electron') const net = require('net') const path = require('path') const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-app-mixed-sandbox' : '/tmp/electron-app-mixed-sandbox' -process.on('uncaughtException', () => { +process.on('uncaughtException', (error) => { app.exit(1) }) @@ -14,9 +14,11 @@ app.once('ready', () => { const client = net.connect(socketPath) client.once('connect', () => { if (lastArg === '--enable-mixed-sandbox') { - dialog.showErrorBox('connected', app.getAppPath()) + ipcMain.on('processArgs', (event, args) => { + client.end(String(args.indexOf('--no-sandbox') >= 0)) + }) let window = new BrowserWindow({ - show: true, + show: false, webPreferences: { preload: path.join(app.getAppPath(), 'electron-app-mixed-sandbox-preload.js'), sandbox: false @@ -24,20 +26,6 @@ app.once('ready', () => { }) window.loadURL('data:,window') - let argv = 'test' - window.webContents.once('did-finish-load', () => { - dialog.showErrorBox('finished-load', 'finished') - window.webContents.executeJavaScript('window.argv', false, (result) => { - dialog.showErrorBox('hello', result) - client.end(String(result)) - }) - }) - // window.webContents.openDevTools() - // window.webContents.executeJavaScript('window.argv', true).then((result) => { - // dialog.showErrorBox('hello', result) - // client.end(String(argv)) - // }) - } else { client.end(String(false)) } From 902b34ba394852981fdd009d7c3f3c15b9de248d Mon Sep 17 00:00:00 2001 From: Hari Krishna Reddy Juturu Date: Mon, 19 Jun 2017 08:03:02 -0700 Subject: [PATCH 10/23] :art: --- .../electron-app-mixed-sandbox-preload.js | 10 +++++----- spec/fixtures/api/mixed-sandbox-app/main.js | 11 +++++------ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js b/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js index aee6c9f24bf..3efe83276cb 100644 --- a/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js +++ b/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js @@ -1,8 +1,8 @@ const { ipcRenderer } = require('electron') if (process) { - process.once('loaded', function () { - ipcRenderer.send('processArgs', process.argv) - }) + process.once('loaded', function () { + ipcRenderer.send('processArgs', process.argv) + }) } else { - ipcRenderer.send('processArgs', 'unable to get process args') -} \ No newline at end of file + ipcRenderer.send('processArgs', 'unable to get process args') +} diff --git a/spec/fixtures/api/mixed-sandbox-app/main.js b/spec/fixtures/api/mixed-sandbox-app/main.js index 4caeafb9a77..fe8c2e45f3e 100644 --- a/spec/fixtures/api/mixed-sandbox-app/main.js +++ b/spec/fixtures/api/mixed-sandbox-app/main.js @@ -4,12 +4,11 @@ const path = require('path') const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-app-mixed-sandbox' : '/tmp/electron-app-mixed-sandbox' -process.on('uncaughtException', (error) => { +process.on('uncaughtException', () => { app.exit(1) }) app.once('ready', () => { - let lastArg = process.argv[process.argv.length - 1] const client = net.connect(socketPath) client.once('connect', () => { @@ -20,14 +19,14 @@ app.once('ready', () => { let window = new BrowserWindow({ show: false, webPreferences: { - preload: path.join(app.getAppPath(), 'electron-app-mixed-sandbox-preload.js'), - sandbox: false + preload: path.join(app.getAppPath(), 'electron-app-mixed-sandbox-preload.js'), + sandbox: false } }) - + window.loadURL('data:,window') } else { - client.end(String(false)) + client.end(String(false)) } }) client.once('end', () => { From cf392e402cc156e3bf27f7a2b460adc9b40a33e0 Mon Sep 17 00:00:00 2001 From: Hari Krishna Reddy Juturu Date: Mon, 19 Jun 2017 11:21:31 -0700 Subject: [PATCH 11/23] Adding docs --- docs/api/sandbox-option.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/api/sandbox-option.md b/docs/api/sandbox-option.md index 9598e47257d..dba63c85e89 100644 --- a/docs/api/sandbox-option.md +++ b/docs/api/sandbox-option.md @@ -60,6 +60,8 @@ It is important to note that this option alone won't enable the OS-enforced sand `--enable-sandbox` command-line argument must be passed to electron, which will force `sandbox: true` for all `BrowserWindow` instances. +To enable OS-enforced sandbox on `BrowserWindow` or `webview` process with `sandbox:true` without causing +entire app to be in sandbox, `--enable-mixed-sandbox` command-line argument must be passed to electron. ```js let win From 14178d982610bcd7dcad65f0547a119c43af25c2 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 26 Jun 2017 13:52:19 -0700 Subject: [PATCH 12/23] Expose argv to preload process object --- atom/renderer/atom_sandboxed_renderer_client.cc | 5 +++++ lib/sandboxed_renderer/init.js | 1 + spec/api-browser-window-spec.js | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/atom/renderer/atom_sandboxed_renderer_client.cc b/atom/renderer/atom_sandboxed_renderer_client.cc index 2768f072cd6..aa4780fa15b 100644 --- a/atom/renderer/atom_sandboxed_renderer_client.cc +++ b/atom/renderer/atom_sandboxed_renderer_client.cc @@ -80,6 +80,10 @@ v8::Local GetBinding(v8::Isolate* isolate, v8::Local key, return exports; } +base::CommandLine::StringVector GetArgv() { + return base::CommandLine::ForCurrentProcess()->argv(); +} + void InitializeBindings(v8::Local binding, v8::Local context) { auto isolate = context->GetIsolate(); @@ -87,6 +91,7 @@ void InitializeBindings(v8::Local binding, b.SetMethod("get", GetBinding); b.SetMethod("crash", AtomBindings::Crash); b.SetMethod("hang", AtomBindings::Hang); + b.SetMethod("getArgv", GetArgv); b.SetMethod("getProcessMemoryInfo", &AtomBindings::GetProcessMemoryInfo); b.SetMethod("getSystemMemoryInfo", &AtomBindings::GetSystemMemoryInfo); } diff --git a/lib/sandboxed_renderer/init.js b/lib/sandboxed_renderer/init.js index 1aec0bc0bfd..119391e7946 100644 --- a/lib/sandboxed_renderer/init.js +++ b/lib/sandboxed_renderer/init.js @@ -41,6 +41,7 @@ preloadProcess.crash = () => binding.crash() preloadProcess.hang = () => binding.hang() preloadProcess.getProcessMemoryInfo = () => binding.getProcessMemoryInfo() preloadProcess.getSystemMemoryInfo = () => binding.getSystemMemoryInfo() +preloadProcess.argv = binding.getArgv() process.platform = preloadProcess.platform = electron.remote.process.platform process.execPath = preloadProcess.execPath = electron.remote.process.execPath process.on('exit', () => preloadProcess.emit('exit')) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 92736315050..ab40331225a 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1286,7 +1286,7 @@ describe('BrowserWindow module', function () { }) }) - it('enable-mixed-sandbox', (done) => { + it('adds --enable-sandbox to render processes created with sandbox: true', (done) => { this.timeout(120000) let state = 'none' From 0eaddd1565b8d184f7acc562e45f7faa22b0725c Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 26 Jun 2017 14:12:03 -0700 Subject: [PATCH 13/23] Verify --enable-sandbox/--no-sandbox command line --- spec/api-browser-window-spec.js | 55 +++++------------ .../electron-app-mixed-sandbox-preload.js | 9 +-- spec/fixtures/api/mixed-sandbox-app/main.js | 61 +++++++++++-------- 3 files changed, 51 insertions(+), 74 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index ab40331225a..c726a1b8b0e 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1262,51 +1262,28 @@ describe('BrowserWindow module', function () { }) describe('mixed sandbox option', function () { - // TOOD (juturu): This test needs to be fixed - let sandboxServer = null - const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-app-mixed-sandbox' : '/tmp/electron-app-mixed-sandbox' + this.timeout(120000) - beforeEach(function (done) { - fs.unlink(socketPath, () => { - sandboxServer = net.createServer() - sandboxServer.listen(socketPath) - done() - }) - }) + let appProcess - afterEach(function (done) { - sandboxServer.close(() => { - if (process.platform === 'win32') { - done() - } else { - fs.unlink(socketPath, () => { - done() - }) - } - }) + afterEach(function () { + if (appProcess != null) { + appProcess.kill() + } }) it('adds --enable-sandbox to render processes created with sandbox: true', (done) => { - this.timeout(120000) - - let state = 'none' - sandboxServer.once('error', (error) => { - done(error) - }) - sandboxServer.on('connection', (client) => { - client.once('data', function (data) { - if (String(data) === 'false' && state === 'none') { - state = 'first-launch' - } else if (String(data) === 'true' && state === 'first-launch') { - done() - } else { - done(`Unexpected state: ${data}`) - } - }) - }) - const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app') - ChildProcess.spawn(remote.process.execPath, [appPath]) + ©appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--enable-mixed-sandbox'], {stdio: ['ignore', 'ipc', 'ignore']}) + appProcess.once('message', (argv) => { + assert.equal(argv.sandbox.includes('--enable-sandbox'), true) + assert.equal(argv.sandbox.includes('--no-sandbox'), false) + + assert.equal(argv.noSandbox.includes('--enable-sandbox'), false) + assert.equal(argv.noSandbox.includes('--no-sandbox'), true) + + done() + }) }) }) diff --git a/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js b/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js index 3efe83276cb..abe0eeea87e 100644 --- a/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js +++ b/spec/fixtures/api/mixed-sandbox-app/electron-app-mixed-sandbox-preload.js @@ -1,8 +1 @@ -const { ipcRenderer } = require('electron') -if (process) { - process.once('loaded', function () { - ipcRenderer.send('processArgs', process.argv) - }) -} else { - ipcRenderer.send('processArgs', 'unable to get process args') -} +require('electron').ipcRenderer.send('argv', process.argv) diff --git a/spec/fixtures/api/mixed-sandbox-app/main.js b/spec/fixtures/api/mixed-sandbox-app/main.js index fe8c2e45f3e..7823208b72e 100644 --- a/spec/fixtures/api/mixed-sandbox-app/main.js +++ b/spec/fixtures/api/mixed-sandbox-app/main.js @@ -1,39 +1,46 @@ -const { app, BrowserWindow, ipcMain } = require('electron') -const net = require('net') +const {app, BrowserWindow, ipcMain} = require('electron') const path = require('path') -const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-app-mixed-sandbox' : '/tmp/electron-app-mixed-sandbox' - process.on('uncaughtException', () => { app.exit(1) }) -app.once('ready', () => { - let lastArg = process.argv[process.argv.length - 1] - const client = net.connect(socketPath) - client.once('connect', () => { - if (lastArg === '--enable-mixed-sandbox') { - ipcMain.on('processArgs', (event, args) => { - client.end(String(args.indexOf('--no-sandbox') >= 0)) - }) - let window = new BrowserWindow({ - show: false, - webPreferences: { - preload: path.join(app.getAppPath(), 'electron-app-mixed-sandbox-preload.js'), - sandbox: false - } - }) +let sandboxWindow +let noSandboxWindow - window.loadURL('data:,window') - } else { - client.end(String(false)) +app.once('ready', () => { + sandboxWindow = new BrowserWindow({ + show: false, + webPreferences: { + preload: path.join(app.getAppPath(), 'electron-app-mixed-sandbox-preload.js'), + sandbox: true } }) - client.once('end', () => { - app.exit(0) - }) + sandboxWindow.loadURL('about:blank') - if (lastArg !== '--enable-mixed-sandbox') { - app.relaunch({ args: process.argv.slice(1).concat('--enable-mixed-sandbox') }) + noSandboxWindow = new BrowserWindow({ + show: false, + webPreferences: { + preload: path.join(app.getAppPath(), 'electron-app-mixed-sandbox-preload.js'), + sandbox: false + } + }) + noSandboxWindow.loadURL('about:blank') + + const argv = { + sandbox: null, + noSandbox: null } + + ipcMain.on('argv', (event, value) => { + if (event.sender === sandboxWindow.webContents) { + argv.sandbox = value + } else if (event.sender === noSandboxWindow.webContents) { + argv.noSandbox = value + } + + if (argv.sandbox != null && argv.noSandbox != null) { + process.send(argv) + } + }) }) From 74196b96a1ebf0a0b4341e48e9a9b7d0167850ef Mon Sep 17 00:00:00 2001 From: Hari Krishna Reddy Juturu Date: Mon, 26 Jun 2017 14:13:41 -0700 Subject: [PATCH 14/23] Adding enableMixedSandbox api --- atom/browser/api/atom_api_app.cc | 27 ++++++++++++++++++++++++++- atom/browser/api/atom_api_app.h | 1 + docs/api/app.md | 6 ++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 8f2d08277ae..9db24ba5490 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -1028,6 +1028,30 @@ v8::Local App::GetGPUFeatureStatus(v8::Isolate* isolate) { status ? *status : base::DictionaryValue()); } +void App::EnableMixedSandbox(mate::Arguments* args) { + if (Browser::Get()->is_ready()) { + args->ThrowError("app.enableMixedSandbox() can only be called " + "before app is ready"); + return; + } + auto command_line = base::CommandLine::ForCurrentProcess(); + if (command_line->HasSwitch(::switches::kNoSandbox)) { + // Remove the --no-sandbox switch + using StringType = base::CommandLine::StringType; + using StringVector = base::CommandLine::StringVector; + using CharType = base::CommandLine::CharType; + auto argv = command_line->argv(); + StringVector modified_command_line; + const CharType* kNoSandboxArg = L"--no-sandbox"; + for (const StringType& arg : argv) { + if (arg.compare(kNoSandboxArg) != 0) + modified_command_line.push_back(arg); + } + command_line->InitFromArgv(modified_command_line); + } + command_line->AppendSwitch(switches::kEnableMixedSandbox); +} + // static mate::Handle App::Create(v8::Isolate* isolate) { return mate::CreateHandle(isolate, new App(isolate)); @@ -1103,7 +1127,8 @@ void App::BuildPrototype( .SetMethod("getAppMetrics", &App::GetAppMetrics) .SetMethod("getGPUFeatureStatus", &App::GetGPUFeatureStatus) // TODO(juturu): Remove in 2.0, deprecate before then with warnings - .SetMethod("getAppMemoryInfo", &App::GetAppMetrics); + .SetMethod("getAppMemoryInfo", &App::GetAppMetrics) + .SetMethod("enableMixedSandbox", &App::EnableMixedSandbox); } } // namespace api diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index f9726594350..3787d383a6d 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -178,6 +178,7 @@ class App : public AtomBrowserClient::Delegate, std::vector GetAppMetrics(v8::Isolate* isolate); v8::Local GetGPUFeatureStatus(v8::Isolate* isolate); + void EnableMixedSandbox(mate::Arguments* args); #if defined(OS_WIN) // Get the current Jump List settings. diff --git a/docs/api/app.md b/docs/api/app.md index 784b20a4730..9e736f0872a 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -903,6 +903,12 @@ correctly. **Note:** This will not affect `process.argv`. +### `app.enableMixedSandbox()` + +Enables mixed sandbox mode on the app. + +This method can only be called before app is ready. + ### `app.dock.bounce([type])` _macOS_ * `type` String (optional) - Can be `critical` or `informational`. The default is From c01248ce4741b6a8ac73b11cbc16a718b798d124 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 26 Jun 2017 14:14:19 -0700 Subject: [PATCH 15/23] Remove unused require --- spec/api-browser-window-spec.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index c726a1b8b0e..27a94edf867 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -7,7 +7,6 @@ const path = require('path') const os = require('os') const qs = require('querystring') const http = require('http') -const net = require('net') const {closeWindow} = require('./window-helpers') const {ipcRenderer, remote, screen} = require('electron') @@ -1274,7 +1273,7 @@ describe('BrowserWindow module', function () { it('adds --enable-sandbox to render processes created with sandbox: true', (done) => { const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app') - ©appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--enable-mixed-sandbox'], {stdio: ['ignore', 'ipc', 'ignore']}) + appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--enable-mixed-sandbox'], {stdio: ['ignore', 'ipc', 'ignore']}) appProcess.once('message', (argv) => { assert.equal(argv.sandbox.includes('--enable-sandbox'), true) assert.equal(argv.sandbox.includes('--no-sandbox'), false) From f7539e6e8d0132851a2125ff40b5585e1c63fcad Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 26 Jun 2017 14:15:26 -0700 Subject: [PATCH 16/23] Remove increased timeout --- spec/api-browser-window-spec.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 27a94edf867..e419c3327cf 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1261,8 +1261,6 @@ describe('BrowserWindow module', function () { }) describe('mixed sandbox option', function () { - this.timeout(120000) - let appProcess afterEach(function () { From 1258240067b1543a2bbd062966e9f649cff51a25 Mon Sep 17 00:00:00 2001 From: Hari Juturu Date: Mon, 26 Jun 2017 16:20:12 -0700 Subject: [PATCH 17/23] fixing build error --- atom/browser/api/atom_api_app.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 9db24ba5490..16f714fe325 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -1042,7 +1042,12 @@ void App::EnableMixedSandbox(mate::Arguments* args) { using CharType = base::CommandLine::CharType; auto argv = command_line->argv(); StringVector modified_command_line; - const CharType* kNoSandboxArg = L"--no-sandbox"; + #if defined(OS_WIN) + const CharType* kNoSandboxArg = L"--no-sandbox"; + #else + const CharType* kNoSandboxArg = "--no-sandbox"; + #endif + for (const StringType& arg : argv) { if (arg.compare(kNoSandboxArg) != 0) modified_command_line.push_back(arg); From 7fcc00f13793cf86c51cd71c00b63e9b76fd4de9 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 28 Jun 2017 08:33:06 -0700 Subject: [PATCH 18/23] Add spec for app.enableMixedSandbox() --- atom/browser/api/atom_api_app.cc | 26 ++++++------- spec/api-app-spec.js | 42 +++++++++++++++++++++ spec/api-browser-window-spec.js | 24 ------------ spec/fixtures/api/mixed-sandbox-app/main.js | 4 ++ 4 files changed, 58 insertions(+), 38 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 16f714fe325..af7b64667fe 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -1034,23 +1034,21 @@ void App::EnableMixedSandbox(mate::Arguments* args) { "before app is ready"); return; } + auto command_line = base::CommandLine::ForCurrentProcess(); if (command_line->HasSwitch(::switches::kNoSandbox)) { - // Remove the --no-sandbox switch - using StringType = base::CommandLine::StringType; - using StringVector = base::CommandLine::StringVector; - using CharType = base::CommandLine::CharType; - auto argv = command_line->argv(); - StringVector modified_command_line; - #if defined(OS_WIN) - const CharType* kNoSandboxArg = L"--no-sandbox"; - #else - const CharType* kNoSandboxArg = "--no-sandbox"; - #endif +#if defined(OS_WIN) + const base::CommandLine::CharType* noSandboxArg = L"--no-sandbox"; +#else + const base::CommandLine::CharType* noSandboxArg = "--no-sandbox"; +#endif - for (const StringType& arg : argv) { - if (arg.compare(kNoSandboxArg) != 0) - modified_command_line.push_back(arg); + // Remove the --no-sandbox switch + base::CommandLine::StringVector modified_command_line; + for (auto& arg : command_line->argv()) { + if (arg.compare(noSandboxArg) != 0) { + modified_command_line.push_back(arg); + } } command_line->InitFromArgv(modified_command_line); } diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index cda0fbea210..4b04f5b026d 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -566,4 +566,46 @@ describe('app module', function () { assert.equal(typeof features.gpu_compositing, 'string') }) }) + + describe('mixed sandbox option', function () { + let appProcess + + afterEach(function () { + if (appProcess != null) { + appProcess.kill() + } + }) + + describe('when app.enableMixedSandbox() is called', () => { + it('adds --enable-sandbox to render processes created with sandbox: true', (done) => { + const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app') + appProcess = ChildProcess.spawn(remote.process.execPath, [appPath], {stdio: ['ignore', 'ipc', 'ignore']}) + appProcess.once('message', (argv) => { + assert.equal(argv.sandbox.includes('--enable-sandbox'), true) + assert.equal(argv.sandbox.includes('--no-sandbox'), false) + + assert.equal(argv.noSandbox.includes('--enable-sandbox'), false) + assert.equal(argv.noSandbox.includes('--no-sandbox'), true) + + done() + }) + }) + }) + + describe('when the app is launched with --enable-mixed-sandbox', () => { + it('adds --enable-sandbox to render processes created with sandbox: true', (done) => { + const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app') + appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--enable-mixed-sandbox'], {stdio: ['ignore', 'ipc', 'ignore']}) + appProcess.once('message', (argv) => { + assert.equal(argv.sandbox.includes('--enable-sandbox'), true) + assert.equal(argv.sandbox.includes('--no-sandbox'), false) + + assert.equal(argv.noSandbox.includes('--enable-sandbox'), false) + assert.equal(argv.noSandbox.includes('--no-sandbox'), true) + + done() + }) + }) + }) + }) }) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index e419c3327cf..379577571fa 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1260,30 +1260,6 @@ describe('BrowserWindow module', function () { }) }) - describe('mixed sandbox option', function () { - let appProcess - - afterEach(function () { - if (appProcess != null) { - appProcess.kill() - } - }) - - it('adds --enable-sandbox to render processes created with sandbox: true', (done) => { - const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app') - appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--enable-mixed-sandbox'], {stdio: ['ignore', 'ipc', 'ignore']}) - appProcess.once('message', (argv) => { - assert.equal(argv.sandbox.includes('--enable-sandbox'), true) - assert.equal(argv.sandbox.includes('--no-sandbox'), false) - - assert.equal(argv.noSandbox.includes('--enable-sandbox'), false) - assert.equal(argv.noSandbox.includes('--no-sandbox'), true) - - done() - }) - }) - }) - describe('nativeWindowOpen option', () => { beforeEach(() => { w.destroy() diff --git a/spec/fixtures/api/mixed-sandbox-app/main.js b/spec/fixtures/api/mixed-sandbox-app/main.js index 7823208b72e..a06823318d8 100644 --- a/spec/fixtures/api/mixed-sandbox-app/main.js +++ b/spec/fixtures/api/mixed-sandbox-app/main.js @@ -5,6 +5,10 @@ process.on('uncaughtException', () => { app.exit(1) }) +if (!process.argv.includes('--enable-mixed-sandbox')) { + app.enableMixedSandbox() +} + let sandboxWindow let noSandboxWindow From 2df9e57b50a20271f641ae9e8f6229b69d513cd7 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 28 Jun 2017 08:35:17 -0700 Subject: [PATCH 19/23] Remove unused require --- spec/api-browser-window-spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 379577571fa..5c084cb178f 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1,7 +1,6 @@ 'use strict' const assert = require('assert') -const ChildProcess = require('child_process') const fs = require('fs') const path = require('path') const os = require('os') From f1dbfb1925c07e7dfad7d577c446d9eb88a67ff7 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 28 Jun 2017 08:36:29 -0700 Subject: [PATCH 20/23] Mark app.enableMixedSandbox() as experimental --- docs/api/app.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api/app.md b/docs/api/app.md index 9e736f0872a..890987bac75 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -903,9 +903,9 @@ correctly. **Note:** This will not affect `process.argv`. -### `app.enableMixedSandbox()` +### `app.enableMixedSandbox()` _Experimental_ -Enables mixed sandbox mode on the app. +Enables mixed sandbox mode on the app. This method can only be called before app is ready. From 628744f9e14c5f8b9950ebaf6c75a7e91fa81e5e Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 28 Jun 2017 08:37:56 -0700 Subject: [PATCH 21/23] Put enableMixedSandbox before deprecation TODO --- atom/browser/api/atom_api_app.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index af7b64667fe..beecaceff40 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -1129,9 +1129,9 @@ void App::BuildPrototype( .SetMethod("getFileIcon", &App::GetFileIcon) .SetMethod("getAppMetrics", &App::GetAppMetrics) .SetMethod("getGPUFeatureStatus", &App::GetGPUFeatureStatus) + .SetMethod("enableMixedSandbox", &App::EnableMixedSandbox) // TODO(juturu): Remove in 2.0, deprecate before then with warnings - .SetMethod("getAppMemoryInfo", &App::GetAppMetrics) - .SetMethod("enableMixedSandbox", &App::EnableMixedSandbox); + .SetMethod("getAppMemoryInfo", &App::GetAppMetrics); } } // namespace api From 523fbe2df926b694cea4b8b032e7b4507bde61e7 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 28 Jun 2017 09:27:19 -0700 Subject: [PATCH 22/23] Send messages between app via socket connection --- spec/api-app-spec.js | 68 ++++++++++++++++----- spec/fixtures/api/mixed-sandbox-app/main.js | 10 ++- 2 files changed, 61 insertions(+), 17 deletions(-) diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index 4b04f5b026d..01ad7b3cbd0 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -568,26 +568,54 @@ describe('app module', function () { }) describe('mixed sandbox option', function () { - let appProcess + let appProcess = null + let server = null + const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-mixed-sandbox' : '/tmp/electron-mixed-sandbox' - afterEach(function () { + beforeEach(function (done) { + fs.unlink(socketPath, () => { + server = net.createServer() + server.listen(socketPath) + done() + }) + }) + + afterEach(function (done) { if (appProcess != null) { appProcess.kill() } + + server.close(() => { + if (process.platform === 'win32') { + done() + } else { + fs.unlink(socketPath, () => { + done() + }) + } + }) }) describe('when app.enableMixedSandbox() is called', () => { it('adds --enable-sandbox to render processes created with sandbox: true', (done) => { const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app') - appProcess = ChildProcess.spawn(remote.process.execPath, [appPath], {stdio: ['ignore', 'ipc', 'ignore']}) - appProcess.once('message', (argv) => { - assert.equal(argv.sandbox.includes('--enable-sandbox'), true) - assert.equal(argv.sandbox.includes('--no-sandbox'), false) + appProcess = ChildProcess.spawn(remote.process.execPath, [appPath]) - assert.equal(argv.noSandbox.includes('--enable-sandbox'), false) - assert.equal(argv.noSandbox.includes('--no-sandbox'), true) + server.once('error', (error) => { + done(error) + }) - done() + server.on('connection', (client) => { + client.once('data', function (data) { + const argv = JSON.parse(data) + assert.equal(argv.sandbox.includes('--enable-sandbox'), true) + assert.equal(argv.sandbox.includes('--no-sandbox'), false) + + assert.equal(argv.noSandbox.includes('--enable-sandbox'), false) + assert.equal(argv.noSandbox.includes('--no-sandbox'), true) + + done() + }) }) }) }) @@ -595,15 +623,23 @@ describe('app module', function () { describe('when the app is launched with --enable-mixed-sandbox', () => { it('adds --enable-sandbox to render processes created with sandbox: true', (done) => { const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app') - appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--enable-mixed-sandbox'], {stdio: ['ignore', 'ipc', 'ignore']}) - appProcess.once('message', (argv) => { - assert.equal(argv.sandbox.includes('--enable-sandbox'), true) - assert.equal(argv.sandbox.includes('--no-sandbox'), false) + appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--enable-mixed-sandbox']) - assert.equal(argv.noSandbox.includes('--enable-sandbox'), false) - assert.equal(argv.noSandbox.includes('--no-sandbox'), true) + server.once('error', (error) => { + done(error) + }) - done() + server.on('connection', (client) => { + client.once('data', function (data) { + const argv = JSON.parse(data) + assert.equal(argv.sandbox.includes('--enable-sandbox'), true) + assert.equal(argv.sandbox.includes('--no-sandbox'), false) + + assert.equal(argv.noSandbox.includes('--enable-sandbox'), false) + assert.equal(argv.noSandbox.includes('--no-sandbox'), true) + + done() + }) }) }) }) diff --git a/spec/fixtures/api/mixed-sandbox-app/main.js b/spec/fixtures/api/mixed-sandbox-app/main.js index a06823318d8..3ff9cce98bd 100644 --- a/spec/fixtures/api/mixed-sandbox-app/main.js +++ b/spec/fixtures/api/mixed-sandbox-app/main.js @@ -1,4 +1,5 @@ const {app, BrowserWindow, ipcMain} = require('electron') +const net = require('net') const path = require('path') process.on('uncaughtException', () => { @@ -44,7 +45,14 @@ app.once('ready', () => { } if (argv.sandbox != null && argv.noSandbox != null) { - process.send(argv) + const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-mixed-sandbox' : '/tmp/electron-mixed-sandbox' + const client = net.connect(socketPath) + client.once('connect', () => { + client.end(JSON.stringify(argv)) + }) + client.once('end', () => { + app.exit(0) + }) } }) }) From 5822e718e6c184a5d4a43c2073049975da83e5ae Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 28 Jun 2017 09:58:23 -0700 Subject: [PATCH 23/23] Add finish helper to ensure connection --- spec/fixtures/api/mixed-sandbox-app/main.js | 33 +++++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/spec/fixtures/api/mixed-sandbox-app/main.js b/spec/fixtures/api/mixed-sandbox-app/main.js index 3ff9cce98bd..a71c593f513 100644 --- a/spec/fixtures/api/mixed-sandbox-app/main.js +++ b/spec/fixtures/api/mixed-sandbox-app/main.js @@ -17,7 +17,7 @@ app.once('ready', () => { sandboxWindow = new BrowserWindow({ show: false, webPreferences: { - preload: path.join(app.getAppPath(), 'electron-app-mixed-sandbox-preload.js'), + preload: path.join(__dirname, 'electron-app-mixed-sandbox-preload.js'), sandbox: true } }) @@ -26,7 +26,7 @@ app.once('ready', () => { noSandboxWindow = new BrowserWindow({ show: false, webPreferences: { - preload: path.join(app.getAppPath(), 'electron-app-mixed-sandbox-preload.js'), + preload: path.join(__dirname, 'electron-app-mixed-sandbox-preload.js'), sandbox: false } }) @@ -37,22 +37,29 @@ app.once('ready', () => { noSandbox: null } + let connected = false + + function finish () { + if (connected && argv.sandbox != null && argv.noSandbox != null) { + client.once('end', () => { + app.exit(0) + }) + client.end(JSON.stringify(argv)) + } + } + + const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-mixed-sandbox' : '/tmp/electron-mixed-sandbox' + const client = net.connect(socketPath, () => { + connected = true + finish() + }) + ipcMain.on('argv', (event, value) => { if (event.sender === sandboxWindow.webContents) { argv.sandbox = value } else if (event.sender === noSandboxWindow.webContents) { argv.noSandbox = value } - - if (argv.sandbox != null && argv.noSandbox != null) { - const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-mixed-sandbox' : '/tmp/electron-mixed-sandbox' - const client = net.connect(socketPath) - client.once('connect', () => { - client.end(JSON.stringify(argv)) - }) - client.once('end', () => { - app.exit(0) - }) - } + finish() }) })