From 121c452d666d4749744dc2089287d0227aae2ed3 Mon Sep 17 00:00:00 2001 From: Agustin Rivera <31522568+eleqtrizit@users.noreply.github.com> Date: Fri, 10 Apr 2026 12:18:53 -0700 Subject: [PATCH] fix(browser): tighten strict browser hostname navigation (#64367) * fix(browser): tighten strict browser hostname navigation * fix(browser): address review follow-ups * chore(changelog): add strict browser hostname navigation entry * fix(browser): remove stale state prop from SelectionDeps call site The PR's SelectionDeps uses getSsrFPolicy instead of the full state object; the state property was leftover from an earlier iteration. --------- Co-authored-by: Devin Robison --- CHANGELOG.md | 1 + .../browser/src/browser/cdp.helpers.test.ts | 94 +++++++++------- extensions/browser/src/browser/cdp.helpers.ts | 104 ++++++++---------- extensions/browser/src/browser/cdp.test.ts | 22 +++- extensions/browser/src/browser/cdp.ts | 3 +- extensions/browser/src/browser/chrome.ts | 20 +++- .../src/browser/navigation-guard.test.ts | 88 +++++++++++++++ .../browser/src/browser/navigation-guard.ts | 41 ++++++- ...ssion.create-page.navigation-guard.test.ts | 14 +++ .../src/browser/server-context.selection.ts | 48 +++----- .../src/browser/server-context.tab-ops.ts | 47 ++++---- .../browser/src/browser/server-context.ts | 2 +- src/infra/net/ssrf.ts | 6 +- src/plugin-sdk/browser-security-runtime.ts | 2 + 14 files changed, 325 insertions(+), 167 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62ab0bd9152..4c20b8ba953 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,7 @@ Docs: https://docs.openclaw.ai - 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. - Hooks/security: mark agent hook system events as untrusted and sanitize hook display names before cron metadata reuse. (#64372) Thanks @eleqtrizit. - Media/security: honor sender-scoped `toolsBySender` policy for outbound host-media reads so denied senders cannot trigger host file disclosure via attachment hydration. (#64459) Thanks @eleqtrizit. +- Browser/security: reject strict-policy hostname navigation unless the hostname is an explicit allowlist exception or IP literal, and route CDP HTTP discovery through the pinned SSRF fetch path. (#64367) Thanks @eleqtrizit. ## 2026.4.9 ### Changes diff --git a/extensions/browser/src/browser/cdp.helpers.test.ts b/extensions/browser/src/browser/cdp.helpers.test.ts index 92003fcc663..bbc42425559 100644 --- a/extensions/browser/src/browser/cdp.helpers.test.ts +++ b/extensions/browser/src/browser/cdp.helpers.test.ts @@ -1,53 +1,65 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { SsrFBlockedError } from "../infra/net/ssrf.js"; -vi.mock("./cdp-proxy-bypass.js", () => ({ - getDirectAgentForCdp: vi.fn(() => null), - withNoProxyForCdpUrl: vi.fn(async (_url: string, fn: () => Promise) => await fn()), -})); +const fetchWithSsrFGuardMock = vi.hoisted(() => vi.fn()); -const { assertCdpEndpointAllowed, fetchCdpChecked } = await import("./cdp.helpers.js"); -const { BrowserCdpEndpointBlockedError } = await import("./errors.js"); +vi.mock("openclaw/plugin-sdk/ssrf-runtime", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetchWithSsrFGuard: (...args: unknown[]) => fetchWithSsrFGuardMock(...args), + }; +}); -describe("fetchCdpChecked", () => { +import { fetchJson, fetchOk } from "./cdp.helpers.js"; + +describe("cdp helpers", () => { afterEach(() => { - vi.unstubAllGlobals(); + fetchWithSsrFGuardMock.mockReset(); }); - it("disables automatic redirect following for CDP HTTP probes", async () => { - const fetchSpy = vi.fn().mockResolvedValue( - new Response(null, { - status: 302, - headers: { Location: "http://127.0.0.1:9222/json/version" }, + it("releases guarded CDP fetches after the response body is consumed", async () => { + const release = vi.fn(async () => {}); + const json = vi.fn(async () => { + expect(release).not.toHaveBeenCalled(); + return { ok: true }; + }); + fetchWithSsrFGuardMock.mockResolvedValueOnce({ + response: { + ok: true, + status: 200, + json, + }, + release, + }); + + await expect( + fetchJson("http://127.0.0.1:9222/json/version", 250, undefined, { + dangerouslyAllowPrivateNetwork: false, + allowedHostnames: ["127.0.0.1"], }), - ); - vi.stubGlobal("fetch", fetchSpy); + ).resolves.toEqual({ ok: true }); - await expect(fetchCdpChecked("https://example.com/json/version", 50)).rejects.toThrow( - "CDP endpoint redirects are not allowed", - ); + expect(json).toHaveBeenCalledTimes(1); + expect(release).toHaveBeenCalledTimes(1); + }); - const init = fetchSpy.mock.calls[0]?.[1]; - expect(init?.redirect).toBe("manual"); - }); -}); - -describe("assertCdpEndpointAllowed", () => { - it("rethrows SSRF policy failures as BrowserCdpEndpointBlockedError so mapping can distinguish endpoint vs navigation", async () => { - await expect( - assertCdpEndpointAllowed("http://10.0.0.42:9222", { dangerouslyAllowPrivateNetwork: false }), - ).rejects.toBeInstanceOf(BrowserCdpEndpointBlockedError); - }); - - it("does not wrap non-SSRF failures", async () => { - await expect( - assertCdpEndpointAllowed("file:///etc/passwd", { dangerouslyAllowPrivateNetwork: false }), - ).rejects.not.toBeInstanceOf(BrowserCdpEndpointBlockedError); - }); - - it("leaves navigation-target SsrFBlockedError alone for callers that never hit the endpoint helper", () => { - // Sanity check that raw SsrFBlockedError is still its own class and is not - // accidentally converted by the endpoint helper import. - expect(new SsrFBlockedError("blocked")).toBeInstanceOf(SsrFBlockedError); + it("releases guarded CDP fetches for bodyless requests", async () => { + const release = vi.fn(async () => {}); + fetchWithSsrFGuardMock.mockResolvedValueOnce({ + response: { + ok: true, + status: 200, + }, + release, + }); + + await expect( + fetchOk("http://127.0.0.1:9222/json/close/TARGET_1", 250, undefined, { + dangerouslyAllowPrivateNetwork: false, + allowedHostnames: ["127.0.0.1"], + }), + ).resolves.toBeUndefined(); + + expect(release).toHaveBeenCalledTimes(1); }); }); diff --git a/extensions/browser/src/browser/cdp.helpers.ts b/extensions/browser/src/browser/cdp.helpers.ts index 6baa214e55f..ce23c4f5659 100644 --- a/extensions/browser/src/browser/cdp.helpers.ts +++ b/extensions/browser/src/browser/cdp.helpers.ts @@ -2,16 +2,11 @@ import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime"; import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/text-runtime"; import WebSocket from "ws"; import { isLoopbackHost } from "../gateway/net.js"; -import { - SsrFBlockedError, - type SsrFPolicy, - resolvePinnedHostnameWithPolicy, -} from "../infra/net/ssrf.js"; +import { type SsrFPolicy, resolvePinnedHostnameWithPolicy } from "../infra/net/ssrf.js"; import { rawDataToString } from "../infra/ws.js"; import { redactSensitiveText } from "../logging/redact.js"; import { getDirectAgentForCdp, withNoProxyForCdpUrl } from "./cdp-proxy-bypass.js"; import { CDP_HTTP_REQUEST_TIMEOUT_MS, CDP_WS_HANDSHAKE_TIMEOUT_MS } from "./cdp-timeouts.js"; -import { BrowserCdpEndpointBlockedError } from "./errors.js"; import { resolveBrowserRateLimitMessage } from "./rate-limit-message.js"; export { isLoopbackHost }; @@ -68,19 +63,9 @@ export async function assertCdpEndpointAllowed( if (!["http:", "https:", "ws:", "wss:"].includes(parsed.protocol)) { throw new Error(`Invalid CDP URL protocol: ${parsed.protocol.replace(":", "")}`); } - try { - await resolvePinnedHostnameWithPolicy(parsed.hostname, { - policy: ssrfPolicy, - }); - } catch (err) { - // Rethrow SSRF policy failures against the CDP endpoint itself as a - // browser-endpoint-scoped error so the route mapping does not confuse - // them with navigation-target policy blocks. - if (err instanceof SsrFBlockedError) { - throw new BrowserCdpEndpointBlockedError({ cause: err }); - } - throw err; - } + await resolvePinnedHostnameWithPolicy(parsed.hostname, { + policy: ssrfPolicy, + }); } export function redactCdpUrl(cdpUrl: string | null | undefined): string | null | undefined { @@ -168,6 +153,11 @@ export function normalizeCdpHttpBaseForJsonEndpoints(cdpUrl: string): string { } } +type CdpFetchResult = { + response: Response; + release: () => Promise; +}; + function createCdpSender(ws: WebSocket) { let nextId = 1; const pending = new Map(); @@ -233,39 +223,50 @@ export async function fetchJson( url: string, timeoutMs = CDP_HTTP_REQUEST_TIMEOUT_MS, init?: RequestInit, + ssrfPolicy?: SsrFPolicy, ): Promise { - const res = await fetchCdpChecked(url, timeoutMs, init); - return (await res.json()) as T; + const { response, release } = await fetchCdpChecked(url, timeoutMs, init, ssrfPolicy); + try { + return (await response.json()) as T; + } finally { + await release(); + } } export async function fetchCdpChecked( url: string, timeoutMs = CDP_HTTP_REQUEST_TIMEOUT_MS, init?: RequestInit, -): Promise { + ssrfPolicy?: SsrFPolicy, +): Promise { const ctrl = new AbortController(); const t = setTimeout(ctrl.abort.bind(ctrl), timeoutMs); - let release: (() => Promise) | undefined; + let guardedRelease: (() => Promise) | undefined; + let released = false; + const release = async () => { + if (released) { + return; + } + released = true; + clearTimeout(t); + await guardedRelease?.(); + }; try { const headers = getHeadersWithAuth(url, (init?.headers as Record) || {}); - // Block redirects on all CDP HTTP paths (not just probes) because a - // redirect to an internal host is an SSRF vector regardless of whether - // the call is /json/version, /json/list, /json/activate, or /json/close. - const guarded = await withNoProxyForCdpUrl(url, () => - fetchWithSsrFGuard({ - url, - init: { ...init, headers }, - maxRedirects: 0, - policy: { allowPrivateNetwork: true }, - signal: ctrl.signal, - auditContext: "browser-cdp", - }), - ); - release = guarded.release; - const res = guarded.response; - if (res.status >= 300 && res.status < 400) { - throw new Error("CDP endpoint redirects are not allowed"); - } + const res = await withNoProxyForCdpUrl(url, async () => { + if (ssrfPolicy) { + const guarded = await fetchWithSsrFGuard({ + url, + init: { ...init, headers }, + signal: ctrl.signal, + policy: ssrfPolicy, + auditContext: "browser-cdp", + }); + guardedRelease = guarded.release; + return guarded.response; + } + return await fetch(url, { ...init, headers, signal: ctrl.signal }); + }); if (!res.ok) { if (res.status === 429) { // Do not reflect upstream response text into the error surface (log/agent injection risk) @@ -273,23 +274,10 @@ export async function fetchCdpChecked( } throw new Error(`HTTP ${res.status}`); } - if (typeof res.arrayBuffer !== "function") { - return res; - } - const body = await res.arrayBuffer(); - return new Response(body, { - headers: res.headers, - status: res.status, - statusText: res.statusText, - }); + return { response: res, release }; } catch (error) { - if (error instanceof Error && error.message.startsWith("Too many redirects")) { - throw new Error("CDP endpoint redirects are not allowed", { cause: error }); - } + await release(); throw error; - } finally { - clearTimeout(t); - await release?.(); } } @@ -297,8 +285,10 @@ export async function fetchOk( url: string, timeoutMs = CDP_HTTP_REQUEST_TIMEOUT_MS, init?: RequestInit, + ssrfPolicy?: SsrFPolicy, ): Promise { - await fetchCdpChecked(url, timeoutMs, init); + const { release } = await fetchCdpChecked(url, timeoutMs, init, ssrfPolicy); + await release(); } export function openCdpWebSocket( diff --git a/extensions/browser/src/browser/cdp.test.ts b/extensions/browser/src/browser/cdp.test.ts index 5f13bed78ab..a846bab80b3 100644 --- a/extensions/browser/src/browser/cdp.test.ts +++ b/extensions/browser/src/browser/cdp.test.ts @@ -186,6 +186,22 @@ describe("cdp", () => { } }); + it("blocks hostname navigation targets when strict SSRF policy is configured", async () => { + const fetchSpy = vi.spyOn(globalThis, "fetch"); + try { + await expect( + createTargetViaCdp({ + cdpUrl: "http://127.0.0.1:9222", + url: "https://example.com", + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + }), + ).rejects.toBeInstanceOf(InvalidBrowserNavigationUrlError); + expect(fetchSpy).not.toHaveBeenCalled(); + } finally { + fetchSpy.mockRestore(); + } + }); + it("blocks unsupported non-network navigation URLs", async () => { const fetchSpy = vi.spyOn(globalThis, "fetch"); try { @@ -236,7 +252,7 @@ describe("cdp", () => { await expect( createTargetViaCdp({ cdpUrl: `http://127.0.0.1:${httpPort}`, - url: "https://example.com", + url: "https://93.184.216.34", ssrfPolicy: { dangerouslyAllowPrivateNetwork: false, allowedHostnames: ["127.0.0.1"], @@ -249,7 +265,7 @@ describe("cdp", () => { await expect( createTargetViaCdp({ cdpUrl: "http://169.254.169.254:9222", - url: "https://example.com", + url: "https://93.184.216.34", ssrfPolicy: { dangerouslyAllowPrivateNetwork: false, allowedHostnames: ["127.0.0.1"], @@ -262,7 +278,7 @@ describe("cdp", () => { await expect( createTargetViaCdp({ cdpUrl: "ws://169.254.169.254:9222/devtools/browser/PIVOT", - url: "https://example.com", + url: "https://93.184.216.34", ssrfPolicy: { dangerouslyAllowPrivateNetwork: false, allowedHostnames: ["127.0.0.1"], diff --git a/extensions/browser/src/browser/cdp.ts b/extensions/browser/src/browser/cdp.ts index f71bb0fd013..a64bde9a8f7 100644 --- a/extensions/browser/src/browser/cdp.ts +++ b/extensions/browser/src/browser/cdp.ts @@ -187,10 +187,11 @@ export async function createTargetViaCdp(opts: { wsUrl = opts.cdpUrl; } else { // Standard HTTP(S) CDP endpoint — discover WebSocket URL via /json/version. - await assertCdpEndpointAllowed(opts.cdpUrl, opts.ssrfPolicy); const version = await fetchJson<{ webSocketDebuggerUrl?: string }>( appendCdpPath(opts.cdpUrl, "/json/version"), 1500, + undefined, + opts.ssrfPolicy, ); const wsUrlRaw = String(version?.webSocketDebuggerUrl ?? "").trim(); wsUrl = wsUrlRaw ? normalizeCdpWsUrl(wsUrlRaw, opts.cdpUrl) : ""; diff --git a/extensions/browser/src/browser/chrome.ts b/extensions/browser/src/browser/chrome.ts index 247eb198a65..0ece64410b3 100644 --- a/extensions/browser/src/browser/chrome.ts +++ b/extensions/browser/src/browser/chrome.ts @@ -171,14 +171,22 @@ async function fetchChromeVersion( const ctrl = new AbortController(); const t = setTimeout(ctrl.abort.bind(ctrl), timeoutMs); try { - await assertCdpEndpointAllowed(cdpUrl, ssrfPolicy); const versionUrl = appendCdpPath(cdpUrl, "/json/version"); - const res = await fetchCdpChecked(versionUrl, timeoutMs, { signal: ctrl.signal }); - const data = (await res.json()) as ChromeVersion; - if (!data || typeof data !== "object") { - return null; + const { response, release } = await fetchCdpChecked( + versionUrl, + timeoutMs, + { signal: ctrl.signal }, + ssrfPolicy, + ); + try { + const data = (await response.json()) as ChromeVersion; + if (!data || typeof data !== "object") { + return null; + } + return data; + } finally { + await release(); } - return data; } catch { return null; } finally { diff --git a/extensions/browser/src/browser/navigation-guard.test.ts b/extensions/browser/src/browser/navigation-guard.test.ts index 16173e1eac0..26e6e147159 100644 --- a/extensions/browser/src/browser/navigation-guard.test.ts +++ b/extensions/browser/src/browser/navigation-guard.test.ts @@ -116,6 +116,85 @@ describe("browser navigation guard", () => { expect(lookupFn).toHaveBeenCalledWith("example.com", { all: true }); }); + it("blocks hostname navigation when strict SSRF policy is explicitly configured", async () => { + const lookupFn = createLookupFn("93.184.216.34"); + await expect( + assertBrowserNavigationAllowed({ + url: "https://example.com", + lookupFn, + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + }), + ).rejects.toThrow(/dns rebinding protections are unavailable/i); + expect(lookupFn).not.toHaveBeenCalled(); + }); + + it("allows explicitly allowed hostnames in strict mode", async () => { + const lookupFn = createLookupFn("93.184.216.34"); + await expect( + assertBrowserNavigationAllowed({ + url: "https://agent.internal", + lookupFn, + ssrfPolicy: { + dangerouslyAllowPrivateNetwork: false, + allowedHostnames: ["agent.internal"], + }, + }), + ).resolves.toBeUndefined(); + }); + + it("allows wildcard-allowlisted hostnames in strict mode", async () => { + const lookupFn = createLookupFn("93.184.216.34"); + await expect( + assertBrowserNavigationAllowed({ + url: "https://sub.example.com", + lookupFn, + ssrfPolicy: { + dangerouslyAllowPrivateNetwork: false, + hostnameAllowlist: ["*.example.com"], + }, + }), + ).resolves.toBeUndefined(); + }); + + it("does not treat the bare suffix as matching a wildcard allowlist entry", async () => { + const lookupFn = createLookupFn("93.184.216.34"); + await expect( + assertBrowserNavigationAllowed({ + url: "https://example.com", + lookupFn, + ssrfPolicy: { + dangerouslyAllowPrivateNetwork: false, + hostnameAllowlist: ["*.example.com"], + }, + }), + ).rejects.toThrow(/dns rebinding protections are unavailable/i); + expect(lookupFn).not.toHaveBeenCalled(); + }); + + it("does not match sibling domains against wildcard allowlist entries", async () => { + const lookupFn = createLookupFn("93.184.216.34"); + await expect( + assertBrowserNavigationAllowed({ + url: "https://evil-example.com", + lookupFn, + ssrfPolicy: { + dangerouslyAllowPrivateNetwork: false, + hostnameAllowlist: ["*.example.com"], + }, + }), + ).rejects.toThrow(/dns rebinding protections are unavailable/i); + expect(lookupFn).not.toHaveBeenCalled(); + }); + + it("treats bracketed IPv6 URL hostnames as IP literals in strict mode", async () => { + await expect( + assertBrowserNavigationAllowed({ + url: "https://[2606:4700:4700::1111]/", + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + }), + ).resolves.toBeUndefined(); + }); + it("blocks strict policy navigation when env proxy is configured", async () => { vi.stubEnv("HTTP_PROXY", "http://127.0.0.1:7890"); const lookupFn = createLookupFn("93.184.216.34"); @@ -165,6 +244,15 @@ describe("browser navigation guard", () => { ).resolves.toBeUndefined(); }); + it("blocks final hostname URLs in strict mode after navigation", async () => { + await expect( + assertBrowserNavigationResultAllowed({ + url: "https://example.com/final", + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + }), + ).rejects.toBeInstanceOf(InvalidBrowserNavigationUrlError); + }); + it("blocks private intermediate redirect hops", async () => { const publicLookup = createLookupFn("93.184.216.34"); const privateLookup = createLookupFn("127.0.0.1"); diff --git a/extensions/browser/src/browser/navigation-guard.ts b/extensions/browser/src/browser/navigation-guard.ts index 35a831b4a62..552a1dcdbc7 100644 --- a/extensions/browser/src/browser/navigation-guard.ts +++ b/extensions/browser/src/browser/navigation-guard.ts @@ -1,3 +1,8 @@ +import { isIP } from "node:net"; +import { + matchesHostnameAllowlist, + normalizeHostname, +} from "openclaw/plugin-sdk/browser-security-runtime"; import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime"; import { hasProxyEnvConfigured } from "../infra/net/proxy-env.js"; import { @@ -41,6 +46,24 @@ export function requiresInspectableBrowserNavigationRedirects(ssrfPolicy?: SsrFP return !isPrivateNetworkAllowedByPolicy(ssrfPolicy); } +function isIpLiteralHostname(hostname: string): boolean { + return isIP(normalizeHostname(hostname)) !== 0; +} + +function isExplicitlyAllowedBrowserHostname(hostname: string, ssrfPolicy?: SsrFPolicy): boolean { + const normalizedHostname = normalizeHostname(hostname); + const exactMatches = ssrfPolicy?.allowedHostnames ?? []; + if (exactMatches.some((value) => normalizeHostname(value) === normalizedHostname)) { + return true; + } + const hostnameAllowlist = (ssrfPolicy?.hostnameAllowlist ?? []) + .map((pattern) => normalizeHostname(pattern)) + .filter(Boolean); + return hostnameAllowlist.length > 0 + ? matchesHostnameAllowlist(normalizedHostname, hostnameAllowlist) + : false; +} + export async function assertBrowserNavigationAllowed( opts: { url: string; @@ -78,6 +101,21 @@ export async function assertBrowserNavigationAllowed( ); } + // Browser navigations happen in Chromium's network stack, not Node's. In + // strict mode, a hostname-based URL would be resolved twice by different + // resolvers, so Node-side pinning cannot guarantee the browser connects to + // the same address that passed policy checks. + if ( + opts.ssrfPolicy && + !isPrivateNetworkAllowedByPolicy(opts.ssrfPolicy) && + !isIpLiteralHostname(parsed.hostname) && + !isExplicitlyAllowedBrowserHostname(parsed.hostname, opts.ssrfPolicy) + ) { + throw new InvalidBrowserNavigationUrlError( + "Navigation blocked: strict browser SSRF policy requires an IP-literal URL because browser DNS rebinding protections are unavailable for hostname-based navigation", + ); + } + await resolvePinnedHostnameWithPolicy(parsed.hostname, { lookupFn: opts.lookupFn, policy: opts.ssrfPolicy, @@ -87,7 +125,8 @@ export async function assertBrowserNavigationAllowed( /** * Best-effort post-navigation guard for final page URLs. * Only validates network URLs (http/https) and about:blank to avoid false - * positives on browser-internal error pages (e.g. chrome-error://). + * positives on browser-internal error pages (e.g. chrome-error://). In strict + * mode this intentionally re-applies the hostname gate after redirects. */ export async function assertBrowserNavigationResultAllowed( opts: { 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 35d4f9095b7..7387511fc04 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 @@ -202,6 +202,20 @@ describe("pw-session createPageViaPlaywright navigation guard", () => { expect(pageGoto).not.toHaveBeenCalled(); }); + it("blocks hostname navigation when strict SSRF policy is configured", async () => { + const { pageGoto } = installBrowserMocks(); + + await expect( + createPageViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + url: "https://example.com", + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + }), + ).rejects.toBeInstanceOf(InvalidBrowserNavigationUrlError); + + expect(pageGoto).not.toHaveBeenCalled(); + }); + it("blocks private intermediate redirect hops", async () => { const { pageGoto, pageClose, getRouteHandler, mainFrame } = installBrowserMocks(); mockBlockedRedirectNavigation({ pageGoto, getRouteHandler, mainFrame }); diff --git a/extensions/browser/src/browser/server-context.selection.ts b/extensions/browser/src/browser/server-context.selection.ts index 9f4d42a040e..25eac33003a 100644 --- a/extensions/browser/src/browser/server-context.selection.ts +++ b/extensions/browser/src/browser/server-context.selection.ts @@ -1,9 +1,6 @@ import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime"; -import { - assertCdpEndpointAllowed, - fetchOk, - normalizeCdpHttpBaseForJsonEndpoints, -} from "./cdp.helpers.js"; +import type { SsrFPolicy } from "../infra/net/ssrf.js"; +import { fetchOk, normalizeCdpHttpBaseForJsonEndpoints } from "./cdp.helpers.js"; import { appendCdpPath } from "./cdp.js"; import { closeChromeMcpTab, focusChromeMcpTab } from "./chrome-mcp.js"; import type { ResolvedBrowserProfile } from "./config.js"; @@ -11,17 +8,13 @@ import { BrowserTabNotFoundError, BrowserTargetAmbiguousError } from "./errors.j import { getBrowserProfileCapabilities } from "./profile-capabilities.js"; import type { PwAiModule } from "./pw-ai-module.js"; import { getPwAiModule } from "./pw-ai-module.js"; -import type { - BrowserServerState, - BrowserTab, - ProfileRuntimeState, -} from "./server-context.types.js"; +import type { BrowserTab, ProfileRuntimeState } from "./server-context.types.js"; import { resolveTargetIdFromTabs } from "./target-id.js"; type SelectionDeps = { profile: ResolvedBrowserProfile; - state: () => BrowserServerState; getProfileState: () => ProfileRuntimeState; + getSsrFPolicy: () => SsrFPolicy | undefined; ensureBrowserAvailable: () => Promise; listTabs: () => Promise; openTab: (url: string) => Promise; @@ -35,27 +28,17 @@ type SelectionOps = { export function createProfileSelectionOps({ profile, - state, getProfileState, + getSsrFPolicy, ensureBrowserAvailable, listTabs, openTab, }: SelectionDeps): SelectionOps { const cdpHttpBase = normalizeCdpHttpBaseForJsonEndpoints(profile.cdpUrl); const capabilities = getBrowserProfileCapabilities(profile); - const assertProfileCdpEndpointAllowed = async (): Promise => { - await assertCdpEndpointAllowed(profile.cdpUrl, state().resolved.ssrfPolicy); - }; - const assertSelectableCdpEndpointAllowed = async (): Promise => { - if (capabilities.usesChromeMcp || !profile.cdpUrl) { - return; - } - await assertProfileCdpEndpointAllowed(); - }; const ensureTabAvailable = async (targetId?: string): Promise => { await ensureBrowserAvailable(); - await assertSelectableCdpEndpointAllowed(); const profileState = getProfileState(); const tabs1 = await listTabs(); if (tabs1.length === 0) { @@ -100,7 +83,6 @@ export function createProfileSelectionOps({ }; const resolveTargetIdOrThrow = async (targetId: string): Promise => { - await assertSelectableCdpEndpointAllowed(); const tabs = await listTabs(); const resolved = resolveTargetIdFromTabs(targetId, tabs); if (!resolved.ok) { @@ -127,11 +109,9 @@ export function createProfileSelectionOps({ const focusPageByTargetIdViaPlaywright = (mod as Partial | null) ?.focusPageByTargetIdViaPlaywright; if (typeof focusPageByTargetIdViaPlaywright === "function") { - // SSRF check runs inside connectBrowser on cache miss. await focusPageByTargetIdViaPlaywright({ cdpUrl: profile.cdpUrl, targetId: resolvedTargetId, - ssrfPolicy: state().resolved.ssrfPolicy, }); const profileState = getProfileState(); profileState.lastTargetId = resolvedTargetId; @@ -139,8 +119,12 @@ export function createProfileSelectionOps({ } } - await assertProfileCdpEndpointAllowed(); - await fetchOk(appendCdpPath(cdpHttpBase, `/json/activate/${resolvedTargetId}`)); + await fetchOk( + appendCdpPath(cdpHttpBase, `/json/activate/${resolvedTargetId}`), + undefined, + undefined, + getSsrFPolicy(), + ); const profileState = getProfileState(); profileState.lastTargetId = resolvedTargetId; }; @@ -159,18 +143,20 @@ export function createProfileSelectionOps({ const closePageByTargetIdViaPlaywright = (mod as Partial | null) ?.closePageByTargetIdViaPlaywright; if (typeof closePageByTargetIdViaPlaywright === "function") { - // SSRF check runs inside connectBrowser on cache miss. await closePageByTargetIdViaPlaywright({ cdpUrl: profile.cdpUrl, targetId: resolvedTargetId, - ssrfPolicy: state().resolved.ssrfPolicy, }); return; } } - await assertProfileCdpEndpointAllowed(); - await fetchOk(appendCdpPath(cdpHttpBase, `/json/close/${resolvedTargetId}`)); + await fetchOk( + appendCdpPath(cdpHttpBase, `/json/close/${resolvedTargetId}`), + undefined, + undefined, + getSsrFPolicy(), + ); }; return { diff --git a/extensions/browser/src/browser/server-context.tab-ops.ts b/extensions/browser/src/browser/server-context.tab-ops.ts index c16a0803e49..92ced121705 100644 --- a/extensions/browser/src/browser/server-context.tab-ops.ts +++ b/extensions/browser/src/browser/server-context.tab-ops.ts @@ -1,10 +1,5 @@ import { CDP_JSON_NEW_TIMEOUT_MS } from "./cdp-timeouts.js"; -import { - assertCdpEndpointAllowed, - fetchJson, - fetchOk, - normalizeCdpHttpBaseForJsonEndpoints, -} from "./cdp.helpers.js"; +import { fetchJson, fetchOk, normalizeCdpHttpBaseForJsonEndpoints } from "./cdp.helpers.js"; import { appendCdpPath, createTargetViaCdp, normalizeCdpWsUrl } from "./cdp.js"; import { listChromeMcpTabs, openChromeMcpTab } from "./chrome-mcp.js"; import type { ResolvedBrowserProfile } from "./config.js"; @@ -69,9 +64,7 @@ export function createProfileTabOps({ }: TabOpsDeps): ProfileTabOps { const cdpHttpBase = normalizeCdpHttpBaseForJsonEndpoints(profile.cdpUrl); const capabilities = getBrowserProfileCapabilities(profile); - const assertProfileCdpEndpointAllowed = async (): Promise => { - await assertCdpEndpointAllowed(profile.cdpUrl, state().resolved.ssrfPolicy); - }; + const getSsrFPolicy = () => state().resolved.ssrfPolicy; const listTabs = async (): Promise => { if (capabilities.usesChromeMcp) { @@ -82,11 +75,7 @@ export function createProfileTabOps({ const mod = await getPwAiModule({ mode: "strict" }); const listPagesViaPlaywright = (mod as Partial | null)?.listPagesViaPlaywright; if (typeof listPagesViaPlaywright === "function") { - await assertProfileCdpEndpointAllowed(); - const pages = await listPagesViaPlaywright({ - cdpUrl: profile.cdpUrl, - ssrfPolicy: state().resolved.ssrfPolicy, - }); + const pages = await listPagesViaPlaywright({ cdpUrl: profile.cdpUrl }); return pages.map((p) => ({ targetId: p.targetId, title: p.title, @@ -96,7 +85,6 @@ export function createProfileTabOps({ } } - await assertProfileCdpEndpointAllowed(); const raw = await fetchJson< Array<{ id?: string; @@ -105,7 +93,7 @@ export function createProfileTabOps({ webSocketDebuggerUrl?: string; type?: string; }> - >(appendCdpPath(cdpHttpBase, "/json/list")); + >(appendCdpPath(cdpHttpBase, "/json/list"), undefined, undefined, getSsrFPolicy()); return raw .map((t) => ({ targetId: t.id ?? "", @@ -136,9 +124,13 @@ export function createProfileTabOps({ const candidates = pageTabs.filter((tab) => tab.targetId !== keepTargetId); const excessCount = pageTabs.length - MANAGED_BROWSER_PAGE_TAB_LIMIT; - await assertProfileCdpEndpointAllowed(); for (const tab of candidates.slice(0, excessCount)) { - void fetchOk(appendCdpPath(cdpHttpBase, `/json/close/${tab.targetId}`)).catch(() => { + void fetchOk( + appendCdpPath(cdpHttpBase, `/json/close/${tab.targetId}`), + undefined, + undefined, + getSsrFPolicy(), + ).catch(() => { // best-effort cleanup only }); } @@ -224,12 +216,21 @@ export function createProfileTabOps({ return endpointUrl.toString(); })() : `${endpointUrl.toString()}?${encoded}`; - await assertProfileCdpEndpointAllowed(); - const created = await fetchJson(endpoint, CDP_JSON_NEW_TIMEOUT_MS, { - method: "PUT", - }).catch(async (err) => { + const created = await fetchJson( + endpoint, + CDP_JSON_NEW_TIMEOUT_MS, + { + method: "PUT", + }, + ssrfPolicyOpts.ssrfPolicy, + ).catch(async (err) => { if (String(err).includes("HTTP 405")) { - return await fetchJson(endpoint, CDP_JSON_NEW_TIMEOUT_MS); + return await fetchJson( + endpoint, + CDP_JSON_NEW_TIMEOUT_MS, + undefined, + ssrfPolicyOpts.ssrfPolicy, + ); } throw err; }); diff --git a/extensions/browser/src/browser/server-context.ts b/extensions/browser/src/browser/server-context.ts index fcf661161f6..0f0c574e290 100644 --- a/extensions/browser/src/browser/server-context.ts +++ b/extensions/browser/src/browser/server-context.ts @@ -85,8 +85,8 @@ function createProfileContext( const { ensureTabAvailable, focusTab, closeTab } = createProfileSelectionOps({ profile, - state, getProfileState, + getSsrFPolicy: () => state().resolved.ssrfPolicy, ensureBrowserAvailable, listTabs, openTab, diff --git a/src/infra/net/ssrf.ts b/src/infra/net/ssrf.ts index 2cff500b038..536148f8a2c 100644 --- a/src/infra/net/ssrf.ts +++ b/src/infra/net/ssrf.ts @@ -57,7 +57,7 @@ function normalizeHostnameSet(values?: string[]): Set { return new Set(values.map((value) => normalizeHostname(value)).filter(Boolean)); } -function normalizeHostnameAllowlist(values?: string[]): string[] { +export function normalizeHostnameAllowlist(values?: string[]): string[] { if (!values || values.length === 0) { return []; } @@ -87,7 +87,7 @@ function resolveIpv4SpecialUseBlockOptions(policy?: SsrFPolicy): Ipv4SpecialUseB }; } -function isHostnameAllowedByPattern(hostname: string, pattern: string): boolean { +export function isHostnameAllowedByPattern(hostname: string, pattern: string): boolean { if (pattern.startsWith("*.")) { const suffix = pattern.slice(2); if (!suffix || hostname === suffix) { @@ -98,7 +98,7 @@ function isHostnameAllowedByPattern(hostname: string, pattern: string): boolean return hostname === pattern; } -function matchesHostnameAllowlist(hostname: string, allowlist: string[]): boolean { +export function matchesHostnameAllowlist(hostname: string, allowlist: string[]): boolean { if (allowlist.length === 0) { return true; } diff --git a/src/plugin-sdk/browser-security-runtime.ts b/src/plugin-sdk/browser-security-runtime.ts index fcda7f387b9..690e8069660 100644 --- a/src/plugin-sdk/browser-security-runtime.ts +++ b/src/plugin-sdk/browser-security-runtime.ts @@ -9,11 +9,13 @@ export { hasProxyEnvConfigured } from "../infra/net/proxy-env.js"; export { SsrFBlockedError, isBlockedHostnameOrIp, + matchesHostnameAllowlist, isPrivateNetworkAllowedByPolicy, resolvePinnedHostnameWithPolicy, type LookupFn, type SsrFPolicy, } from "../infra/net/ssrf.js"; +export { normalizeHostname } from "../infra/net/hostname.js"; export { isNotFoundPathError, isPathInside } from "../infra/path-guards.js"; export { ensurePortAvailable } from "../infra/ports.js"; export { generateSecureToken } from "../infra/secure-random.js";