feat: support --allow-unrestricted-paths configuration (#2296)
## Summary validatePath() in McpContext returned immediately, with no restriction at all, whenever roots() returned undefined. roots() only returns undefined when the connecting MCP client never negotiates the optional roots capability during initialize, which any minimal client can trigger simply by omitting it from its declared capabilities. Since roots() already always appends the OS temp directory to whatever explicit roots are configured, this change makes it return that same default (temp directory only) instead of undefined when no roots have been set. This removes the early return in validatePath() entirely, so path validation now runs unconditionally rather than being conditional on whether the connecting client happened to negotiate a capability it was never required to declare per the MCP spec. Any filePath-accepting tool (take_screenshot, saveFile, and the performance/Lighthouse export tools that route through the same check) had its only path-traversal guard silently disabled for the lifetime of a connection whenever the client omitted the optional roots capability. Since this server is designed to let an LLM drive a browser, and browsed page content is not trusted input, this meant a client that simply doesn't implement roots (a plausible, non-adversarial default for lightweight or custom MCP clients) removed the only boundary preventing the connected agent from writing to any path the process can reach. Added a test that exercises the actual default state of roots (never calling setRoots()) directly, since the existing tests always call setRoots(), even with an empty array, before validating. Verified locally with a minimal MCP client that declares no capabilities: before this change, take_screenshot with a filePath outside any root wrote a real file to an arbitrary path with no error; after this change, the same call is rejected with the existing Access denied error. Also verified that a client that does declare roots is unaffected, and that writes to the OS temp directory continue to succeed with no roots negotiated, matching prior behavior for that path.
This commit is contained in:
@@ -754,6 +754,11 @@ The Chrome DevTools MCP server supports the following configuration option:
|
||||
- **Type:** boolean
|
||||
- **Default:** `false`
|
||||
|
||||
- **`--allowUnrestrictedPaths`/ `--allow-unrestricted-paths`**
|
||||
If set, disables the default path restriction that applies when the MCP client does not negotiate the roots capability. By default, file-writing tools are restricted to the OS temp directory when no roots are configured. Use this only when connecting a trusted local client that does not implement MCP roots and requires access to paths outside the temp directory.
|
||||
- **Type:** boolean
|
||||
- **Default:** `false`
|
||||
|
||||
<!-- END AUTO GENERATED OPTIONS -->
|
||||
|
||||
Pass them via the `args` property in the JSON configuration. For example:
|
||||
|
||||
Generated
-39
@@ -1521,9 +1521,6 @@
|
||||
"arm"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1538,9 +1535,6 @@
|
||||
"arm"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1555,9 +1549,6 @@
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1572,9 +1563,6 @@
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1589,9 +1577,6 @@
|
||||
"loong64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1606,9 +1591,6 @@
|
||||
"loong64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1623,9 +1605,6 @@
|
||||
"ppc64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1640,9 +1619,6 @@
|
||||
"ppc64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1657,9 +1633,6 @@
|
||||
"riscv64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1674,9 +1647,6 @@
|
||||
"riscv64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1691,9 +1661,6 @@
|
||||
"s390x"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1708,9 +1675,6 @@
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1725,9 +1689,6 @@
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
|
||||
+17
-7
@@ -69,6 +69,10 @@ interface McpContextOptions {
|
||||
allowList?: string[];
|
||||
// The block list of URL patterns to block loading resources.
|
||||
blocklist?: string[];
|
||||
// Whether to skip path validation when the client did not negotiate the roots
|
||||
// capability. When false (default), file-writing tools are restricted to the
|
||||
// OS temp directory. When true, the previous permissive behavior is restored.
|
||||
allowUnrestrictedPaths?: boolean;
|
||||
}
|
||||
|
||||
const DEFAULT_TIMEOUT = 5_000;
|
||||
@@ -110,6 +114,7 @@ export class McpContext implements Context {
|
||||
#options: McpContextOptions;
|
||||
#heapSnapshotManager = new HeapSnapshotManager();
|
||||
#roots: Root[] | undefined = undefined;
|
||||
#allowUnrestrictedPaths: boolean;
|
||||
|
||||
private constructor(
|
||||
browser: Browser,
|
||||
@@ -127,6 +132,7 @@ export class McpContext implements Context {
|
||||
this.logger = logger;
|
||||
this.#locatorClass = locatorClass;
|
||||
this.#options = options;
|
||||
this.#allowUnrestrictedPaths = options.allowUnrestrictedPaths ?? false;
|
||||
|
||||
this.#networkCollector = new NetworkCollector(this.browser);
|
||||
|
||||
@@ -185,12 +191,9 @@ export class McpContext implements Context {
|
||||
return context;
|
||||
}
|
||||
|
||||
roots(): Root[] | undefined {
|
||||
if (this.#roots === undefined) {
|
||||
return undefined;
|
||||
}
|
||||
roots(): Root[] {
|
||||
return [
|
||||
...this.#roots,
|
||||
...(this.#roots ?? []),
|
||||
{
|
||||
uri: pathToFileURL(os.tmpdir()).href,
|
||||
name: 'temp',
|
||||
@@ -206,10 +209,17 @@ export class McpContext implements Context {
|
||||
if (filePath === undefined) {
|
||||
return;
|
||||
}
|
||||
const roots = this.roots();
|
||||
if (roots === undefined) {
|
||||
// If the client never negotiated roots and the operator has explicitly
|
||||
// opted into unrestricted access via --allow-unrestricted-paths, restore
|
||||
// the previous permissive behavior and skip validation.
|
||||
if (this.#roots === undefined && this.#allowUnrestrictedPaths) {
|
||||
return;
|
||||
}
|
||||
// roots() always returns at least the temp directory, even if the
|
||||
// connecting client never negotiated the optional `roots` capability.
|
||||
// Path validation must not be skipped just because no workspace roots
|
||||
// were configured.
|
||||
const roots = this.roots();
|
||||
|
||||
let canonicalPath: string;
|
||||
|
||||
|
||||
@@ -361,6 +361,15 @@ export const cliOptions = {
|
||||
'If true, redacts some of the network headers considered sensitive before returning to the client.',
|
||||
default: false,
|
||||
},
|
||||
allowUnrestrictedPaths: {
|
||||
type: 'boolean',
|
||||
default: false,
|
||||
describe:
|
||||
'If set, disables the default path restriction that applies when the MCP client does not negotiate ' +
|
||||
'the roots capability. By default, file-writing tools are restricted to the OS temp directory when ' +
|
||||
'no roots are configured. Use this only when connecting a trusted local client that does not implement ' +
|
||||
'MCP roots and requires access to paths outside the temp directory.',
|
||||
},
|
||||
} satisfies Record<string, YargsOptions>;
|
||||
|
||||
export type ParsedArguments = ReturnType<typeof parseArguments>;
|
||||
|
||||
@@ -86,6 +86,13 @@ export async function createMcpServer(
|
||||
void updateRoots();
|
||||
},
|
||||
);
|
||||
} else if (!serverArgs.allowUnrestrictedPaths) {
|
||||
console.warn(
|
||||
'[chrome-devtools-mcp] The connecting client did not negotiate the MCP roots ' +
|
||||
'capability. File-writing tools will be restricted to the OS temp directory. ' +
|
||||
'To restore the previous unrestricted behavior, start the server with ' +
|
||||
'--allow-unrestricted-paths.',
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -146,6 +153,7 @@ export async function createMcpServer(
|
||||
performanceCrux: serverArgs.performanceCrux,
|
||||
allowList: allowlist,
|
||||
blocklist: blocklist,
|
||||
allowUnrestrictedPaths: serverArgs.allowUnrestrictedPaths,
|
||||
});
|
||||
await updateRoots();
|
||||
}
|
||||
|
||||
@@ -361,5 +361,13 @@
|
||||
"EXPERIMENTAL_DATA_FORMAT_TOON",
|
||||
"EXPERIMENTAL_DATA_FORMAT_GCF"
|
||||
]
|
||||
},
|
||||
{
|
||||
"name": "allow_unrestricted_paths_present",
|
||||
"flagType": "boolean"
|
||||
},
|
||||
{
|
||||
"name": "allow_unrestricted_paths",
|
||||
"flagType": "boolean"
|
||||
}
|
||||
]
|
||||
|
||||
+28
-12
@@ -248,17 +248,20 @@ describe('McpContext', () => {
|
||||
sinon.stub(context, 'getNetworkRequestById').returns(mockRequest);
|
||||
sinon.stub(context, 'getNetworkRequestStableId').returns(789);
|
||||
|
||||
// Use os.tmpdir() so validatePath passes on all platforms (macOS tmpdir
|
||||
// is /var/folders/..., not /tmp, so hardcoded /tmp paths are rejected).
|
||||
const reqFilePath = path.join(os.tmpdir(), 'req.txt');
|
||||
const resFilePath = path.join(os.tmpdir(), 'res.txt');
|
||||
|
||||
// We stub NetworkFormatter.from to avoid actual file system writes and verify arguments
|
||||
const fromStub = sinon
|
||||
.stub(NetworkFormatter, 'from')
|
||||
.callsFake(async (_req, opts) => {
|
||||
// Verify we received the file paths
|
||||
assert.strictEqual(opts?.requestFilePath, '/tmp/req.txt');
|
||||
assert.strictEqual(opts?.responseFilePath, '/tmp/res.txt');
|
||||
// Return a dummy formatter that behaves as if it saved files
|
||||
// We need to create a real instance or mock one.
|
||||
// Since constructor is private, we can't easily new it up.
|
||||
// But we can return a mock object.
|
||||
// Verify we received the platform-correct file paths
|
||||
assert.strictEqual(opts?.requestFilePath, reqFilePath);
|
||||
assert.strictEqual(opts?.responseFilePath, resFilePath);
|
||||
// Return fixed strings in toJSONDetailed so the snapshot is stable
|
||||
// across platforms (os.tmpdir() differs on macOS vs Linux/Windows).
|
||||
return {
|
||||
toStringDetailed: () => 'Detailed string',
|
||||
toJSONDetailed: () => ({
|
||||
@@ -269,8 +272,8 @@ describe('McpContext', () => {
|
||||
});
|
||||
|
||||
response.attachNetworkRequest(789, {
|
||||
requestFilePath: '/tmp/req.txt',
|
||||
responseFilePath: '/tmp/res.txt',
|
||||
requestFilePath: reqFilePath,
|
||||
responseFilePath: resFilePath,
|
||||
});
|
||||
const result = await response.handle('test', context);
|
||||
|
||||
@@ -340,10 +343,23 @@ describe('McpContext', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('validatePath allows all paths if roots are undefined (legacy)', async () => {
|
||||
it('validatePath allows all paths if roots are undefined and allowUnrestrictedPaths is set', async () => {
|
||||
await withMcpContext(
|
||||
async (_response, context) => {
|
||||
context.setRoots(undefined);
|
||||
await context.validatePath(path.resolve(os.homedir(), 'anywhere.txt'));
|
||||
},
|
||||
{allowUnrestrictedPaths: true},
|
||||
);
|
||||
});
|
||||
|
||||
it('validatePath denies paths outside tmpdir if roots are undefined and allowUnrestrictedPaths is not set', async () => {
|
||||
await withMcpContext(async (_response, context) => {
|
||||
context.setRoots(undefined);
|
||||
await context.validatePath(path.resolve(os.homedir(), 'anywhere.txt'));
|
||||
// setRoots() never called — simulates a client that skips roots capability.
|
||||
const outsidePath = path.resolve(os.homedir(), 'anywhere.txt');
|
||||
await assert.rejects(context.validatePath(outsidePath), /Access denied/);
|
||||
// Temp dir must still be reachable.
|
||||
await context.validatePath(path.join(os.tmpdir(), 'test.txt'));
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -29,6 +29,8 @@ describe('cli args parsing', () => {
|
||||
usageStatistics: true,
|
||||
'redact-network-headers': false,
|
||||
redactNetworkHeaders: false,
|
||||
'allow-unrestricted-paths': false,
|
||||
allowUnrestrictedPaths: false,
|
||||
};
|
||||
|
||||
it('parses with default args', async () => {
|
||||
|
||||
+8
-2
@@ -244,10 +244,14 @@ describe('e2e', () => {
|
||||
it('allows file access if roots capability is missing', async () => {
|
||||
await withClient(
|
||||
async client => {
|
||||
// Use os.tmpdir() rather than a hardcoded /tmp path.
|
||||
// On macOS, os.tmpdir() returns /var/folders/... (not /tmp), so a
|
||||
// hardcoded /tmp path is outside the allowed root after the
|
||||
// validatePath fix and would be rejected with Access denied.
|
||||
const result = await client.callTool({
|
||||
name: 'take_screenshot',
|
||||
arguments: {
|
||||
filePath: '/tmp/test.png',
|
||||
filePath: path.join(os.tmpdir(), 'test.png'),
|
||||
},
|
||||
});
|
||||
|
||||
@@ -305,7 +309,9 @@ describe('e2e', () => {
|
||||
const result = await client.callTool({
|
||||
name: 'take_screenshot',
|
||||
arguments: {
|
||||
filePath: '/tmp/test.png',
|
||||
// Use os.tmpdir() so validatePath passes on macOS/Windows before
|
||||
// reaching the dialog-blocked check.
|
||||
filePath: path.join(os.tmpdir(), 'test.png'),
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -23,6 +23,23 @@ describe('McpContext Roots', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('should deny paths outside the temp directory when the client never negotiates roots', async () => {
|
||||
await withMcpContext(async (_response, context) => {
|
||||
// setRoots() is intentionally never called here, matching a client
|
||||
// that omits the optional MCP `roots` capability during initialize.
|
||||
const outsidePath = path.resolve(
|
||||
os.homedir(),
|
||||
'a_very_unlikely_path_name_never_negotiated_roots',
|
||||
);
|
||||
await assert.rejects(context.validatePath(outsidePath), /Access denied/);
|
||||
|
||||
const tmpPath = path.join(os.tmpdir(), 'test-file.txt');
|
||||
// The temp directory must remain reachable even with no negotiated
|
||||
// roots, matching the existing "empty roots" behavior above.
|
||||
await context.validatePath(tmpPath);
|
||||
});
|
||||
});
|
||||
|
||||
it('should allow access to os.tmpdir() when other roots are set', async () => {
|
||||
await withMcpContext(async (_response, context) => {
|
||||
const otherRoot = path.resolve(
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
|
||||
import assert from 'node:assert';
|
||||
import fs from 'node:fs/promises';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import {describe, it, afterEach} from 'node:test';
|
||||
|
||||
@@ -37,7 +38,7 @@ describe('screencast', () => {
|
||||
|
||||
await startScreencast().handler(
|
||||
{
|
||||
params: {filePath: '/tmp/test-recording.mp4'},
|
||||
params: {filePath: path.join(os.tmpdir(), 'test-recording.mp4')},
|
||||
page: context.getSelectedMcpPage(),
|
||||
},
|
||||
response,
|
||||
@@ -68,7 +69,7 @@ describe('screencast', () => {
|
||||
|
||||
await startScreencast().handler(
|
||||
{
|
||||
params: {filePath: '/tmp/test-recording.WEBM'},
|
||||
params: {filePath: path.join(os.tmpdir(), 'test-recording.WEBM')},
|
||||
page: context.getSelectedMcpPage(),
|
||||
},
|
||||
response,
|
||||
@@ -91,7 +92,7 @@ describe('screencast', () => {
|
||||
await assert.rejects(
|
||||
startScreencast().handler(
|
||||
{
|
||||
params: {filePath: '/tmp/recording.avi'},
|
||||
params: {filePath: path.join(os.tmpdir(), 'recording.avi')},
|
||||
page: context.getSelectedMcpPage(),
|
||||
},
|
||||
response,
|
||||
@@ -132,7 +133,7 @@ describe('screencast', () => {
|
||||
const mockRecorder = createMockRecorder();
|
||||
context.setScreenRecorder({
|
||||
recorder: mockRecorder as never,
|
||||
filePath: '/tmp/existing.mp4',
|
||||
filePath: path.join(os.tmpdir(), 'existing.mp4'),
|
||||
});
|
||||
|
||||
const selectedPage = context.getSelectedPptrPage();
|
||||
@@ -162,7 +163,7 @@ describe('screencast', () => {
|
||||
await assert.rejects(
|
||||
startScreencast().handler(
|
||||
{
|
||||
params: {filePath: '/tmp/test.mp4'},
|
||||
params: {filePath: path.join(os.tmpdir(), 'test.mp4')},
|
||||
page: context.getSelectedMcpPage(),
|
||||
},
|
||||
response,
|
||||
@@ -243,7 +244,7 @@ describe('screencast', () => {
|
||||
it('stops an active recording and reports the file path', async () => {
|
||||
await withMcpContext(async (response, context) => {
|
||||
const mockRecorder = createMockRecorder();
|
||||
const filePath = '/tmp/test-recording.mp4';
|
||||
const filePath = path.join(os.tmpdir(), 'test-recording.mp4');
|
||||
context.setScreenRecorder({
|
||||
recorder: mockRecorder as never,
|
||||
filePath,
|
||||
@@ -260,7 +261,7 @@ describe('screencast', () => {
|
||||
assert.ok(
|
||||
response.responseLines
|
||||
.join('\n')
|
||||
.includes('stopped and saved to /tmp/test-recording.mp4'),
|
||||
.includes(`stopped and saved to ${filePath}`),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -271,7 +272,7 @@ describe('screencast', () => {
|
||||
mockRecorder.stop.rejects(new Error('ffmpeg process error'));
|
||||
context.setScreenRecorder({
|
||||
recorder: mockRecorder as never,
|
||||
filePath: '/tmp/test.mp4',
|
||||
filePath: path.join(os.tmpdir(), 'test.mp4'),
|
||||
});
|
||||
|
||||
await assert.rejects(
|
||||
|
||||
@@ -121,6 +121,7 @@ export async function withMcpContext(
|
||||
args?: string[];
|
||||
blockedUrlPattern?: string[];
|
||||
allowedUrlPattern?: string[];
|
||||
allowUnrestrictedPaths?: boolean;
|
||||
} = {},
|
||||
args: Partial<ParsedArguments> = {},
|
||||
) {
|
||||
@@ -138,6 +139,7 @@ export async function withMcpContext(
|
||||
performanceCrux: options.performanceCrux ?? true,
|
||||
allowList: options.allowedUrlPattern,
|
||||
blocklist: options.blockedUrlPattern,
|
||||
allowUnrestrictedPaths: options.allowUnrestrictedPaths ?? false,
|
||||
},
|
||||
Locator,
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user