fix(mcp): lax file path sanitization (#37502)

Author: pavelfeldman

packages/playwright/src/mcp/browser/browserContextFactory.ts

@@ -106,7 +106,7 @@ class IsolatedContextFactory extends BaseContextFactory {
   protected override async _doObtainBrowser(clientInfo: ClientInfo): Promise<playwright.Browser> {
     await injectCdpPort(this.config.browser);
     const browserType = playwright[this.config.browser.browserName];
-    const tracesDir = await outputFile(this.config, clientInfo, `traces`);
+    const tracesDir = await outputFile(this.config, clientInfo, `traces`, { origin: 'code' });
     if (this.config.saveTrace)
       await startTraceServer(this.config, tracesDir);
     return browserType.launch({
@@ -173,7 +173,7 @@ class PersistentContextFactory implements BrowserContextFactory {
     await injectCdpPort(this.config.browser);
     testDebug('create browser context (persistent)');
     const userDataDir = this.config.browser.userDataDir ?? await this._createUserDataDir(clientInfo);
-    const tracesDir = await outputFile(this.config, clientInfo, `traces`);
+    const tracesDir = await outputFile(this.config, clientInfo, `traces`, { origin: 'code' });
     if (this.config.saveTrace)
       await startTraceServer(this.config, tracesDir);
 

packages/playwright/src/mcp/browser/config.ts

@@ -271,15 +271,27 @@ async function loadConfig(configFile: string | undefined): Promise<Config> {
   }
 }
 
-export async function outputFile(config: FullConfig, clientInfo: ClientInfo, name: string): Promise<string> {
+export async function outputFile(config: FullConfig, clientInfo: ClientInfo, fileName: string, options: { origin: 'code' | 'llm' | 'web' }): Promise<string> {
   const rootPath = firstRootPath(clientInfo);
   const outputDir = config.outputDir
     ?? (rootPath ? path.join(rootPath, '.playwright-mcp') : undefined)
     ?? path.join(process.env.PW_TMPDIR_FOR_TEST ?? os.tmpdir(), 'playwright-mcp-output', String(clientInfo.timestamp));
 
-  await fs.promises.mkdir(outputDir, { recursive: true });
-  const fileName = sanitizeForFilePath(name);
-  return path.join(outputDir, fileName);
+  // Trust code.
+  if (options.origin === 'code')
+    return path.resolve(outputDir, fileName);
+
+  // Trust llm to use valid characters in file names.
+  if (options.origin === 'llm') {
+    fileName = fileName.split('\\').join('/');
+    const resolvedFile = path.resolve(outputDir, fileName);
+    if (!resolvedFile.startsWith(path.resolve(outputDir) + path.sep))
+      throw new Error(`Resolved file path for ${fileName} is outside of the output directory`);
+    return resolvedFile;
+  }
+
+  // Do not trust web, at all.
+  return path.join(outputDir, sanitizeForFilePath(fileName));
 }
 
 function pickDefined<T extends object>(obj: T | undefined): Partial<T> {

packages/playwright/src/mcp/browser/context.ts

@@ -113,8 +113,8 @@ export class Context {
     return url;
   }
 
-  async outputFile(name: string): Promise<string> {
-    return outputFile(this.config, this._clientInfo, name);
+  async outputFile(fileName: string, options: { origin: 'code' | 'llm' | 'web' }): Promise<string> {
+    return outputFile(this.config, this._clientInfo, fileName, options);
   }
 
   private _onPageCreated(page: playwright.Page) {

packages/playwright/src/mcp/browser/sessionLog.ts

@@ -53,7 +53,7 @@ export class SessionLog {
   }
 
   static async create(config: FullConfig, clientInfo: mcpServer.ClientInfo): Promise<SessionLog> {
-    const sessionFolder = await outputFile(config, clientInfo, `session-${Date.now()}`);
+    const sessionFolder = await outputFile(config, clientInfo, `session-${Date.now()}`, { origin: 'code' });
     await fs.promises.mkdir(sessionFolder, { recursive: true });
     // eslint-disable-next-line no-console
     console.error(`Session: ${sessionFolder}`);

packages/playwright/src/mcp/browser/tab.ts

@@ -117,7 +117,7 @@ export class Tab extends EventEmitter<TabEventsInterface> {
     const entry = {
       download,
       finished: false,
-      outputFile: await this.context.outputFile(download.suggestedFilename())
+      outputFile: await this.context.outputFile(download.suggestedFilename(), { origin: 'web' })
     };
     this._downloads.push(entry);
     await download.saveAs(entry.outputFile);

packages/playwright/src/mcp/browser/tools/pdf.ts

@@ -17,6 +17,7 @@
 import { z } from '../../sdk/bundle';
 import { defineTabTool } from './tool';
 import * as javascript from '../codegen';
+import { dateAsFileName } from './utils';
 
 const pdfSchema = z.object({
   filename: z.string().optional().describe('File name to save the pdf to. Defaults to `page-{timestamp}.pdf` if not specified.'),
@@ -34,7 +35,7 @@ const pdf = defineTabTool({
   },
 
   handle: async (tab, params, response) => {
-    const fileName = await tab.context.outputFile(params.filename ?? `page-${new Date().toISOString()}.pdf`);
+    const fileName = await tab.context.outputFile(params.filename ?? `page-${dateAsFileName()}.pdf`, { origin: 'llm' });
     response.addCode(`await page.pdf(${javascript.formatObject({ path: fileName })});`);
     response.addResult(`Saved page as ${fileName}`);
     await tab.page.pdf({ path: fileName });

packages/playwright/src/mcp/browser/tools/screenshot.ts

@@ -17,7 +17,7 @@
 import { z } from '../../sdk/bundle';
 import { defineTabTool } from './tool';
 import * as javascript from '../codegen';
-import { generateLocator } from './utils';
+import { generateLocator, dateAsFileName } from './utils';
 
 import type * as playwright from 'playwright-core';
 
@@ -51,7 +51,7 @@ const screenshot = defineTabTool({
 
   handle: async (tab, params, response) => {
     const fileType = params.type || 'png';
-    const fileName = await tab.context.outputFile(params.filename ?? `page-${new Date().toISOString()}.${fileType}`);
+    const fileName = await tab.context.outputFile(params.filename ?? `page-${dateAsFileName()}.${fileType}`, { origin: 'llm' });
     const options: playwright.PageScreenshotOptions = {
       type: fileType,
       quality: fileType === 'png' ? undefined : 90,

packages/playwright/src/mcp/browser/tools/tracing.ts

@@ -32,7 +32,7 @@ const tracingStart = defineTool({
 
   handle: async (context, params, response) => {
     const browserContext = await context.ensureBrowserContext();
-    const tracesDir = await context.outputFile(`traces`);
+    const tracesDir = await context.outputFile(`traces`, { origin: 'code' });
     const name = 'trace-' + Date.now();
     await (browserContext.tracing as Tracing).start({
       name,

packages/playwright/src/mcp/browser/tools/utils.ts

@@ -82,3 +82,8 @@ export async function generateLocator(locator: playwright.Locator): Promise<stri
 export async function callOnPageNoTrace<T>(page: playwright.Page, callback: (page: playwright.Page) => Promise<T>): Promise<T> {
   return await (page as any)._wrapApiCall(() => callback(page), { internal: true });
 }
+
+export function dateAsFileName(): string {
+  const date = new Date();
+  return date.toISOString().replace(/[:.]/g, '-');
+}