Skip to content

Commit e13a23c

Browse files
authored
chore: fix the mcp sandbox settings (microsoft#38977)
1 parent fc8e4f3 commit e13a23c

4 files changed

Lines changed: 28 additions & 18 deletions

File tree

packages/playwright/src/mcp/browser/config.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,6 @@ export const defaultConfig: FullConfig = {
8282
launchOptions: {
8383
channel: 'chrome',
8484
headless: os.platform() === 'linux' && !process.env.DISPLAY,
85-
chromiumSandbox: true,
8685
},
8786
contextOptions: {
8887
viewport: null,
@@ -127,7 +126,7 @@ const defaultDaemonConfig = (cliOptions: CLIOptions) => mergeConfig(defaultConfi
127126
type BrowserUserConfig = NonNullable<Config['browser']>;
128127

129128
export type FullConfig = Config & {
130-
browser: Omit<BrowserUserConfig, 'browserName'> & {
129+
browser: Omit<BrowserUserConfig, 'browserName' | 'launchOptions' | 'contextOptions'> & {
131130
browserName: 'chromium' | 'firefox' | 'webkit';
132131
launchOptions: NonNullable<BrowserUserConfig['launchOptions']>;
133132
contextOptions: NonNullable<BrowserUserConfig['contextOptions']>;
@@ -161,6 +160,7 @@ export async function resolveCLIConfig(cliOptions: CLIOptions): Promise<FullConf
161160
result = mergeConfig(result, configInFile);
162161
result = mergeConfig(result, envOverrides);
163162
result = mergeConfig(result, cliOverrides);
163+
164164
if (cliOptions.daemon)
165165
result.skillMode = true;
166166

@@ -170,6 +170,13 @@ export async function resolveCLIConfig(cliOptions: CLIOptions): Promise<FullConf
170170
result.browser.userDataDir = `${cliOptions.daemonDataDir}-${browserToken}`;
171171
}
172172

173+
if (result.browser.browserName === 'chromium' && result.browser.launchOptions.chromiumSandbox === undefined) {
174+
if (process.platform === 'linux')
175+
result.browser.launchOptions.chromiumSandbox = result.browser.launchOptions.channel !== 'chromium';
176+
else
177+
result.browser.launchOptions.chromiumSandbox = true;
178+
}
179+
173180
await validateConfig(result);
174181
return result;
175182
}
@@ -222,11 +229,10 @@ export function configFromCLIOptions(cliOptions: CLIOptions): Config {
222229
headless: cliOptions.headless,
223230
};
224231

232+
// --sandbox was passed, enable the sandbox
225233
// --no-sandbox was passed, disable the sandbox
226-
if (cliOptions.sandbox === false)
227-
launchOptions.chromiumSandbox = false;
228-
if (process.env.CI && process.platform === 'linux')
229-
launchOptions.chromiumSandbox = false;
234+
if (cliOptions.sandbox !== undefined)
235+
launchOptions.chromiumSandbox = cliOptions.sandbox;
230236

231237
if (cliOptions.proxyServer) {
232238
launchOptions.proxy = {

packages/playwright/src/mcp/program.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ export function decorateCommand(command: Command, version: string) {
6262
.option('--port <port>', 'port to listen on for SSE transport.')
6363
.option('--proxy-bypass <bypass>', 'comma-separated domains to bypass proxy, for example ".com,chromium.org,.domain.com"')
6464
.option('--proxy-server <proxy>', 'specify proxy server, for example "http://myproxy:3128" or "socks5://myproxy:8080"')
65+
.option('--sandbox', 'enable the sandbox for all process types that are normally not sandboxed.')
6566
.option('--save-session', 'Whether to save the Playwright MCP session into the output directory.')
6667
.option('--save-trace', 'Whether to save the Playwright Trace of the session into the output directory.')
6768
.option('--save-video <size>', 'Whether to save the video of the session into the output directory. For example "--save-video=800x600"', resolutionParser.bind(null, '--save-video'))
@@ -80,6 +81,10 @@ export function decorateCommand(command: Command, version: string) {
8081
.addOption(new ProgramOption('--daemon-data-dir <path>', 'path to the daemon data directory.').hideHelp())
8182
.addOption(new ProgramOption('--daemon-headed', 'run daemon in headed mode').hideHelp())
8283
.action(async options => {
84+
85+
// normalize the --no-sandbox option: sandbox = true => nothing was passed, sandbox = false => --no-sandbox was passed.
86+
options.sandbox = options.sandbox === true ? undefined : false;
87+
8388
setupExitWatchdog();
8489

8590
if (options.vision) {

tests/mcp/config.spec.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import fs from 'node:fs';
1818

1919
import { test, expect } from './fixtures';
20-
import { configFromCLIOptions } from '../../packages/playwright/lib/mcp/browser/config';
20+
import { resolveCLIConfig } from '../../packages/playwright/lib/mcp/browser/config';
2121
import type { Config } from '../../packages/playwright/src/mcp/config';
2222

2323
test('config user data dir', async ({ startClient, server }, testInfo) => {
@@ -78,14 +78,15 @@ test.describe(() => {
7878
});
7979
});
8080

81-
test.describe('sandbox configuration', () => {
82-
test('should enable sandbox by default (no --no-sandbox flag)', async () => {
83-
const config = configFromCLIOptions({ sandbox: undefined });
84-
expect(config.browser?.launchOptions?.chromiumSandbox).toBeUndefined();
85-
});
81+
async function sandboxOption(cli: any) {
82+
const config: any = await resolveCLIConfig(cli);
83+
return config.browser.launchOptions.chromiumSandbox;
84+
}
8685

87-
test('should disable sandbox when --no-sandbox flag is passed', async () => {
88-
const config = configFromCLIOptions({ sandbox: false });
89-
expect(config.browser?.launchOptions?.chromiumSandbox).toBe(false);
90-
});
86+
test('test sandbox configuration', async ({}) => {
87+
expect(await sandboxOption({ browser: 'chromium' })).toBe(process.platform !== 'linux');
88+
expect(await sandboxOption({ browser: 'chromium', sandbox: true })).toBe(true);
89+
expect(await sandboxOption({ browser: 'chrome', sandbox: false })).toBe(false);
90+
expect(await sandboxOption({ browser: 'chrome' })).toBe(true);
91+
expect(await sandboxOption({ browser: 'msedge' })).toBe(true);
9192
});

tests/mcp/fixtures.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,6 @@ export const test = serverTest.extend<TestFixtures & TestOptions, WorkerFixtures
9898
if (!options?.args?.some(arg => arg.startsWith('--config')))
9999
args.push(`--config=${test.info().outputPath()}`);
100100
} else {
101-
if (process.env.CI && process.platform === 'linux')
102-
args.push('--no-sandbox');
103101
if (mcpBrowser)
104102
args.push(`--browser=${mcpBrowser}`);
105103
if (options?.config) {

0 commit comments

Comments
 (0)