diff --git a/CHANGELOG.md b/CHANGELOG.md index e4f2a80a15f..e0c949eda2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ Docs: https://docs.openclaw.ai +## Unreleased + +### Fixes + +- Browser/SSRF: block private-network intermediate redirect hops in strict browser navigation flows and fail closed when remote tab-open paths cannot inspect redirect chains. Thanks @zpbrent. + ## 2026.3.8 ### Changes diff --git a/src/browser/navigation-guard.test.ts b/src/browser/navigation-guard.test.ts index 8a8350cdb62..af6e7fba434 100644 --- a/src/browser/navigation-guard.test.ts +++ b/src/browser/navigation-guard.test.ts @@ -2,8 +2,10 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { SsrFBlockedError, type LookupFn } from "../infra/net/ssrf.js"; import { assertBrowserNavigationAllowed, + assertBrowserNavigationRedirectChainAllowed, assertBrowserNavigationResultAllowed, InvalidBrowserNavigationUrlError, + requiresInspectableBrowserNavigationRedirects, } from "./navigation-guard.js"; function createLookupFn(address: string): LookupFn { @@ -147,4 +149,58 @@ describe("browser navigation guard", () => { }), ).resolves.toBeUndefined(); }); + + it("blocks private intermediate redirect hops", async () => { + const publicLookup = createLookupFn("93.184.216.34"); + const privateLookup = createLookupFn("127.0.0.1"); + const finalRequest = { + url: () => "https://public.example/final", + redirectedFrom: () => ({ + url: () => "http://private.example/internal", + redirectedFrom: () => ({ + url: () => "https://public.example/start", + redirectedFrom: () => null, + }), + }), + }; + + await expect( + assertBrowserNavigationRedirectChainAllowed({ + request: finalRequest, + lookupFn: vi.fn(async (hostname: string) => + hostname === "private.example" + ? privateLookup(hostname, { all: true }) + : publicLookup(hostname, { all: true }), + ) as unknown as LookupFn, + }), + ).rejects.toBeInstanceOf(SsrFBlockedError); + }); + + it("allows redirect chains when every hop is public", async () => { + const lookupFn = createLookupFn("93.184.216.34"); + const finalRequest = { + url: () => "https://public.example/final", + redirectedFrom: () => ({ + url: () => "https://public.example/middle", + redirectedFrom: () => ({ + url: () => "https://public.example/start", + redirectedFrom: () => null, + }), + }), + }; + + await expect( + assertBrowserNavigationRedirectChainAllowed({ + request: finalRequest, + lookupFn, + }), + ).resolves.toBeUndefined(); + }); + + it("treats default browser SSRF mode as requiring redirect-hop inspection", () => { + expect(requiresInspectableBrowserNavigationRedirects()).toBe(true); + expect(requiresInspectableBrowserNavigationRedirects({ allowPrivateNetwork: true })).toBe( + false, + ); + }); }); diff --git a/src/browser/navigation-guard.ts b/src/browser/navigation-guard.ts index 496dee19469..216140aba98 100644 --- a/src/browser/navigation-guard.ts +++ b/src/browser/navigation-guard.ts @@ -25,12 +25,21 @@ export type BrowserNavigationPolicyOptions = { ssrfPolicy?: SsrFPolicy; }; +export type BrowserNavigationRequestLike = { + url(): string; + redirectedFrom(): BrowserNavigationRequestLike | null; +}; + export function withBrowserNavigationPolicy( ssrfPolicy?: SsrFPolicy, ): BrowserNavigationPolicyOptions { return ssrfPolicy ? { ssrfPolicy } : {}; } +export function requiresInspectableBrowserNavigationRedirects(ssrfPolicy?: SsrFPolicy): boolean { + return !isPrivateNetworkAllowedByPolicy(ssrfPolicy); +} + export async function assertBrowserNavigationAllowed( opts: { url: string; @@ -102,3 +111,24 @@ export async function assertBrowserNavigationResultAllowed( await assertBrowserNavigationAllowed(opts); } } + +export async function assertBrowserNavigationRedirectChainAllowed( + opts: { + request?: BrowserNavigationRequestLike | null; + lookupFn?: LookupFn; + } & BrowserNavigationPolicyOptions, +): Promise { + const chain: string[] = []; + let current = opts.request ?? null; + while (current) { + chain.push(current.url()); + current = current.redirectedFrom(); + } + for (const url of chain.toReversed()) { + await assertBrowserNavigationAllowed({ + url, + lookupFn: opts.lookupFn, + ssrfPolicy: opts.ssrfPolicy, + }); + } +} diff --git a/src/browser/pw-session.create-page.navigation-guard.test.ts b/src/browser/pw-session.create-page.navigation-guard.test.ts index 95a09273001..ae20e43c230 100644 --- a/src/browser/pw-session.create-page.navigation-guard.test.ts +++ b/src/browser/pw-session.create-page.navigation-guard.test.ts @@ -1,5 +1,6 @@ import { chromium } from "playwright-core"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { SsrFBlockedError } from "../infra/net/ssrf.js"; import * as chromeModule from "./chrome.js"; import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js"; import { closePlaywrightBrowserConnection, createPageViaPlaywright } from "./pw-session.js"; @@ -9,7 +10,9 @@ const getChromeWebSocketUrlSpy = vi.spyOn(chromeModule, "getChromeWebSocketUrl") function installBrowserMocks() { const pageOn = vi.fn(); - const pageGoto = vi.fn(async () => {}); + const pageGoto = vi.fn< + (...args: unknown[]) => Promise Record }> + >(async () => null); const pageTitle = vi.fn(async () => ""); const pageUrl = vi.fn(() => "about:blank"); const contextOn = vi.fn(); @@ -84,4 +87,27 @@ describe("pw-session createPageViaPlaywright navigation guard", () => { expect(created.targetId).toBe("TARGET_1"); expect(pageGoto).not.toHaveBeenCalled(); }); + + it("blocks private intermediate redirect hops", async () => { + const { pageGoto } = installBrowserMocks(); + pageGoto.mockResolvedValueOnce({ + request: () => ({ + url: () => "https://93.184.216.34/final", + redirectedFrom: () => ({ + url: () => "http://127.0.0.1:18080/internal-hop", + redirectedFrom: () => ({ + url: () => "https://93.184.216.34/start", + redirectedFrom: () => null, + }), + }), + }), + }); + + await expect( + createPageViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + url: "https://93.184.216.34/start", + }), + ).rejects.toBeInstanceOf(SsrFBlockedError); + }); }); diff --git a/src/browser/pw-session.ts b/src/browser/pw-session.ts index a058ba07791..a7103c1174c 100644 --- a/src/browser/pw-session.ts +++ b/src/browser/pw-session.ts @@ -22,6 +22,7 @@ import { getChromeWebSocketUrl } from "./chrome.js"; import { BrowserTabNotFoundError } from "./errors.js"; import { assertBrowserNavigationAllowed, + assertBrowserNavigationRedirectChainAllowed, assertBrowserNavigationResultAllowed, withBrowserNavigationPolicy, } from "./navigation-guard.js"; @@ -787,8 +788,13 @@ export async function createPageViaPlaywright(opts: { url: targetUrl, ...navigationPolicy, }); - await page.goto(targetUrl, { timeout: 30_000 }).catch(() => { + const response = await page.goto(targetUrl, { timeout: 30_000 }).catch(() => { // Navigation might fail for some URLs, but page is still created + return null; + }); + await assertBrowserNavigationRedirectChainAllowed({ + request: response?.request(), + ...navigationPolicy, }); await assertBrowserNavigationResultAllowed({ url: page.url(), diff --git a/src/browser/pw-tools-core.snapshot.navigate-guard.test.ts b/src/browser/pw-tools-core.snapshot.navigate-guard.test.ts index ef54087eb38..993bbfcc3b1 100644 --- a/src/browser/pw-tools-core.snapshot.navigate-guard.test.ts +++ b/src/browser/pw-tools-core.snapshot.navigate-guard.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import { SsrFBlockedError } from "../infra/net/ssrf.js"; import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js"; import { getPwToolsCoreSessionMocks, @@ -75,4 +76,32 @@ describe("pw-tools-core.snapshot navigate guard", () => { expect(goto).toHaveBeenCalledTimes(2); expect(result.url).toBe("https://example.com/recovered"); }); + + it("blocks private intermediate redirect hops during navigation", async () => { + const goto = vi.fn(async () => ({ + request: () => ({ + url: () => "https://93.184.216.34/final", + redirectedFrom: () => ({ + url: () => "http://127.0.0.1:18080/internal-hop", + redirectedFrom: () => ({ + url: () => "https://93.184.216.34/start", + redirectedFrom: () => null, + }), + }), + }), + })); + setPwToolsCoreCurrentPage({ + goto, + url: vi.fn(() => "https://93.184.216.34/final"), + }); + + await expect( + mod.navigateViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + url: "https://93.184.216.34/start", + }), + ).rejects.toBeInstanceOf(SsrFBlockedError); + + expect(goto).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/browser/pw-tools-core.snapshot.ts b/src/browser/pw-tools-core.snapshot.ts index b3dc8dec7b0..09926626db1 100644 --- a/src/browser/pw-tools-core.snapshot.ts +++ b/src/browser/pw-tools-core.snapshot.ts @@ -2,6 +2,7 @@ import type { SsrFPolicy } from "../infra/net/ssrf.js"; import { type AriaSnapshotNode, formatAriaSnapshot, type RawAXNode } from "./cdp.js"; import { assertBrowserNavigationAllowed, + assertBrowserNavigationRedirectChainAllowed, assertBrowserNavigationResultAllowed, withBrowserNavigationPolicy, } from "./navigation-guard.js"; @@ -196,8 +197,10 @@ export async function navigateViaPlaywright(opts: { const timeout = Math.max(1000, Math.min(120_000, opts.timeoutMs ?? 20_000)); let page = await getPageForTargetId(opts); ensurePageState(page); + const navigate = async () => await page.goto(url, { timeout }); + let response; try { - await page.goto(url, { timeout }); + response = await navigate(); } catch (err) { if (!isRetryableNavigateError(err)) { throw err; @@ -211,8 +214,12 @@ export async function navigateViaPlaywright(opts: { }).catch(() => {}); page = await getPageForTargetId(opts); ensurePageState(page); - await page.goto(url, { timeout }); + response = await navigate(); } + await assertBrowserNavigationRedirectChainAllowed({ + request: response?.request(), + ...withBrowserNavigationPolicy(opts.ssrfPolicy), + }); const finalUrl = page.url(); await assertBrowserNavigationResultAllowed({ url: finalUrl, diff --git a/src/browser/server-context.remote-profile-tab-ops.suite.ts b/src/browser/server-context.remote-profile-tab-ops.suite.ts index e0bd5815199..a2020f559e5 100644 --- a/src/browser/server-context.remote-profile-tab-ops.suite.ts +++ b/src/browser/server-context.remote-profile-tab-ops.suite.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import "./server-context.chrome-test-harness.js"; import * as chromeModule from "./chrome.js"; +import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js"; import * as pwAiModule from "./pw-ai-module.js"; import { createBrowserRouteContext } from "./server-context.js"; import { @@ -230,6 +231,17 @@ describe("browser server-context remote profile tab operations", () => { expect(tabs.map((t) => t.targetId)).toEqual(["T1"]); }); + it("fails closed for remote tab opens in strict mode without Playwright", async () => { + vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue(null); + const { state, remote, fetchMock } = createRemoteRouteHarness(); + state.resolved.ssrfPolicy = {}; + + await expect(remote.openTab("https://example.com")).rejects.toBeInstanceOf( + InvalidBrowserNavigationUrlError, + ); + expect(fetchMock).not.toHaveBeenCalled(); + }); + it("does not enforce managed tab cap for remote openclaw profiles", async () => { const listPagesViaPlaywright = vi .fn() diff --git a/src/browser/server-context.tab-ops.ts b/src/browser/server-context.tab-ops.ts index 5adbd45923e..24985430bdc 100644 --- a/src/browser/server-context.tab-ops.ts +++ b/src/browser/server-context.tab-ops.ts @@ -5,6 +5,8 @@ import type { ResolvedBrowserProfile } from "./config.js"; import { assertBrowserNavigationAllowed, assertBrowserNavigationResultAllowed, + InvalidBrowserNavigationUrlError, + requiresInspectableBrowserNavigationRedirects, withBrowserNavigationPolicy, } from "./navigation-guard.js"; import { getBrowserProfileCapabilities } from "./profile-capabilities.js"; @@ -153,6 +155,12 @@ export function createProfileTabOps({ } } + if (requiresInspectableBrowserNavigationRedirects(state().resolved.ssrfPolicy)) { + throw new InvalidBrowserNavigationUrlError( + "Navigation blocked: strict browser SSRF policy requires Playwright-backed redirect-hop inspection", + ); + } + const createdViaCdp = await createTargetViaCdp({ cdpUrl: profile.cdpUrl, url,