Skip to content

Commit 176eb69

Browse files
OrKoNOrKoN
authored
fix: use realpath for MCP roots validation (#2127)
1 parent 2eab509 commit 176eb69

15 files changed

Lines changed: 270 additions & 64 deletions

‎src/McpContext.ts‎

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

77
import fs from 'node:fs/promises';
8+
import fsPromises from 'node:fs/promises';
89
import os from 'node:os';
910
import path from 'node:path';
1011
import {fileURLToPath, pathToFileURL} from 'node:url';
@@ -45,7 +46,11 @@ import type {
4546
GeolocationOptions,
4647
ExtensionServiceWorker,
4748
} from './types.js';
48-
import {ensureExtension, getTempFilePath} from './utils/files.js';
49+
import {
50+
ensureExtension,
51+
getTempFilePath,
52+
resolveCanonicalPath,
53+
} from './utils/files.js';
4954
import {getNetworkMultiplierFromString} from './WaitForHelper.js';
5055

5156
interface McpContextOptions {
@@ -175,27 +180,58 @@ export class McpContext implements Context {
175180
this.#roots = roots;
176181
}
177182

178-
validatePath(filePath?: string): void {
183+
async validatePath(filePath?: string): Promise<void> {
179184
if (filePath === undefined) {
180185
return;
181186
}
182187
const roots = this.roots();
183188
if (roots === undefined) {
184189
return;
185190
}
186-
const absolutePath = path.resolve(filePath);
191+
192+
let canonicalPath: string;
193+
194+
try {
195+
canonicalPath = await resolveCanonicalPath(filePath);
196+
} catch (err) {
197+
const errMsg = err instanceof Error ? err.message : String(err);
198+
console.error(
199+
`[MCP Context] Error resolving real path for ${filePath}: ${errMsg}`,
200+
);
201+
throw new Error(
202+
`Access denied: Cannot resolve base path for ${filePath}.`,
203+
);
204+
}
205+
206+
let allowed = false;
187207
for (const root of roots) {
188-
const rootPath = path.resolve(fileURLToPath(root.uri));
189-
if (
190-
absolutePath === rootPath ||
191-
absolutePath.startsWith(rootPath + path.sep)
192-
) {
193-
return;
208+
try {
209+
const rootPathUri = root.uri;
210+
const rootPath = path.resolve(fileURLToPath(rootPathUri));
211+
const canonicalRoot = await fsPromises.realpath(rootPath);
212+
213+
if (
214+
canonicalPath === canonicalRoot ||
215+
canonicalPath.startsWith(canonicalRoot + path.sep)
216+
) {
217+
allowed = true;
218+
break;
219+
}
220+
} catch (rootErr) {
221+
const errMsg =
222+
rootErr instanceof Error ? rootErr.message : String(rootErr);
223+
console.warn(
224+
`[MCP Context] Could not resolve configured root ${root.uri}: ${errMsg}`,
225+
);
226+
// Skip this root if it cannot be resolved.
194227
}
195228
}
196-
throw new Error(
197-
`Access denied: path ${filePath} is not within any of the workspace roots ${JSON.stringify(roots)}.`,
198-
);
229+
230+
if (!allowed) {
231+
throw new Error(
232+
`Access denied: path ${filePath} (canonical: ${canonicalPath}) is not within any of the configured workspace roots.`,
233+
);
234+
}
199235
}
200236

201237
resolveCdpRequestId(page: McpPage, cdpRequestId: string): number | undefined {
@@ -708,7 +744,7 @@ export class McpContext implements Context {
708744
filename: string,
709745
): Promise<{filepath: string}> {
710746
const filepath = await getTempFilePath(filename);
711-
this.validatePath(filepath);
747+
await this.validatePath(filepath);
712748
try {
713749
await fs.writeFile(filepath, data);
714750
} catch (err) {
@@ -722,7 +758,7 @@ export class McpContext implements Context {
722758
clientProvidedFilePath: string,
723759
extension: SupportedExtensions,
724760
): Promise<{filename: string}> {
725-
this.validatePath(clientProvidedFilePath);
761+
await this.validatePath(clientProvidedFilePath);
726762
try {
727763
const filePath = ensureExtension(
728764
path.resolve(clientProvidedFilePath),
@@ -794,7 +830,7 @@ export class McpContext implements Context {
794830
}
795831

796832
async installExtension(extensionPath: string): Promise<string> {
797-
this.validatePath(extensionPath);
833+
await this.validatePath(extensionPath);
798834
const id = await this.browser.installExtension(extensionPath);
799835
return id;
800836
}
@@ -825,36 +861,37 @@ export class McpContext implements Context {
825861
async getHeapSnapshotAggregates(
826862
filePath: string,
827863
): Promise<Record<string, AggregatedInfoWithId>> {
828-
this.validatePath(filePath);
864+
await this.validatePath(filePath);
829865
return await this.#heapSnapshotManager.getAggregates(filePath);
830866
}
831867

832868
async getHeapSnapshotStats(
833869
filePath: string,
834870
): Promise<DevTools.HeapSnapshotModel.HeapSnapshotModel.Statistics> {
835-
this.validatePath(filePath);
871+
await this.validatePath(filePath);
836872
return await this.#heapSnapshotManager.getStats(filePath);
837873
}
838874

839875
async getHeapSnapshotStaticData(
840876
filePath: string,
841877
): Promise<DevTools.HeapSnapshotModel.HeapSnapshotModel.StaticData | null> {
842-
this.validatePath(filePath);
878+
await this.validatePath(filePath);
843879
return await this.#heapSnapshotManager.getStaticData(filePath);
844880
}
845881

846882
async getHeapSnapshotNodesById(
847883
filePath: string,
848884
id: number,
849885
): Promise<DevTools.HeapSnapshotModel.HeapSnapshotModel.ItemsRange> {
850-
this.validatePath(filePath);
886+
await this.validatePath(filePath);
851887
return await this.#heapSnapshotManager.getNodesById(filePath, id);
852888
}
853889

854890
async getHeapSnapshotRetainers(
855891
filePath: string,
856892
nodeId: number,
857893
): Promise<DevTools.HeapSnapshotModel.HeapSnapshotModel.ItemsRange> {
894+
await this.validatePath(filePath);
858895
return await this.#heapSnapshotManager.getRetainers(filePath, nodeId);
859896
}
860897
}

‎src/tools/ToolDefinition.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ export type SupportedExtensions =
173173
* Only add methods used by tools/*.
174174
*/
175175
export type Context = Readonly<{
176-
validatePath(filePath?: string): void;
176+
validatePath(filePath?: string): Promise<void>;
177177
isRunningPerformanceTrace(): boolean;
178178
setIsRunningPerformanceTrace(x: boolean): void;
179179
isCruxEnabled(): boolean;

‎src/tools/input.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ export const uploadFile = definePageTool({
463463
blockedByDialog: true,
464464
handler: async (request, response, context) => {
465465
const {uid, filePath} = request.params;
466-
context.validatePath(filePath);
466+
await context.validatePath(filePath);
467467
const handle = (await request.page.getElementByUid(
468468
uid,
469469
)) as ElementHandle<HTMLInputElement>;

‎src/tools/lighthouse.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export const lighthouseAudit = definePageTool({
5959
outputDirPath,
6060
} = request.params;
6161

62-
context.validatePath(outputDirPath);
62+
await context.validatePath(outputDirPath);
6363

6464
const flags: Flags = {
6565
onlyCategories: categories,

‎src/tools/memory.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export const takeHeapSnapshot = definePageTool({
2525
blockedByDialog: true,
2626
handler: async (request, response, context) => {
2727
const page = request.page;
28-
context.validatePath(request.params.filePath);
28+
await context.validatePath(request.params.filePath);
2929

3030
await page.pptrPage.captureHeapSnapshot({
3131
path: ensureExtension(request.params.filePath, '.heapsnapshot'),
@@ -51,7 +51,7 @@ export const getHeapSnapshotSummary = defineTool({
5151
},
5252
blockedByDialog: false,
5353
handler: async (request, response, context) => {
54-
context.validatePath(request.params.filePath);
54+
await context.validatePath(request.params.filePath);
5555
const stats = await context.getHeapSnapshotStats(request.params.filePath);
5656
const staticData = await context.getHeapSnapshotStaticData(
5757
request.params.filePath,
@@ -83,7 +83,7 @@ export const getHeapSnapshotDetails = defineTool({
8383
},
8484
blockedByDialog: false,
8585
handler: async (request, response, context) => {
86-
context.validatePath(request.params.filePath);
86+
await context.validatePath(request.params.filePath);
8787
const aggregates = await context.getHeapSnapshotAggregates(
8888
request.params.filePath,
8989
);
@@ -112,7 +112,7 @@ export const getHeapSnapshotClassNodes = defineTool({
112112
},
113113
blockedByDialog: false,
114114
handler: async (request, response, context) => {
115-
context.validatePath(request.params.filePath);
115+
await context.validatePath(request.params.filePath);
116116
const nodes = await context.getHeapSnapshotNodesById(
117117
request.params.filePath,
118118
request.params.id,
@@ -142,7 +142,7 @@ export const getHeapSnapshotRetainers = defineTool({
142142
pageSize: zod.number().optional().describe('The page size for pagination.'),
143143
},
144144
handler: async (request, response, context) => {
145-
context.validatePath(request.params.filePath);
145+
await context.validatePath(request.params.filePath);
146146

147147
const retainers = await context.getHeapSnapshotRetainers(
148148
request.params.filePath,

‎src/tools/network.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ export const getNetworkRequest = definePageTool({
116116
},
117117
blockedByDialog: true,
118118
handler: async (request, response, context) => {
119-
context.validatePath(request.params.requestFilePath);
120-
context.validatePath(request.params.responseFilePath);
119+
await context.validatePath(request.params.requestFilePath);
120+
await context.validatePath(request.params.responseFilePath);
121121
if (request.params.reqid) {
122122
response.attachNetworkRequest(request.params.reqid, {
123123
requestFilePath: request.params.requestFilePath,

‎src/tools/performance.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export const startTrace = definePageTool({
4949
},
5050
blockedByDialog: true,
5151
handler: async (request, response, context) => {
52-
context.validatePath(request.params.filePath);
52+
await context.validatePath(request.params.filePath);
5353
if (context.isRunningPerformanceTrace()) {
5454
response.appendResponseLine(
5555
'Error: a performance trace is already running. Use performance_stop_trace to stop it. Only one trace can be running at any given time.',
@@ -128,7 +128,7 @@ export const stopTrace = definePageTool({
128128
},
129129
blockedByDialog: true,
130130
handler: async (request, response, context) => {
131-
context.validatePath(request.params.filePath);
131+
await context.validatePath(request.params.filePath);
132132
if (!context.isRunningPerformanceTrace()) {
133133
return;
134134
}

‎src/tools/screencast.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export const startScreencast = definePageTool(args => ({
4040
},
4141
blockedByDialog: false,
4242
handler: async (request, response, context) => {
43-
context.validatePath(request.params.filePath);
43+
await context.validatePath(request.params.filePath);
4444
if (context.getScreenRecorder() !== null) {
4545
response.appendResponseLine(
4646
'Error: a screencast recording is already in progress. Use screencast_stop to stop it before starting a new one.',

‎src/tools/screenshot.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export const screenshot = definePageTool({
5252
},
5353
blockedByDialog: true,
5454
handler: async (request, response, context) => {
55-
context.validatePath(request.params.filePath);
55+
await context.validatePath(request.params.filePath);
5656
if (request.params.uid && request.params.fullPage) {
5757
throw new Error('Providing both "uid" and "fullPage" is not allowed.');
5858
}

‎src/tools/script.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ Example with arguments: \`(el) => {
8181
filePath,
8282
} = request.params;
8383

84-
context.validatePath(filePath);
84+
await context.validatePath(filePath);
8585

8686
if (cliArgs?.categoryExtensions && serviceWorkerId) {
8787
if (uidArgs && uidArgs.length > 0) {

0 commit comments

Comments
 (0)