Skip to content

Commit 121c452

Browse files
eleqtrizitdrobison00
eleqtrizit
and
drobison00
authored
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 <[email protected]>
1 parent 4164d6f commit 121c452

14 files changed

Lines changed: 321 additions & 163 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ Docs: https://docs.openclaw.ai
133133
- 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.
134134
- Hooks/security: mark agent hook system events as untrusted and sanitize hook display names before cron metadata reuse. (#64372) Thanks @eleqtrizit.
135135
- 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.
136+
- 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.
136137
## 2026.4.9
137138

138139
### Changes

‎extensions/browser/src/browser/cdp.helpers.test.ts‎

Lines changed: 49 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,65 @@
11
import { afterEach, describe, expect, it, vi } from "vitest";
2-
import { SsrFBlockedError } from "../infra/net/ssrf.js";
32

4-
vi.mock("./cdp-proxy-bypass.js", () => ({
5-
getDirectAgentForCdp: vi.fn(() => null),
6-
withNoProxyForCdpUrl: vi.fn(async (_url: string, fn: () => Promise<unknown>) => await fn()),
7-
}));
3+
const fetchWithSsrFGuardMock = vi.hoisted(() => vi.fn());
84

9-
const { assertCdpEndpointAllowed, fetchCdpChecked } = await import("./cdp.helpers.js");
10-
const { BrowserCdpEndpointBlockedError } = await import("./errors.js");
5+
vi.mock("openclaw/plugin-sdk/ssrf-runtime", async (importOriginal) => {
6+
const actual = await importOriginal<typeof import("openclaw/plugin-sdk/ssrf-runtime")>();
7+
return {
8+
...actual,
9+
fetchWithSsrFGuard: (...args: unknown[]) => fetchWithSsrFGuardMock(...args),
10+
};
11+
});
12+
13+
import { fetchJson, fetchOk } from "./cdp.helpers.js";
1114

12-
describe("fetchCdpChecked", () => {
15+
describe("cdp helpers", () => {
1316
afterEach(() => {
14-
vi.unstubAllGlobals();
17+
fetchWithSsrFGuardMock.mockReset();
1518
});
1619

17-
it("disables automatic redirect following for CDP HTTP probes", async () => {
18-
const fetchSpy = vi.fn().mockResolvedValue(
19-
new Response(null, {
20-
status: 302,
21-
headers: { Location: "http://127.0.0.1:9222/json/version" },
22-
}),
23-
);
24-
vi.stubGlobal("fetch", fetchSpy);
20+
it("releases guarded CDP fetches after the response body is consumed", async () => {
21+
const release = vi.fn(async () => {});
22+
const json = vi.fn(async () => {
23+
expect(release).not.toHaveBeenCalled();
24+
return { ok: true };
25+
});
26+
fetchWithSsrFGuardMock.mockResolvedValueOnce({
27+
response: {
28+
ok: true,
29+
status: 200,
30+
json,
31+
},
32+
release,
33+
});
2534

26-
await expect(fetchCdpChecked("https://example.com/json/version", 50)).rejects.toThrow(
27-
"CDP endpoint redirects are not allowed",
28-
);
35+
await expect(
36+
fetchJson("http://127.0.0.1:9222/json/version", 250, undefined, {
37+
dangerouslyAllowPrivateNetwork: false,
38+
allowedHostnames: ["127.0.0.1"],
39+
}),
40+
).resolves.toEqual({ ok: true });
2941

30-
const init = fetchSpy.mock.calls[0]?.[1];
31-
expect(init?.redirect).toBe("manual");
42+
expect(json).toHaveBeenCalledTimes(1);
43+
expect(release).toHaveBeenCalledTimes(1);
3244
});
33-
});
3445

35-
describe("assertCdpEndpointAllowed", () => {
36-
it("rethrows SSRF policy failures as BrowserCdpEndpointBlockedError so mapping can distinguish endpoint vs navigation", async () => {
37-
await expect(
38-
assertCdpEndpointAllowed("http://10.0.0.42:9222", { dangerouslyAllowPrivateNetwork: false }),
39-
).rejects.toBeInstanceOf(BrowserCdpEndpointBlockedError);
40-
});
46+
it("releases guarded CDP fetches for bodyless requests", async () => {
47+
const release = vi.fn(async () => {});
48+
fetchWithSsrFGuardMock.mockResolvedValueOnce({
49+
response: {
50+
ok: true,
51+
status: 200,
52+
},
53+
release,
54+
});
4155

42-
it("does not wrap non-SSRF failures", async () => {
4356
await expect(
44-
assertCdpEndpointAllowed("file:///etc/passwd", { dangerouslyAllowPrivateNetwork: false }),
45-
).rejects.not.toBeInstanceOf(BrowserCdpEndpointBlockedError);
46-
});
57+
fetchOk("http://127.0.0.1:9222/json/close/TARGET_1", 250, undefined, {
58+
dangerouslyAllowPrivateNetwork: false,
59+
allowedHostnames: ["127.0.0.1"],
60+
}),
61+
).resolves.toBeUndefined();
4762

48-
it("leaves navigation-target SsrFBlockedError alone for callers that never hit the endpoint helper", () => {
49-
// Sanity check that raw SsrFBlockedError is still its own class and is not
50-
// accidentally converted by the endpoint helper import.
51-
expect(new SsrFBlockedError("blocked")).toBeInstanceOf(SsrFBlockedError);
63+
expect(release).toHaveBeenCalledTimes(1);
5264
});
5365
});

‎extensions/browser/src/browser/cdp.helpers.ts‎

Lines changed: 47 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,11 @@ import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime";
22
import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/text-runtime";
33
import WebSocket from "ws";
44
import { isLoopbackHost } from "../gateway/net.js";
5-
import {
6-
SsrFBlockedError,
7-
type SsrFPolicy,
8-
resolvePinnedHostnameWithPolicy,
9-
} from "../infra/net/ssrf.js";
5+
import { type SsrFPolicy, resolvePinnedHostnameWithPolicy } from "../infra/net/ssrf.js";
106
import { rawDataToString } from "../infra/ws.js";
117
import { redactSensitiveText } from "../logging/redact.js";
128
import { getDirectAgentForCdp, withNoProxyForCdpUrl } from "./cdp-proxy-bypass.js";
139
import { CDP_HTTP_REQUEST_TIMEOUT_MS, CDP_WS_HANDSHAKE_TIMEOUT_MS } from "./cdp-timeouts.js";
14-
import { BrowserCdpEndpointBlockedError } from "./errors.js";
1510
import { resolveBrowserRateLimitMessage } from "./rate-limit-message.js";
1611

1712
export { isLoopbackHost };
@@ -68,19 +63,9 @@ export async function assertCdpEndpointAllowed(
6863
if (!["http:", "https:", "ws:", "wss:"].includes(parsed.protocol)) {
6964
throw new Error(`Invalid CDP URL protocol: ${parsed.protocol.replace(":", "")}`);
7065
}
71-
try {
72-
await resolvePinnedHostnameWithPolicy(parsed.hostname, {
73-
policy: ssrfPolicy,
74-
});
75-
} catch (err) {
76-
// Rethrow SSRF policy failures against the CDP endpoint itself as a
77-
// browser-endpoint-scoped error so the route mapping does not confuse
78-
// them with navigation-target policy blocks.
79-
if (err instanceof SsrFBlockedError) {
80-
throw new BrowserCdpEndpointBlockedError({ cause: err });
81-
}
82-
throw err;
83-
}
66+
await resolvePinnedHostnameWithPolicy(parsed.hostname, {
67+
policy: ssrfPolicy,
68+
});
8469
}
8570

8671
export function redactCdpUrl(cdpUrl: string | null | undefined): string | null | undefined {
@@ -168,6 +153,11 @@ export function normalizeCdpHttpBaseForJsonEndpoints(cdpUrl: string): string {
168153
}
169154
}
170155

156+
type CdpFetchResult = {
157+
response: Response;
158+
release: () => Promise<void>;
159+
};
160+
171161
function createCdpSender(ws: WebSocket) {
172162
let nextId = 1;
173163
const pending = new Map<number, Pending>();
@@ -233,72 +223,72 @@ export async function fetchJson<T>(
233223
url: string,
234224
timeoutMs = CDP_HTTP_REQUEST_TIMEOUT_MS,
235225
init?: RequestInit,
226+
ssrfPolicy?: SsrFPolicy,
236227
): Promise<T> {
237-
const res = await fetchCdpChecked(url, timeoutMs, init);
238-
return (await res.json()) as T;
228+
const { response, release } = await fetchCdpChecked(url, timeoutMs, init, ssrfPolicy);
229+
try {
230+
return (await response.json()) as T;
231+
} finally {
232+
await release();
233+
}
239234
}
240235

241236
export async function fetchCdpChecked(
242237
url: string,
243238
timeoutMs = CDP_HTTP_REQUEST_TIMEOUT_MS,
244239
init?: RequestInit,
245-
): Promise<Response> {
240+
ssrfPolicy?: SsrFPolicy,
241+
): Promise<CdpFetchResult> {
246242
const ctrl = new AbortController();
247243
const t = setTimeout(ctrl.abort.bind(ctrl), timeoutMs);
248-
let release: (() => Promise<void>) | undefined;
244+
let guardedRelease: (() => Promise<void>) | undefined;
245+
let released = false;
246+
const release = async () => {
247+
if (released) {
248+
return;
249+
}
250+
released = true;
251+
clearTimeout(t);
252+
await guardedRelease?.();
253+
};
249254
try {
250255
const headers = getHeadersWithAuth(url, (init?.headers as Record<string, string>) || {});
251-
// Block redirects on all CDP HTTP paths (not just probes) because a
252-
// redirect to an internal host is an SSRF vector regardless of whether
253-
// the call is /json/version, /json/list, /json/activate, or /json/close.
254-
const guarded = await withNoProxyForCdpUrl(url, () =>
255-
fetchWithSsrFGuard({
256-
url,
257-
init: { ...init, headers },
258-
maxRedirects: 0,
259-
policy: { allowPrivateNetwork: true },
260-
signal: ctrl.signal,
261-
auditContext: "browser-cdp",
262-
}),
263-
);
264-
release = guarded.release;
265-
const res = guarded.response;
266-
if (res.status >= 300 && res.status < 400) {
267-
throw new Error("CDP endpoint redirects are not allowed");
268-
}
256+
const res = await withNoProxyForCdpUrl(url, async () => {
257+
if (ssrfPolicy) {
258+
const guarded = await fetchWithSsrFGuard({
259+
url,
260+
init: { ...init, headers },
261+
signal: ctrl.signal,
262+
policy: ssrfPolicy,
263+
auditContext: "browser-cdp",
264+
});
265+
guardedRelease = guarded.release;
266+
return guarded.response;
267+
}
268+
return await fetch(url, { ...init, headers, signal: ctrl.signal });
269+
});
269270
if (!res.ok) {
270271
if (res.status === 429) {
271272
// Do not reflect upstream response text into the error surface (log/agent injection risk)
272273
throw new Error(`${resolveBrowserRateLimitMessage(url)} Do NOT retry the browser tool.`);
273274
}
274275
throw new Error(`HTTP ${res.status}`);
275276
}
276-
if (typeof res.arrayBuffer !== "function") {
277-
return res;
278-
}
279-
const body = await res.arrayBuffer();
280-
return new Response(body, {
281-
headers: res.headers,
282-
status: res.status,
283-
statusText: res.statusText,
284-
});
277+
return { response: res, release };
285278
} catch (error) {
286-
if (error instanceof Error && error.message.startsWith("Too many redirects")) {
287-
throw new Error("CDP endpoint redirects are not allowed", { cause: error });
288-
}
279+
await release();
289280
throw error;
290-
} finally {
291-
clearTimeout(t);
292-
await release?.();
293281
}
294282
}
295283

296284
export async function fetchOk(
297285
url: string,
298286
timeoutMs = CDP_HTTP_REQUEST_TIMEOUT_MS,
299287
init?: RequestInit,
288+
ssrfPolicy?: SsrFPolicy,
300289
): Promise<void> {
301-
await fetchCdpChecked(url, timeoutMs, init);
290+
const { release } = await fetchCdpChecked(url, timeoutMs, init, ssrfPolicy);
291+
await release();
302292
}
303293

304294
export function openCdpWebSocket(

‎extensions/browser/src/browser/cdp.test.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,22 @@ describe("cdp", () => {
186186
}
187187
});
188188

189+
it("blocks hostname navigation targets when strict SSRF policy is configured", async () => {
190+
const fetchSpy = vi.spyOn(globalThis, "fetch");
191+
try {
192+
await expect(
193+
createTargetViaCdp({
194+
cdpUrl: "http://127.0.0.1:9222",
195+
url: "https://example.com",
196+
ssrfPolicy: { dangerouslyAllowPrivateNetwork: false },
197+
}),
198+
).rejects.toBeInstanceOf(InvalidBrowserNavigationUrlError);
199+
expect(fetchSpy).not.toHaveBeenCalled();
200+
} finally {
201+
fetchSpy.mockRestore();
202+
}
203+
});
204+
189205
it("blocks unsupported non-network navigation URLs", async () => {
190206
const fetchSpy = vi.spyOn(globalThis, "fetch");
191207
try {
@@ -236,7 +252,7 @@ describe("cdp", () => {
236252
await expect(
237253
createTargetViaCdp({
238254
cdpUrl: `http://127.0.0.1:${httpPort}`,
239-
url: "https://example.com",
255+
url: "https://93.184.216.34",
240256
ssrfPolicy: {
241257
dangerouslyAllowPrivateNetwork: false,
242258
allowedHostnames: ["127.0.0.1"],
@@ -249,7 +265,7 @@ describe("cdp", () => {
249265
await expect(
250266
createTargetViaCdp({
251267
cdpUrl: "http://169.254.169.254:9222",
252-
url: "https://example.com",
268+
url: "https://93.184.216.34",
253269
ssrfPolicy: {
254270
dangerouslyAllowPrivateNetwork: false,
255271
allowedHostnames: ["127.0.0.1"],
@@ -262,7 +278,7 @@ describe("cdp", () => {
262278
await expect(
263279
createTargetViaCdp({
264280
cdpUrl: "ws://169.254.169.254:9222/devtools/browser/PIVOT",
265-
url: "https://example.com",
281+
url: "https://93.184.216.34",
266282
ssrfPolicy: {
267283
dangerouslyAllowPrivateNetwork: false,
268284
allowedHostnames: ["127.0.0.1"],

‎extensions/browser/src/browser/cdp.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,11 @@ export async function createTargetViaCdp(opts: {
187187
wsUrl = opts.cdpUrl;
188188
} else {
189189
// Standard HTTP(S) CDP endpoint — discover WebSocket URL via /json/version.
190-
await assertCdpEndpointAllowed(opts.cdpUrl, opts.ssrfPolicy);
191190
const version = await fetchJson<{ webSocketDebuggerUrl?: string }>(
192191
appendCdpPath(opts.cdpUrl, "/json/version"),
193192
1500,
193+
undefined,
194+
opts.ssrfPolicy,
194195
);
195196
const wsUrlRaw = String(version?.webSocketDebuggerUrl ?? "").trim();
196197
wsUrl = wsUrlRaw ? normalizeCdpWsUrl(wsUrlRaw, opts.cdpUrl) : "";

‎extensions/browser/src/browser/chrome.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -171,14 +171,22 @@ async function fetchChromeVersion(
171171
const ctrl = new AbortController();
172172
const t = setTimeout(ctrl.abort.bind(ctrl), timeoutMs);
173173
try {
174-
await assertCdpEndpointAllowed(cdpUrl, ssrfPolicy);
175174
const versionUrl = appendCdpPath(cdpUrl, "/json/version");
176-
const res = await fetchCdpChecked(versionUrl, timeoutMs, { signal: ctrl.signal });
177-
const data = (await res.json()) as ChromeVersion;
178-
if (!data || typeof data !== "object") {
179-
return null;
175+
const { response, release } = await fetchCdpChecked(
176+
versionUrl,
177+
timeoutMs,
178+
{ signal: ctrl.signal },
179+
ssrfPolicy,
180+
);
181+
try {
182+
const data = (await response.json()) as ChromeVersion;
183+
if (!data || typeof data !== "object") {
184+
return null;
185+
}
186+
return data;
187+
} finally {
188+
await release();
180189
}
181-
return data;
182190
} catch {
183191
return null;
184192
} finally {

0 commit comments

Comments
 (0)