From 5aecbaa2c6c883061613aa8ea86d0fdb6ce12432 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 29 Apr 2026 16:45:24 -0700 Subject: [PATCH 1/5] fix(mcp): preserve env arg and don't auto-set channel without browser CLI (#40501) --- .../playwright-core/src/tools/mcp/config.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/playwright-core/src/tools/mcp/config.ts b/packages/playwright-core/src/tools/mcp/config.ts index 91935f40bae82..d042ec924827a 100644 --- a/packages/playwright-core/src/tools/mcp/config.ts +++ b/packages/playwright-core/src/tools/mcp/config.ts @@ -110,8 +110,8 @@ export async function resolveConfig(config: Config): Promise { return { ...merged, browser }; } -export async function resolveCLIConfigForMCP(cliOptions: CLIOptions): Promise { - const envOverrides = configFromEnv(); +export async function resolveCLIConfigForMCP(cliOptions: CLIOptions, env?: NodeJS.ProcessEnv): Promise { + const envOverrides = configFromEnv(env); const cliOverrides = configFromCLIOptions(cliOptions); const configFile = cliOverrides.configFile ?? envOverrides.configFile; const configInFile = await loadConfig(configFile); @@ -129,7 +129,7 @@ export async function resolveCLIConfigForMCP(cliOptions: CLIOptions): Promise { +export async function resolveCLIConfigForCLI(daemonProfilesDir: string, sessionName: string, options: any, env?: NodeJS.ProcessEnv): Promise { const config = options.config ? path.resolve(options.config) : undefined; try { const defaultConfigFile = path.resolve('.playwright', 'cli.config.json'); @@ -149,11 +149,11 @@ export async function resolveCLIConfigForCLI(daemonProfilesDir: string, sessionN snapshotMode: 'full', }); - const envOverrides = configFromEnv(); + const envOverrides = configFromEnv(env); const configFile = daemonOverrides.configFile ?? envOverrides.configFile; const configInFile = await loadConfig(configFile); const configDir = configFile ? path.dirname(path.resolve(configFile)) : process.cwd(); - const globalConfigPath = path.join(process.env['PWTEST_CLI_GLOBAL_CONFIG'] ?? os.homedir(), '.playwright', 'cli.config.json'); + const globalConfigPath = path.join((env ?? process.env)['PWTEST_CLI_GLOBAL_CONFIG'] ?? os.homedir(), '.playwright', 'cli.config.json'); const globalConfigExists = fs.existsSync(globalConfigPath); const globalConfigInFile = await loadConfig(globalConfigExists ? globalConfigPath : undefined); const globalConfigDir = globalConfigExists ? path.dirname(globalConfigPath) : process.cwd(); @@ -235,7 +235,7 @@ async function validateBrowserConfig(browser: MergedConfig['browser']): Promise< return { ...browser, browserName }; } -function resolveBrowserParam(browserOption: string | undefined): { browserName: 'chromium' | 'firefox' | 'webkit', channel?: string } { +function resolveBrowserParam(browserOption: string | undefined): { browserName?: 'chromium' | 'firefox' | 'webkit', channel?: string } { switch (browserOption) { case 'chrome': case 'chrome-beta': @@ -253,7 +253,7 @@ function resolveBrowserParam(browserOption: string | undefined): { browserName: case 'webkit': return { browserName: 'webkit' }; default: - return { browserName: 'chromium', channel: 'chrome' }; + return {}; } } @@ -352,8 +352,8 @@ function configFromCLIOptions(cliOptions: CLIOptions): Config & { configFile?: s return { ...config, configFile: cliOptions.config }; } -export function configFromEnv(): Config & { configFile?: string } { - const e = process.env; +export function configFromEnv(env?: NodeJS.ProcessEnv): Config & { configFile?: string } { + const e = env ?? process.env; const options: CLIOptions = {}; options.allowedHosts = commaSeparatedList(e.PLAYWRIGHT_MCP_ALLOWED_HOSTS); options.allowedOrigins = semicolonSeparatedList(e.PLAYWRIGHT_MCP_ALLOWED_ORIGINS); From 3a47f3e5d50d86913eafb8eb6b5a2a35d17ae10e Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 29 Apr 2026 16:53:21 -0700 Subject: [PATCH 2/5] chore: use isPathInside consistently (#40495) --- packages/playwright-core/src/cli/driver.ts | 4 +- packages/playwright-core/src/cli/program.ts | 2 + .../src/remote/playwrightServer.ts | 22 +++++------ .../src/server/bidi/bidiPage.ts | 2 +- .../playwright-core/src/server/harBackend.ts | 11 ++++-- .../src/server/registry/index.ts | 14 +++++-- .../src/server/trace/viewer/traceViewer.ts | 28 +++++++++++-- .../src/tools/backend/context.ts | 4 +- .../playwright/src/mcp/test/generatorTools.ts | 11 +++--- .../playwright/src/mcp/test/plannerTools.ts | 8 +++- packages/playwright/src/reporters/merge.ts | 17 ++++++-- packages/utils/fileUtils.ts | 15 +++++++ packages/utils/httpServer.ts | 39 ++++++++++++++++++- 13 files changed, 142 insertions(+), 35 deletions(-) diff --git a/packages/playwright-core/src/cli/driver.ts b/packages/playwright-core/src/cli/driver.ts index aac39382b4f0e..05c28bc836f52 100644 --- a/packages/playwright-core/src/cli/driver.ts +++ b/packages/playwright-core/src/cli/driver.ts @@ -72,6 +72,7 @@ export type RunServerOptions = { browserProxyMode?: 'client' | 'tether', ownedByTetherClient?: boolean, artifactsDir?: string, + unsafe?: boolean, }; export async function runServer(options: RunServerOptions) { @@ -82,8 +83,9 @@ export async function runServer(options: RunServerOptions) { maxConnections = Infinity, extension, artifactsDir, + unsafe, } = options; - const server = new PlaywrightServer({ mode: extension ? 'extension' : 'default', path, maxConnections, artifactsDir }); + const server = new PlaywrightServer({ mode: extension ? 'extension' : 'default', path, maxConnections, artifactsDir, unsafe }); const wsEndpoint = await server.listen(port, host); process.on('exit', () => server.close().catch(console.error)); console.log('Listening on ' + wsEndpoint); diff --git a/packages/playwright-core/src/cli/program.ts b/packages/playwright-core/src/cli/program.ts index 59a95c3b06dbc..87d97354128d8 100644 --- a/packages/playwright-core/src/cli/program.ts +++ b/packages/playwright-core/src/cli/program.ts @@ -177,6 +177,7 @@ export function decorateProgram(program: Command) { .option('--max-clients ', 'Maximum clients') .option('--mode ', 'Server mode, either "default" or "extension"') .option('--artifacts-dir ', 'Artifacts directory') + .option('--unsafe', 'Allow clients to set unsafe launch options (args, executablePath, ignoreAllDefaultArgs, etc)') .action(async function(options) { runServer({ port: options.port ? +options.port : undefined, @@ -185,6 +186,7 @@ export function decorateProgram(program: Command) { maxConnections: options.maxClients ? +options.maxClients : Infinity, extension: options.mode === 'extension' || !!process.env.PW_EXTENSION_MODE, artifactsDir: options.artifactsDir, + unsafe: !!options.unsafe, }).catch(logErrorAndExit); }); diff --git a/packages/playwright-core/src/remote/playwrightServer.ts b/packages/playwright-core/src/remote/playwrightServer.ts index 7c45778cb9345..7b8a30d2ca8e4 100644 --- a/packages/playwright-core/src/remote/playwrightServer.ts +++ b/packages/playwright-core/src/remote/playwrightServer.ts @@ -42,6 +42,7 @@ type ServerOptions = { preLaunchedAndroidDevice?: AndroidDevice; preLaunchedSocksProxy?: SocksProxy; artifactsDir?: string; + unsafe?: boolean; }; export class PlaywrightServer { @@ -106,8 +107,7 @@ export class PlaywrightServer { } const isExtension = this._options.mode === 'extension'; - const allowFSPaths = isExtension; - launchOptions = filterLaunchOptions(launchOptions, allowFSPaths); + launchOptions = filterLaunchOptions(launchOptions, isExtension || !!this._options.unsafe); // Always override artifacts dir with the one from server options. if (this._options.artifactsDir) @@ -363,21 +363,21 @@ function launchOptionsHash(options: LaunchOptionsWithTimeout) { return JSON.stringify(copy); } -function filterLaunchOptions(options: LaunchOptionsWithTimeout, allowFSPaths: boolean): LaunchOptionsWithTimeout { +function filterLaunchOptions(options: LaunchOptionsWithTimeout, allowUnsafe: boolean): LaunchOptionsWithTimeout { return { channel: options.channel, - args: options.args, - ignoreAllDefaultArgs: options.ignoreAllDefaultArgs, - ignoreDefaultArgs: options.ignoreDefaultArgs, + args: allowUnsafe ? options.args : undefined, + ignoreAllDefaultArgs: allowUnsafe ? options.ignoreAllDefaultArgs : undefined, + ignoreDefaultArgs: allowUnsafe ? options.ignoreDefaultArgs : undefined, timeout: options.timeout, headless: options.headless, proxy: options.proxy, - chromiumSandbox: options.chromiumSandbox, - firefoxUserPrefs: options.firefoxUserPrefs, + chromiumSandbox: allowUnsafe ? options.chromiumSandbox : undefined, + firefoxUserPrefs: allowUnsafe ? options.firefoxUserPrefs : undefined, slowMo: options.slowMo, - executablePath: (isUnderTest() || allowFSPaths) ? options.executablePath : undefined, - downloadsPath: allowFSPaths ? options.downloadsPath : undefined, - artifactsDir: (isUnderTest() || allowFSPaths) ? options.artifactsDir : undefined, + executablePath: (isUnderTest() || allowUnsafe) ? options.executablePath : undefined, + downloadsPath: allowUnsafe ? options.downloadsPath : undefined, + artifactsDir: (isUnderTest() || allowUnsafe) ? options.artifactsDir : undefined, }; } diff --git a/packages/playwright-core/src/server/bidi/bidiPage.ts b/packages/playwright-core/src/server/bidi/bidiPage.ts index 6637b0d46ee96..eb6fe320bd652 100644 --- a/packages/playwright-core/src/server/bidi/bidiPage.ts +++ b/packages/playwright-core/src/server/bidi/bidiPage.ts @@ -255,7 +255,7 @@ export class BidiPage implements PageDelegate { if (!originPage) return; - this._browserContext._browser.downloadCreated(originPage, event.navigation, event.url, event.suggestedFilename, event.suggestedFilename); + this._browserContext._browser.downloadCreated(originPage, event.navigation, event.url, event.suggestedFilename); } private _onDownloadEnded(event: bidi.BrowsingContext.DownloadEndParams) { diff --git a/packages/playwright-core/src/server/harBackend.ts b/packages/playwright-core/src/server/harBackend.ts index d85a07dc5ffd9..bff11879e112b 100644 --- a/packages/playwright-core/src/server/harBackend.ts +++ b/packages/playwright-core/src/server/harBackend.ts @@ -18,6 +18,7 @@ import fs from 'fs'; import path from 'path'; import { createGuid } from '@utils/crypto'; +import { isPathInside } from '@utils/fileUtils'; import { ZipFile } from '@utils/zipFile'; import type { HeadersArray } from '@isomorphic/types'; @@ -78,10 +79,14 @@ export class HarBackend { const file = content._file; let buffer: Buffer; if (file) { - if (this._zipFile) + if (this._zipFile) { buffer = await this._zipFile.read(file); - else - buffer = await fs.promises.readFile(path.resolve(this._baseDir!, file)); + } else { + const resolved = path.resolve(this._baseDir!, file); + if (!isPathInside(this._baseDir!, resolved)) + throw new Error(`HAR entry _file escapes base directory: ${file}`); + buffer = await fs.promises.readFile(resolved); + } } else { buffer = Buffer.from(content.text || '', content.encoding === 'base64' ? 'base64' : 'utf-8'); } diff --git a/packages/playwright-core/src/server/registry/index.ts b/packages/playwright-core/src/server/registry/index.ts index 31e1b9cb90e23..c5bc0c8fc8c22 100644 --- a/packages/playwright-core/src/server/registry/index.ts +++ b/packages/playwright-core/src/server/registry/index.ts @@ -1287,10 +1287,15 @@ export class Registry { } as any)[process.platform]; const release = searchConfig ? product.releases.find((release: any) => release.platform === searchConfig.platform && release.architecture === searchConfig.arch && release.artifacts.length > 0) : null; const artifact = release ? release.artifacts.find((artifact: any) => artifact.artifactname === searchConfig.artifact) : null; - if (artifact) - scriptArgs.push(artifact.location /* url */); - else + if (!artifact) throw new Error(`Cannot install ${channel} on ${process.platform}`); + const location = String(artifact.location); + if (!URL.canParse(location)) + throw new Error(`Cannot install ${channel}: invalid artifact url`); + const parsed = new URL(location); + if (parsed.protocol !== 'https:') + throw new Error(`Cannot install ${channel}: artifact url must be https`); + scriptArgs.push(location); } await this._installChromiumChannel(channel, scripts, scriptArgs); } @@ -1311,7 +1316,8 @@ export class Registry { if (code !== 0) throw new Error(`Failed to install ${channel}`); } else { - const { command, args, elevatedPermissions } = await transformCommandsForRoot([`bash "${path.join(BIN_PATH, scriptName)}" ${scriptArgs.join('')}`]); + const shellArgs = scriptArgs.map(a => `'${a.replace(/'/g, `'\\''`)}'`).join(' '); + const { command, args, elevatedPermissions } = await transformCommandsForRoot([`bash "${path.join(BIN_PATH, scriptName)}" ${shellArgs}`]); if (elevatedPermissions) console.log('Switching to root user to install dependencies...'); // eslint-disable-line no-console const { code } = await spawnAsync(command, args, { cwd, stdio: 'inherit' }); diff --git a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts index bf26e56466461..cd27f9b4cc95a 100644 --- a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts +++ b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts @@ -16,12 +16,14 @@ import fs from 'fs'; import path from 'path'; +import url from 'url'; import open from 'open'; import { HttpServer } from '@utils/httpServer'; import { gracefullyProcessExitDoNotHang } from '@utils/processLauncher'; import { isUnderTest } from '@utils/debug'; import { isCodingAgent } from '@utils/env'; +import { isPathInside } from '@utils/fileUtils'; import { libPath } from '../../../package'; import { syncLocalStorageWithSettings } from '../../launchApp'; import { launchApp } from '../../launchApp'; @@ -43,6 +45,7 @@ export type TraceViewerServerOptions = { port?: number; isServer?: boolean; transport?: Transport; + allowedFileRoots?: string[]; }; export type TraceViewerRedirectOptions = { @@ -87,13 +90,20 @@ function validateTraceUrlOrPath(traceFileOrUrl: string | undefined): string | un export async function startTraceViewerServer(options?: TraceViewerServerOptions): Promise { const server = new HttpServer(); + const allowedRoots = (options?.allowedFileRoots ?? [process.cwd()]).map(r => path.resolve(r)); + const isAllowed = (filePath: string) => allowedRoots.some(root => isPathInside(root, filePath)); const serveTraceDataRoute = (request: http.IncomingMessage, response: http.ServerResponse, relativePath: string): boolean => { if (!relativePath.startsWith('/file')) return false; const url = new URL('http://localhost' + request.url!); try { - const filePath = url.searchParams.get('path')!; + const filePath = path.resolve(url.searchParams.get('path')!); + if (!isAllowed(filePath)) { + response.statusCode = 403; + response.end(); + return true; + } if (fs.existsSync(filePath)) return server.serveFile(request, response, filePath); @@ -187,7 +197,7 @@ export async function installRootRedirect(server: HttpServer, traceUrl: string | export async function runTraceViewerApp(traceUrl: string | undefined, browserName: string, options: TraceViewerServerOptions & { headless?: boolean }) { traceUrl = validateTraceUrlOrPath(traceUrl); - const server = await startTraceViewerServer(options); + const server = await startTraceViewerServer({ ...options, allowedFileRoots: traceFileRoots(traceUrl, options.allowedFileRoots) }); await installRootRedirect(server, traceUrl, options); const page = await openTraceViewerApp(server.urlPrefix('precise'), browserName, options); page.on('close', () => gracefullyProcessExitDoNotHang(0)); @@ -196,11 +206,23 @@ export async function runTraceViewerApp(traceUrl: string | undefined, browserNam export async function runTraceInBrowser(traceUrl: string | undefined, options: TraceViewerServerOptions) { traceUrl = validateTraceUrlOrPath(traceUrl); - const server = await startTraceViewerServer(options); + const server = await startTraceViewerServer({ ...options, allowedFileRoots: traceFileRoots(traceUrl, options.allowedFileRoots) }); await installRootRedirect(server, traceUrl, options); await openTraceInBrowser(server.urlPrefix('human-readable')); } +function traceFileRoots(traceUrl: string | undefined, configured: string[] | undefined): string[] { + if (configured) + return configured; + if (traceUrl?.startsWith('file://')) { + try { + return [path.dirname(url.fileURLToPath(traceUrl))]; + } catch { + } + } + return [process.cwd()]; +} + export async function openTraceViewerApp(url: string, browserName: string, options?: TraceViewerAppOptions): Promise { const traceViewerPlaywright = createPlaywright({ sdkLanguage: 'javascript', isInternalPlaywright: true }); const traceViewerBrowser = isUnderTest() ? 'chromium' : browserName; diff --git a/packages/playwright-core/src/tools/backend/context.ts b/packages/playwright-core/src/tools/backend/context.ts index a505652025e9a..e0ba8dc3d439c 100644 --- a/packages/playwright-core/src/tools/backend/context.ts +++ b/packages/playwright-core/src/tools/backend/context.ts @@ -21,6 +21,7 @@ import debug from 'debug'; import { escapeWithQuotes } from '@isomorphic/stringUtils'; import { disposeAll } from '@isomorphic/disposable'; import { eventsHelper } from '@utils/eventsHelper'; +import { isPathInside } from '@utils/fileUtils'; import { playwright } from '../../inprocess'; import { Tab } from './tab'; @@ -405,7 +406,6 @@ async function checkFile(options: ContextOptions, resolvedFilename: string, flag // Trust llm to use valid characters in file names. const output = outputDir(options); const workspace = options.cwd; - const withinDir = (root: string) => resolvedFilename === root || resolvedFilename.startsWith(root + path.sep); - if (!withinDir(output) && !withinDir(workspace)) + if (!isPathInside(output, resolvedFilename) && !isPathInside(workspace, resolvedFilename)) throw new Error(`File access denied: ${resolvedFilename} is outside allowed roots. Allowed roots: ${output}, ${workspace}`); } diff --git a/packages/playwright/src/mcp/test/generatorTools.ts b/packages/playwright/src/mcp/test/generatorTools.ts index 4b8e5cb12b33a..4552ae888f8ee 100644 --- a/packages/playwright/src/mcp/test/generatorTools.ts +++ b/packages/playwright/src/mcp/test/generatorTools.ts @@ -18,6 +18,8 @@ import fs from 'fs'; import path from 'path'; import * as z from 'zod'; +import { isPathInside, resolveWithinRoot } from '@utils/fileUtils'; + import { defineTestTool } from './testTool'; import { GeneratorJournal } from './testContext'; @@ -83,12 +85,11 @@ export const generatorWriteTest = defineTestTool({ throw new Error('No test runner found, please setup page and perform actions first.'); const config = await testRunner.loadConfig(); + const resolvedFile = resolveWithinRoot(context.rootPath, params.fileName); const dirs: string[] = []; for (const project of config.projects) { - const testDir = path.relative(context.rootPath, project.project.testDir).replace(/\\/g, '/'); - const fileName = params.fileName.replace(/\\/g, '/'); - if (fileName.startsWith(testDir)) { - const resolvedFile = path.resolve(context.rootPath, fileName); + const projectTestDir = project.project.testDir; + if (resolvedFile && isPathInside(projectTestDir, resolvedFile)) { await fs.promises.mkdir(path.dirname(resolvedFile), { recursive: true }); await fs.promises.writeFile(resolvedFile, params.code); return { @@ -98,7 +99,7 @@ export const generatorWriteTest = defineTestTool({ }] }; } - dirs.push(testDir); + dirs.push(path.relative(context.rootPath, projectTestDir).replace(/\\/g, '/')); } throw new Error(`Test file did not match any of the test dirs: ${dirs.join(', ')}`); }, diff --git a/packages/playwright/src/mcp/test/plannerTools.ts b/packages/playwright/src/mcp/test/plannerTools.ts index 1e6e9deaf8eba..800afd229e670 100644 --- a/packages/playwright/src/mcp/test/plannerTools.ts +++ b/packages/playwright/src/mcp/test/plannerTools.ts @@ -18,6 +18,8 @@ import fs from 'fs'; import path from 'path'; import * as z from 'zod'; +import { resolveWithinRoot } from '@utils/fileUtils'; + import { defineTestTool } from './testTool'; export const setupPage = defineTestTool({ @@ -117,7 +119,11 @@ export const saveTestPlan = defineTestTool({ } } lines.push(``); - await fs.promises.writeFile(path.resolve(context.rootPath, params.fileName), lines.join('\n')); + const resolvedFile = resolveWithinRoot(context.rootPath, params.fileName); + if (!resolvedFile) + throw new Error(`Plan file name must be a relative path inside the workspace: ${params.fileName}`); + await fs.promises.mkdir(path.dirname(resolvedFile), { recursive: true }); + await fs.promises.writeFile(resolvedFile, lines.join('\n')); return { content: [{ type: 'text', diff --git a/packages/playwright/src/reporters/merge.ts b/packages/playwright/src/reporters/merge.ts index 9680bd5221d55..131985684f951 100644 --- a/packages/playwright/src/reporters/merge.ts +++ b/packages/playwright/src/reporters/merge.ts @@ -17,6 +17,7 @@ import fs from 'fs'; import path from 'path'; +import { isPathInside } from '@utils/fileUtils'; import { ZipFile } from '@utils/zipFile'; import { currentBlobReportVersion } from './blob'; @@ -68,7 +69,12 @@ export async function createMergedReport(config: FullConfigInternal, dir: string // a relative path on posix, and vice versa. // Therefore, we cannot use `path.resolve()` here - it will resolve relative-looking paths // against `process.cwd()`, while we just want to normalize ".." and "." segments. - resolvePath: (rootDir, relativePath) => stringPool.internString(pathPackage.normalize(pathPackage.join(rootDir, relativePath))), + resolvePath: (rootDir, relativePath) => { + const resolved = pathPackage.normalize(pathPackage.join(rootDir, relativePath)); + const escapes = pathPackage.isAbsolute(relativePath) + || (resolved !== rootDir && !resolved.startsWith(rootDir + pathPackage.sep)); + return stringPool.internString(escapes ? pathPackage.join(rootDir, '') : resolved); + }, configOverrides: config.config, }); printStatus(`processing test events`); @@ -494,11 +500,16 @@ class AttachmentPathPatcher { } private _patchAttachments(attachments: JsonAttachment[]) { + const resourceRoot = path.resolve(this._resourceDir); for (const attachment of attachments) { if (!attachment.path) continue; - - attachment.path = path.join(this._resourceDir, attachment.path); + const joined = path.resolve(resourceRoot, attachment.path); + if (!isPathInside(resourceRoot, joined)) { + attachment.path = undefined; + continue; + } + attachment.path = joined; } } } diff --git a/packages/utils/fileUtils.ts b/packages/utils/fileUtils.ts index 9cca36eda6cc9..fefe17a30bcec 100644 --- a/packages/utils/fileUtils.ts +++ b/packages/utils/fileUtils.ts @@ -60,6 +60,21 @@ export function sanitizeForFilePath(s: string) { return s.replace(/[\x00-\x2C\x2E-\x2F\x3A-\x40\x5B-\x60\x7B-\x7F]+/g, '-'); } +export function isPathInside(root: string, candidate: string): boolean { + const resolvedRoot = path.resolve(root); + const resolvedCandidate = path.resolve(candidate); + if (resolvedCandidate === resolvedRoot) + return true; + return resolvedCandidate.startsWith(resolvedRoot + path.sep); +} + +export function resolveWithinRoot(root: string, fileName: string): string | null { + if (path.isAbsolute(fileName)) + return null; + const resolvedFile = path.resolve(root, fileName); + return isPathInside(root, resolvedFile) ? resolvedFile : null; +} + export function toPosixPath(aPath: string): string { return aPath.split(path.sep).join(path.posix.sep); } diff --git a/packages/utils/httpServer.ts b/packages/utils/httpServer.ts index ca7599adc51b4..8691634ef4d36 100644 --- a/packages/utils/httpServer.ts +++ b/packages/utils/httpServer.ts @@ -21,6 +21,7 @@ import mime from 'mime'; import { WebSocketServer as wsServer } from 'ws'; import { assert } from '@isomorphic/assert'; import { createGuid } from './crypto'; +import { isPathInside } from './fileUtils'; import { createHttpServer, startHttpServer } from './network'; import type http from 'http'; @@ -51,6 +52,8 @@ export class HttpServer { private _started = false; private _routes: { prefix?: string, exact?: string, handler: ServerRouteHandler }[] = []; private _wsGuid: string | undefined; + // Allowed Host headers; null disables the check (host bound to a public address). + private _allowedHosts: Set | null = null; constructor() { this._server = createHttpServer(this._onRequest.bind(this)); @@ -169,6 +172,7 @@ export class HttpServer { const resolvedHost = address.family === 'IPv4' ? address.address : `[${address.address}]`; this._urlPrefixPrecise = `http://${resolvedHost}:${address.port}`; this._urlPrefixHumanReadable = `http://${host ?? 'localhost'}:${address.port}`; + this._allowedHosts = computeAllowedHosts(host, address.address, this._port); } } @@ -258,6 +262,15 @@ export class HttpServer { return; } + if (this._allowedHosts) { + const host = request.headers.host?.toLowerCase(); + if (!host || !this._allowedHosts.has(host)) { + response.statusCode = 403; + response.end(); + return; + } + } + request.on('error', () => response.end()); try { if (!request.url) { @@ -279,14 +292,38 @@ export class HttpServer { } } +function computeAllowedHosts(requested: string | undefined, bound: string, port: number): Set | null { + const loopback = new Set(['127.0.0.1', '::1', 'localhost']); + const isLoopback = (h: string | undefined) => h !== undefined && loopback.has(h.toLowerCase()); + if (!isLoopback(requested) && requested !== undefined) + return null; + if (!isLoopback(bound) && requested === undefined) + return null; + return new Set([ + `localhost:${port}`, + `127.0.0.1:${port}`, + `[::1]:${port}`, + ]); +} + export function serveFolder(folder: string): HttpServer { const server = new HttpServer(); + const folderRoot = path.resolve(folder); server.routePrefix('/', (request, response) => { let relativePath = new URL('http://localhost' + request.url).pathname; if (relativePath.startsWith('/trace/file')) { const url = new URL('http://localhost' + request.url!); + const requested = url.searchParams.get('path'); + if (!requested) + return false; + const resolved = path.resolve(requested); + if (!isPathInside(folderRoot, resolved)) { + response.statusCode = 403; + response.end(); + return true; + } try { - return server.serveFile(request, response, url.searchParams.get('path')!); + return server.serveFile(request, response, resolved); } catch (e) { return false; } From d73823e63502a6386a408a25d74ebcbeb4a58f1c Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 29 Apr 2026 16:53:32 -0700 Subject: [PATCH 3/5] feat(mcp): rename browser_run_code to browser_run_code_unsafe (#40496) --- docs/src/getting-started-mcp.md | 2 +- .../.claude/agents/playwright-test-planner.md | 2 +- .../agents/playwright-test-planner.agent.md | 2 +- .../src/tools/backend/runCode.ts | 6 ++-- .../src/tools/cli-daemon/commands.ts | 2 +- .../agents/playwright-test-planner.agent.md | 2 +- tests/extension/extension.spec.ts | 6 ++-- tests/mcp/capabilities.spec.ts | 2 +- tests/mcp/generator.spec.ts | 2 +- tests/mcp/run-code.spec.ts | 28 +++++++++---------- 10 files changed, 27 insertions(+), 27 deletions(-) diff --git a/docs/src/getting-started-mcp.md b/docs/src/getting-started-mcp.md index 72a26421405a8..8e9be5aa8de41 100644 --- a/docs/src/getting-started-mcp.md +++ b/docs/src/getting-started-mcp.md @@ -103,7 +103,7 @@ Playwright MCP provides tools for all common browser interactions: ### Running Playwright code -For complex interactions that go beyond individual tool calls, use the `browser_run_code` tool to execute Playwright scripts directly: +For complex interactions that go beyond individual tool calls, use the `browser_run_code_unsafe` tool to execute Playwright scripts directly. This tool runs arbitrary JavaScript in the Playwright server process and is RCE-equivalent — only enable it for trusted MCP clients: ```txt Run this Playwright code to verify the todo count: diff --git a/examples/todomvc/.claude/agents/playwright-test-planner.md b/examples/todomvc/.claude/agents/playwright-test-planner.md index b33d6ba96e82a..19ce156daec6f 100644 --- a/examples/todomvc/.claude/agents/playwright-test-planner.md +++ b/examples/todomvc/.claude/agents/playwright-test-planner.md @@ -1,7 +1,7 @@ --- name: playwright-test-planner description: Use this agent when you need to create comprehensive test plan for a web application or website -tools: Glob, Grep, Read, LS, mcp__playwright-test__browser_click, mcp__playwright-test__browser_close, mcp__playwright-test__browser_console_messages, mcp__playwright-test__browser_drag, mcp__playwright-test__browser_evaluate, mcp__playwright-test__browser_file_upload, mcp__playwright-test__browser_handle_dialog, mcp__playwright-test__browser_hover, mcp__playwright-test__browser_navigate, mcp__playwright-test__browser_navigate_back, mcp__playwright-test__browser_network_requests, mcp__playwright-test__browser_press_key, mcp__playwright-test__browser_run_code, mcp__playwright-test__browser_select_option, mcp__playwright-test__browser_snapshot, mcp__playwright-test__browser_take_screenshot, mcp__playwright-test__browser_type, mcp__playwright-test__browser_wait_for, mcp__playwright-test__planner_setup_page, mcp__playwright-test__planner_save_plan +tools: Glob, Grep, Read, LS, mcp__playwright-test__browser_click, mcp__playwright-test__browser_close, mcp__playwright-test__browser_console_messages, mcp__playwright-test__browser_drag, mcp__playwright-test__browser_evaluate, mcp__playwright-test__browser_file_upload, mcp__playwright-test__browser_handle_dialog, mcp__playwright-test__browser_hover, mcp__playwright-test__browser_navigate, mcp__playwright-test__browser_navigate_back, mcp__playwright-test__browser_network_requests, mcp__playwright-test__browser_press_key, mcp__playwright-test__browser_run_code_unsafe, mcp__playwright-test__browser_select_option, mcp__playwright-test__browser_snapshot, mcp__playwright-test__browser_take_screenshot, mcp__playwright-test__browser_type, mcp__playwright-test__browser_wait_for, mcp__playwright-test__planner_setup_page, mcp__playwright-test__planner_save_plan model: sonnet color: green --- diff --git a/examples/todomvc/.github/agents/playwright-test-planner.agent.md b/examples/todomvc/.github/agents/playwright-test-planner.agent.md index b5e04a1ed6729..53d450025caf3 100644 --- a/examples/todomvc/.github/agents/playwright-test-planner.agent.md +++ b/examples/todomvc/.github/agents/playwright-test-planner.agent.md @@ -15,7 +15,7 @@ tools: - playwright-test/browser_navigate_back - playwright-test/browser_network_requests - playwright-test/browser_press_key - - playwright-test/browser_run_code + - playwright-test/browser_run_code_unsafe - playwright-test/browser_select_option - playwright-test/browser_snapshot - playwright-test/browser_take_screenshot diff --git a/packages/playwright-core/src/tools/backend/runCode.ts b/packages/playwright-core/src/tools/backend/runCode.ts index f6ea7fd72b36d..ecada28e95102 100644 --- a/packages/playwright-core/src/tools/backend/runCode.ts +++ b/packages/playwright-core/src/tools/backend/runCode.ts @@ -30,9 +30,9 @@ const codeSchema = z.object({ const runCode = defineTabTool({ capability: 'core', schema: { - name: 'browser_run_code', - title: 'Run Playwright code', - description: 'Run Playwright code snippet', + name: 'browser_run_code_unsafe', + title: 'Run Playwright code (unsafe)', + description: 'Run a Playwright code snippet. Unsafe: executes arbitrary JavaScript in the Playwright server process and is RCE-equivalent.', inputSchema: codeSchema, type: 'action', }, diff --git a/packages/playwright-core/src/tools/cli-daemon/commands.ts b/packages/playwright-core/src/tools/cli-daemon/commands.ts index 26f4abd681ae6..0a6596ad8acc8 100644 --- a/packages/playwright-core/src/tools/cli-daemon/commands.ts +++ b/packages/playwright-core/src/tools/cli-daemon/commands.ts @@ -465,7 +465,7 @@ const runCode = declareCommand({ options: z.object({ filename: z.string().optional().describe('Load code from the specified file.'), }), - toolName: 'browser_run_code', + toolName: 'browser_run_code_unsafe', toolParams: ({ code, filename }) => ({ code, filename }), }); diff --git a/packages/playwright/src/agents/playwright-test-planner.agent.md b/packages/playwright/src/agents/playwright-test-planner.agent.md index 801dda573d398..ad72d4ac01146 100644 --- a/packages/playwright/src/agents/playwright-test-planner.agent.md +++ b/packages/playwright/src/agents/playwright-test-planner.agent.md @@ -18,7 +18,7 @@ tools: - playwright-test/browser_network_request - playwright-test/browser_network_requests - playwright-test/browser_press_key - - playwright-test/browser_run_code + - playwright-test/browser_run_code_unsafe - playwright-test/browser_select_option - playwright-test/browser_snapshot - playwright-test/browser_take_screenshot diff --git a/tests/extension/extension.spec.ts b/tests/extension/extension.spec.ts index 42b9b19c1c578..ef1519695a1e0 100644 --- a/tests/extension/extension.spec.ts +++ b/tests/extension/extension.spec.ts @@ -76,7 +76,7 @@ test(`protocolVersion defaults to 1`, async ({ startExtensionClient, server, pro process.env.PLAYWRIGHT_EXTENSION_PROTOCOL = saved; }); -test(`browser_run_code can evaluate in a web worker`, async ({ startExtensionClient, server, protocolVersion }) => { +test(`browser_run_code_unsafe can evaluate in a web worker`, async ({ startExtensionClient, server, protocolVersion }) => { test.skip(protocolVersion === 1, 'Multi-tab not supported in protocol v1'); server.setContent('/worker.js', ` self.onmessage = (e) => self.postMessage('echo:' + e.data); @@ -108,7 +108,7 @@ test(`browser_run_code can evaluate in a web worker`, async ({ startExtensionCli await navigateResponse; const runCodeResponse = await client.callTool({ - name: 'browser_run_code', + name: 'browser_run_code_unsafe', arguments: { code: `async (page) => { const worker = page.workers().length ? page.workers()[0] : await page.waitForEvent('worker'); @@ -143,7 +143,7 @@ test(`browser_run_code can evaluate in a web worker`, async ({ startExtensionCli }); const runCodeResponse2 = await client.callTool({ - name: 'browser_run_code', + name: 'browser_run_code_unsafe', arguments: { code: `async (page) => { const worker = page.workers().length ? page.workers()[0] : await page.waitForEvent('worker'); diff --git a/tests/mcp/capabilities.spec.ts b/tests/mcp/capabilities.spec.ts index d99d401e2ad39..931646a891249 100644 --- a/tests/mcp/capabilities.spec.ts +++ b/tests/mcp/capabilities.spec.ts @@ -37,7 +37,7 @@ test('test snapshot tool list', async ({ client }) => { 'browser_network_requests', 'browser_press_key', 'browser_resize', - 'browser_run_code', + 'browser_run_code_unsafe', 'browser_snapshot', 'browser_tabs', 'browser_take_screenshot', diff --git a/tests/mcp/generator.spec.ts b/tests/mcp/generator.spec.ts index b163ac49920f2..d4f7ed697686d 100644 --- a/tests/mcp/generator.spec.ts +++ b/tests/mcp/generator.spec.ts @@ -46,7 +46,7 @@ test('generator tools intent', async ({ startClient }) => { expect(toolsWithIntent).toContain('browser_press_key'); expect(toolsWithIntent).toContain('browser_press_sequentially'); expect(toolsWithIntent).toContain('browser_resize'); - expect(toolsWithIntent).toContain('browser_run_code'); + expect(toolsWithIntent).toContain('browser_run_code_unsafe'); expect(toolsWithIntent).toContain('browser_select_option'); expect(toolsWithIntent).toContain('browser_tabs'); }); diff --git a/tests/mcp/run-code.spec.ts b/tests/mcp/run-code.spec.ts index 1d958c7be8b74..257e70c86ff56 100644 --- a/tests/mcp/run-code.spec.ts +++ b/tests/mcp/run-code.spec.ts @@ -18,7 +18,7 @@ import fs from 'fs'; import { test, expect, parseResponse, consoleEntries } from './fixtures'; -test('browser_run_code', async ({ client, server }) => { +test('browser_run_code_unsafe', async ({ client, server }) => { server.setContent('/', ` `, 'text/html'); @@ -29,7 +29,7 @@ test('browser_run_code', async ({ client, server }) => { const code = 'async (page) => await page.getByRole("button", { name: "Submit" }).click()'; const response = parseResponse(await client.callTool({ - name: 'browser_run_code', + name: 'browser_run_code_unsafe', arguments: { code, }, @@ -38,7 +38,7 @@ test('browser_run_code', async ({ client, server }) => { expect(content).toContain('[LOG] Submit'); }); -test('browser_run_code block', async ({ client, server }) => { +test('browser_run_code_unsafe block', async ({ client, server }) => { server.setContent('/', ` `, 'text/html'); @@ -48,7 +48,7 @@ test('browser_run_code block', async ({ client, server }) => { }); const response = parseResponse(await client.callTool({ - name: 'browser_run_code', + name: 'browser_run_code_unsafe', arguments: { code: 'async (page) => { await page.getByRole("button", { name: "Submit" }).click(); await page.getByRole("button", { name: "Submit" }).click(); }', }, @@ -62,7 +62,7 @@ test('browser_run_code block', async ({ client, server }) => { expect(content).toMatch(/\[LOG\] Submit.*\n.*\[LOG\] Submit/); }); -test('browser_run_code no-require', async ({ client, server }) => { +test('browser_run_code_unsafe no-require', async ({ client, server }) => { server.setContent('/', ` `, 'text/html'); @@ -72,7 +72,7 @@ test('browser_run_code no-require', async ({ client, server }) => { }); expect(await client.callTool({ - name: 'browser_run_code', + name: 'browser_run_code_unsafe', arguments: { code: `(page) => { require('fs'); }`, }, @@ -82,7 +82,7 @@ test('browser_run_code no-require', async ({ client, server }) => { }); }); -test('browser_run_code return value', async ({ client, server }) => { +test('browser_run_code_unsafe return value', async ({ client, server }) => { server.setContent('/', ` `, 'text/html'); @@ -94,7 +94,7 @@ test('browser_run_code return value', async ({ client, server }) => { const code = 'async (page) => { await page.getByRole("button", { name: "Submit" }).click(); return { message: "Hello, world!" }; await page.getByRole("banner").click(); }'; const response = parseResponse(await client.callTool({ - name: 'browser_run_code', + name: 'browser_run_code_unsafe', arguments: { code, }, @@ -108,7 +108,7 @@ test('browser_run_code return value', async ({ client, server }) => { expect(content).toContain('[LOG] Submit'); }); -test('browser_run_code route handler exception keeps server alive', async ({ client, server }) => { +test('browser_run_code_unsafe route handler exception keeps server alive', async ({ client, server }) => { server.setContent('/', '', 'text/html'); await client.callTool({ name: 'browser_navigate', @@ -127,7 +127,7 @@ test('browser_run_code route handler exception keeps server alive', async ({ cli }); }`; expect(await client.callTool({ - name: 'browser_run_code', + name: 'browser_run_code_unsafe', arguments: { code }, })).toHaveResponse({ error: expect.stringContaining('ReferenceError: URL is not defined'), @@ -142,7 +142,7 @@ test('browser_run_code route handler exception keeps server alive', async ({ cli expect(followUp.isError).toBeFalsy(); }); -test('browser_run_code with filename', async ({ client, server }) => { +test('browser_run_code_unsafe with filename', async ({ client, server }) => { server.setContent('/', ` `, 'text/html'); @@ -156,14 +156,14 @@ test('browser_run_code with filename', async ({ client, server }) => { await fs.promises.writeFile(filePath, code); const response = parseResponse(await client.callTool({ - name: 'browser_run_code', + name: 'browser_run_code_unsafe', arguments: { filename: 'test-code.js' }, })); const content = await consoleEntries(response); expect(content).toContain('[LOG] Clicked'); }); -test('browser_run_code with filename containing template literals', async ({ client, server }) => { +test('browser_run_code_unsafe with filename containing template literals', async ({ client, server }) => { server.setContent('/', ` `, 'text/html'); @@ -177,7 +177,7 @@ test('browser_run_code with filename containing template literals', async ({ cli await fs.promises.writeFile(filePath, code); const response = parseResponse(await client.callTool({ - name: 'browser_run_code', + name: 'browser_run_code_unsafe', arguments: { filename: 'template-code.js' }, })); const content = await consoleEntries(response); From a8ad57fcb4566146d4be6587c50bd7a095383ee4 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 29 Apr 2026 16:54:25 -0700 Subject: [PATCH 4/5] fix(server): default run-server and launchServer to localhost (#40500) --- docs/src/api/class-android.md | 2 +- docs/src/api/class-browserserver.md | 2 -- docs/src/api/class-browsertype.md | 2 +- packages/playwright-client/types/types.d.ts | 15 ++++++--------- packages/playwright-core/types/types.d.ts | 15 ++++++--------- packages/utils/wsServer.ts | 6 +++++- 6 files changed, 19 insertions(+), 23 deletions(-) diff --git a/docs/src/api/class-android.md b/docs/src/api/class-android.md index ffef68509dc6d..4427622b6af51 100644 --- a/docs/src/api/class-android.md +++ b/docs/src/api/class-android.md @@ -206,7 +206,7 @@ throw if multiple devices are connected. * since: v1.45 - `host` <[string]> -Host to use for the web socket. It is optional and if it is omitted, the server will accept connections on the unspecified IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address (0.0.0.0) otherwise. Consider hardening it with picking a specific interface. +Host to use for the web socket. It is optional and defaults to `localhost`, accepting connections only from the loopback interface. Pass an explicit address (e.g. `0.0.0.0`) to accept connections from the network — be aware this exposes the device RPC to anything that can reach the listening port. ### option: Android.launchServer.port * since: v1.28 diff --git a/docs/src/api/class-browserserver.md b/docs/src/api/class-browserserver.md index 0de5c1228bd11..21f318a70abcb 100644 --- a/docs/src/api/class-browserserver.md +++ b/docs/src/api/class-browserserver.md @@ -31,5 +31,3 @@ Browser websocket url. Browser websocket endpoint which can be used as an argument to [`method: BrowserType.connect`] to establish connection to the browser. - -Note that if the listen `host` option in `launchServer` options is not specified, localhost will be output anyway, even if the actual listening address is an unspecified address. diff --git a/docs/src/api/class-browsertype.md b/docs/src/api/class-browsertype.md index ce815d6e8a79d..e3f52a8503d74 100644 --- a/docs/src/api/class-browsertype.md +++ b/docs/src/api/class-browsertype.md @@ -405,7 +405,7 @@ const { chromium } = require('playwright'); // Or 'webkit' or 'firefox'. * since: v1.45 - `host` <[string]> -Host to use for the web socket. It is optional and if it is omitted, the server will accept connections on the unspecified IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address (0.0.0.0) otherwise. Consider hardening it with picking a specific interface. +Host to use for the web socket. It is optional and defaults to `localhost`, accepting connections only from the loopback interface. Pass an explicit address (e.g. `0.0.0.0`) to accept connections from the network — be aware this exposes the browser RPC to anything that can reach the listening port. ### option: BrowserType.launchServer.port * since: v1.8 diff --git a/packages/playwright-client/types/types.d.ts b/packages/playwright-client/types/types.d.ts index 527243cd566b7..69ce47e5b6c6c 100644 --- a/packages/playwright-client/types/types.d.ts +++ b/packages/playwright-client/types/types.d.ts @@ -16100,9 +16100,9 @@ export interface BrowserType { headless?: boolean; /** - * Host to use for the web socket. It is optional and if it is omitted, the server will accept connections on the - * unspecified IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address (0.0.0.0) otherwise. Consider - * hardening it with picking a specific interface. + * Host to use for the web socket. It is optional and defaults to `localhost`, accepting connections only from the + * loopback interface. Pass an explicit address (e.g. `0.0.0.0`) to accept connections from the network — be aware + * this exposes the browser RPC to anything that can reach the listening port. */ host?: string; @@ -17348,9 +17348,9 @@ export interface Android { deviceSerialNumber?: string; /** - * Host to use for the web socket. It is optional and if it is omitted, the server will accept connections on the - * unspecified IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address (0.0.0.0) otherwise. Consider - * hardening it with picking a specific interface. + * Host to use for the web socket. It is optional and defaults to `localhost`, accepting connections only from the + * loopback interface. Pass an explicit address (e.g. `0.0.0.0`) to accept connections from the network — be aware + * this exposes the device RPC to anything that can reach the listening port. */ host?: string; @@ -19526,9 +19526,6 @@ export interface BrowserServer { * Browser websocket endpoint which can be used as an argument to * [browserType.connect(endpoint[, options])](https://playwright.dev/docs/api/class-browsertype#browser-type-connect) * to establish connection to the browser. - * - * Note that if the listen `host` option in `launchServer` options is not specified, localhost will be output anyway, - * even if the actual listening address is an unspecified address. */ wsEndpoint(): string; diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index 527243cd566b7..69ce47e5b6c6c 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -16100,9 +16100,9 @@ export interface BrowserType { headless?: boolean; /** - * Host to use for the web socket. It is optional and if it is omitted, the server will accept connections on the - * unspecified IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address (0.0.0.0) otherwise. Consider - * hardening it with picking a specific interface. + * Host to use for the web socket. It is optional and defaults to `localhost`, accepting connections only from the + * loopback interface. Pass an explicit address (e.g. `0.0.0.0`) to accept connections from the network — be aware + * this exposes the browser RPC to anything that can reach the listening port. */ host?: string; @@ -17348,9 +17348,9 @@ export interface Android { deviceSerialNumber?: string; /** - * Host to use for the web socket. It is optional and if it is omitted, the server will accept connections on the - * unspecified IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address (0.0.0.0) otherwise. Consider - * hardening it with picking a specific interface. + * Host to use for the web socket. It is optional and defaults to `localhost`, accepting connections only from the + * loopback interface. Pass an explicit address (e.g. `0.0.0.0`) to accept connections from the network — be aware + * this exposes the device RPC to anything that can reach the listening port. */ host?: string; @@ -19526,9 +19526,6 @@ export interface BrowserServer { * Browser websocket endpoint which can be used as an argument to * [browserType.connect(endpoint[, options])](https://playwright.dev/docs/api/class-browsertype#browser-type-connect) * to establish connection to the browser. - * - * Note that if the listen `host` option in `launchServer` options is not specified, localhost will be output anyway, - * even if the actual listening address is an unspecified address. */ wsEndpoint(): string; diff --git a/packages/utils/wsServer.ts b/packages/utils/wsServer.ts index ffc83a6a58377..921a24dbd7ad9 100644 --- a/packages/utils/wsServer.ts +++ b/packages/utils/wsServer.ts @@ -60,6 +60,10 @@ export class WSServer { async listen(port: number = 0, hostname: string | undefined, path: string): Promise { debugLogger.log('server', `Server started at ${new Date()}`); + // Default to loopback so the WebSocket RPC is not exposed to the network unless + // the caller explicitly opts in by passing a host (e.g. '0.0.0.0'). + hostname ??= 'localhost'; + const server = createHttpServer(this._delegate.onRequest); server.on('error', error => debugLogger.log('server', String(error))); this.server = server; @@ -71,7 +75,7 @@ export class WSServer { reject(new Error('Could not bind server socket')); return; } - const wsEndpoint = typeof address === 'string' ? `${address}${path}` : `ws://${hostname || 'localhost'}:${address.port}${path}`; + const wsEndpoint = typeof address === 'string' ? `${address}${path}` : `ws://${hostname}:${address.port}${path}`; resolve(wsEndpoint); }).on('error', reject); }); From 37ff94b562723f7cfda1e6955e23b9cf1c649f6a Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 29 Apr 2026 17:01:13 -0700 Subject: [PATCH 5/5] fix(trace): escape script-context interpolation in snapshot renderer (#40497) --- CLAUDE.md | 1 + packages/isomorphic/trace/snapshotRenderer.ts | 5 +- tests/library/snapshot-renderer.spec.ts | 48 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tests/library/snapshot-renderer.spec.ts diff --git a/CLAUDE.md b/CLAUDE.md index 9110c0b092916..76da6a4b9c9aa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -121,6 +121,7 @@ fix(proxy): handle SOCKS proxy authentication Fixes: https://github.com/microsoft/playwright/issues/39562 EOF )" +# **Never `git push` without an explicit instruction to push.** git push origin fix-39562 gh pr create --repo microsoft/playwright --head username:fix-39562 \ --title "fix(proxy): handle SOCKS proxy authentication" \ diff --git a/packages/isomorphic/trace/snapshotRenderer.ts b/packages/isomorphic/trace/snapshotRenderer.ts index 72bf324bb53d6..368bd33448101 100644 --- a/packages/isomorphic/trace/snapshotRenderer.ts +++ b/packages/isomorphic/trace/snapshotRenderer.ts @@ -568,7 +568,10 @@ function snapshotScript(viewport: ViewportSize, ...targetIds: (string | undefine win.addEventListener('DOMContentLoaded', onDOMContentLoaded); } - return `\n(${applyPlaywrightAttributes.toString()})(${JSON.stringify(blankSnapshotUrl)},${JSON.stringify(viewport)}${targetIds.map(id => `, "${id}"`).join('')})`; + // Trace data is untrusted; escape `<` so attacker-controlled targetIds/viewport + // cannot terminate the surrounding ". + const safe = (value: unknown) => JSON.stringify(value).replace(/ `, ${safe(String(id))}`).join('')})`; } diff --git a/tests/library/snapshot-renderer.spec.ts b/tests/library/snapshot-renderer.spec.ts new file mode 100644 index 0000000000000..c92fcc0871df6 --- /dev/null +++ b/tests/library/snapshot-renderer.spec.ts @@ -0,0 +1,48 @@ +/** + * 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 { test, expect } from '@playwright/test'; +import { SnapshotRenderer } from '../../packages/isomorphic/trace/snapshotRenderer'; +import { LRUCache } from '../../packages/isomorphic/lruCache'; +import type { FrameSnapshot } from '../../packages/trace/src/snapshot'; + +function makeSnapshot(overrides: Partial = {}): FrameSnapshot { + return { + callId: 'call-1', + pageId: 'page-1', + frameId: 'frame-1', + frameUrl: 'http://example.com/', + timestamp: 0, + collectionTime: 0, + html: ['HTML', {}, ['BODY', {}]], + resourceOverrides: [], + viewport: { width: 1280, height: 720 }, + isMainFrame: true, + ...overrides, + }; +} + +for (const [name, overrides] of [ + ['callId', { callId: '' }], +] as const) { + test(`snapshot renderer escapes attacker-controlled ${name} in script context`, () => { + const renderer = new SnapshotRenderer(new LRUCache(1_000_000), [], [makeSnapshot(overrides)], [], 0); + const { html } = renderer.render(); + expect(html.match(/