Skip to content

Commit 4b4df0e

Browse files
committed
fix: always allow tmpdir access in client roots
1 parent 7ee5e86 commit 4b4df0e

4 files changed

Lines changed: 84 additions & 10 deletions

File tree

src/McpContext.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55
*/
66

77
import fs from 'node:fs/promises';
8+
import os from 'node:os';
89
import path from 'node:path';
9-
import {fileURLToPath} from 'node:url';
10+
import {fileURLToPath, pathToFileURL} from 'node:url';
1011

1112
import type {TargetUniverse} from './DevtoolsUtils.js';
1213
import {UniverseManager} from './DevtoolsUtils.js';
@@ -158,7 +159,16 @@ export class McpContext implements Context {
158159
}
159160

160161
roots(): Root[] | undefined {
161-
return this.#roots;
162+
if (this.#roots === undefined) {
163+
return undefined;
164+
}
165+
return [
166+
...this.#roots,
167+
{
168+
uri: pathToFileURL(os.tmpdir()).href,
169+
name: 'temp',
170+
},
171+
];
162172
}
163173

164174
setRoots(roots: Root[] | undefined): void {

tests/McpContext.test.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66

77
import assert from 'node:assert';
8+
import os from 'node:os';
89
import path from 'node:path';
910
import {afterEach, describe, it} from 'node:test';
1011
import {pathToFileURL} from 'node:url';
@@ -220,13 +221,21 @@ describe('McpContext', () => {
220221
await withMcpContext(async (_response, context) => {
221222
const roots = [{uri: 'file:///test', name: 'test'}];
222223
context.setRoots(roots);
223-
assert.deepEqual(context.roots(), roots);
224+
const actualRoots = context.roots();
225+
assert.ok(
226+
actualRoots?.some(r => r.name === 'test'),
227+
'Should contain the set root',
228+
);
229+
assert.ok(
230+
actualRoots?.some(r => r.name === 'temp'),
231+
'Should contain the temp root',
232+
);
224233
});
225234
});
226235

227236
it('validatePath allows paths within roots', async () => {
228237
await withMcpContext(async (_response, context) => {
229-
const workspacePath = path.resolve('/tmp/workspace');
238+
const workspacePath = path.resolve(os.homedir(), 'workspace-test');
230239
const roots = [
231240
{uri: pathToFileURL(workspacePath).href, name: 'workspace'},
232241
];
@@ -235,24 +244,28 @@ describe('McpContext', () => {
235244
context.validatePath(path.join(workspacePath, 'test.txt'));
236245
context.validatePath(workspacePath);
237246

238-
// Invalid path outside root
239-
const outsidePath = path.resolve('/tmp/outside.txt');
247+
// Invalid path outside root and outside temp dir
248+
const outsidePath = path.resolve(os.homedir(), 'outside-test.txt');
240249
assert.throws(() => context.validatePath(outsidePath), /Access denied/);
241250
});
242251
});
243252

244253
it('validatePath allows all paths if roots are undefined (legacy)', async () => {
245254
await withMcpContext(async (_response, context) => {
246255
context.setRoots(undefined);
247-
context.validatePath(path.resolve('/tmp/anywhere.txt'));
256+
context.validatePath(path.resolve(os.homedir(), 'anywhere.txt'));
248257
});
249258
});
250259

251-
it('validatePath denies all paths if roots list is empty', async () => {
260+
it('validatePath denies paths outside os.tmpdir() if roots list is empty', async () => {
252261
await withMcpContext(async (_response, context) => {
253262
context.setRoots([]);
263+
// Should allow temp dir
264+
context.validatePath(path.join(os.tmpdir(), 'test.txt'));
265+
266+
// Should deny outside temp dir
254267
assert.throws(
255-
() => context.validatePath(path.resolve('/tmp/anywhere.txt')),
268+
() => context.validatePath(path.resolve(os.homedir(), 'anywhere.txt')),
256269
/Access denied/,
257270
);
258271
});

tests/index.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
import assert from 'node:assert';
88
import fs from 'node:fs';
9+
import os from 'node:os';
10+
import path from 'node:path';
911
import {describe, it} from 'node:test';
1012

1113
import {Client} from '@modelcontextprotocol/sdk/client/index.js';
@@ -208,7 +210,7 @@ describe('e2e', () => {
208210
const result = await client.callTool({
209211
name: 'take_screenshot',
210212
arguments: {
211-
filePath: '/tmp/test.png',
213+
filePath: path.resolve(os.homedir(), 'test.png'),
212214
},
213215
});
214216

tests/roots.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* @license
3+
* Copyright 2026 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import assert from 'node:assert';
8+
import os from 'node:os';
9+
import path from 'node:path';
10+
import {describe, it} from 'node:test';
11+
import {pathToFileURL} from 'node:url';
12+
13+
import {withMcpContext} from './utils.js';
14+
15+
describe('McpContext Roots', () => {
16+
it('should allow access to os.tmpdir() even if roots are empty', async () => {
17+
await withMcpContext(async (_response, context) => {
18+
context.setRoots([]);
19+
const tmpPath = path.join(os.tmpdir(), 'test-file.txt');
20+
// This should not throw
21+
context.validatePath(tmpPath);
22+
});
23+
});
24+
25+
it('should allow access to os.tmpdir() when other roots are set', async () => {
26+
await withMcpContext(async (_response, context) => {
27+
const otherRoot = path.resolve(
28+
os.tmpdir(),
29+
'..',
30+
'other_workspace_root_for_test',
31+
);
32+
context.setRoots([{uri: pathToFileURL(otherRoot).href, name: 'other'}]);
33+
34+
const tmpPath = path.join(os.tmpdir(), 'test-file.txt');
35+
// This should not throw.
36+
context.validatePath(tmpPath);
37+
38+
// Other root should also be allowed.
39+
context.validatePath(path.join(otherRoot, 'file.txt'));
40+
41+
// Outside should still be denied. Use a path that is definitely not a root or temp dir.
42+
const outsidePath = path.resolve(
43+
os.homedir(),
44+
'a_very_unlikely_path_name_12345',
45+
);
46+
assert.throws(() => context.validatePath(outsidePath), /Access denied/);
47+
});
48+
});
49+
});

0 commit comments

Comments
 (0)