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

Jump to bottom

feat: add app.enableSandbox() #14999

Merged
merged 1 commit into from Oct 10, 2018
Merged

feat: add app.enableSandbox() #14999

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 29 additions & 16 deletions atom/browser/api/atom_api_app.cc
View file
Open in desktop
Expand Up @@ -1184,23 +1184,10 @@ v8::Local<v8::Promise> App::GetGPUInfo(v8::Isolate* isolate,
return promise->GetHandle();
}

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();
static void RemoveNoSandboxSwitch(base::CommandLine* command_line) {
if (command_line->HasSwitch(service_manager::switches::kNoSandbox)) {
#if defined(OS_WIN)
const base::CommandLine::CharType* noSandboxArg = L"--no-sandbox";
#else
const base::CommandLine::CharType* noSandboxArg = "--no-sandbox";
#endif

// Remove the --no-sandbox switch
const base::CommandLine::CharType* noSandboxArg =
FILE_PATH_LITERAL("--no-sandbox");
base::CommandLine::StringVector modified_command_line;
for (auto& arg : command_line->argv()) {
if (arg.compare(noSandboxArg) != 0) {
Expand All @@ -1209,6 +1196,31 @@ void App::EnableMixedSandbox(mate::Arguments* args) {
}
command_line->InitFromArgv(modified_command_line);
}
}

void App::EnableSandbox(mate::Arguments* args) {
if (Browser::Get()->is_ready()) {
args->ThrowError(
"app.enableSandbox() can only be called "
"before app is ready");
return;
}

auto* command_line = base::CommandLine::ForCurrentProcess();
RemoveNoSandboxSwitch(command_line);
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);
}

Expand Down Expand Up @@ -1316,6 +1328,7 @@ void App::BuildPrototype(v8::Isolate* isolate,
.SetMethod("startAccessingSecurityScopedResource",
&App::StartAccessingSecurityScopedResource)
#endif
.SetMethod("enableSandbox", &App::EnableSandbox)
.SetMethod("enableMixedSandbox", &App::EnableMixedSandbox);
}

Expand Down
1 change: 1 addition & 0 deletions atom/browser/api/atom_api_app.h
View file
Open in desktop
Expand Up @@ -203,6 +203,7 @@ class App : public AtomBrowserClient::Delegate,
v8::Local<v8::Value> GetGPUFeatureStatus(v8::Isolate* isolate);
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)
Expand Down
6 changes: 6 additions & 0 deletions docs/api/app.md
View file
Open in desktop
Expand Up @@ -1089,6 +1089,12 @@ correctly.

**Note:** This will not affect `process.argv`.

### `app.enableSandbox()` _Experimental_ _macOS_ _Windows_

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.
Expand Down
61 changes: 57 additions & 4 deletions spec/api-app-spec.js
View file
Open in desktop
Expand Up @@ -871,7 +871,7 @@ describe('app module', () => {
})
})

describe('mixed sandbox option', () => {
describe('sandbox options', () => {
let appProcess = null
let server = null
const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-mixed-sandbox' : '/tmp/electron-mixed-sandbox'
Expand Down Expand Up @@ -903,10 +903,60 @@ describe('app module', () => {
})
})

describe('when app.enableSandbox() is called', () => {
it('adds --enable-sandbox to all renderer processes', done => {
const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app')
appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--app-enable-sandbox'])

server.once('error', error => { done(error) })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to use async/await, but does it look cleaner here?

// Throws on error.
const client = await Promise.race([
  emittedOnce(server, 'connection').then(([, client]) => client),
  emittedOnce(server, 'error').then(([, error]) => Promise.reject(error))
])

const data = await emittedOnce(client, 'data')
const argv = JSON.parse(data)
<...>


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.include('--enable-sandbox')
expect(argv.noSandbox).to.not.include('--no-sandbox')

expect(argv.noSandboxDevtools).to.be.true()
expect(argv.sandboxDevtools).to.be.true()

done()
})
})
})
})

describe('when the app is launched with --enable-sandbox', () => {
it('adds --enable-sandbox to all renderer processes', done => {
const appPath = path.join(__dirname, 'fixtures', 'api', 'mixed-sandbox-app')
appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--enable-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.include('--enable-sandbox')
expect(argv.noSandbox).to.not.include('--no-sandbox')

expect(argv.noSandboxDevtools).to.be.true()
expect(argv.sandboxDevtools).to.be.true()

done()
})
})
})
})

describe('when app.enableMixedSandbox() is called', () => {
it('adds --enable-sandbox to render processes created with sandbox: true', done => {
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])
appProcess = ChildProcess.spawn(remote.process.execPath, [appPath, '--app-enable-mixed-sandbox'])

server.once('error', error => { done(error) })

Expand All @@ -919,14 +969,17 @@ describe('app module', () => {
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 render processes created with sandbox: true', done => {
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'])

Expand Down
6 changes: 5 additions & 1 deletion spec/fixtures/api/mixed-sandbox-app/main.js
View file
Open in desktop
Expand Up @@ -6,7 +6,11 @@ process.on('uncaughtException', () => {
app.exit(1)
})

if (!process.argv.includes('--enable-mixed-sandbox')) {
if (process.argv.includes('--app-enable-sandbox')) {
app.enableSandbox()
}

if (process.argv.includes('--app-enable-mixed-sandbox')) {
app.enableMixedSandbox()
}

Expand Down