Skip to content

Commit a90378a

Browse files
authored
fix: always allow tmpdir access with client roots (#1984)
This PR changes the implementation to allow access to tmpdir even if the client is not configured it explicitly because several tools default to the tmpdir for outputs. Many clients like gemini-cli/claude do not configure the tmp dir as a root by default.
1 parent b2b3e99 commit a90378a

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)