Skip to content

Commit ade5ac0

Browse files
eleqtriziteleqtrizit
authored
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
1 parent ac21e89 commit ade5ac0

10 files changed

Lines changed: 248 additions & 12 deletions

‎extensions/browser/src/browser/pw-session.termination-cdp-ssrf.test.ts‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
// Browser tests cover pw session termination CDP SSRF guard plugin behavior.
2+
import { chromium } from "playwright-core";
3+
import { afterEach, describe, expect, it, vi } from "vitest";
4+
import * as chromeModule from "./chrome.js";
5+
import {
6+
closePlaywrightBrowserConnection,
7+
forceDisconnectPlaywrightForTarget,
8+
listPagesViaPlaywright,
9+
} from "./pw-session.js";
10+
11+
const wsMockState = vi.hoisted(() => ({
12+
constructorUrls: [] as string[],
13+
}));
14+
15+
vi.mock("ws", () => {
16+
class MockWebSocket {
17+
static OPEN = 1;
18+
19+
readyState = 0;
20+
private readonly handlers = new Map<string, (error?: Error) => void>();
21+
22+
constructor(url: string) {
23+
wsMockState.constructorUrls.push(url);
24+
setTimeout(() => {
25+
this.handlers.get("error")?.(new Error("test socket should not open"));
26+
}, 0);
27+
}
28+
29+
on(event: string, handler: (error?: Error) => void) {
30+
this.handlers.set(event, handler);
31+
return this;
32+
}
33+
34+
close() {
35+
this.readyState = 3;
36+
this.handlers.get("close")?.();
37+
}
38+
39+
send() {}
40+
}
41+
42+
return { default: MockWebSocket };
43+
});
44+
45+
const connectOverCdpSpy = vi.spyOn(chromium, "connectOverCDP");
46+
const getChromeWebSocketUrlSpy = vi.spyOn(chromeModule, "getChromeWebSocketUrl");
47+
48+
function installBrowserMock() {
49+
const sessionSend = vi.fn(async (method: string) => {
50+
if (method === "Target.getTargetInfo") {
51+
return { targetInfo: { targetId: "TARGET_1" } };
52+
}
53+
return {};
54+
});
55+
const sessionDetach = vi.fn(async () => {});
56+
const page = {
57+
on: vi.fn(),
58+
context: () => context,
59+
title: vi.fn(async () => "target"),
60+
url: vi.fn(() => "https://example.com"),
61+
} as unknown as import("playwright-core").Page;
62+
const context = {
63+
pages: () => [page],
64+
on: vi.fn(),
65+
newCDPSession: vi.fn(async () => ({
66+
send: sessionSend,
67+
detach: sessionDetach,
68+
})),
69+
} as unknown as import("playwright-core").BrowserContext;
70+
const browserClose = vi.fn(async () => {});
71+
const browser = {
72+
contexts: () => [context],
73+
on: vi.fn(),
74+
off: vi.fn(),
75+
close: browserClose,
76+
} as unknown as import("playwright-core").Browser;
77+
78+
connectOverCdpSpy.mockResolvedValue(browser);
79+
getChromeWebSocketUrlSpy.mockResolvedValue(null);
80+
return { browserClose };
81+
}
82+
83+
afterEach(async () => {
84+
connectOverCdpSpy.mockReset();
85+
getChromeWebSocketUrlSpy.mockReset();
86+
wsMockState.constructorUrls = [];
87+
await closePlaywrightBrowserConnection().catch(() => {});
88+
});
89+
90+
describe("pw-session termination CDP SSRF guard", () => {
91+
it("blocks discovered target WebSocket URLs before best-effort termination opens a socket", async () => {
92+
const { browserClose } = installBrowserMock();
93+
const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(
94+
new Response(
95+
JSON.stringify([
96+
{
97+
id: "TARGET_1",
98+
webSocketDebuggerUrl: "ws://169.254.169.254/devtools/page/TARGET_1",
99+
},
100+
]),
101+
{ status: 200 },
102+
),
103+
);
104+
105+
try {
106+
await listPagesViaPlaywright({
107+
cdpUrl: "http://127.0.0.1:18792",
108+
ssrfPolicy: { dangerouslyAllowPrivateNetwork: false },
109+
});
110+
111+
await forceDisconnectPlaywrightForTarget({
112+
cdpUrl: "http://127.0.0.1:18792",
113+
targetId: "TARGET_1",
114+
ssrfPolicy: { dangerouslyAllowPrivateNetwork: false },
115+
});
116+
117+
expect(fetchSpy).toHaveBeenCalledTimes(1);
118+
expect(fetchSpy.mock.calls[0]?.[0]).toBe("http://127.0.0.1:18792/json/list");
119+
expect(wsMockState.constructorUrls).toEqual([]);
120+
expect(browserClose).toHaveBeenCalledTimes(1);
121+
} finally {
122+
fetchSpy.mockRestore();
123+
}
124+
});
125+
});

