diff --git a/CHANGELOG.md b/CHANGELOG.md index 0253db8b6a1..1e5e4de0808 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -130,6 +130,7 @@ Docs: https://docs.openclaw.ai - Browser/security: guard existing-session Chrome MCP interaction routes with SSRF post-checks so delayed navigation from click, type, press, and evaluate cannot bypass the configured policy. (#64370) Thanks @eleqtrizit. - Browser/security: default browser SSRF policy to strict mode so unconfigured installs block private-network navigation, and align external-content marker span mapping so ZWS-injected boundary spoofs are fully sanitized. (#63885) Thanks @eleqtrizit. +- Browser/security: apply SSRF navigation policy to subframe document navigations so iframe-targeted private-network hops are blocked without quarantining the parent page. (#64371) Thanks @eleqtrizit. ## 2026.4.9 ### Changes diff --git a/extensions/browser/src/browser/pw-session.create-page.navigation-guard.test.ts b/extensions/browser/src/browser/pw-session.create-page.navigation-guard.test.ts index e6321c98b4c..35d4f9095b7 100644 --- a/extensions/browser/src/browser/pw-session.create-page.navigation-guard.test.ts +++ b/extensions/browser/src/browser/pw-session.create-page.navigation-guard.test.ts @@ -126,6 +126,7 @@ async function dispatchMockNavigation(params: { getRouteHandler: () => MockRouteHandler | null; mainFrame: object; url: string; + frame?: object; isNavigationRequest?: boolean; resourceType?: string; route?: Partial; @@ -137,7 +138,7 @@ async function dispatchMockNavigation(params: { const { resourceType } = params; await handler(createMockRoute(params.route), { isNavigationRequest: () => params.isNavigationRequest ?? true, - frame: () => params.mainFrame, + frame: () => params.frame ?? params.mainFrame, ...(resourceType ? { resourceType: () => resourceType } : {}), url: () => params.url, }); @@ -237,6 +238,41 @@ describe("pw-session createPageViaPlaywright navigation guard", () => { expect(pageClose).toHaveBeenCalledTimes(1); }); + it("aborts private subframe document hops without quarantining the page", async () => { + const { pageGoto, pageClose, getRouteHandler, mainFrame } = installBrowserMocks(); + const subframe = {}; + const subframeRoute = createMockRoute(); + pageGoto.mockImplementationOnce(async () => { + await dispatchMockNavigation({ + getRouteHandler, + mainFrame, + url: "https://93.184.216.34/start", + }); + await dispatchMockNavigation({ + getRouteHandler, + mainFrame, + frame: subframe, + url: "http://127.0.0.1:18080/internal-hop", + route: subframeRoute, + }); + return { + request: () => ({ + url: () => "https://93.184.216.34/start", + redirectedFrom: () => null, + }), + }; + }); + + const created = await createPageViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + url: "https://93.184.216.34/start", + }); + + expect(created.targetId).toBe("TARGET_1"); + expect(subframeRoute.abort).toHaveBeenCalledTimes(1); + expect(pageClose).not.toHaveBeenCalled(); + }); + it("preserves the created tab on ordinary navigation failure", async () => { const { pageGoto, pageClose } = installBrowserMocks(); pageGoto.mockRejectedValueOnce(new Error("page.goto: net::ERR_NAME_NOT_RESOLVED")); diff --git a/extensions/browser/src/browser/pw-session.ts b/extensions/browser/src/browser/pw-session.ts index 71daadd28fc..8502286dcdd 100644 --- a/extensions/browser/src/browser/pw-session.ts +++ b/extensions/browser/src/browser/pw-session.ts @@ -704,6 +704,36 @@ function isTopLevelNavigationRequest(page: Page, request: Request): boolean { } } +function isSubframeDocumentNavigationRequest(page: Page, request: Request): boolean { + let sameMainFrame = false; + try { + sameMainFrame = request.frame() === page.mainFrame(); + } catch { + // Fail closed: if frame resolution throws after the top-level check already + // determined this is NOT the main frame, treat it as a subframe document + // navigation so the SSRF guard still fires. Returning false here would let + // transient renderer churn skip the policy check entirely. + return true; + } + if (sameMainFrame) { + return false; + } + + try { + if (request.isNavigationRequest()) { + return true; + } + } catch { + // Fall through to the resource-type check. + } + + try { + return request.resourceType() === "document"; + } catch { + return false; + } +} + function isPolicyDenyNavigationError(err: unknown): boolean { return err instanceof SsrFBlockedError || err instanceof InvalidBrowserNavigationUrlError; } @@ -769,7 +799,10 @@ export async function gotoPageWithNavigationGuard(opts: { await route.abort().catch(() => {}); return; } - if (!isTopLevelNavigationRequest(opts.page, request)) { + const isTopLevel = isTopLevelNavigationRequest(opts.page, request); + const isSubframeDocument = + !isTopLevel && isSubframeDocumentNavigationRequest(opts.page, request); + if (!isTopLevel && !isSubframeDocument) { await route.continue(); return; } @@ -780,7 +813,9 @@ export async function gotoPageWithNavigationGuard(opts: { }); } catch (err) { if (isPolicyDenyNavigationError(err)) { - blockedError = err; + if (isTopLevel) { + blockedError = err; + } await route.abort().catch(() => {}); return; } diff --git a/extensions/browser/src/browser/pw-tools-core.interactions.navigation-guard.test.ts b/extensions/browser/src/browser/pw-tools-core.interactions.navigation-guard.test.ts index ac5364e1d4f..e5576078219 100644 --- a/extensions/browser/src/browser/pw-tools-core.interactions.navigation-guard.test.ts +++ b/extensions/browser/src/browser/pw-tools-core.interactions.navigation-guard.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import { + getPwToolsCoreNavigationGuardMocks, getPwToolsCoreSessionMocks, installPwToolsCoreTestHooks, setPwToolsCoreCurrentPage, @@ -9,6 +10,18 @@ import { installPwToolsCoreTestHooks(); const mod = await import("./pw-tools-core.js"); +function createMutableFrame(initialUrl: string) { + let currentUrl = initialUrl; + return { + frame: { + url: vi.fn(() => currentUrl), + }, + setUrl: (nextUrl: string) => { + currentUrl = nextUrl; + }, + }; +} + describe("pw-tools-core interaction navigation guard", () => { it("waits for the grace window before completing a successful non-navigating click", async () => { vi.useFakeTimers(); @@ -120,12 +133,12 @@ describe("pw-tools-core interaction navigation guard", () => { } }); - it("ignores subframe framenavigated events before the main frame navigates", async () => { + it("checks subframe navigations before a later main-frame navigation", async () => { vi.useFakeTimers(); try { const listeners = new Set<(frame: object) => void>(); const mainFrame = {}; - const subframe = {}; + const subframe = { url: () => "https://example.com/embed" }; let currentUrl = "http://127.0.0.1:9222/json/version"; const click = vi.fn(async () => { setTimeout(() => { @@ -169,10 +182,449 @@ describe("pw-tools-core interaction navigation guard", () => { expect( getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely, ).not.toHaveBeenCalled(); + expect( + getPwToolsCoreNavigationGuardMocks().assertBrowserNavigationResultAllowed, + ).not.toHaveBeenCalled(); await vi.advanceTimersByTimeAsync(10); await task; + expect( + getPwToolsCoreNavigationGuardMocks().assertBrowserNavigationResultAllowed, + ).toHaveBeenCalledWith({ + ssrfPolicy: { allowPrivateNetwork: false }, + url: "https://example.com/embed", + }); + expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).toHaveBeenCalledWith( + { + cdpUrl: "http://127.0.0.1:18792", + page, + response: null, + ssrfPolicy: { allowPrivateNetwork: false }, + targetId: "T1", + }, + ); + } finally { + vi.useRealTimers(); + } + }); + + it("blocks subframe-only navigation to a private URL during the post-action grace window", async () => { + vi.useFakeTimers(); + try { + const listeners = new Set<(frame: object) => void>(); + const mainFrame = {}; + const subframe = { url: () => "http://169.254.169.254/latest/meta-data/" }; + const click = vi.fn(async () => { + setTimeout(() => { + for (const listener of listeners) { + listener(subframe); + } + }, 10); + }); + const page = { + mainFrame: vi.fn(() => mainFrame), + on: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.add(listener); + } + }), + off: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.delete(listener); + } + }), + url: vi.fn(() => "https://attacker.example.com/page"), + }; + setPwToolsCoreCurrentRefLocator({ click }); + setPwToolsCoreCurrentPage(page); + + const blocked = new Error("SSRF blocked: private network"); + getPwToolsCoreNavigationGuardMocks().assertBrowserNavigationResultAllowed.mockRejectedValueOnce( + blocked, + ); + + const task = mod.clickViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + ref: "1", + ssrfPolicy: { allowPrivateNetwork: false }, + }); + const rejection = expect(task).rejects.toThrow("SSRF blocked: private network"); + + await vi.advanceTimersByTimeAsync(10); + await vi.advanceTimersByTimeAsync(240); + await rejection; + expect( + getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely, + ).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + it("snapshots delayed subframe URLs before later rewrites make them look safe", async () => { + vi.useFakeTimers(); + try { + const listeners = new Set<(frame: object) => void>(); + const mainFrame = {}; + const subframe = createMutableFrame("http://169.254.169.254/latest/meta-data/"); + const click = vi.fn(async () => { + setTimeout(() => { + for (const listener of listeners) { + listener(subframe.frame); + } + }, 10); + setTimeout(() => { + subframe.setUrl("https://example.com/embed"); + }, 20); + }); + const page = { + mainFrame: vi.fn(() => mainFrame), + on: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.add(listener); + } + }), + off: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.delete(listener); + } + }), + url: vi.fn(() => "https://attacker.example.com/page"), + }; + setPwToolsCoreCurrentRefLocator({ click }); + setPwToolsCoreCurrentPage(page); + + const task = mod.clickViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + ref: "1", + ssrfPolicy: { allowPrivateNetwork: false }, + }); + + await vi.advanceTimersByTimeAsync(20); + await vi.advanceTimersByTimeAsync(230); + await task; + + expect( + getPwToolsCoreNavigationGuardMocks().assertBrowserNavigationResultAllowed, + ).toHaveBeenCalledWith({ + ssrfPolicy: { allowPrivateNetwork: false }, + url: "http://169.254.169.254/latest/meta-data/", + }); + } finally { + vi.useRealTimers(); + } + }); + + it("still quarantines the main frame when a delayed subframe block fires first", async () => { + vi.useFakeTimers(); + try { + const listeners = new Set<(frame: object) => void>(); + const mainFrame = {}; + const subframe = { url: () => "http://169.254.169.254/latest/meta-data/" }; + let currentUrl = "https://attacker.example.com/page"; + const click = vi.fn(async () => { + setTimeout(() => { + for (const listener of listeners) { + listener(subframe); + } + }, 10); + setTimeout(() => { + currentUrl = "http://127.0.0.1:8080/internal"; + for (const listener of listeners) { + listener(mainFrame); + } + }, 20); + }); + const page = { + mainFrame: vi.fn(() => mainFrame), + on: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.add(listener); + } + }), + off: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.delete(listener); + } + }), + url: vi.fn(() => currentUrl), + }; + setPwToolsCoreCurrentRefLocator({ click }); + setPwToolsCoreCurrentPage(page); + + const subframeBlocked = new Error("subframe blocked"); + const mainFrameBlocked = new Error("main frame blocked"); + getPwToolsCoreNavigationGuardMocks().assertBrowserNavigationResultAllowed.mockRejectedValueOnce( + subframeBlocked, + ); + getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely.mockRejectedValueOnce( + mainFrameBlocked, + ); + + const task = mod.clickViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + ref: "1", + ssrfPolicy: { allowPrivateNetwork: false }, + }); + const rejection = expect(task).rejects.toThrow("main frame blocked"); + + await vi.advanceTimersByTimeAsync(20); + await rejection; + expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).toHaveBeenCalledWith( + { + cdpUrl: "http://127.0.0.1:18792", + page, + response: null, + ssrfPolicy: { allowPrivateNetwork: false }, + targetId: "T1", + }, + ); + } finally { + vi.useRealTimers(); + } + }); + + it("does not stop watching for a later main-frame navigation after a harmless subframe hop", async () => { + vi.useFakeTimers(); + try { + const listeners = new Set<(frame: object) => void>(); + const mainFrame = {}; + const subframe = { url: () => "about:blank" }; + let currentUrl = "http://127.0.0.1:9222/json/version"; + const click = vi.fn(async () => { + setTimeout(() => { + for (const listener of listeners) { + listener(subframe); + } + }, 10); + setTimeout(() => { + currentUrl = "http://127.0.0.1:9222/json/list"; + for (const listener of listeners) { + listener(mainFrame); + } + }, 20); + }); + const page = { + mainFrame: vi.fn(() => mainFrame), + on: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.add(listener); + } + }), + off: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.delete(listener); + } + }), + url: vi.fn(() => currentUrl), + }; + setPwToolsCoreCurrentRefLocator({ click }); + setPwToolsCoreCurrentPage(page); + + const task = mod.clickViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + ref: "1", + ssrfPolicy: { allowPrivateNetwork: false }, + }); + + await vi.advanceTimersByTimeAsync(20); + await task; + + expect( + getPwToolsCoreNavigationGuardMocks().assertBrowserNavigationResultAllowed, + ).not.toHaveBeenCalled(); + expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).toHaveBeenCalledWith( + { + cdpUrl: "http://127.0.0.1:18792", + page, + response: null, + ssrfPolicy: { allowPrivateNetwork: false }, + targetId: "T1", + }, + ); + } finally { + vi.useRealTimers(); + } + }); + + it("checks delayed subframe navigations in the action-error recovery path", async () => { + vi.useFakeTimers(); + try { + const listeners = new Set<(frame: object) => void>(); + const mainFrame = {}; + const subframe = { url: () => "http://169.254.169.254/latest/meta-data/" }; + const page = { + mainFrame: vi.fn(() => mainFrame), + evaluate: vi.fn(async () => { + setTimeout(() => { + for (const listener of listeners) { + listener(subframe); + } + }, 10); + throw new Error("evaluate failed"); + }), + on: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.add(listener); + } + }), + off: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.delete(listener); + } + }), + url: vi.fn(() => "https://attacker.example.com/page"), + }; + setPwToolsCoreCurrentPage(page); + + const blocked = new Error("SSRF blocked: private network"); + getPwToolsCoreNavigationGuardMocks().assertBrowserNavigationResultAllowed.mockRejectedValueOnce( + blocked, + ); + + const task = mod.evaluateViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + fn: "() => 1", + ssrfPolicy: { allowPrivateNetwork: false }, + }); + const rejection = expect(task).rejects.toThrow("SSRF blocked: private network"); + + await vi.advanceTimersByTimeAsync(10); + await vi.advanceTimersByTimeAsync(240); + await rejection; + expect( + getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely, + ).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + it("snapshots subframe URLs observed during the action before they change", async () => { + vi.useFakeTimers(); + try { + const listeners = new Set<(frame: object) => void>(); + const mainFrame = {}; + const subframe = createMutableFrame("http://169.254.169.254/latest/meta-data/"); + const click = vi.fn( + () => + new Promise((resolve) => { + setTimeout(() => { + for (const listener of listeners) { + listener(subframe.frame); + } + }, 10); + setTimeout(() => { + subframe.setUrl("https://example.com/embed"); + }, 20); + setTimeout(resolve, 30); + }), + ); + const page = { + mainFrame: vi.fn(() => mainFrame), + on: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.add(listener); + } + }), + off: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.delete(listener); + } + }), + url: vi.fn(() => "https://attacker.example.com/page"), + }; + setPwToolsCoreCurrentRefLocator({ click }); + setPwToolsCoreCurrentPage(page); + + const task = mod.clickViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + ref: "1", + ssrfPolicy: { allowPrivateNetwork: false }, + }); + + await vi.advanceTimersByTimeAsync(30); + await vi.advanceTimersByTimeAsync(250); + await task; + + expect( + getPwToolsCoreNavigationGuardMocks().assertBrowserNavigationResultAllowed, + ).toHaveBeenCalledWith({ + ssrfPolicy: { allowPrivateNetwork: false }, + url: "http://169.254.169.254/latest/meta-data/", + }); + } finally { + vi.useRealTimers(); + } + }); + + it("still quarantines the main frame when an in-flight subframe block fires first", async () => { + vi.useFakeTimers(); + try { + const listeners = new Set<(frame: object) => void>(); + const mainFrame = {}; + const subframe = { url: () => "http://169.254.169.254/latest/meta-data/" }; + let currentUrl = "https://attacker.example.com/page"; + const click = vi.fn( + () => + new Promise((resolve) => { + setTimeout(() => { + for (const listener of listeners) { + listener(subframe); + } + }, 10); + setTimeout(() => { + currentUrl = "http://127.0.0.1:8080/internal"; + for (const listener of listeners) { + listener(mainFrame); + } + }, 20); + setTimeout(resolve, 30); + }), + ); + const page = { + mainFrame: vi.fn(() => mainFrame), + on: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.add(listener); + } + }), + off: vi.fn((event: string, listener: (frame: object) => void) => { + if (event === "framenavigated") { + listeners.delete(listener); + } + }), + url: vi.fn(() => currentUrl), + }; + setPwToolsCoreCurrentRefLocator({ click }); + setPwToolsCoreCurrentPage(page); + + const subframeBlocked = new Error("subframe blocked"); + const mainFrameBlocked = new Error("main frame blocked"); + getPwToolsCoreNavigationGuardMocks().assertBrowserNavigationResultAllowed.mockRejectedValueOnce( + subframeBlocked, + ); + getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely.mockRejectedValueOnce( + mainFrameBlocked, + ); + + const task = mod.clickViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + ref: "1", + ssrfPolicy: { allowPrivateNetwork: false }, + }); + const rejection = expect(task).rejects.toThrow("main frame blocked"); + + await vi.advanceTimersByTimeAsync(30); + await rejection; expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).toHaveBeenCalledWith( { cdpUrl: "http://127.0.0.1:18792", diff --git a/extensions/browser/src/browser/pw-tools-core.interactions.ts b/extensions/browser/src/browser/pw-tools-core.interactions.ts index a1d66d72c9b..133f368b48f 100644 --- a/extensions/browser/src/browser/pw-tools-core.interactions.ts +++ b/extensions/browser/src/browser/pw-tools-core.interactions.ts @@ -12,6 +12,10 @@ import { } from "./act-policy.js"; import type { BrowserActRequest, BrowserFormField } from "./client-actions.types.js"; import { DEFAULT_FILL_FIELD_TYPE } from "./form-fields.js"; +import { + assertBrowserNavigationResultAllowed, + withBrowserNavigationPolicy, +} from "./navigation-guard.js"; import { DEFAULT_UPLOAD_DIR, resolveStrictExistingPathsWithinRoot } from "./paths.js"; import { assertPageNavigationCompletedSafely, @@ -119,20 +123,84 @@ function isMainFrameNavigation(page: NavigationObservablePage, frame: Frame): bo return frame === page.mainFrame(); } +async function assertSubframeNavigationAllowed( + frameUrl: string, + ssrfPolicy?: SsrFPolicy, +): Promise { + if (!ssrfPolicy || (!frameUrl.startsWith("http://") && !frameUrl.startsWith("https://"))) { + // Non-network frame URLs like about:blank and about:srcdoc do not cross the + // browser SSRF boundary, so they should not trigger the navigation policy. + return; + } + + await assertBrowserNavigationResultAllowed({ + url: frameUrl, + ...withBrowserNavigationPolicy(ssrfPolicy), + }); +} + +type ObservedDelayedNavigations = { + mainFrameNavigated: boolean; + subframes: string[]; +}; + +function snapshotNetworkFrameUrl(frame: Frame): string | null { + try { + const frameUrl = frame.url(); + return frameUrl.startsWith("http://") || frameUrl.startsWith("https://") ? frameUrl : null; + } catch { + return null; + } +} + +async function assertObservedDelayedNavigations(opts: { + cdpUrl: string; + page: Page; + ssrfPolicy?: SsrFPolicy; + targetId?: string; + observed: ObservedDelayedNavigations; +}): Promise { + let subframeError: unknown; + try { + for (const frameUrl of opts.observed.subframes) { + await assertSubframeNavigationAllowed(frameUrl, opts.ssrfPolicy); + } + } catch (err) { + subframeError = err; + } + if (opts.observed.mainFrameNavigated) { + await assertPageNavigationCompletedSafely({ + cdpUrl: opts.cdpUrl, + page: opts.page, + response: null, + ssrfPolicy: opts.ssrfPolicy, + targetId: opts.targetId, + }); + } + if (subframeError) { + throw subframeError; + } +} + function observeDelayedInteractionNavigation( page: NavigationObservablePage, previousUrl: string, -): Promise { +): Promise { if (didCrossDocumentUrlChange(page, previousUrl)) { - return Promise.resolve(true); + return Promise.resolve({ mainFrameNavigated: true, subframes: [] }); } if (typeof page.on !== "function" || typeof page.off !== "function") { - return Promise.resolve(false); + return Promise.resolve({ mainFrameNavigated: false, subframes: [] }); } - return new Promise((resolve) => { + return new Promise((resolve) => { + const subframes: string[] = []; const onFrameNavigated = (frame: Frame) => { if (!isMainFrameNavigation(page, frame)) { + const frameUrl = snapshotNetworkFrameUrl(frame); + if (frameUrl) { + subframes.push(frameUrl); + } return; } // Use isHashOnlyNavigation rather than !didCrossDocumentUrlChange: the @@ -142,11 +210,14 @@ function observeDelayedInteractionNavigation( return; } cleanup(); - resolve(true); + resolve({ mainFrameNavigated: true, subframes }); }; const timeout = setTimeout(() => { cleanup(); - resolve(didCrossDocumentUrlChange(page, previousUrl)); + resolve({ + mainFrameNavigated: didCrossDocumentUrlChange(page, previousUrl), + subframes, + }); }, INTERACTION_NAVIGATION_GRACE_MS); const cleanup = () => { clearTimeout(timeout); @@ -196,8 +267,13 @@ function scheduleDelayedInteractionNavigationGuard(opts: { } resolve(); }; + const subframes: string[] = []; const onFrameNavigated = (frame: Frame) => { if (!isMainFrameNavigation(page, frame)) { + const frameUrl = snapshotNetworkFrameUrl(frame); + if (frameUrl) { + subframes.push(frameUrl); + } return; } // Use isHashOnlyNavigation rather than !didCrossDocumentUrlChange: the @@ -207,16 +283,26 @@ function scheduleDelayedInteractionNavigationGuard(opts: { return; } cleanup(); - void assertPageNavigationCompletedSafely({ + void assertObservedDelayedNavigations({ cdpUrl: opts.cdpUrl, page: opts.page, - response: null, ssrfPolicy: opts.ssrfPolicy, targetId: opts.targetId, + observed: { mainFrameNavigated: true, subframes }, }).then(() => settle(), settle); }; const timeout = setTimeout(() => { - settle(); + cleanup(); + void assertObservedDelayedNavigations({ + cdpUrl: opts.cdpUrl, + page: opts.page, + ssrfPolicy: opts.ssrfPolicy, + targetId: opts.targetId, + observed: { + mainFrameNavigated: didCrossDocumentUrlChange(page, opts.previousUrl), + subframes, + }, + }).then(() => settle(), settle); }, INTERACTION_NAVIGATION_GRACE_MS); const cleanup = () => { clearTimeout(timeout); @@ -248,8 +334,13 @@ async function assertInteractionNavigationCompletedSafely(opts: { // slow interactions, silently bypassing the SSRF guard. const navPage = opts.page as unknown as NavigationObservablePage; let navigatedDuringAction = false; + const subframeNavigationsDuringAction: string[] = []; const onFrameNavigated = (frame: Frame) => { if (!isMainFrameNavigation(navPage, frame)) { + const frameUrl = snapshotNetworkFrameUrl(frame); + if (frameUrl) { + subframeNavigationsDuringAction.push(frameUrl); + } return; } // Use isHashOnlyNavigation rather than didCrossDocumentUrlChange: the event @@ -278,6 +369,15 @@ async function assertInteractionNavigationCompletedSafely(opts: { const navigationObserved = navigatedDuringAction || didCrossDocumentUrlChange(opts.page, opts.previousUrl); + let subframeError: unknown; + try { + for (const frameUrl of subframeNavigationsDuringAction) { + await assertSubframeNavigationAllowed(frameUrl, opts.ssrfPolicy); + } + } catch (err) { + subframeError = err; + } + if (navigationObserved) { await assertPageNavigationCompletedSafely({ cdpUrl: opts.cdpUrl, @@ -290,17 +390,14 @@ async function assertInteractionNavigationCompletedSafely(opts: { // Preserve the action-error path semantics: if a rejected click/evaluate still // triggers a delayed navigation, the SSRF block must win over the original // action error instead of surfacing a stale interaction failure. - const delayedNavigationObserved = await observeDelayedInteractionNavigation( - opts.page, - opts.previousUrl, - ); - if (delayedNavigationObserved) { - await assertPageNavigationCompletedSafely({ + const observed = await observeDelayedInteractionNavigation(opts.page, opts.previousUrl); + if (observed.mainFrameNavigated || observed.subframes.length > 0) { + await assertObservedDelayedNavigations({ cdpUrl: opts.cdpUrl, page: opts.page, - response: null, ssrfPolicy: opts.ssrfPolicy, targetId: opts.targetId, + observed, }); } } else { @@ -316,6 +413,10 @@ async function assertInteractionNavigationCompletedSafely(opts: { }); } + if (subframeError) { + throw subframeError; + } + if (actionError) { throw actionError; } diff --git a/extensions/browser/src/browser/pw-tools-core.test-harness.ts b/extensions/browser/src/browser/pw-tools-core.test-harness.ts index 03926701253..2d71558cc42 100644 --- a/extensions/browser/src/browser/pw-tools-core.test-harness.ts +++ b/extensions/browser/src/browser/pw-tools-core.test-harness.ts @@ -42,12 +42,28 @@ const sessionMocks = vi.hoisted(() => ({ rememberRoleRefsForTarget: vi.fn(() => {}), })); +const navigationGuardMocks = vi.hoisted(() => ({ + assertBrowserNavigationResultAllowed: vi.fn(async () => {}), + withBrowserNavigationPolicy: vi.fn((ssrfPolicy?: unknown) => ({ ssrfPolicy })), +})); + vi.mock("./pw-session.js", () => sessionMocks); +vi.mock("./navigation-guard.js", async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + ...navigationGuardMocks, + }; +}); export function getPwToolsCoreSessionMocks() { return sessionMocks; } +export function getPwToolsCoreNavigationGuardMocks() { + return navigationGuardMocks; +} + export function setPwToolsCoreCurrentPage(page: Record | null) { currentPage = page; } @@ -70,5 +86,8 @@ export function installPwToolsCoreTestHooks() { for (const fn of Object.values(sessionMocks)) { fn.mockClear(); } + for (const fn of Object.values(navigationGuardMocks)) { + fn.mockClear(); + } }); }