Skip to content

Commit 6e56c02

Browse files
authored
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.
1 parent ff53b7b commit 6e56c02

12 files changed

Lines changed: 113 additions & 68 deletions

README.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,11 @@ The Chrome DevTools MCP server supports the following configuration option:
754754
- **Type:** boolean
755755
- **Default:** `false`
756756

757+
- **`--allowUnrestrictedPaths`/ `--allow-unrestricted-paths`**
758+
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.
759+
- **Type:** boolean
760+
- **Default:** `false`
761+
757762
<!-- END AUTO GENERATED OPTIONS -->
758763

759764
Pass them via the `args` property in the JSON configuration. For example:

package-lock.json

Lines changed: 0 additions & 39 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/McpContext.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ interface McpContextOptions {
6969
allowList?: string[];
7070
// The block list of URL patterns to block loading resources.
7171
blocklist?: string[];
72+
// Whether to skip path validation when the client did not negotiate the roots
73+
// capability. When false (default), file-writing tools are restricted to the
74+
// OS temp directory. When true, the previous permissive behavior is restored.
75+
allowUnrestrictedPaths?: boolean;
7276
}
7377

7478
const DEFAULT_TIMEOUT = 5_000;
@@ -110,6 +114,7 @@ export class McpContext implements Context {
110114
#options: McpContextOptions;
111115
#heapSnapshotManager = new HeapSnapshotManager();
112116
#roots: Root[] | undefined = undefined;
117+
#allowUnrestrictedPaths: boolean;
113118

114119
private constructor(
115120
browser: Browser,
@@ -127,6 +132,7 @@ export class McpContext implements Context {
127132
this.logger = logger;
128133
this.#locatorClass = locatorClass;
129134
this.#options = options;
135+
this.#allowUnrestrictedPaths = options.allowUnrestrictedPaths ?? false;
130136

131137
this.#networkCollector = new NetworkCollector(this.browser);
132138

@@ -185,12 +191,9 @@ export class McpContext implements Context {
185191
return context;
186192
}
187193

188-
roots(): Root[] | undefined {
189-
if (this.#roots === undefined) {
190-
return undefined;
191-
}
194+
roots(): Root[] {
192195
return [
193-
...this.#roots,
196+
...(this.#roots ?? []),
194197
{
195198
uri: pathToFileURL(os.tmpdir()).href,
196199
name: 'temp',
@@ -206,10 +209,17 @@ export class McpContext implements Context {
206209
if (filePath === undefined) {
207210
return;
208211
}
209-
const roots = this.roots();
210-
if (roots === undefined) {
212+
// If the client never negotiated roots and the operator has explicitly
213+
// opted into unrestricted access via --allow-unrestricted-paths, restore
214+
// the previous permissive behavior and skip validation.
215+
if (this.#roots === undefined && this.#allowUnrestrictedPaths) {
211216
return;
212217
}
218+
// roots() always returns at least the temp directory, even if the
219+
// connecting client never negotiated the optional `roots` capability.
220+
// Path validation must not be skipped just because no workspace roots
221+
// were configured.
222+
const roots = this.roots();
213223

214224
let canonicalPath: string;
215225

src/bin/chrome-devtools-mcp-cli-options.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,15 @@ export const cliOptions = {
361361
'If true, redacts some of the network headers considered sensitive before returning to the client.',
362362
default: false,
363363
},
364+
allowUnrestrictedPaths: {
365+
type: 'boolean',
366+
default: false,
367+
describe:
368+
'If set, disables the default path restriction that applies when the MCP client does not negotiate ' +
369+
'the roots capability. By default, file-writing tools are restricted to the OS temp directory when ' +
370+
'no roots are configured. Use this only when connecting a trusted local client that does not implement ' +
371+
'MCP roots and requires access to paths outside the temp directory.',
372+
},
364373
} satisfies Record<string, YargsOptions>;
365374

366375
export type ParsedArguments = ReturnType<typeof parseArguments>;

src/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,13 @@ export async function createMcpServer(
8686
void updateRoots();
8787
},
8888
);
89+
} else if (!serverArgs.allowUnrestrictedPaths) {
90+
console.warn(
91+
'[chrome-devtools-mcp] The connecting client did not negotiate the MCP roots ' +
92+
'capability. File-writing tools will be restricted to the OS temp directory. ' +
93+
'To restore the previous unrestricted behavior, start the server with ' +
94+
'--allow-unrestricted-paths.',
95+
);
8996
}
9097
};
9198

@@ -146,6 +153,7 @@ export async function createMcpServer(
146153
performanceCrux: serverArgs.performanceCrux,
147154
allowList: allowlist,
148155
blocklist: blocklist,
156+
allowUnrestrictedPaths: serverArgs.allowUnrestrictedPaths,
149157
});
150158
await updateRoots();
151159
}

src/telemetry/flag_usage_metrics.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,5 +361,13 @@
361361
"EXPERIMENTAL_DATA_FORMAT_TOON",
362362
"EXPERIMENTAL_DATA_FORMAT_GCF"
363363
]
364+
},
365+
{
366+
"name": "allow_unrestricted_paths_present",
367+
"flagType": "boolean"
368+
},
369+
{
370+
"name": "allow_unrestricted_paths",
371+
"flagType": "boolean"
364372
}
365373
]

tests/McpContext.test.ts

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -248,17 +248,20 @@ describe('McpContext', () => {
248248
sinon.stub(context, 'getNetworkRequestById').returns(mockRequest);
249249
sinon.stub(context, 'getNetworkRequestStableId').returns(789);
250250

251+
// Use os.tmpdir() so validatePath passes on all platforms (macOS tmpdir
252+
// is /var/folders/..., not /tmp, so hardcoded /tmp paths are rejected).
253+
const reqFilePath = path.join(os.tmpdir(), 'req.txt');
254+
const resFilePath = path.join(os.tmpdir(), 'res.txt');
255+
251256
// We stub NetworkFormatter.from to avoid actual file system writes and verify arguments
252257
const fromStub = sinon
253258
.stub(NetworkFormatter, 'from')
254259
.callsFake(async (_req, opts) => {
255-
// Verify we received the file paths
256-
assert.strictEqual(opts?.requestFilePath, '/tmp/req.txt');
257-
assert.strictEqual(opts?.responseFilePath, '/tmp/res.txt');
258-
// Return a dummy formatter that behaves as if it saved files
259-
// We need to create a real instance or mock one.
260-
// Since constructor is private, we can't easily new it up.
261-
// But we can return a mock object.
260+
// Verify we received the platform-correct file paths
261+
assert.strictEqual(opts?.requestFilePath, reqFilePath);
262+
assert.strictEqual(opts?.responseFilePath, resFilePath);
263+
// Return fixed strings in toJSONDetailed so the snapshot is stable
264+
// across platforms (os.tmpdir() differs on macOS vs Linux/Windows).
262265
return {
263266
toStringDetailed: () => 'Detailed string',
264267
toJSONDetailed: () => ({
@@ -269,8 +272,8 @@ describe('McpContext', () => {
269272
});
270273

271274
response.attachNetworkRequest(789, {
272-
requestFilePath: '/tmp/req.txt',
273-
responseFilePath: '/tmp/res.txt',
275+
requestFilePath: reqFilePath,
276+
responseFilePath: resFilePath,
274277
});
275278
const result = await response.handle('test', context);
276279

@@ -340,10 +343,23 @@ describe('McpContext', () => {
340343
});
341344
});
342345

343-
it('validatePath allows all paths if roots are undefined (legacy)', async () => {
346+
it('validatePath allows all paths if roots are undefined and allowUnrestrictedPaths is set', async () => {
347+
await withMcpContext(
348+
async (_response, context) => {
349+
context.setRoots(undefined);
350+
await context.validatePath(path.resolve(os.homedir(), 'anywhere.txt'));
351+
},
352+
{allowUnrestrictedPaths: true},
353+
);
354+
});
355+
356+
it('validatePath denies paths outside tmpdir if roots are undefined and allowUnrestrictedPaths is not set', async () => {
344357
await withMcpContext(async (_response, context) => {
345-
context.setRoots(undefined);
346-
await context.validatePath(path.resolve(os.homedir(), 'anywhere.txt'));
358+
// setRoots() never called — simulates a client that skips roots capability.
359+
const outsidePath = path.resolve(os.homedir(), 'anywhere.txt');
360+
await assert.rejects(context.validatePath(outsidePath), /Access denied/);
361+
// Temp dir must still be reachable.
362+
await context.validatePath(path.join(os.tmpdir(), 'test.txt'));
347363
});
348364
});
349365

tests/cli.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ describe('cli args parsing', () => {
2929
usageStatistics: true,
3030
'redact-network-headers': false,
3131
redactNetworkHeaders: false,
32+
'allow-unrestricted-paths': false,
33+
allowUnrestrictedPaths: false,
3234
};
3335

3436
it('parses with default args', async () => {

tests/index.test.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,10 +244,14 @@ describe('e2e', () => {
244244
it('allows file access if roots capability is missing', async () => {
245245
await withClient(
246246
async client => {
247+
// Use os.tmpdir() rather than a hardcoded /tmp path.
248+
// On macOS, os.tmpdir() returns /var/folders/... (not /tmp), so a
249+
// hardcoded /tmp path is outside the allowed root after the
250+
// validatePath fix and would be rejected with Access denied.
247251
const result = await client.callTool({
248252
name: 'take_screenshot',
249253
arguments: {
250-
filePath: '/tmp/test.png',
254+
filePath: path.join(os.tmpdir(), 'test.png'),
251255
},
252256
});
253257

@@ -305,7 +309,9 @@ describe('e2e', () => {
305309
const result = await client.callTool({
306310
name: 'take_screenshot',
307311
arguments: {
308-
filePath: '/tmp/test.png',
312+
// Use os.tmpdir() so validatePath passes on macOS/Windows before
313+
// reaching the dialog-blocked check.
314+
filePath: path.join(os.tmpdir(), 'test.png'),
309315
},
310316
});
311317

tests/roots.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,23 @@ describe('McpContext Roots', () => {
2323
});
2424
});
2525

26+
it('should deny paths outside the temp directory when the client never negotiates roots', async () => {
27+
await withMcpContext(async (_response, context) => {
28+
// setRoots() is intentionally never called here, matching a client
29+
// that omits the optional MCP `roots` capability during initialize.
30+
const outsidePath = path.resolve(
31+
os.homedir(),
32+
'a_very_unlikely_path_name_never_negotiated_roots',
33+
);
34+
await assert.rejects(context.validatePath(outsidePath), /Access denied/);
35+
36+
const tmpPath = path.join(os.tmpdir(), 'test-file.txt');
37+
// The temp directory must remain reachable even with no negotiated
38+
// roots, matching the existing "empty roots" behavior above.
39+
await context.validatePath(tmpPath);
40+
});
41+
});
42+
2643
it('should allow access to os.tmpdir() when other roots are set', async () => {
2744
await withMcpContext(async (_response, context) => {
2845
const otherRoot = path.resolve(

0 commit comments

Comments
 (0)