Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: allow disabling browser sandbox #141

Merged
merged 1 commit into from
Feb 8, 2022
Merged

chore: allow disabling browser sandbox #141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
25 changes: 12 additions & 13 deletions src/yaml.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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 () {
Expand Down
8 changes: 4 additions & 4 deletions test/yaml.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down