Merge pull request #141 from electron-userland/disable-browser-sandbox

chore: allow disabling browser sandbox
electron-userland · Feb 8, 2022 · eb0998b · eb0998b
2 parents 49da95d + 36769bd
commit eb0998b
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 19 deletions.
diff --git a/src/index.d.ts b/src/index.d.ts
@@ -157,10 +157,12 @@ declare namespace createSnap {
       alsa?: true;
       /**
        * [Web browser functionality](https://github.com/snapcore/snapd/wiki/Interfaces#browser-support).
-       * This is enabled by default when using Electron ≥ 5.0.0, due to the
+       * This was originally enabled by default when using Electron ≥ 5.0.0, due to the
        * [setuid sandbox support](https://github.com/electron/electron/pull/17269).
+       * However, Snapcraft allows for use of the snap confined sandbox, particularly within
+       * strict confinement. We should encourage but not enforce the browser-sandbox plug.
        */
-      browserSandbox?: true;
+      browserSandbox?: false;
       /**
        * [MPRIS](https://specifications.freedesktop.org/mpris-spec/latest/) support.
        *

diff --git a/src/yaml.js b/src/yaml.js
@@ -20,7 +20,6 @@ const common = require('electron-installer-common')
 const fs = require('fs-extra')
 const { merge, pull } = require('lodash')
 const path = require('path')
-const semver = require('semver')
 const { spawn } = require('@malept/cross-spawn-promise')
 const which = require('which')
 const yaml = require('js-yaml')
@@ -155,26 +154,26 @@ class SnapcraftYAML {
   }
 
   transformFeatures () {
-    if (semver.satisfies(this.electronVersion, '>= 5.0.0') && !this.features.browserSandbox) {
-      this.features.browserSandbox = true
-    }
     for (const feature of Object.keys(this.features)) {
       this.transformFeature(feature)
     }
   }
 
   transformBrowserSandbox () {
     debug('Replacing browser-support plug with browser-sandbox')
-    pull(this.app.plugs, 'browser-support')
-    this.app.plugs.push('browser-sandbox')
-    if (!this.data.plugs) {
-      this.data.plugs = {}
-    }
-    this.data.plugs['browser-sandbox'] = {
-      'allow-sandbox': true,
-      interface: 'browser-support'
+    if (this.app.plugs.includes('browser-sandbox') ||
+       (this.features.browserSandbox && this.features.browserSandbox === true)) {
+      pull(this.app.plugs, 'browser-support')
+      this.app.plugs.push('browser-sandbox')
+      if (!this.data.plugs) {
+        this.data.plugs = {}
+      }
+      this.data.plugs['browser-sandbox'] = {
+        'allow-sandbox': true,
+        interface: 'browser-support'
+      }
+      console.warn('The browser-sandbox feature will trigger a manual review in the Snap store.')
     }
-    console.warn('The browser-sandbox feature will trigger a manual review in the Snap store.')
   }
 
   transformMPRIS () {

diff --git a/test/yaml.js b/test/yaml.js
@@ -85,10 +85,10 @@ test('browserSandbox feature', async t => {
   t.deepEqual(plugs['browser-sandbox'], { interface: 'browser-support', 'allow-sandbox': true }, 'browser-sandbox plug exists')
 })
 
-test('browserSandbox is always on for Electron >= 5.0.0', async t => {
-  const { apps } = await createYaml(t, { name: 'electronAppName' }, '5.0.0')
-  util.assertNotIncludes(t, apps.electronAppName.plugs, 'browser-support', 'browser-support is not in app plugs')
-  util.assertIncludes(t, apps.electronAppName.plugs, 'browser-sandbox', 'browser-sandbox is in app plugs')
+test('browserSandbox feature allow both true and false', async t => {
+  const { apps } = await createYaml(t, { name: 'electronAppName', features: { browserSandbox: false } })
+  util.assertIncludes(t, apps.electronAppName.plugs, 'browser-support', 'browser-support is not in app plugs')
+  util.assertNotIncludes(t, apps.electronAppName.plugs, 'browser-sandbox', 'browser-sandbox is in app plugs')
 })
 
 test('browserSandbox feature with custom plugs', async t => {