From d98ab24c07b878da052d4062448cfe58cac14b4d Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 29 Jun 2026 16:36:34 -0700 Subject: [PATCH] chore(mcp): remove roots support, use cwd as workspace root MCP roots are deprecated (SEP-2577) and server-initiated roots/list is unreliable across clients. Drop server-side roots listing, the test client's roots handler, and the initialization indirection; always use process.cwd() as the workspace root, matching the previous default. --- .../playwright-core/src/tools/mcp/program.ts | 2 +- .../src/tools/utils/mcp/server.ts | 43 +----- packages/playwright-core/src/utilsBundle.ts | 2 +- tests/mcp/core.spec.ts | 7 - tests/mcp/files.spec.ts | 79 +---------- tests/mcp/fixtures.ts | 14 +- tests/mcp/http.spec.ts | 26 +--- tests/mcp/planner.spec.ts | 3 +- tests/mcp/roots.spec.ts | 130 ------------------ 9 files changed, 9 insertions(+), 297 deletions(-) delete mode 100644 tests/mcp/roots.spec.ts diff --git a/packages/playwright-core/src/tools/mcp/program.ts b/packages/playwright-core/src/tools/mcp/program.ts index e29d72fed016c..12deb5b183259 100644 --- a/packages/playwright-core/src/tools/mcp/program.ts +++ b/packages/playwright-core/src/tools/mcp/program.ts @@ -34,7 +34,7 @@ export function decorateMCPCommand(command: Command) { command .option('--allowed-hosts ', 'comma-separated list of hosts this server is allowed to serve from. Defaults to the host the server is bound to. Pass \'*\' to disable the host check.', commaSeparatedList) .option('--allowed-origins ', 'semicolon-separated list of TRUSTED origins to allow the browser to request. Default is to allow all.\nImportant: *does not* serve as a security boundary and *does not* affect redirects. ', semicolonSeparatedList) - .option('--allow-unrestricted-file-access', 'allow access to files outside of the workspace roots. Also allows unrestricted access to file:// URLs. By default access to file system is restricted to workspace root directories (or cwd if no roots are configured) only, and navigation to file:// URLs is blocked.') + .option('--allow-unrestricted-file-access', 'allow access to files outside of the workspace directory. Also allows unrestricted access to file:// URLs. By default access to file system is restricted to the current working directory only, and navigation to file:// URLs is blocked.') .option('--blocked-origins ', 'semicolon-separated list of origins to block the browser from requesting. Blocklist is evaluated before allowlist. If used without the allowlist, requests not matching the blocklist are still allowed.\nImportant: *does not* serve as a security boundary and *does not* affect redirects.', semicolonSeparatedList) .option('--block-service-workers', 'block service workers') .option('--browser ', 'browser or chrome channel to use, possible values: chrome, firefox, webkit, msedge.') diff --git a/packages/playwright-core/src/tools/utils/mcp/server.ts b/packages/playwright-core/src/tools/utils/mcp/server.ts index 4bd2ea7eef76f..17ca1752fd40d 100644 --- a/packages/playwright-core/src/tools/utils/mcp/server.ts +++ b/packages/playwright-core/src/tools/utils/mcp/server.ts @@ -14,8 +14,6 @@ * limitations under the License. */ -import { fileURLToPath } from 'url'; - import { CallToolRequestSchema, ListToolsRequestSchema } from '@modelcontextprotocol/sdk/types.js'; import debug from 'debug'; import { Server } from '@modelcontextprotocol/sdk/server/index.js'; @@ -23,10 +21,10 @@ import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' import { startMcpHttpServer } from './http'; import { toMcpTool } from './tool'; -import type { CallToolResult, CallToolRequest, Root } from '@modelcontextprotocol/sdk/types.js'; +import type { CallToolResult, CallToolRequest } from '@modelcontextprotocol/sdk/types.js'; import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; export type { Server } from '@modelcontextprotocol/sdk/server/index.js'; -export type { Tool, CallToolResult, CallToolRequest, Root } from '@modelcontextprotocol/sdk/types.js'; +export type { Tool, CallToolResult, CallToolRequest } from '@modelcontextprotocol/sdk/types.js'; import type { Server as ServerType } from '@modelcontextprotocol/sdk/server/index.js'; import type { ToolSchema } from './tool'; @@ -130,18 +128,8 @@ export function createServer(name: string, version: string, factory: ServerBacke } const initializeServer = async (server: ServerType, factory: ServerBackendFactory, runHeartbeat: boolean): Promise => { - const capabilities = server.getClientCapabilities(); - let clientRoots: Root[] = []; - if (capabilities?.roots) { - const { roots } = await server.listRoots().catch(e => { - serverDebug(e); - return { roots: [] }; - }); - clientRoots = roots; - } - const clientInfo: ClientInfo = { - cwd: firstRootPath(clientRoots), + cwd: process.cwd(), clientName: server.getClientVersion()?.name ?? 'Playwright MCP', }; @@ -216,31 +204,6 @@ export async function start(serverBackendFactory: ServerBackendFactory, options: console.error(message); } -export function firstRootPath(roots: Root[]): string { - return allRootPaths(roots)[0]; -} - -export function allRootPaths(roots: Root[]): string[] { - const paths: string[] = []; - for (const root of roots) { - const url = new URL(root.uri); - let rootPath; - try { - rootPath = fileURLToPath(url); - } catch (e) { - // Support WSL paths on Windows. - if (e.code === 'ERR_INVALID_FILE_URL_PATH' && process.platform === 'win32') - rootPath = decodeURIComponent(url.pathname); - } - if (!rootPath) - continue; - paths.push(rootPath); - } - if (paths.length === 0) - paths.push(process.cwd()); - return paths; -} - function mergeTextParts(result: CallToolResult): CallToolResult { const content: CallToolResult['content'] = []; const testParts: string[] = []; diff --git a/packages/playwright-core/src/utilsBundle.ts b/packages/playwright-core/src/utilsBundle.ts index af57d4cf10f19..ddb49fe35f484 100644 --- a/packages/playwright-core/src/utilsBundle.ts +++ b/packages/playwright-core/src/utilsBundle.ts @@ -113,5 +113,5 @@ export { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js' export { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; export { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; export { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; -export { CallToolRequestSchema, ListRootsRequestSchema, ListToolsRequestSchema, PingRequestSchema, ProgressNotificationSchema } from '@modelcontextprotocol/sdk/types.js'; +export { CallToolRequestSchema, ListToolsRequestSchema, PingRequestSchema, ProgressNotificationSchema } from '@modelcontextprotocol/sdk/types.js'; export { zodToJsonSchema } from 'zod-to-json-schema'; diff --git a/tests/mcp/core.spec.ts b/tests/mcp/core.spec.ts index 8e6040a3f5781..bf6e1d66b2e71 100644 --- a/tests/mcp/core.spec.ts +++ b/tests/mcp/core.spec.ts @@ -61,19 +61,12 @@ test('browser_navigate allows about:, data: and javascript: protocols', async ({ }); test('browser_navigate can navigate to file:// URLs allowUnrestrictedFileAccess is true', async ({ startClient }, testInfo) => { - const rootDir = testInfo.outputPath(); const fileOutsideRoot = testInfo.outputPath('test.txt'); await fs.writeFile(fileOutsideRoot, 'Test file content'); const { client } = await startClient({ config: { allowUnrestrictedFileAccess: true, }, - roots: [ - { - name: 'workspace', - uri: pathToFileURL(rootDir).href, - } - ], }); const url = pathToFileURL(fileOutsideRoot).href; diff --git a/tests/mcp/files.spec.ts b/tests/mcp/files.spec.ts index 3ed6693140ff1..06d79b76017e3 100644 --- a/tests/mcp/files.spec.ts +++ b/tests/mcp/files.spec.ts @@ -161,75 +161,7 @@ test('navigating to download link emits download', async ({ startClient, server, }); }); -test('file upload restricted to roots by default', async ({ startClient, server }, testInfo) => { - const rootDir = testInfo.outputPath('workspace'); - await fs.mkdir(rootDir, { recursive: true }); - - const { client } = await startClient({ - roots: [ - { - name: 'workspace', - uri: `file://${rootDir}`, - } - ], - }); - - server.setContent('/', ``, 'text/html'); - - await client.callTool({ - name: 'browser_navigate', - arguments: { url: server.PREFIX }, - }); - - // Click on file input to trigger file chooser - await client.callTool({ - name: 'browser_click', - arguments: { - element: 'Textbox', - target: 'e2', - }, - }); - - // Create a file inside the root - const fileInsideRoot = testInfo.outputPath('workspace', 'inside.txt'); - await fs.writeFile(fileInsideRoot, 'Inside root'); - - // Should succeed - file is inside root - expect(await client.callTool({ - name: 'browser_file_upload', - arguments: { - paths: [fileInsideRoot], - }, - })).toHaveResponse({ - code: expect.stringContaining(`await fileChooser.setFiles(`), - }); - - // Click again to open file chooser - await client.callTool({ - name: 'browser_click', - arguments: { - element: 'Textbox', - target: 'e2', - }, - }); - - // Create a file outside the root - const fileOutsideRoot = testInfo.outputPath('outside.txt'); - await fs.writeFile(fileOutsideRoot, 'Outside root'); - - // Should fail - file is outside root - expect(await client.callTool({ - name: 'browser_file_upload', - arguments: { - paths: [fileOutsideRoot], - }, - })).toHaveResponse({ - isError: true, - error: expect.stringMatching('File access denied: .* is outside allowed roots'), - }); -}); - -test('file upload is restricted to cwd if no roots are configured', async ({ startClient, server }, testInfo) => { +test('file upload is restricted to cwd', async ({ startClient, server }, testInfo) => { const rootDir = testInfo.outputPath('workspace'); await fs.mkdir(rootDir, { recursive: true }); @@ -291,19 +223,10 @@ test('file upload is restricted to cwd if no roots are configured', async ({ sta }); test('file upload unrestricted when flag is set', async ({ startClient, server }, testInfo) => { - const rootDir = testInfo.outputPath('workspace'); - await fs.mkdir(rootDir, { recursive: true }); - const { client } = await startClient({ config: { allowUnrestrictedFileAccess: true, }, - roots: [ - { - name: 'workspace', - uri: `file://${rootDir}`, - } - ], }); server.setContent('/', ``, 'text/html'); diff --git a/tests/mcp/fixtures.ts b/tests/mcp/fixtures.ts index 98ffc26e144ce..f473255eed5a9 100644 --- a/tests/mcp/fixtures.ts +++ b/tests/mcp/fixtures.ts @@ -21,7 +21,6 @@ import { chromium } from 'playwright'; import { test as baseTest, expect as baseExpect } from '@playwright/test'; import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; -import { ListRootsRequestSchema } from '@modelcontextprotocol/sdk/types.js'; import { TestServer } from '../config/testserver'; import { serverFixtures } from '../config/serverFixtures'; import { tools } from '../../packages/playwright-core/lib/coreBundle'; @@ -57,8 +56,6 @@ export type StartClient = (options?: { omitArgs?: string[], cwd?: string, config?: Config | string, - roots?: { name: string, uri: string }[], - rootsResponseDelay?: number, env?: NodeJS.ProcessEnv, noTimeoutForTest?: boolean, }) => Promise<{ client: Client, stderr: () => string }>; @@ -127,16 +124,7 @@ export const test = serverTest.extend !options.omitArgs?.includes(arg)); - const client = new Client({ name: options?.clientName ?? 'test', version: '1.0.0' }, options?.roots ? { capabilities: { roots: {} } } : undefined); - if (options?.roots) { - client.setRequestHandler(ListRootsRequestSchema, async request => { - if (options.rootsResponseDelay) - await new Promise(resolve => setTimeout(resolve, options.rootsResponseDelay)); - return { - roots: options.roots, - }; - }); - } + const client = new Client({ name: options?.clientName ?? 'test', version: '1.0.0' }); const env = inheritAndCleanEnv({ PW_TMPDIR_FOR_TEST: testInfo.outputPath('tmp'), ...options?.env diff --git a/tests/mcp/http.spec.ts b/tests/mcp/http.spec.ts index 292e7e1275aea..2a37dba013ccc 100644 --- a/tests/mcp/http.spec.ts +++ b/tests/mcp/http.spec.ts @@ -24,7 +24,7 @@ import { test as baseTest, expect, mcpServerPath, formatLog } from './fixtures'; import { inheritAndCleanEnv } from '../config/utils'; import type { Config } from '../../packages/playwright-core/src/tools/mcp/config.d'; -import { ListRootsRequestSchema, PingRequestSchema } from 'playwright-core/lib/utilsBundle'; +import { PingRequestSchema } from 'playwright-core/lib/utilsBundle'; const test = baseTest.extend<{ serverEndpoint: (options?: { args?: string[], noPort?: boolean, env?: Record }) => Promise<{ url: URL, stderr: () => string }> }>({ serverEndpoint: async ({ mcpHeadless }, use, testInfo) => { @@ -346,30 +346,6 @@ test('http transport (default)', async ({ serverEndpoint }) => { expect(transport.sessionId, 'has session support').toBeDefined(); }); -test('client should receive list roots request', async ({ serverEndpoint, server }) => { - const { url } = await serverEndpoint(); - const transport = new StreamableHTTPClientTransport(url); - const client = new Client({ name: 'test', version: '1.0.0' }, { capabilities: { roots: {} } }); - const requests = []; - client.setRequestHandler(ListRootsRequestSchema, async request => { - requests.push(request); - return { - roots: [ - { - name: 'test', - uri: 'file://tmp/', - } - ], - }; - }); - await client.connect(transport); - await client.callTool({ - name: 'browser_navigate', - arguments: { url: server.HELLO_WORLD }, - }); - await expect.poll(() => requests).toEqual([{ method: 'roots/list' }]); -}); - test('should close session when heartbeat ping is not answered', async ({ serverEndpoint, server }) => { const { url, stderr } = await serverEndpoint({ env: { PLAYWRIGHT_MCP_PING_TIMEOUT_MS: '500' } }); diff --git a/tests/mcp/planner.spec.ts b/tests/mcp/planner.spec.ts index 2e68d647805c8..357077eaa2bd5 100644 --- a/tests/mcp/planner.spec.ts +++ b/tests/mcp/planner.spec.ts @@ -18,7 +18,6 @@ import { test, expect, writeFiles } from './fixtures'; import fs from 'fs'; import path from 'path'; -import url from 'url'; test.use({ mcpServerType: 'test-mcp' }); @@ -117,7 +116,7 @@ test('planner_setup_page seed resolution - rootPath', async ({ startClient }) => const { client } = await startClient({ args: ['--config=packages/my-app/configs/playwright.config.ts'], - roots: [{ name: 'root', uri: url.pathToFileURL(test.info().outputPath('')).toString() }], + cwd: test.info().outputPath(''), }); expect(await client.callTool({ diff --git a/tests/mcp/roots.spec.ts b/tests/mcp/roots.spec.ts deleted file mode 100644 index 96e8a2daf60f4..0000000000000 --- a/tests/mcp/roots.spec.ts +++ /dev/null @@ -1,130 +0,0 @@ -/** - * Copyright (c) Microsoft Corporation. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import crypto from 'crypto'; -import fs from 'fs'; -import { pathToFileURL } from 'url'; - -import { test, expect } from './fixtures'; - -const p = process.platform === 'win32' ? 'c:\\non\\existent\\folder' : '/non/existent/folder'; - -test('should use separate user data by root path', async ({ startClient, server }, testInfo) => { - const { client } = await startClient({ - clientName: 'Visual Studio Code', - roots: [ - { - name: 'test', - uri: 'file://' + p.replace(/\\/g, '/'), - } - ], - }); - - await client.callTool({ - name: 'browser_navigate', - arguments: { url: server.HELLO_WORLD }, - }); - - const hash = createHash(p); - const [file] = await fs.promises.readdir(testInfo.outputPath('ms-playwright')); - expect(file).toContain(hash); -}); - -test('should list all tools when listRoots is slow', async ({ startClient }) => { - const { client } = await startClient({ - clientName: 'Another custom client', - roots: [], - rootsResponseDelay: 1000, - }); - const tools = await client.listTools(); - expect(tools.tools.length).toBeGreaterThan(10); -}); - -test('should tolerate malformed roots', async ({ startClient, server }, testInfo) => { - const { client } = await startClient({ - clientName: 'Visual Studio Code', - roots: [ - { - name: 'test', - uri: 'bogus://' + p.replace(/\\/g, '/'), - } - ], - }); - - expect(await client.callTool({ - name: 'browser_navigate', - arguments: { url: server.HELLO_WORLD }, - })).toHaveResponse({ - code: expect.stringContaining(`page.goto('http://localhost`), - }); - - const [file] = await fs.promises.readdir(testInfo.outputPath('ms-playwright')); - expect(file).toMatch(/mcp-.*/); -}); - -test('should tolerate WSL client roots on Windows server', async ({ startClient, server }, testInfo) => { - test.skip(process.platform !== 'win32'); - const { client } = await startClient({ - clientName: 'client', - roots: [ - { - name: 'test', - uri: 'file:///mnt/c/users/username/Desktop', - } - ], - }); - - expect(await client.callTool({ - name: 'browser_navigate', - arguments: { url: server.HELLO_WORLD }, - })).toHaveResponse({ - code: expect.stringContaining(`page.goto('http://localhost`), - }); - - const [file] = await fs.promises.readdir(testInfo.outputPath('ms-playwright')); - expect(file).toMatch(/mcp-.*/); -}); - -function createHash(data: string): string { - return crypto.createHash('sha256').update(data).digest('hex').slice(0, 7); -} - -test('should return relative paths when root is specified', async ({ startClient, server }, testInfo) => { - const rootPath = testInfo.outputPath('workspace'); - await fs.promises.mkdir(rootPath, { recursive: true }); - - const { client } = await startClient({ - clientName: 'test-client', - roots: [ - { - name: 'workspace', - uri: pathToFileURL(rootPath).toString(), - }, - ], - }); - - await client.callTool({ - name: 'browser_navigate', - arguments: { url: server.HELLO_WORLD }, - }); - - expect(await client.callTool({ - name: 'browser_take_screenshot', - arguments: { filename: 'screenshot.png' }, - })).toHaveResponse({ - result: expect.stringContaining(`[Screenshot of viewport](./screenshot.png)`), - }); -});