diff --git a/packages/playwright-core/src/remote/playwrightServer.ts b/packages/playwright-core/src/remote/playwrightServer.ts index 960a4a6e278a0..c1928ce1e6b5e 100644 --- a/packages/playwright-core/src/remote/playwrightServer.ts +++ b/packages/playwright-core/src/remote/playwrightServer.ts @@ -84,6 +84,8 @@ export class PlaywrightServer { return { error: `HTTP/${request.httpVersion} 428 Precondition Required\r\n\r\n${uaError}` }; }, + isAllowedPathname: pathname => pathname === this._options.path, + onHeaders: headers => { if (process.env.PWTEST_SERVER_WS_HEADERS) headers.push(process.env.PWTEST_SERVER_WS_HEADERS!); diff --git a/packages/playwright-core/src/remote/playwrightWebSocketServer.ts b/packages/playwright-core/src/remote/playwrightWebSocketServer.ts index 11e9c3786dc8d..9465f8b442633 100644 --- a/packages/playwright-core/src/remote/playwrightWebSocketServer.ts +++ b/packages/playwright-core/src/remote/playwrightWebSocketServer.ts @@ -26,9 +26,12 @@ import type { PlaywrightInitializeResult } from './playwrightConnection'; export class PlaywrightWebSocketServer { private _wsServer: WSServer; private _browser: Browser; + private _path: string; constructor(browser: Browser, path: string) { this._browser = browser; + this._path = path; + browser.on(Browser.Events.Disconnected, () => this.close()); const semaphore = new Semaphore(Infinity); @@ -38,6 +41,7 @@ export class PlaywrightWebSocketServer { }, onUpgrade: () => undefined, onHeaders: () => {}, + isAllowedPathname: pathname => pathname === this._path, onConnection: (request, url, ws, id) => { debugLogger.log('server', `[${id}] ws client connected`); return new PlaywrightConnection( @@ -61,8 +65,8 @@ export class PlaywrightWebSocketServer { }; } - async listen(port: number = 0, hostname?: string, path?: string): Promise { - return await this._wsServer.listen(port, hostname, path || '/'); + async listen(port: number = 0, hostname?: string): Promise { + return await this._wsServer.listen(port, hostname, this._path); } async close() { diff --git a/packages/playwright-core/src/server/browser.ts b/packages/playwright-core/src/server/browser.ts index 56fff6415a054..608fdab72caf2 100644 --- a/packages/playwright-core/src/server/browser.ts +++ b/packages/playwright-core/src/server/browser.ts @@ -222,8 +222,8 @@ export class BrowserServer { let endpoint: string; if (options.host !== undefined || options.port !== undefined) { - this._wsServer = new PlaywrightWebSocketServer(this._browser, '/'); - endpoint = await this._wsServer.listen(options.port ?? 0, options.host, '/' + createGuid()); + this._wsServer = new PlaywrightWebSocketServer(this._browser, '/' + createGuid()); + endpoint = await this._wsServer.listen(options.port ?? 0, options.host); } else { this._pipeServer = new PlaywrightPipeServer(this._browser); this._pipeSocketPath = await this._socketPath(); diff --git a/packages/playwright-core/src/tools/mcp/cdpRelay.ts b/packages/playwright-core/src/tools/mcp/cdpRelay.ts index 2de9348bf62b7..e24ae1a6c42d8 100644 --- a/packages/playwright-core/src/tools/mcp/cdpRelay.ts +++ b/packages/playwright-core/src/tools/mcp/cdpRelay.ts @@ -26,16 +26,15 @@ */ import { spawn } from 'child_process'; -import http from 'http'; import os from 'os'; import debug from 'debug'; -import ws, { WebSocketServer as wsServer } from 'ws'; +import ws from 'ws'; import { ManualPromise } from '@isomorphic/manualPromise'; +import { WSServer } from '@utils/wsServer'; import { registry } from '../../server/registry/index'; import { findPlaywrightExtensionProfile, playwrightExtensionId } from '../utils/extension'; -import { addressToString } from '../utils/mcp/http'; import { logUnhandledError } from './log'; import { ExtensionProtocolV2 } from './cdpRelayV2'; import * as protocol from './protocol'; @@ -43,7 +42,7 @@ import * as protocol from './protocol'; import type websocket from 'ws'; import type { ExtensionCommandV2, ExtensionEventsV2 } from './protocol'; import type { CDPMessage } from './browserModel'; -import type { WebSocket, WebSocketServer } from 'ws'; +import type { WebSocket } from 'ws'; const debugLogger = debug('pw:mcp:relay'); @@ -58,23 +57,20 @@ type CDPCommand = { type CDPResponse = CDPMessage; export class CDPRelayServer { - private _httpServer: http.Server; - private _wsHost: string; + private _wsServer: WSServer; + private _wsHost!: string; private _browserChannel: string; private _executablePath?: string; private _userDataDir?: string; private _cdpPath: string; private _extensionPath: string; - private _wss: WebSocketServer; private _cdpConnection: WebSocket | null = null; private _extensionConnection: ExtensionConnection | null = null; private _protocolVersion: number; private _handler: ExtensionProtocolV2; private _extensionConnectionPromise = new ManualPromise(); - constructor(server: http.Server, browserChannel: string, executablePath?: string, userDataDir?: string) { - this._httpServer = server; - this._wsHost = addressToString(server.address(), { protocol: 'ws' }); + constructor(browserChannel: string, executablePath?: string, userDataDir?: string) { this._browserChannel = browserChannel; this._executablePath = executablePath; this._userDataDir = userDataDir; @@ -92,8 +88,27 @@ export class CDPRelayServer { this._extensionPath = `/extension/${uuid}`; void this._extensionConnectionPromise.catch(logUnhandledError); - this._wss = new wsServer({ server }); - this._wss.on('connection', this._onConnection.bind(this)); + this._wsServer = new WSServer({ + onRequest: (request, response) => { + response.statusCode = 404; + response.end(); + }, + onHeaders: () => {}, + onUpgrade: () => undefined, + isAllowedPathname: pathname => pathname === this._cdpPath || pathname === this._extensionPath, + onConnection: (request, url, ws) => { + debugLogger(`New connection to ${url.pathname}`); + if (url.pathname === this._cdpPath) + this._handlePlaywrightConnection(ws); + else + this._handleExtensionConnection(ws); + return undefined; + }, + }); + } + + async start(): Promise { + this._wsHost = await this._wsServer.listen(0, undefined, ''); } cdpEndpoint() { @@ -161,8 +176,7 @@ export class CDPRelayServer { stop(): void { this._closeConnections('Server stopped'); - this._wss.close(); - this._httpServer.close(); + void this._wsServer.close().catch(logUnhandledError); } private _closeConnections(reason: string) { @@ -170,19 +184,6 @@ export class CDPRelayServer { this._closeExtensionConnection(reason); } - private _onConnection(ws: WebSocket, request: http.IncomingMessage): void { - const url = new URL(`http://localhost${request.url}`); - debugLogger(`New connection to ${url.pathname}`); - if (url.pathname === this._cdpPath) { - this._handlePlaywrightConnection(ws); - } else if (url.pathname === this._extensionPath) { - this._handleExtensionConnection(ws); - } else { - debugLogger(`Invalid path: ${url.pathname}`); - ws.close(4004, 'Invalid path'); - } - } - private _handlePlaywrightConnection(ws: WebSocket): void { if (!this._extensionConnection) { debugLogger('Rejecting Playwright connection: extension not connected'); diff --git a/packages/playwright-core/src/tools/mcp/extensionContextFactory.ts b/packages/playwright-core/src/tools/mcp/extensionContextFactory.ts index 8c3f6f3e8bac7..c8c8d1d8cca6e 100644 --- a/packages/playwright-core/src/tools/mcp/extensionContextFactory.ts +++ b/packages/playwright-core/src/tools/mcp/extensionContextFactory.ts @@ -15,7 +15,6 @@ */ import debug from 'debug'; -import { createHttpServer, startHttpServer } from '@utils/network'; import { defaultUserDataDirForChannel } from '@utils/chromiumChannels'; import { playwright } from '../../inprocess'; import { isPlaywrightExtensionInstalled, playwrightExtensionInstallUrl } from '../utils/extension'; @@ -34,9 +33,8 @@ export async function createExtensionBrowser(channel: string, executablePath: st throw new Error(`Playwright Extension not found in "${userDataDir}". Install it from ${playwrightExtensionInstallUrl}`); } - const httpServer = createHttpServer(); - await startHttpServer(httpServer, {}); - const relay = new CDPRelayServer(httpServer, channel, executablePath, userDataDir); + const relay = new CDPRelayServer(channel, executablePath, userDataDir); + await relay.start(); debugLogger(`CDP relay server started, extension endpoint: ${relay.extensionEndpoint()}.`); try { diff --git a/packages/utils/httpServer.ts b/packages/utils/httpServer.ts index 4e36467248c14..e85b30d4af686 100644 --- a/packages/utils/httpServer.ts +++ b/packages/utils/httpServer.ts @@ -89,6 +89,11 @@ export class HttpServer { const pathname = new URL(request.url ?? '/', 'http://localhost').pathname; if (pathname !== wsPath) return; + if (!isAllowedHost(request, this._allowedHosts)) { + socket.write(`HTTP/${request.httpVersion} 403 Forbidden\r\n\r\n`); + socket.destroy(); + return; + } wss.handleUpgrade(request, socket, head, ws => wss.emit('connection', ws, request)); }); // HMR end @@ -268,14 +273,10 @@ export class HttpServer { return; } - if (this._allowedHosts) { - const host = request.headers.host?.toLowerCase(); - const hostname = host ? hostnameFromHostHeader(host) : undefined; - if (!hostname || !this._allowedHosts.has(hostname)) { - response.statusCode = 403; - response.end(); - return; - } + if (!isAllowedHost(request, this._allowedHosts)) { + response.statusCode = 403; + response.end(); + return; } request.on('error', () => response.end()); @@ -309,12 +310,21 @@ export function computeAllowedHosts(requested: string | undefined, bound: string return new Set(['localhost', '127.0.0.1', '[::1]']); } +// A null allowlist disables the check (server deliberately bound to a public address). +export function isAllowedHost(request: http.IncomingMessage, allowedHosts: Set | null): boolean { + if (!allowedHosts) + return true; + const host = request.headers.host?.toLowerCase(); + const hostname = host ? hostnameFromHostHeader(host) : undefined; + return !!hostname && allowedHosts.has(hostname); +} + // Bracket IPv6 literals so they can be used as the host part of a URL. export function urlHostFromAddress(address: { address: string, family: string }): string { return address.family === 'IPv6' ? `[${address.address}]` : address.address; } -export function hostnameFromHostHeader(host: string): string { +function hostnameFromHostHeader(host: string): string { if (host.startsWith('[')) { const end = host.indexOf(']'); return end < 0 ? host : host.substring(0, end + 1); diff --git a/packages/utils/wsServer.ts b/packages/utils/wsServer.ts index 93625ecb60ed0..1715dea87c4f8 100644 --- a/packages/utils/wsServer.ts +++ b/packages/utils/wsServer.ts @@ -15,7 +15,7 @@ */ import { WebSocketServer as wsServer } from 'ws'; -import { computeAllowedHosts, hostnameFromHostHeader, urlHostFromAddress } from './httpServer'; +import { computeAllowedHosts, isAllowedHost, urlHostFromAddress } from './httpServer'; import { createHttpServer } from './network'; import { debugLogger } from './debugLogger'; @@ -46,7 +46,8 @@ export type WSServerDelegate = { onRequest: (request: http.IncomingMessage, response: http.ServerResponse) => void; onHeaders: (headers: string[]) => void; onUpgrade: (request: http.IncomingMessage, socket: stream.Duplex) => { error: string } | undefined; - onConnection: (request: http.IncomingMessage, url: URL, ws: WebSocket, id: string) => WSConnection; + onConnection: (request: http.IncomingMessage, url: URL, ws: WebSocket, id: string) => WSConnection | undefined; + isAllowedPathname: (pathname: string) => boolean; }; export class WSServer { @@ -60,7 +61,7 @@ export class WSServer { this._delegate = delegate; } - async listen(port: number = 0, hostname: string | undefined, path: string): Promise { + async listen(port: number = 0, hostname: string | undefined, defaultPath: string): Promise { debugLogger.log('server', `Server started at ${new Date()}`); // Default to loopback so the WebSocket RPC is not exposed to the network unless @@ -79,14 +80,14 @@ export class WSServer { return; } if (typeof address === 'string') { - resolve(`${address}${path}`); + resolve(`${address}${defaultPath}`); return; } // Advertise the bound IP literal in the wsEndpoint so the client connects to // the same address family the server bound to. Otherwise the client and // server resolvers can disagree on what 'localhost' means (see #40605). this._allowedHosts = computeAllowedHosts(hostname, address.address); - resolve(`ws://${urlHostFromAddress(address)}:${address.port}${path}`); + resolve(`ws://${urlHostFromAddress(address)}:${address.port}${defaultPath}`); }).on('error', reject); }); @@ -101,12 +102,12 @@ export class WSServer { server.on('upgrade', (request, socket, head) => { const pathname = new URL('http://localhost' + request.url!).pathname; - if (pathname !== path) { + if (!this._delegate.isAllowedPathname(pathname)) { socket.write(`HTTP/${request.httpVersion} 400 Bad Request\r\n\r\n`); socket.destroy(); return; } - if (this._allowedHosts && !this._isAllowedOrigin(request.headers.origin)) { + if (!isAllowedHost(request, this._allowedHosts) || !this._isAllowedOrigin(request.headers.origin)) { socket.write(`HTTP/${request.httpVersion} 403 Forbidden\r\n\r\n`); socket.destroy(); return; @@ -133,25 +134,26 @@ export class WSServer { } private _onRequest(request: http.IncomingMessage, response: http.ServerResponse) { - if (this._allowedHosts) { - const host = request.headers.host?.toLowerCase(); - const hostname = host ? hostnameFromHostHeader(host) : undefined; - if (!hostname || !this._allowedHosts.has(hostname)) { - response.statusCode = 403; - response.end(); - return; - } + if (!isAllowedHost(request, this._allowedHosts)) { + response.statusCode = 403; + response.end(); + return; } this._delegate.onRequest(request, response); } private _isAllowedOrigin(origin: string | undefined): boolean { - if (!origin) + if (!this._allowedHosts || !origin) return true; try { - const hostname = new URL(origin).hostname.toLowerCase(); + const url = new URL(origin); + // Only web page origins are subject to the check; e.g. browser + // extensions are allowed. + if (url.protocol !== 'http:' && url.protocol !== 'https:') + return true; + const hostname = url.hostname.toLowerCase(); const bracketed = hostname.includes(':') ? `[${hostname}]` : hostname; - return this._allowedHosts!.has(hostname) || this._allowedHosts!.has(bracketed); + return this._allowedHosts.has(hostname) || this._allowedHosts.has(bracketed); } catch { return false; } diff --git a/tests/extension/extension.spec.ts b/tests/extension/extension.spec.ts index 17574346072f0..9945ca5a74106 100644 --- a/tests/extension/extension.spec.ts +++ b/tests/extension/extension.spec.ts @@ -17,6 +17,8 @@ import fs from 'fs/promises'; import path from 'path'; +import WebSocket from 'ws'; + import { test, testWithOldExtensionVersion, expect, extensionId, clickAllowAndSelect, connectAndNavigate, readExtensionToken, startWithExtensionFlag } from './extension-fixtures'; import { utils } from '../../packages/playwright-core/lib/coreBundle'; @@ -405,3 +407,45 @@ test(`reconnects after the extension connection drops`, { snapshot: expect.stringContaining(`Hello, world!`), }); }); + +test(`relay rejects websocket upgrades with forged host or origin`, { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright-mcp/issues/1694' }, +}, async ({ startExtensionClient, server }) => { + const { browserContext, client } = await startExtensionClient(); + + const confirmationPagePromise = browserContext.waitForEvent('page', page => { + return page.url().startsWith(`chrome-extension://${extensionId}/connect.html`); + }); + const navigateResponse = client.callTool({ + name: 'browser_navigate', + arguments: { url: server.HELLO_WORLD }, + }); + const connectPage = await confirmationPagePromise; + const relayUrl = new URL(connectPage.url()).searchParams.get('mcpRelayUrl')!; + expect(relayUrl).toBeTruthy(); + await clickAllowAndSelect(connectPage, 'Welcome'); + await navigateResponse; + + expect(await wsUpgradeResult(relayUrl, { host: 'evil.com' })).toBe(403); + expect(await wsUpgradeResult(relayUrl, { host: 'evil.com:80' })).toBe(403); + expect(await wsUpgradeResult(relayUrl, { origin: 'http://evil.com' })).toBe(403); + expect(await wsUpgradeResult(relayUrl, { origin: 'https://evil.com' })).toBe(403); + + // Control: default headers pass the upgrade validation. + expect(await wsUpgradeResult(relayUrl)).toBe('connected'); +}); + +function wsUpgradeResult(url: string, headers?: Record): Promise { + return new Promise((resolve, reject) => { + const ws = new WebSocket(url, { headers }); + ws.on('open', () => { + ws.close(); + resolve('connected'); + }); + ws.on('unexpected-response', (request, response) => { + request.destroy(); + resolve(response.statusCode!); + }); + ws.on('error', reject); + }); +}