feat: enable mixed-sandbox mode by default (#15894)

atom/app/atom_main_delegate.cc

@@ -238,12 +238,6 @@ void AtomMainDelegate::PreSandboxStartup() {
   // linux (namespace sandbox is available on most distros).
   command_line->AppendSwitch(service_manager::switches::kDisableSetuidSandbox);
 
-  if (!command_line->HasSwitch(switches::kEnableMixedSandbox) &&
-      !command_line->HasSwitch(switches::kEnableSandbox)) {
-    // Disable renderer sandbox for most of node's functions.
-    command_line->AppendSwitch(service_manager::switches::kNoSandbox);
-  }
-
   // Allow file:// URIs to read other file:// URIs by default.
   command_line->AppendSwitch(::switches::kAllowFileAccessFromFiles);
 

atom/browser/api/atom_api_app.cc

@@ -1250,19 +1250,6 @@ void App::EnableSandbox(mate::Arguments* args) {
   command_line->AppendSwitch(switches::kEnableSandbox);
 }
 
-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();
-  RemoveNoSandboxSwitch(command_line);
-  command_line->AppendSwitch(switches::kEnableMixedSandbox);
-}
-
 #if defined(OS_MACOSX)
 bool App::MoveToApplicationsFolder(mate::Arguments* args) {
   return ui::cocoa::AtomBundleMover::Move(args);
@@ -1370,8 +1357,7 @@ void App::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("startAccessingSecurityScopedResource",
                  &App::StartAccessingSecurityScopedResource)
 #endif
-      .SetMethod("enableSandbox", &App::EnableSandbox)
+      .SetMethod("enableSandbox", &App::EnableSandbox);
-      .SetMethod("enableMixedSandbox", &App::EnableMixedSandbox);
 }
 
 }  // namespace api

atom/browser/api/atom_api_app.h

@@ -206,7 +206,6 @@ class App : public AtomBrowserClient::Delegate,
   v8::Local<v8::Promise> GetGPUInfo(v8::Isolate* isolate,
                                     const std::string& info_type);
   void EnableSandbox(mate::Arguments* args);
-  void EnableMixedSandbox(mate::Arguments* args);
 
 #if defined(OS_MACOSX)
   bool MoveToApplicationsFolder(mate::Arguments* args);

atom/browser/atom_browser_client.cc

@@ -71,6 +71,7 @@
 #include "services/device/public/cpp/geolocation/location_provider.h"
 #include "services/network/public/cpp/resource_request_body.h"
 #include "services/proxy_resolver/public/mojom/proxy_resolver.mojom.h"
