diff --git a/src/ToolHandler.ts b/src/ToolHandler.ts index 80950c97b..9d7a82ed0 100644 --- a/src/ToolHandler.ts +++ b/src/ToolHandler.ts @@ -142,6 +142,44 @@ function buildUnknownArgumentsMessage( return `Unknown ${unknownLabel} for tool "${toolName}": ${formatArgumentNames(unknownArgumentNames)}. ${expectedArguments} ${correction} and retry.`; } +function getFieldPathValue( + params: Record, + fieldPath: string, +): unknown { + let value: unknown = params; + for (const field of fieldPath.split('.')) { + if (!field) { + throw new Error(`Invalid empty field in file path annotation.`); + } + if ( + value === null || + typeof value !== 'object' || + !Object.hasOwn(value, field) + ) { + return undefined; + } + value = Object.getOwnPropertyDescriptor(value, field)?.value; + } + return value; +} + +async function validateFilePathFields( + context: McpContext, + params: Record, + filePathFields: string[], +): Promise { + for (const fieldPath of filePathFields) { + const value = getFieldPathValue(params, fieldPath); + if (value === undefined) { + continue; + } + if (typeof value !== 'string') { + throw new Error(`File path field "${fieldPath}" must be a string.`); + } + await context.validatePath(value); + } +} + export class ToolHandler { readonly inputSchema: zod.ZodRawShape; readonly registeredInputSchema: zod.ZodTypeAny; @@ -214,6 +252,11 @@ export class ToolHandler { const context = await this.getContext(); logger(`${this.tool.name} context: resolved`); await context.detectOpenDevToolsWindows(); + await validateFilePathFields( + context, + params, + this.tool.annotations.filePathFields, + ); const response = this.serverArgs.slim ? new SlimMcpResponse(this.serverArgs) : new McpResponse(this.serverArgs); diff --git a/src/tools/ToolDefinition.ts b/src/tools/ToolDefinition.ts index dd8cba89b..3bb7ca4b7 100644 --- a/src/tools/ToolDefinition.ts +++ b/src/tools/ToolDefinition.ts @@ -40,6 +40,11 @@ export interface BaseToolDefinition< annotations: { title?: string; category: ToolCategory; + /** + * Request parameter field paths that need workspace root validation before + * the tool handler runs. + */ + filePathFields: string[]; /** * If true, the tool does not modify its environment. */ diff --git a/src/tools/console.ts b/src/tools/console.ts index c0c0fd6c6..d92e0d46c 100644 --- a/src/tools/console.ts +++ b/src/tools/console.ts @@ -46,6 +46,7 @@ export const listConsoleMessages = definePageTool(cliArgs => { annotations: { category: ToolCategory.DEBUGGING, readOnlyHint: true, + filePathFields: [], }, schema: { pageSize: zod @@ -96,6 +97,7 @@ export const getConsoleMessage = definePageTool({ annotations: { category: ToolCategory.DEBUGGING, readOnlyHint: true, + filePathFields: [], }, schema: { msgid: zod diff --git a/src/tools/emulation.ts b/src/tools/emulation.ts index 519ad5a10..26d52cd54 100644 --- a/src/tools/emulation.ts +++ b/src/tools/emulation.ts @@ -51,6 +51,7 @@ export const emulate = definePageTool({ annotations: { category: ToolCategory.EMULATION, readOnlyHint: false, + filePathFields: [], }, schema: { networkConditions: zod diff --git a/src/tools/extensions.ts b/src/tools/extensions.ts index f0656deda..b30a54d23 100644 --- a/src/tools/extensions.ts +++ b/src/tools/extensions.ts @@ -15,6 +15,7 @@ export const installExtension = defineTool({ annotations: { category: ToolCategory.EXTENSIONS, readOnlyHint: false, + filePathFields: ['path'], }, schema: { path: zod @@ -24,7 +25,6 @@ export const installExtension = defineTool({ blockedByDialog: false, handler: async (request, response, context) => { const {path} = request.params; - await context.validatePath(path); const id = await context.installExtension(path); response.appendResponseLine(`Extension installed. Id: ${id}`); }, @@ -36,6 +36,7 @@ export const uninstallExtension = defineTool({ annotations: { category: ToolCategory.EXTENSIONS, readOnlyHint: false, + filePathFields: [], }, schema: { id: zod.string().describe('ID of the extension to uninstall.'), @@ -55,6 +56,7 @@ export const listExtensions = defineTool({ annotations: { category: ToolCategory.EXTENSIONS, readOnlyHint: true, + filePathFields: [], }, schema: {}, blockedByDialog: false, @@ -69,6 +71,7 @@ export const reloadExtension = defineTool({ annotations: { category: ToolCategory.EXTENSIONS, readOnlyHint: false, + filePathFields: [], }, schema: { id: zod.string().describe('ID of the extension to reload.'), @@ -92,6 +95,7 @@ export const triggerExtensionAction = defineTool({ annotations: { category: ToolCategory.EXTENSIONS, readOnlyHint: false, + filePathFields: [], }, schema: { id: zod.string().describe('ID of the extension to trigger the action for.'), diff --git a/src/tools/input.ts b/src/tools/input.ts index 7ff11e40f..a7e5c54c8 100644 --- a/src/tools/input.ts +++ b/src/tools/input.ts @@ -92,6 +92,7 @@ export const click = definePageTool({ annotations: { category: ToolCategory.INPUT, readOnlyHint: false, + filePathFields: [], }, schema: { uid: zod @@ -145,6 +146,7 @@ export const clickAt = definePageTool({ annotations: { category: ToolCategory.INPUT, readOnlyHint: false, + filePathFields: [], conditions: ['experimentalVision'], }, schema: { @@ -179,6 +181,7 @@ export const hover = definePageTool({ annotations: { category: ToolCategory.INPUT, readOnlyHint: false, + filePathFields: [], }, schema: { uid: zod @@ -301,6 +304,7 @@ export const fill = definePageTool({ annotations: { category: ToolCategory.INPUT, readOnlyHint: false, + filePathFields: [], }, schema: { uid: zod @@ -340,6 +344,7 @@ export const typeText = definePageTool({ annotations: { category: ToolCategory.INPUT, readOnlyHint: false, + filePathFields: [], }, schema: { text: zod.string().describe('The text to type'), @@ -369,6 +374,7 @@ export const drag = definePageTool({ annotations: { category: ToolCategory.INPUT, readOnlyHint: false, + filePathFields: [], }, schema: { from_uid: zod.string().describe('The uid of the element to drag'), @@ -405,6 +411,7 @@ export const fillForm = definePageTool({ annotations: { category: ToolCategory.INPUT, readOnlyHint: false, + filePathFields: [], }, schema: { elements: zod @@ -450,6 +457,7 @@ export const uploadFile = definePageTool({ annotations: { category: ToolCategory.INPUT, readOnlyHint: false, + filePathFields: ['filePath'], }, schema: { uid: zod @@ -461,9 +469,8 @@ export const uploadFile = definePageTool({ includeSnapshot: includeSnapshotSchema, }, blockedByDialog: true, - handler: async (request, response, context) => { + handler: async (request, response, _context) => { const {uid, filePath} = request.params; - await context.validatePath(filePath); const handle = (await request.page.getElementByUid( uid, )) as ElementHandle; @@ -502,6 +509,7 @@ export const pressKey = definePageTool({ annotations: { category: ToolCategory.INPUT, readOnlyHint: false, + filePathFields: [], }, schema: { key: zod diff --git a/src/tools/lighthouse.ts b/src/tools/lighthouse.ts index da0a7267c..a94e77bd6 100644 --- a/src/tools/lighthouse.ts +++ b/src/tools/lighthouse.ts @@ -26,6 +26,7 @@ export const lighthouseAudit = definePageTool({ annotations: { category: ToolCategory.DEBUGGING, readOnlyHint: false, + filePathFields: ['outputDirPath'], }, schema: { mode: zod @@ -58,9 +59,6 @@ export const lighthouseAudit = definePageTool({ device = 'desktop', outputDirPath, } = request.params; - - await context.validatePath(outputDirPath); - const flags: Flags = { onlyCategories: categories, output: formats, diff --git a/src/tools/memory.ts b/src/tools/memory.ts index c2dc409b4..feabab62f 100644 --- a/src/tools/memory.ts +++ b/src/tools/memory.ts @@ -16,6 +16,7 @@ export const takeHeapSnapshot = definePageTool({ annotations: { category: ToolCategory.MEMORY, readOnlyHint: false, + filePathFields: ['filePath'], }, schema: { filePath: zod @@ -23,9 +24,8 @@ export const takeHeapSnapshot = definePageTool({ .describe('A path to a .heapsnapshot file to save the heapsnapshot to.'), }, blockedByDialog: true, - handler: async (request, response, context) => { + handler: async (request, response, _context) => { const page = request.page; - await context.validatePath(request.params.filePath); await page.pptrPage.captureHeapSnapshot({ path: ensureExtension(request.params.filePath, '.heapsnapshot'), @@ -44,6 +44,7 @@ export const getHeapSnapshotSummary = defineTool({ annotations: { category: ToolCategory.MEMORY, readOnlyHint: true, + filePathFields: ['filePath'], conditions: ['experimentalMemory'], }, schema: { @@ -51,7 +52,6 @@ export const getHeapSnapshotSummary = defineTool({ }, blockedByDialog: false, handler: async (request, response, context) => { - await context.validatePath(request.params.filePath); const stats = await context.getHeapSnapshotStats(request.params.filePath); const staticData = await context.getHeapSnapshotStaticData( request.params.filePath, @@ -68,6 +68,7 @@ export const getHeapSnapshotDetails = defineTool({ annotations: { category: ToolCategory.MEMORY, readOnlyHint: true, + filePathFields: ['filePath'], conditions: ['experimentalMemory'], }, schema: { @@ -83,7 +84,6 @@ export const getHeapSnapshotDetails = defineTool({ }, blockedByDialog: false, handler: async (request, response, context) => { - await context.validatePath(request.params.filePath); const aggregates = await context.getHeapSnapshotAggregates( request.params.filePath, ); @@ -102,6 +102,7 @@ export const getHeapSnapshotClassNodes = defineTool({ annotations: { category: ToolCategory.MEMORY, readOnlyHint: true, + filePathFields: ['filePath'], conditions: ['experimentalMemory'], }, schema: { @@ -112,7 +113,6 @@ export const getHeapSnapshotClassNodes = defineTool({ }, blockedByDialog: false, handler: async (request, response, context) => { - await context.validatePath(request.params.filePath); const nodes = await context.getHeapSnapshotNodesById( request.params.filePath, request.params.id, @@ -132,6 +132,7 @@ export const getHeapSnapshotRetainers = defineTool({ annotations: { category: ToolCategory.MEMORY, readOnlyHint: true, + filePathFields: ['filePath'], conditions: ['experimentalMemory'], }, blockedByDialog: false, @@ -142,8 +143,6 @@ export const getHeapSnapshotRetainers = defineTool({ pageSize: zod.number().optional().describe('The page size for pagination.'), }, handler: async (request, response, context) => { - await context.validatePath(request.params.filePath); - const retainers = await context.getHeapSnapshotRetainers( request.params.filePath, request.params.nodeId, diff --git a/src/tools/network.ts b/src/tools/network.ts index 1df56a8ff..c45f7e527 100644 --- a/src/tools/network.ts +++ b/src/tools/network.ts @@ -38,6 +38,7 @@ export const listNetworkRequests = definePageTool({ annotations: { category: ToolCategory.NETWORK, readOnlyHint: true, + filePathFields: [], }, schema: { pageSize: zod @@ -93,6 +94,7 @@ export const getNetworkRequest = definePageTool({ annotations: { category: ToolCategory.NETWORK, readOnlyHint: false, + filePathFields: ['requestFilePath', 'responseFilePath'], }, schema: { reqid: zod @@ -116,8 +118,6 @@ export const getNetworkRequest = definePageTool({ }, blockedByDialog: true, handler: async (request, response, context) => { - await context.validatePath(request.params.requestFilePath); - await context.validatePath(request.params.responseFilePath); if (request.params.reqid) { response.attachNetworkRequest(request.params.reqid, { requestFilePath: request.params.requestFilePath, diff --git a/src/tools/pages.ts b/src/tools/pages.ts index faaf5d9e8..237c34729 100644 --- a/src/tools/pages.ts +++ b/src/tools/pages.ts @@ -82,6 +82,7 @@ export const listPages = defineTool(args => { annotations: { category: ToolCategory.NAVIGATION, readOnlyHint: true, + filePathFields: [], }, schema: {}, blockedByDialog: false, @@ -99,6 +100,7 @@ export const selectPage = defineTool({ annotations: { category: ToolCategory.NAVIGATION, readOnlyHint: true, + filePathFields: [], }, schema: { pageId: zod @@ -130,6 +132,7 @@ export const closePage = defineTool({ annotations: { category: ToolCategory.NAVIGATION, readOnlyHint: false, + filePathFields: [], }, schema: { pageId: zod @@ -159,6 +162,7 @@ export const newPage = defineTool(args => { annotations: { category: ToolCategory.NAVIGATION, readOnlyHint: false, + filePathFields: [], }, schema: { url: zod.string().describe('URL to load in a new page.'), @@ -218,6 +222,7 @@ export const navigatePage = definePageTool(args => { annotations: { category: ToolCategory.NAVIGATION, readOnlyHint: false, + filePathFields: [], }, schema: { type: zod @@ -385,6 +390,7 @@ export const resizePage = definePageTool({ annotations: { category: ToolCategory.EMULATION, readOnlyHint: false, + filePathFields: [], }, schema: { width: zod.number().describe('Page width'), @@ -425,6 +431,7 @@ export const handleDialog = definePageTool({ annotations: { category: ToolCategory.INPUT, readOnlyHint: false, + filePathFields: [], }, schema: { action: zod @@ -477,6 +484,7 @@ export const getTabId = definePageTool({ annotations: { category: ToolCategory.NAVIGATION, readOnlyHint: true, + filePathFields: [], conditions: ['experimentalInteropTools'], }, schema: { diff --git a/src/tools/performance.ts b/src/tools/performance.ts index 4ea5f201e..65790d57e 100644 --- a/src/tools/performance.ts +++ b/src/tools/performance.ts @@ -31,6 +31,7 @@ export const startTrace = definePageTool({ annotations: { category: ToolCategory.PERFORMANCE, readOnlyHint: false, + filePathFields: ['filePath'], }, schema: { reload: zod @@ -49,7 +50,6 @@ export const startTrace = definePageTool({ }, blockedByDialog: true, handler: async (request, response, context) => { - await context.validatePath(request.params.filePath); if (context.isRunningPerformanceTrace()) { response.appendResponseLine( 'Error: a performance trace is already running. Use performance_stop_trace to stop it. Only one trace can be running at any given time.', @@ -122,13 +122,13 @@ export const stopTrace = definePageTool({ annotations: { category: ToolCategory.PERFORMANCE, readOnlyHint: false, + filePathFields: ['filePath'], }, schema: { filePath: filePathSchema, }, blockedByDialog: true, handler: async (request, response, context) => { - await context.validatePath(request.params.filePath); if (!context.isRunningPerformanceTrace()) { return; } @@ -149,6 +149,7 @@ export const analyzeInsight = definePageTool({ annotations: { category: ToolCategory.PERFORMANCE, readOnlyHint: true, + filePathFields: [], }, schema: { insightSetId: zod diff --git a/src/tools/screencast.ts b/src/tools/screencast.ts index 11af9dfda..3e60087f9 100644 --- a/src/tools/screencast.ts +++ b/src/tools/screencast.ts @@ -28,6 +28,7 @@ export const startScreencast = definePageTool(args => ({ annotations: { category: ToolCategory.DEBUGGING, readOnlyHint: false, + filePathFields: ['filePath'], conditions: ['experimentalScreencast'], }, schema: { @@ -40,7 +41,6 @@ export const startScreencast = definePageTool(args => ({ }, blockedByDialog: false, handler: async (request, response, context) => { - await context.validatePath(request.params.filePath); if (context.getScreenRecorder() !== null) { response.appendResponseLine( 'Error: a screencast recording is already in progress. Use screencast_stop to stop it before starting a new one.', @@ -99,6 +99,7 @@ export const stopScreencast = definePageTool({ annotations: { category: ToolCategory.DEBUGGING, readOnlyHint: false, + filePathFields: [], conditions: ['experimentalScreencast'], }, schema: {}, diff --git a/src/tools/screenshot.ts b/src/tools/screenshot.ts index 746a6c11f..c19cbd28f 100644 --- a/src/tools/screenshot.ts +++ b/src/tools/screenshot.ts @@ -17,6 +17,7 @@ export const screenshot = definePageTool({ category: ToolCategory.DEBUGGING, // Not read-only due to filePath param. readOnlyHint: false, + filePathFields: ['filePath'], }, schema: { format: zod @@ -52,7 +53,6 @@ export const screenshot = definePageTool({ }, blockedByDialog: true, handler: async (request, response, context) => { - await context.validatePath(request.params.filePath); if (request.params.uid && request.params.fullPage) { throw new Error('Providing both "uid" and "fullPage" is not allowed.'); } diff --git a/src/tools/script.ts b/src/tools/script.ts index 037200d86..1800b565e 100644 --- a/src/tools/script.ts +++ b/src/tools/script.ts @@ -22,6 +22,7 @@ so returned values have to be JSON-serializable.`, annotations: { category: ToolCategory.DEBUGGING, readOnlyHint: false, + filePathFields: ['filePath'], }, schema: { ...(cliArgs?.experimentalPageIdRouting ? pageIdSchema : {}), @@ -81,8 +82,6 @@ Example with arguments: \`(el) => { filePath, } = request.params; - await context.validatePath(filePath); - if (cliArgs?.categoryExtensions && serviceWorkerId) { if (uidArgs && uidArgs.length > 0) { throw new Error( diff --git a/src/tools/slim/tools.ts b/src/tools/slim/tools.ts index c0132da19..5a851dc2f 100644 --- a/src/tools/slim/tools.ts +++ b/src/tools/slim/tools.ts @@ -16,6 +16,7 @@ export const screenshot = definePageTool({ category: ToolCategory.DEBUGGING, // Not read-only due to filePath param. readOnlyHint: false, + filePathFields: [], }, schema: {}, blockedByDialog: true, @@ -39,6 +40,7 @@ export const navigate = definePageTool({ annotations: { category: ToolCategory.NAVIGATION, readOnlyHint: false, + filePathFields: [], }, schema: { url: zod.string().describe('URL to navigate to'), @@ -77,6 +79,7 @@ export const evaluate = definePageTool({ annotations: { category: ToolCategory.DEBUGGING, readOnlyHint: false, + filePathFields: [], }, schema: { script: zod.string().describe(`JS script to run on the page`), diff --git a/src/tools/snapshot.ts b/src/tools/snapshot.ts index 8c0c90ef6..5f21865dd 100644 --- a/src/tools/snapshot.ts +++ b/src/tools/snapshot.ts @@ -18,6 +18,7 @@ in the DevTools Elements panel (if any).`, category: ToolCategory.DEBUGGING, // Not read-only due to filePath param. readOnlyHint: false, + filePathFields: ['filePath'], }, schema: { verbose: zod @@ -34,8 +35,7 @@ in the DevTools Elements panel (if any).`, ), }, blockedByDialog: true, - handler: async (request, response, context) => { - await context.validatePath(request.params.filePath); + handler: async (request, response, _context) => { response.includeSnapshot({ verbose: request.params.verbose ?? false, filePath: request.params.filePath, @@ -49,6 +49,7 @@ export const waitFor = definePageTool({ annotations: { category: ToolCategory.NAVIGATION, readOnlyHint: true, + filePathFields: [], }, schema: { text: zod diff --git a/src/tools/thirdPartyDeveloper.ts b/src/tools/thirdPartyDeveloper.ts index c0cdd23a0..d1f2b4eac 100644 --- a/src/tools/thirdPartyDeveloper.ts +++ b/src/tools/thirdPartyDeveloper.ts @@ -48,6 +48,7 @@ export const listThirdPartyDeveloperTools = definePageTool({ annotations: { category: ToolCategory.THIRD_PARTY, readOnlyHint: true, + filePathFields: [], }, schema: {}, blockedByDialog: false, @@ -62,6 +63,7 @@ export const executeThirdPartyDeveloperTool = definePageTool({ annotations: { category: ToolCategory.THIRD_PARTY, readOnlyHint: false, + filePathFields: [], }, schema: { toolName: zod.string().describe('The name of the tool to execute'), diff --git a/src/tools/webmcp.ts b/src/tools/webmcp.ts index 3655e554e..a3c9727f1 100644 --- a/src/tools/webmcp.ts +++ b/src/tools/webmcp.ts @@ -15,6 +15,7 @@ export const listWebMcpTools = definePageTool({ annotations: { category: ToolCategory.WEBMCP, readOnlyHint: true, + filePathFields: [], }, schema: {}, blockedByDialog: false, @@ -29,6 +30,7 @@ export const executeWebMcpTool = definePageTool({ annotations: { category: ToolCategory.WEBMCP, readOnlyHint: false, + filePathFields: [], }, schema: { toolName: zod.string().describe('The name of the WebMCP tool to execute'), diff --git a/tests/ToolHandler.test.ts b/tests/ToolHandler.test.ts index b86516d41..03666a9c9 100644 --- a/tests/ToolHandler.test.ts +++ b/tests/ToolHandler.test.ts @@ -34,6 +34,7 @@ describe('ToolHandler', () => { annotations: { category: ToolCategory.INPUT, readOnlyHint: false, + filePathFields: [], }, schema: {}, blockedByDialog: false, @@ -75,6 +76,7 @@ describe('ToolHandler', () => { annotations: { category: ToolCategory.NAVIGATION, readOnlyHint: true, + filePathFields: [], }, schema: {}, blockedByDialog: false, @@ -115,6 +117,7 @@ describe('ToolHandler', () => { annotations: { category: ToolCategory.NAVIGATION, readOnlyHint: true, + filePathFields: [], }, schema: { url: zod.string(), @@ -164,6 +167,7 @@ describe('ToolHandler', () => { annotations: { category: ToolCategory.EMULATION, readOnlyHint: true, + filePathFields: [], }, schema: {}, blockedByDialog: false, @@ -197,4 +201,53 @@ describe('ToolHandler', () => { ); assert.strictEqual(handlerCalled, false); }); + + it('validates annotated file path fields before running the handler', async () => { + let handlerCalled = false; + const tool: ToolDefinition = { + name: 'file_tool', + description: 'A tool with a file path', + annotations: { + category: ToolCategory.DEBUGGING, + readOnlyHint: false, + filePathFields: ['filePath'], + }, + schema: { + filePath: zod.string(), + }, + blockedByDialog: false, + handler: async () => { + handlerCalled = true; + }, + }; + + const mockContext = sinon.createStubInstance(McpContext); + mockContext.detectOpenDevToolsWindows.resolves(); + mockContext.validatePath.rejects(new Error('Access denied')); + + const toolMutex = new Mutex(); + const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], { + CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true', + }); + + const toolHandler = new ToolHandler( + tool, + serverArgs, + async () => mockContext, + toolMutex, + ); + + const result = await toolHandler.handle({filePath: '/outside.txt'}); + + assert.strictEqual(result.isError, true); + assert.match( + result.content[0].type === 'text' ? result.content[0].text : '', + /Access denied/, + ); + sinon.assert.calledOnceWithExactly( + mockContext.validatePath, + '/outside.txt', + ); + assert.strictEqual(handlerCalled, false); + }); }); diff --git a/tests/telemetry/metricsRegistry.test.ts b/tests/telemetry/metricsRegistry.test.ts index 53c335abb..72fd21464 100644 --- a/tests/telemetry/metricsRegistry.test.ts +++ b/tests/telemetry/metricsRegistry.test.ts @@ -41,6 +41,7 @@ describe('metricsRegistry', () => { annotations: { category: ToolCategory.INPUT, readOnlyHint: true, + filePathFields: [], }, schema: { argStr: zod.string(), @@ -67,6 +68,7 @@ describe('metricsRegistry', () => { annotations: { category: ToolCategory.INPUT, readOnlyHint: true, + filePathFields: [], }, schema: { argEnum: zod.enum(['foo', 'bar']), @@ -90,6 +92,7 @@ describe('metricsRegistry', () => { annotations: { category: ToolCategory.THIRD_PARTY, readOnlyHint: true, + filePathFields: [], }, schema: {}, blockedByDialog: false,