diff --git a/src/browser/CdpConnection.ts b/src/browser/CdpConnection.ts index 280134ee5..fc4d069b8 100644 --- a/src/browser/CdpConnection.ts +++ b/src/browser/CdpConnection.ts @@ -154,20 +154,35 @@ export class CdpConnection { this.#failPending("protocol_error"); return; } - if ("id" in message && typeof message.id === "number") { + if ("id" in message) { + if ( + typeof message.id !== "number" || + !Number.isSafeInteger(message.id) || + message.id < 1 + ) { + this.#failPending("protocol_error"); + return; + } this.#receiveResponse(message, message.id); return; } - if ("method" in message && typeof message.method === "string") { - const event: CdpEvent = { - method: message.method, - params: "params" in message ? message.params : {}, - ...(typeof message.sessionId === "string" - ? { sessionId: message.sessionId } - : {}), - }; - for (const listener of this.#listeners) listener(event); + if ( + typeof message.method !== "string" || + message.method.length === 0 || + ("params" in message && !isRecord(message.params)) || + ("sessionId" in message && typeof message.sessionId !== "string") + ) { + this.#failPending("protocol_error"); + return; } + const event: CdpEvent = { + method: message.method, + params: "params" in message ? message.params : {}, + ...(typeof message.sessionId === "string" + ? { sessionId: message.sessionId } + : {}), + }; + for (const listener of this.#listeners) listener(event); } #receiveResponse(message: Record, id: number): void { @@ -216,7 +231,7 @@ const waitForOpen = async ( await new Promise((resolve, reject) => { const timer = setTimeout(() => { cleanup(); - socket.terminate(); + terminateSocket(socket); reject(new AnalysisTimeoutError("inspect_web_page", CONNECT_TIMEOUT_MS)); }, CONNECT_TIMEOUT_MS); const onOpen = (): void => { @@ -236,7 +251,7 @@ const waitForOpen = async ( ); }; const onAbort = (): void => { - socket.terminate(); + terminateSocket(socket); onFailure(new AnalysisCancelledError("inspect_web_page")); }; const cleanup = (): void => { @@ -250,3 +265,8 @@ const waitForOpen = async ( signal?.addEventListener("abort", onAbort, { once: true }); if (signal?.aborted === true) onAbort(); }); + +const terminateSocket = (socket: WebSocket): void => { + socket.once("error", () => undefined); + socket.terminate(); +}; diff --git a/src/browser/CdpPageCapture.ts b/src/browser/CdpPageCapture.ts index 80b396e20..1be836703 100644 --- a/src/browser/CdpPageCapture.ts +++ b/src/browser/CdpPageCapture.ts @@ -64,9 +64,10 @@ export const capturePage = async ( ): Promise => { const allowedOrigins = new Set(context.input.allowed_origins); const events = new CdpCaptureEvents(context.input, allowedOrigins); - const removeListener = context.connection.onEvent((event) => - events.ingest(event), - ); + const removeListener = context.connection.onEvent((event) => { + if (event.sessionId !== context.sessionId) return; + events.ingest(event); + }); const state: CaptureState = { context, events, diff --git a/tests/browserCli.test.ts b/tests/browserCli.test.ts index 364479d37..6d4177bd4 100644 --- a/tests/browserCli.test.ts +++ b/tests/browserCli.test.ts @@ -84,27 +84,31 @@ describe("browser CLI parity", () => { INTEGRATION_TEST_TIMEOUT_MS, ); - it("requires explicit approval before opening the CDP endpoint", async () => { - const browser = await startFakeCdpBrowser(); - browsers.push(browser); - const result = await runCli( - ["list-browser-targets", browser.endpoint, "--json"], - { - ...process.env, - REA_BROWSER_OBSERVE_ENABLED: "true", - REA_BROWSER_CDP_ENDPOINTS_JSON: JSON.stringify([browser.endpoint]), - REA_BROWSER_ALLOWED_ORIGINS_JSON: JSON.stringify([ - browser.allowedOrigin, - ]), - }, - ); - expect(result).toMatchObject({ - error: "Browser observation failed", - code: "invalid_request", - category: "invalid_input", - }); - expect(browser.commands).toHaveLength(0); - }); + it( + "requires explicit approval before opening the CDP endpoint", + async () => { + const browser = await startFakeCdpBrowser(); + browsers.push(browser); + const result = await runCli( + ["list-browser-targets", browser.endpoint, "--json"], + { + ...process.env, + REA_BROWSER_OBSERVE_ENABLED: "true", + REA_BROWSER_CDP_ENDPOINTS_JSON: JSON.stringify([browser.endpoint]), + REA_BROWSER_ALLOWED_ORIGINS_JSON: JSON.stringify([ + browser.allowedOrigin, + ]), + }, + ); + expect(result).toMatchObject({ + error: "Browser observation failed", + code: "invalid_request", + category: "invalid_input", + }); + expect(browser.commands).toHaveLength(0); + }, + INTEGRATION_TEST_TIMEOUT_MS, + ); }); const runCli = async ( diff --git a/tests/cdpBrowserProvider.test.ts b/tests/cdpBrowserProvider.test.ts index 5db899f8c..a2b4649a9 100644 --- a/tests/cdpBrowserProvider.test.ts +++ b/tests/cdpBrowserProvider.test.ts @@ -50,7 +50,7 @@ describe("CdpBrowserProvider", () => { }); it("captures bounded passive evidence without retaining sensitive values", async () => { - const browser = await startFakeCdpBrowser(); + const browser = await startFakeCdpBrowser({ foreignSessionEvents: true }); browsers.push(browser); const provider = new CdpBrowserProvider(); const result = await provider.inspectPage( @@ -71,6 +71,7 @@ describe("CdpBrowserProvider", () => { expect(result.value.dom.nodes[1]?.attribute_names).toEqual(["token"]); expect(result.value.accessibility.nodes[0]?.name).toBe("Submit report"); expect(result.value.scripts.items).toHaveLength(1); + expect(result.value.scripts.items[0]?.script_id).toBe("script-allowed"); expect(result.value.scripts.items[0]?.source).toEqual({ included: false, reason: "source capture was not approved", @@ -384,6 +385,24 @@ describe("CdpBrowserProvider", () => { ok: false, error: { _tag: "BrowserObservationError", reason: "protocol_error" }, }); + + const malformedEventShape = await startFakeCdpBrowser({ + malformedEventShapeOnMethod: "Debugger.enable", + }); + browsers.push(malformedEventShape); + const eventShapeProtocol = await new CdpBrowserProvider().inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: malformedEventShape.endpoint, + allowed_origins: [malformedEventShape.allowedOrigin], + target_id: "allowed-page", + approved: true, + observation_ms: 0, + }), + ); + expect(eventShapeProtocol).toMatchObject({ + ok: false, + error: { _tag: "BrowserObservationError", reason: "protocol_error" }, + }); }); it("cancels observation and still detaches without closing the page", async () => { @@ -400,7 +419,8 @@ describe("CdpBrowserProvider", () => { }), { signal: controller.signal }, ); - setTimeout(() => controller.abort(), 20); + await waitForCommand(browser, "Target.attachToTarget"); + controller.abort(); const result = await pending; expect(result).toMatchObject({ ok: false, @@ -495,3 +515,14 @@ describe("CdpBrowserProvider", () => { }); }); }); + +const waitForCommand = async ( + browser: FakeCdpBrowser, + method: string, +): Promise => { + for (let attempt = 0; attempt < 200; attempt += 1) { + if (browser.commands.some((command) => command.method === method)) return; + await new Promise((resolve) => setTimeout(resolve, 5)); + } + throw new Error(`Timed out waiting for fake CDP command ${method}`); +}; diff --git a/tests/fixtures/fakeCdpBrowser.ts b/tests/fixtures/fakeCdpBrowser.ts index 6183a728f..0b3b43a94 100644 --- a/tests/fixtures/fakeCdpBrowser.ts +++ b/tests/fixtures/fakeCdpBrowser.ts @@ -23,6 +23,7 @@ interface FakeOptions { readonly invalidBrowserWebSocket?: boolean; readonly malformedMessageOnMethod?: string; readonly malformedEventOnMethod?: string; + readonly malformedEventShapeOnMethod?: string; readonly closeOnMethod?: string; readonly hangOnMethod?: string; readonly unsupportedMethods?: readonly string[]; @@ -31,6 +32,7 @@ interface FakeOptions { readonly navigateDuringObservationUrl?: string; readonly navigateDuringCaptureUrl?: string; readonly extraCollections?: boolean; + readonly foreignSessionEvents?: boolean; } /** Start a real HTTP/WebSocket fake at the same seams as a user-owned browser. */ @@ -114,6 +116,8 @@ export const startFakeCdpBrowser = async ( ); if (options.malformedEventOnMethod === command.method) socket.send("{not-json"); + if (options.malformedEventShapeOnMethod === command.method) + socket.send(JSON.stringify({ method: 42 })); emitEvents(socket, command, port, options); }); }); @@ -426,6 +430,14 @@ const emitEvents = ( options: FakeOptions, ): void => { if (command.method === "Debugger.enable") { + if (options.foreignSessionEvents === true) + event(socket, "Debugger.scriptParsed", "foreign-session", { + scriptId: "script-foreign", + url: `http://127.0.0.1:${String(port)}/foreign.js`, + hash: "foreign-hash", + length: 20, + isModule: false, + }); event(socket, "Debugger.scriptParsed", command.sessionId, { scriptId: "script-allowed", url: `http://127.0.0.1:${String(port)}/app.js?token=script-secret`,