‎extensions/browser/src/browser/pw-session.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1531,7 +1531,7 @@ async function tryTerminateExecutionViaCdp(opts: {
15311531
id?: string;
15321532
webSocketDebuggerUrl?: string;
15331533
}>
1534-
>(listUrl, 2000).catch(() => null);
1534+
>(listUrl, 2000, undefined, opts.ssrfPolicy).catch(() => null);
15351535
if (!pages || pages.length === 0) {
15361536
return;
15371537
}
@@ -1543,6 +1543,7 @@ async function tryTerminateExecutionViaCdp(opts: {
15431543
return;
15441544
}
15451545
const wsUrl = normalizeCdpWsUrl(wsUrlRaw, cdpHttpBase);
1546+
await assertCdpEndpointAllowed(wsUrl, opts.ssrfPolicy);
15461547
const needsAttach = cdpSocketNeedsAttach(wsUrl);
15471548

15481549
const runWithTimeout = async <T>(work: Promise<T>, ms: number): Promise<T> => {

‎extensions/browser/src/browser/pw-tools-core.browser-ssrf-guard.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,40 @@ describe("pw-tools-core browser SSRF guards", () => {
8282
});
8383
});
8484

85+
it("preserves SSRF policy when aborting a pending click", async () => {
86+
const ctrl = new AbortController();
87+
let clickStarted: () => void = () => {};
88+
const clickStartedPromise = new Promise<void>((resolve) => {
89+
clickStarted = resolve;
90+
});
91+
pageState.page = { url: vi.fn(() => "https://example.com") };
92+
pageState.locator = {
93+
click: vi.fn(() => {
94+
clickStarted();
95+
return new Promise(() => {});
96+
}),
97+
};
98+
99+
const task = interactions.clickViaPlaywright({
100+
cdpUrl: "http://127.0.0.1:18792",
101+
targetId: "tab-1",
102+
ref: "1",
103+
ssrfPolicy: { dangerouslyAllowPrivateNetwork: false },
104+
signal: ctrl.signal,
105+
});
106+
107+
await clickStartedPromise;
108+
ctrl.abort(new Error("aborted by test"));
109+
110+
await expect(task).rejects.toThrow("aborted by test");
111+
expect(sessionMocks.forceDisconnectPlaywrightForTarget).toHaveBeenCalledWith({
112+
cdpUrl: "http://127.0.0.1:18792",
113+
targetId: "tab-1",
114+
ssrfPolicy: { dangerouslyAllowPrivateNetwork: false },
115+
reason: "click aborted",
116+
});
117+
});
118+
85119
it("re-checks select-triggered navigations with the session safety helper", async () => {
86120
let currentUrl = "https://example.com";
87121
pageState.page = { url: vi.fn(() => currentUrl) };

‎extensions/browser/src/browser/pw-tools-core.interactions.evaluate.abort.test.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,20 @@ describe("evaluateViaPlaywright (abort)", () => {
9191
cdpUrl: "http://127.0.0.1:9222",
9292
fn,
9393
ref,
94+
ssrfPolicy: { dangerouslyAllowPrivateNetwork: false },
9495
signal: ctrl.signal,
9596
});
9697

9798
await pending.evalCalledPromise;
9899
ctrl.abort(new Error("aborted by test"));
99100

100101
await expect(p).rejects.toThrow("aborted by test");
101-
expect(forceDisconnectPlaywrightForTarget).toHaveBeenCalled();
102+
expect(forceDisconnectPlaywrightForTarget).toHaveBeenCalledWith({
103+
cdpUrl: "http://127.0.0.1:9222",
104+
targetId: undefined,
105+
ssrfPolicy: { dangerouslyAllowPrivateNetwork: false },
106+
reason: "evaluate aborted",
107+
});
102108
});
103109

104110
it("does not disconnect when evaluate is blocked by an observed dialog", async () => {

‎extensions/browser/src/browser/pw-tools-core.interactions.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,7 @@ export async function clickViaPlaywright(opts: {
558558
void forceDisconnectPlaywrightForTarget({
559559
cdpUrl: opts.cdpUrl,
560560
targetId: opts.targetId,
561+
ssrfPolicy: opts.ssrfPolicy,
561562
reason: "click aborted",
562563
}).catch(() => {});
563564
};
@@ -1025,6 +1026,7 @@ export async function evaluateViaPlaywright(opts: {
10251026
void forceDisconnectPlaywrightForTarget({
10261027
cdpUrl: opts.cdpUrl,
10271028
targetId: opts.targetId,
1029+
ssrfPolicy: opts.ssrfPolicy,
10281030
reason: "evaluate aborted",
10291031
}).catch(() => {});
10301032
});

‎extensions/browser/src/browser/pw-tools-core.snapshot.navigate-guard.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ describe("pw-tools-core.snapshot navigate guard", () => {
108108
expect(getPwToolsCoreSessionMocks().forceDisconnectPlaywrightForTarget).toHaveBeenCalledWith({
109109
cdpUrl: "http://127.0.0.1:18792",
110110
targetId: "tab-1",
111+
ssrfPolicy: { allowPrivateNetwork: true },
111112
reason: "retry navigate after detached frame",
112113
});
113114
expect(getPwToolsCoreSessionMocks().gotoPageWithNavigationGuard).toHaveBeenCalledTimes(2);

‎extensions/browser/src/browser/pw-tools-core.snapshot.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ export async function navigateViaPlaywright(opts: {
435435
await forceDisconnectPlaywrightForTarget({
436436
cdpUrl: opts.cdpUrl,
437437
targetId: opts.targetId,
438+
ssrfPolicy: opts.ssrfPolicy,
438439
reason: "retry navigate after detached frame",
439440
}).catch(() => {});
440441
page = await getPageForTargetId(opts);

‎extensions/browser/src/browser/server-context.remote-profile-tab-ops.fallback.test.ts‎

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ describe("browser remote profile fallback and attachOnly behavior", () => {
6868
id: "T1",
6969
title: "Tab 1",
7070
url: "https://example.com",
71-
webSocketDebuggerUrl: "wss://browserless.example/devtools/page/T1",
71+
webSocketDebuggerUrl: "wss://1.1.1.1:9222/devtools/page/T1",
7272
type: "page",
7373
},
7474
]),
@@ -88,21 +88,21 @@ describe("browser remote profile fallback and attachOnly behavior", () => {
8888
id: "OMNI",
8989
title: "Omnibox Popup",
9090
url: "chrome://omnibox-popup.top-chrome/",
91-
webSocketDebuggerUrl: "wss://browserless.example/devtools/page/OMNI",
91+
webSocketDebuggerUrl: "wss://1.1.1.1:9222/devtools/page/OMNI",
9292
type: "page",
9393
},
9494
{
9595
id: "UNTRUSTED",
9696
title: "Untrusted",
9797
url: "chrome-untrusted://foo/",
98-
webSocketDebuggerUrl: "wss://browserless.example/devtools/page/UNTRUSTED",
98+
webSocketDebuggerUrl: "wss://1.1.1.1:9222/devtools/page/UNTRUSTED",
9999
type: "page",
100100
},
101101
{
102102
id: "T1",
103103
title: "Tab 1",
104104
url: "https://example.com",
105-
webSocketDebuggerUrl: "wss://browserless.example/devtools/page/T1",
105+
webSocketDebuggerUrl: "wss://1.1.1.1:9222/devtools/page/T1",
106106
type: "page",
107107
},
108108
]),
@@ -113,6 +113,55 @@ describe("browser remote profile fallback and attachOnly behavior", () => {
113113
expect(tabs.map((t) => t.targetId)).toEqual(["T1"]);
114114
});
115115

116+
it("rejects policy-blocked discovered CDP websocket URLs from raw tab listings", async () => {
117+
vi.spyOn(deps.pwAiModule, "getPwAiModule").mockResolvedValue(null);
118+
const { state, remote } = deps.createRemoteRouteHarness(
119+
vi.fn(
120+
deps.createJsonListFetchMock([
121+
{
122+
id: "T_BLOCKED",
123+
title: "Blocked",
124+
url: "https://example.com",
125+
webSocketDebuggerUrl: "ws://169.254.169.254/devtools/page/T_BLOCKED",
126+
type: "page",
127+
},
128+
]),
129+
),
130+
);
131+
state.resolved.ssrfPolicy = { dangerouslyAllowPrivateNetwork: false };
132+
133+
await expect(remote.listTabs()).rejects.toBeInstanceOf(deps.BrowserCdpEndpointBlockedError);
134+
});
135+
136+
it("rejects policy-blocked discovered CDP websocket URLs from raw tab creation", async () => {
137+
vi.spyOn(deps.pwAiModule, "getPwAiModule").mockResolvedValue(null);
138+
vi.spyOn(deps.cdpModule, "createTargetViaCdp").mockRejectedValue(
139+
new Error("Target.createTarget unavailable"),
140+
);
141+
const fetchMock = vi.fn(async (url: unknown) => {
142+
const u = String(url);
143+
if (!u.includes("/json/new")) {
144+
throw new Error(`unexpected fetch: ${u}`);
145+
}
146+
return {
147+
ok: true,
148+
json: async () => ({
149+
id: "T_BLOCKED",
150+
title: "Blocked",
151+
url: "about:blank",
152+
webSocketDebuggerUrl: "ws://169.254.169.254/devtools/page/T_BLOCKED",
153+
type: "page",
154+
}),
155+
} as unknown as Response;
156+
});
157+
const { state, remote } = deps.createRemoteRouteHarness(fetchMock);
158+
state.resolved.ssrfPolicy = { dangerouslyAllowPrivateNetwork: false };
159+
160+
await expect(remote.openTab("about:blank")).rejects.toBeInstanceOf(
161+
deps.BrowserCdpEndpointBlockedError,
162+
);
163+
});
164+
116165
it("fails closed for remote tab opens in strict mode without Playwright", async () => {
117166
vi.spyOn(deps.pwAiModule, "getPwAiModule").mockResolvedValue(null);
118167
const { state, remote, fetchMock } = deps.createRemoteRouteHarness();
@@ -176,7 +225,7 @@ describe("browser remote profile fallback and attachOnly behavior", () => {
176225
id: "T_REMOTE",
177226
title: "Remote Tab",
178227
url: "https://example.com",
179-
webSocketDebuggerUrl: "wss://browserless.example/devtools/page/T_REMOTE",
228+
webSocketDebuggerUrl: "wss://1.1.1.1:9222/devtools/page/T_REMOTE",
180229
type: "page",
181230
},
182231
]),

‎extensions/browser/src/browser/server-context.remote-profile-tab-ops.test-helpers.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { afterEach, beforeEach, vi } from "vitest";
77
export type RemoteProfileTestDeps = {
88
cdpModule: typeof import("./cdp.js");
99
chromeModule: typeof import("./chrome.js");
10+
BrowserCdpEndpointBlockedError: typeof import("./errors.js").BrowserCdpEndpointBlockedError;
1011
InvalidBrowserNavigationUrlError: typeof import("./navigation-guard.js").InvalidBrowserNavigationUrlError;
1112
pwAiModule: typeof import("./pw-ai-module.js");
1213
closePlaywrightBrowserConnection: typeof import("./pw-session.js").closePlaywrightBrowserConnection;
@@ -26,6 +27,7 @@ export async function loadRemoteProfileTestDeps(): Promise<RemoteProfileTestDeps
2627
await import("./server-context.chrome-test-harness.js");
2728
const cdpModule = await import("./cdp.js");
2829
const chromeModule = await import("./chrome.js");
30+
const { BrowserCdpEndpointBlockedError } = await import("./errors.js");
2931
const { InvalidBrowserNavigationUrlError } = await import("./navigation-guard.js");
3032
const pwAiModule = await import("./pw-ai-module.js");
3133
const { closePlaywrightBrowserConnection } = await import("./pw-session.js");
@@ -40,6 +42,7 @@ export async function loadRemoteProfileTestDeps(): Promise<RemoteProfileTestDeps
4042
return {
4143
cdpModule,
4244
chromeModule,
45+
BrowserCdpEndpointBlockedError,
4346
InvalidBrowserNavigationUrlError,
4447
pwAiModule,
4548
closePlaywrightBrowserConnection,

0 commit comments

Comments
 (0)