From ade5ac03506e4a8f4d06ffb99c8b5500a3f0db28 Mon Sep 17 00:00:00 2001 From: Agustin Rivera <31522568+eleqtrizit@users.noreply.github.com> Date: Wed, 10 Jun 2026 07:59:29 -0700 Subject: [PATCH] fix(browser): validate discovered CDP websocket URLs (#91747) * fix(browser): validate discovered cdp websocket urls * fix(browser): validate cdp tab creation websockets * fix(browser): guard termination cdp websocket * fix(browser): use .toString() instead of String() to satisfy oxlint no-base-to-string * fix(browser): avoid cdp termination assertion stringification * fix(browser): preserve cdp ssrf policy --- .../pw-session.termination-cdp-ssrf.test.ts | 125 ++++++++++++++++++ extensions/browser/src/browser/pw-session.ts | 3 +- .../pw-tools-core.browser-ssrf-guard.test.ts | 34 +++++ ...s-core.interactions.evaluate.abort.test.ts | 8 +- .../src/browser/pw-tools-core.interactions.ts | 2 + ...tools-core.snapshot.navigate-guard.test.ts | 1 + .../src/browser/pw-tools-core.snapshot.ts | 1 + ...xt.remote-profile-tab-ops.fallback.test.ts | 59 ++++++++- ...ext.remote-profile-tab-ops.test-helpers.ts | 3 + .../src/browser/server-context.tab-ops.ts | 24 +++- 10 files changed, 248 insertions(+), 12 deletions(-) create mode 100644 extensions/browser/src/browser/pw-session.termination-cdp-ssrf.test.ts diff --git a/extensions/browser/src/browser/pw-session.termination-cdp-ssrf.test.ts b/extensions/browser/src/browser/pw-session.termination-cdp-ssrf.test.ts new file mode 100644 index 00000000000..2a8a00d1830 --- /dev/null +++ b/extensions/browser/src/browser/pw-session.termination-cdp-ssrf.test.ts @@ -0,0 +1,125 @@ +// Browser tests cover pw session termination CDP SSRF guard plugin behavior. +import { chromium } from "playwright-core"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import * as chromeModule from "./chrome.js"; +import { + closePlaywrightBrowserConnection, + forceDisconnectPlaywrightForTarget, + listPagesViaPlaywright, +} from "./pw-session.js"; + +const wsMockState = vi.hoisted(() => ({ + constructorUrls: [] as string[], +})); + +vi.mock("ws", () => { + class MockWebSocket { + static OPEN = 1; + + readyState = 0; + private readonly handlers = new Map void>(); + + constructor(url: string) { + wsMockState.constructorUrls.push(url); + setTimeout(() => { + this.handlers.get("error")?.(new Error("test socket should not open")); + }, 0); + } + + on(event: string, handler: (error?: Error) => void) { + this.handlers.set(event, handler); + return this; + } + + close() { + this.readyState = 3; + this.handlers.get("close")?.(); + } + + send() {} + } + + return { default: MockWebSocket }; +}); + +const connectOverCdpSpy = vi.spyOn(chromium, "connectOverCDP"); +const getChromeWebSocketUrlSpy = vi.spyOn(chromeModule, "getChromeWebSocketUrl"); + +function installBrowserMock() { + const sessionSend = vi.fn(async (method: string) => { + if (method === "Target.getTargetInfo") { + return { targetInfo: { targetId: "TARGET_1" } }; + } + return {}; + }); + const sessionDetach = vi.fn(async () => {}); + const page = { + on: vi.fn(), + context: () => context, + title: vi.fn(async () => "target"), + url: vi.fn(() => "https://example.com"), + } as unknown as import("playwright-core").Page; + const context = { + pages: () => [page], + on: vi.fn(), + newCDPSession: vi.fn(async () => ({ + send: sessionSend, + detach: sessionDetach, + })), + } as unknown as import("playwright-core").BrowserContext; + const browserClose = vi.fn(async () => {}); + const browser = { + contexts: () => [context], + on: vi.fn(), + off: vi.fn(), + close: browserClose, + } as unknown as import("playwright-core").Browser; + + connectOverCdpSpy.mockResolvedValue(browser); + getChromeWebSocketUrlSpy.mockResolvedValue(null); + return { browserClose }; +} + +afterEach(async () => { + connectOverCdpSpy.mockReset(); + getChromeWebSocketUrlSpy.mockReset(); + wsMockState.constructorUrls = []; + await closePlaywrightBrowserConnection().catch(() => {}); +}); + +describe("pw-session termination CDP SSRF guard", () => { + it("blocks discovered target WebSocket URLs before best-effort termination opens a socket", async () => { + const { browserClose } = installBrowserMock(); + const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response( + JSON.stringify([ + { + id: "TARGET_1", + webSocketDebuggerUrl: "ws://169.254.169.254/devtools/page/TARGET_1", + }, + ]), + { status: 200 }, + ), + ); + + try { + await listPagesViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + }); + + await forceDisconnectPlaywrightForTarget({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "TARGET_1", + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + }); + + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(fetchSpy.mock.calls[0]?.[0]).toBe("http://127.0.0.1:18792/json/list"); + expect(wsMockState.constructorUrls).toEqual([]); + expect(browserClose).toHaveBeenCalledTimes(1); + } finally { + fetchSpy.mockRestore(); + } + }); +}); diff --git a/extensions/browser/src/browser/pw-session.ts b/extensions/browser/src/browser/pw-session.ts index a70e8e42ee1..ce98df2d98b 100644 --- a/extensions/browser/src/browser/pw-session.ts +++ b/extensions/browser/src/browser/pw-session.ts @@ -1531,7 +1531,7 @@ async function tryTerminateExecutionViaCdp(opts: { id?: string; webSocketDebuggerUrl?: string; }> - >(listUrl, 2000).catch(() => null); + >(listUrl, 2000, undefined, opts.ssrfPolicy).catch(() => null); if (!pages || pages.length === 0) { return; } @@ -1543,6 +1543,7 @@ async function tryTerminateExecutionViaCdp(opts: { return; } const wsUrl = normalizeCdpWsUrl(wsUrlRaw, cdpHttpBase); + await assertCdpEndpointAllowed(wsUrl, opts.ssrfPolicy); const needsAttach = cdpSocketNeedsAttach(wsUrl); const runWithTimeout = async (work: Promise, ms: number): Promise => { diff --git a/extensions/browser/src/browser/pw-tools-core.browser-ssrf-guard.test.ts b/extensions/browser/src/browser/pw-tools-core.browser-ssrf-guard.test.ts index 4b1c3b2f287..bbc85b3acbb 100644 --- a/extensions/browser/src/browser/pw-tools-core.browser-ssrf-guard.test.ts +++ b/extensions/browser/src/browser/pw-tools-core.browser-ssrf-guard.test.ts @@ -82,6 +82,40 @@ describe("pw-tools-core browser SSRF guards", () => { }); }); + it("preserves SSRF policy when aborting a pending click", async () => { + const ctrl = new AbortController(); + let clickStarted: () => void = () => {}; + const clickStartedPromise = new Promise((resolve) => { + clickStarted = resolve; + }); + pageState.page = { url: vi.fn(() => "https://example.com") }; + pageState.locator = { + click: vi.fn(() => { + clickStarted(); + return new Promise(() => {}); + }), + }; + + const task = interactions.clickViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "tab-1", + ref: "1", + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + signal: ctrl.signal, + }); + + await clickStartedPromise; + ctrl.abort(new Error("aborted by test")); + + await expect(task).rejects.toThrow("aborted by test"); + expect(sessionMocks.forceDisconnectPlaywrightForTarget).toHaveBeenCalledWith({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "tab-1", + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + reason: "click aborted", + }); + }); + it("re-checks select-triggered navigations with the session safety helper", async () => { let currentUrl = "https://example.com"; pageState.page = { url: vi.fn(() => currentUrl) }; diff --git a/extensions/browser/src/browser/pw-tools-core.interactions.evaluate.abort.test.ts b/extensions/browser/src/browser/pw-tools-core.interactions.evaluate.abort.test.ts index ddd40a203a9..f8d6bf8ffae 100644 --- a/extensions/browser/src/browser/pw-tools-core.interactions.evaluate.abort.test.ts +++ b/extensions/browser/src/browser/pw-tools-core.interactions.evaluate.abort.test.ts @@ -91,6 +91,7 @@ describe("evaluateViaPlaywright (abort)", () => { cdpUrl: "http://127.0.0.1:9222", fn, ref, + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, signal: ctrl.signal, }); @@ -98,7 +99,12 @@ describe("evaluateViaPlaywright (abort)", () => { ctrl.abort(new Error("aborted by test")); await expect(p).rejects.toThrow("aborted by test"); - expect(forceDisconnectPlaywrightForTarget).toHaveBeenCalled(); + expect(forceDisconnectPlaywrightForTarget).toHaveBeenCalledWith({ + cdpUrl: "http://127.0.0.1:9222", + targetId: undefined, + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + reason: "evaluate aborted", + }); }); it("does not disconnect when evaluate is blocked by an observed dialog", async () => { diff --git a/extensions/browser/src/browser/pw-tools-core.interactions.ts b/extensions/browser/src/browser/pw-tools-core.interactions.ts index 7d94710aaee..b7119754bad 100644 --- a/extensions/browser/src/browser/pw-tools-core.interactions.ts +++ b/extensions/browser/src/browser/pw-tools-core.interactions.ts @@ -558,6 +558,7 @@ export async function clickViaPlaywright(opts: { void forceDisconnectPlaywrightForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, + ssrfPolicy: opts.ssrfPolicy, reason: "click aborted", }).catch(() => {}); }; @@ -1025,6 +1026,7 @@ export async function evaluateViaPlaywright(opts: { void forceDisconnectPlaywrightForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, + ssrfPolicy: opts.ssrfPolicy, reason: "evaluate aborted", }).catch(() => {}); }); diff --git a/extensions/browser/src/browser/pw-tools-core.snapshot.navigate-guard.test.ts b/extensions/browser/src/browser/pw-tools-core.snapshot.navigate-guard.test.ts index 5b19c7d1655..a81ab58700c 100644 --- a/extensions/browser/src/browser/pw-tools-core.snapshot.navigate-guard.test.ts +++ b/extensions/browser/src/browser/pw-tools-core.snapshot.navigate-guard.test.ts @@ -108,6 +108,7 @@ describe("pw-tools-core.snapshot navigate guard", () => { expect(getPwToolsCoreSessionMocks().forceDisconnectPlaywrightForTarget).toHaveBeenCalledWith({ cdpUrl: "http://127.0.0.1:18792", targetId: "tab-1", + ssrfPolicy: { allowPrivateNetwork: true }, reason: "retry navigate after detached frame", }); expect(getPwToolsCoreSessionMocks().gotoPageWithNavigationGuard).toHaveBeenCalledTimes(2); diff --git a/extensions/browser/src/browser/pw-tools-core.snapshot.ts b/extensions/browser/src/browser/pw-tools-core.snapshot.ts index 1a74f129b5d..eb18416486a 100644 --- a/extensions/browser/src/browser/pw-tools-core.snapshot.ts +++ b/extensions/browser/src/browser/pw-tools-core.snapshot.ts @@ -435,6 +435,7 @@ export async function navigateViaPlaywright(opts: { await forceDisconnectPlaywrightForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, + ssrfPolicy: opts.ssrfPolicy, reason: "retry navigate after detached frame", }).catch(() => {}); page = await getPageForTargetId(opts); diff --git a/extensions/browser/src/browser/server-context.remote-profile-tab-ops.fallback.test.ts b/extensions/browser/src/browser/server-context.remote-profile-tab-ops.fallback.test.ts index eb880b51edb..162fd775d55 100644 --- a/extensions/browser/src/browser/server-context.remote-profile-tab-ops.fallback.test.ts +++ b/extensions/browser/src/browser/server-context.remote-profile-tab-ops.fallback.test.ts @@ -68,7 +68,7 @@ describe("browser remote profile fallback and attachOnly behavior", () => { id: "T1", title: "Tab 1", url: "https://example.com", - webSocketDebuggerUrl: "wss://browserless.example/devtools/page/T1", + webSocketDebuggerUrl: "wss://1.1.1.1:9222/devtools/page/T1", type: "page", }, ]), @@ -88,21 +88,21 @@ describe("browser remote profile fallback and attachOnly behavior", () => { id: "OMNI", title: "Omnibox Popup", url: "chrome://omnibox-popup.top-chrome/", - webSocketDebuggerUrl: "wss://browserless.example/devtools/page/OMNI", + webSocketDebuggerUrl: "wss://1.1.1.1:9222/devtools/page/OMNI", type: "page", }, { id: "UNTRUSTED", title: "Untrusted", url: "chrome-untrusted://foo/", - webSocketDebuggerUrl: "wss://browserless.example/devtools/page/UNTRUSTED", + webSocketDebuggerUrl: "wss://1.1.1.1:9222/devtools/page/UNTRUSTED", type: "page", }, { id: "T1", title: "Tab 1", url: "https://example.com", - webSocketDebuggerUrl: "wss://browserless.example/devtools/page/T1", + webSocketDebuggerUrl: "wss://1.1.1.1:9222/devtools/page/T1", type: "page", }, ]), @@ -113,6 +113,55 @@ describe("browser remote profile fallback and attachOnly behavior", () => { expect(tabs.map((t) => t.targetId)).toEqual(["T1"]); }); + it("rejects policy-blocked discovered CDP websocket URLs from raw tab listings", async () => { + vi.spyOn(deps.pwAiModule, "getPwAiModule").mockResolvedValue(null); + const { state, remote } = deps.createRemoteRouteHarness( + vi.fn( + deps.createJsonListFetchMock([ + { + id: "T_BLOCKED", + title: "Blocked", + url: "https://example.com", + webSocketDebuggerUrl: "ws://169.254.169.254/devtools/page/T_BLOCKED", + type: "page", + }, + ]), + ), + ); + state.resolved.ssrfPolicy = { dangerouslyAllowPrivateNetwork: false }; + + await expect(remote.listTabs()).rejects.toBeInstanceOf(deps.BrowserCdpEndpointBlockedError); + }); + + it("rejects policy-blocked discovered CDP websocket URLs from raw tab creation", async () => { + vi.spyOn(deps.pwAiModule, "getPwAiModule").mockResolvedValue(null); + vi.spyOn(deps.cdpModule, "createTargetViaCdp").mockRejectedValue( + new Error("Target.createTarget unavailable"), + ); + const fetchMock = vi.fn(async (url: unknown) => { + const u = String(url); + if (!u.includes("/json/new")) { + throw new Error(`unexpected fetch: ${u}`); + } + return { + ok: true, + json: async () => ({ + id: "T_BLOCKED", + title: "Blocked", + url: "about:blank", + webSocketDebuggerUrl: "ws://169.254.169.254/devtools/page/T_BLOCKED", + type: "page", + }), + } as unknown as Response; + }); + const { state, remote } = deps.createRemoteRouteHarness(fetchMock); + state.resolved.ssrfPolicy = { dangerouslyAllowPrivateNetwork: false }; + + await expect(remote.openTab("about:blank")).rejects.toBeInstanceOf( + deps.BrowserCdpEndpointBlockedError, + ); + }); + it("fails closed for remote tab opens in strict mode without Playwright", async () => { vi.spyOn(deps.pwAiModule, "getPwAiModule").mockResolvedValue(null); const { state, remote, fetchMock } = deps.createRemoteRouteHarness(); @@ -176,7 +225,7 @@ describe("browser remote profile fallback and attachOnly behavior", () => { id: "T_REMOTE", title: "Remote Tab", url: "https://example.com", - webSocketDebuggerUrl: "wss://browserless.example/devtools/page/T_REMOTE", + webSocketDebuggerUrl: "wss://1.1.1.1:9222/devtools/page/T_REMOTE", type: "page", }, ]), diff --git a/extensions/browser/src/browser/server-context.remote-profile-tab-ops.test-helpers.ts b/extensions/browser/src/browser/server-context.remote-profile-tab-ops.test-helpers.ts index c880b693c38..9568a96e20f 100644 --- a/extensions/browser/src/browser/server-context.remote-profile-tab-ops.test-helpers.ts +++ b/extensions/browser/src/browser/server-context.remote-profile-tab-ops.test-helpers.ts @@ -7,6 +7,7 @@ import { afterEach, beforeEach, vi } from "vitest"; export type RemoteProfileTestDeps = { cdpModule: typeof import("./cdp.js"); chromeModule: typeof import("./chrome.js"); + BrowserCdpEndpointBlockedError: typeof import("./errors.js").BrowserCdpEndpointBlockedError; InvalidBrowserNavigationUrlError: typeof import("./navigation-guard.js").InvalidBrowserNavigationUrlError; pwAiModule: typeof import("./pw-ai-module.js"); closePlaywrightBrowserConnection: typeof import("./pw-session.js").closePlaywrightBrowserConnection; @@ -26,6 +27,7 @@ export async function loadRemoteProfileTestDeps(): Promise >(appendCdpPath(cdpHttpBase, "/json/list"), undefined, undefined, getCdpControlPolicy()); - return raw - .map((t) => ({ + const cdpControlPolicy = getCdpControlPolicy(); + const tabs: BrowserTab[] = []; + for (const t of raw) { + const tab: BrowserTab = { targetId: t.id ?? "", title: t.title ?? "", url: t.url ?? "", wsUrl: normalizeWsUrl(t.webSocketDebuggerUrl, profile.cdpUrl), type: t.type, - })) - .filter((t) => Boolean(t.targetId) && isSelectableCdpBrowserTarget(t)); + }; + if (!tab.targetId || !isSelectableCdpBrowserTarget(tab)) { + continue; + } + if (tab.wsUrl) { + await assertCdpEndpointAllowed(tab.wsUrl, cdpControlPolicy); + } + tabs.push(tab); + } + return tabs; }; const listTabs = async (): Promise => { @@ -400,6 +410,10 @@ export function createProfileTabOps({ profileState.lastTargetId = created.id; const resolvedUrl = created.url ?? url; await assertBrowserNavigationResultAllowed({ url: resolvedUrl, ...ssrfPolicyOpts }); + const wsUrl = normalizeWsUrl(created.webSocketDebuggerUrl, profile.cdpUrl); + if (wsUrl) { + await assertCdpEndpointAllowed(wsUrl, getCdpControlPolicy()); + } triggerManagedTabLimit(created.id); return assignTabAlias({ profileState, @@ -408,7 +422,7 @@ export function createProfileTabOps({ targetId: created.id, title: created.title ?? "", url: resolvedUrl, - wsUrl: normalizeWsUrl(created.webSocketDebuggerUrl, profile.cdpUrl), + wsUrl, type: created.type, }, });