+#include "services/service_manager/sandbox/switches.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "v8/include/v8.h"
@@ -504,6 +505,16 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches(
 
   content::WebContents* web_contents = GetWebContentsFromProcessID(process_id);
   if (web_contents) {
+    // devtools processes must be launched unsandboxed in order for the remote
+    // API to work in devtools extensions. This is due to the fact that the
+    // remote API assumes that it will only be used from the main frame, but
+    // devtools extensions are loaded from an iframe.
+    // It would be possible to sandbox devtools extensions processes by default
+    // if we made the remote API work with multiple frames.
+    if (web_contents->GetVisibleURL().SchemeIs("chrome-devtools")) {
+      command_line->AppendSwitch(service_manager::switches::kNoSandbox);
+      command_line->AppendSwitch(::switches::kNoZygote);
+    }
     auto* web_preferences = WebContentsPreferences::From(web_contents);
     if (web_preferences)
       web_preferences->AppendCommandLineSwitches(command_line);

atom/common/options_switches.cc

@@ -161,9 +161,6 @@ 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";
 

atom/common/options_switches.h

@@ -83,7 +83,6 @@ extern const char kOffscreen[];
 namespace switches {
 
 extern const char kEnableSandbox[];
-extern const char kEnableMixedSandbox[];
 extern const char kEnablePlugins[];
 extern const char kPpapiFlashPath[];
 extern const char kPpapiFlashVersion[];

docs/api/app.md

@@ -1225,12 +1225,6 @@ Enables full sandbox mode on the app.
 
 This method can only be called before app is ready.
 
-### `app.enableMixedSandbox()` _Experimental_ _macOS_ _Windows_
-
-Enables mixed sandbox mode on the app.
-
-This method can only be called before app is ready.
-
 ### `app.isInApplicationsFolder()` _macOS_
 
 Returns `Boolean` - Whether the application is currently running from the

docs/api/sandbox-option.md

@@ -60,10 +60,6 @@ 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.
-This option is currently only supported on macOS and Windows.
-
 ```js
 let win
 app.on('ready', () => {

lib/browser/api/app.js

@@ -30,6 +30,9 @@ Object.assign(app, {
     getSwitchValue: (...args) => commandLine.getSwitchValue(...args.map(String)),
     appendSwitch: (...args) => commandLine.appendSwitch(...args.map(String)),
     appendArgument: (...args) => commandLine.appendArgument(...args.map(String))
+  },
+  enableMixedSandbox () {
+    deprecate.log(`'enableMixedSandbox' is deprecated. Mixed-sandbox mode is now enabled by default. You can safely remove the call to enableMixedSandbox().`)
   }
 })
 

spec/api-app-spec.js

@@ -1111,56 +1111,6 @@ describe('app module', () => {
         })
       })
     })
-
-    describe('when app.enableMixedSandbox() is called', () => {
-      it('adds --enable-sandbox to renderer processes created with sandbox: true', done => {
-        const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app')
-        appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--app-enable-mixed-sandbox'])
-
-        server.once('error', error => { done(error) })
-
-        server.on('connection', client => {
-          client.once('data', data => {
-            const argv = JSON.parse(data)
-            expect(argv.sandbox).to.include('--enable-sandbox')
-            expect(argv.sandbox).to.not.include('--no-sandbox')
-
-            expect(argv.noSandbox).to.not.include('--enable-sandbox')
-            expect(argv.noSandbox).to.include('--no-sandbox')
-
-            expect(argv.noSandboxDevtools).to.be.true()
-            expect(argv.sandboxDevtools).to.be.true()
-
-            done()
-          })
-        })
-      })
-    })
-
-    describe('when the app is launched with --enable-mixed-sandbox', () => {
-      it('adds --enable-sandbox to renderer 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'])
-
-        server.once('error', error => { done(error) })
-
-        server.on('connection', client => {
-          client.once('data', data => {
-            const argv = JSON.parse(data)
-            expect(argv.sandbox).to.include('--enable-sandbox')
-            expect(argv.sandbox).to.not.include('--no-sandbox')
-
-            expect(argv.noSandbox).to.not.include('--enable-sandbox')
-            expect(argv.noSandbox).to.include('--no-sandbox')
-
-            expect(argv.noSandboxDevtools).to.be.true()
-            expect(argv.sandboxDevtools).to.be.true()
-
-            done()
-          })
-        })
-      })
-    })
   })
 
   describe('disableDomainBlockingFor3DAPIs() API', () => {

spec/api-browser-window-spec.js

@@ -1873,7 +1873,6 @@ describe('BrowserWindow module', () => {
 
       it('validates process APIs access in sandboxed renderer', (done) => {
         ipcMain.once('answer', function (event, test) {
-          assert.strictEqual(test.pid, w.webContents.getOSProcessId())
           assert.strictEqual(test.arch, remote.process.arch)
           assert.strictEqual(test.platform, remote.process.platform)
           assert.deepStrictEqual(...resolveGetters(test.env, remote.process.env))

spec/fixtures/api/mixed-sandbox-app/main.js

@@ -10,10 +10,6 @@ if (process.argv.includes('--app-enable-sandbox')) {
   app.enableSandbox()
 }
 
-if (process.argv.includes('--app-enable-mixed-sandbox')) {
-  app.enableMixedSandbox()
-}
-
 let currentWindowSandboxed = false
 
 app.once('ready', () => {