Skip to content

Commit 6195660

Browse files
committed
fix(browser): unify SSRF guard path for navigation
1 parent 3c419b7 commit 6195660

15 files changed

+269
-18
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ Docs: https://docs.openclaw.ai
6565
- Commands/Doctor: avoid rewriting invalid configs with new `gateway.auth.token` defaults during repair and only write when real config changes are detected, preventing accidental token duplication and backup churn.
6666
- Sandbox/Registry: serialize container and browser registry writes with shared file locks and atomic replacement to prevent lost updates and delete rollback races from desyncing `sandbox list`, `prune`, and `recreate --all`. Thanks @kexinoh.
6767
- Security/Exec: require `tools.exec.safeBins` binaries to resolve from trusted bin directories (system defaults plus gateway startup `PATH`) so PATH-hijacked trojan binaries cannot bypass allowlist checks. Thanks @jackhax for reporting.
68+
- Security/Browser: route browser URL navigation through one SSRF-guarded validation path for tab-open/CDP-target/Playwright navigation flows and block private/metadata destinations by default (configurable via `browser.ssrfPolicy`). This ships in the next npm release. Thanks @dorjoos for reporting.
6869
- Cron/Webhooks: protect cron webhook POST delivery with SSRF-guarded outbound fetch (`fetchWithSsrFGuard`) to block private/metadata destinations before request dispatch. Thanks @Adam55A-code.
6970
- Security/Net: block SSRF bypass via NAT64 (`64:ff9b::/96`, `64:ff9b:1::/48`), 6to4 (`2002::/16`), and Teredo (`2001:0000::/32`) IPv6 transition addresses, and fail closed on IPv6 parse errors. Thanks @jackhax.
7071

src/browser/cdp.test.ts

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createServer } from "node:http";
2-
import { afterEach, describe, expect, it } from "vitest";
2+
import { afterEach, describe, expect, it, vi } from "vitest";
33
import { type WebSocket, WebSocketServer } from "ws";
4+
import { SsrFBlockedError } from "../infra/net/ssrf.js";
45
import { rawDataToString } from "../infra/ws.js";
56
import { createTargetViaCdp, evaluateJavaScript, normalizeCdpWsUrl, snapshotAria } from "./cdp.js";
67

@@ -92,6 +93,61 @@ describe("cdp", () => {
9293
expect(created.targetId).toBe("TARGET_123");
9394
});
9495

96+
it("blocks private navigation targets by default", async () => {
97+
const fetchSpy = vi.spyOn(globalThis, "fetch");
98+
try {
99+
await expect(
100+
createTargetViaCdp({
101+
cdpUrl: "http://127.0.0.1:9222",
102+
url: "http://127.0.0.1:8080",
103+
}),
104+
).rejects.toBeInstanceOf(SsrFBlockedError);
105+
expect(fetchSpy).not.toHaveBeenCalled();
106+
} finally {
107+
fetchSpy.mockRestore();
108+
}
109+
});
110+
111+
it("allows private navigation targets when explicitly configured", async () => {
112+
const wsPort = await startWsServerWithMessages((msg, socket) => {
113+
if (msg.method !== "Target.createTarget") {
114+
return;
115+
}
116+
expect(msg.params?.url).toBe("http://127.0.0.1:8080");
117+
socket.send(
118+
JSON.stringify({
119+
id: msg.id,
120+
result: { targetId: "TARGET_LOCAL" },
121+
}),
122+
);
123+
});
124+
125+
httpServer = createServer((req, res) => {
126+
if (req.url === "/json/version") {
127+
res.setHeader("content-type", "application/json");
128+
res.end(
129+
JSON.stringify({
130+
webSocketDebuggerUrl: `ws://127.0.0.1:${wsPort}/devtools/browser/TEST`,
131+
}),
132+
);
133+
return;
134+
}
135+
res.statusCode = 404;
136+
res.end("not found");
137+
});
138+
139+
await new Promise<void>((resolve) => httpServer?.listen(0, "127.0.0.1", resolve));
140+
const httpPort = (httpServer.address() as { port: number }).port;
141+
142+
const created = await createTargetViaCdp({
143+
cdpUrl: `http://127.0.0.1:${httpPort}`,
144+
url: "http://127.0.0.1:8080",
145+
ssrfPolicy: { allowPrivateNetwork: true },
146+
});
147+
148+
expect(created.targetId).toBe("TARGET_LOCAL");
149+
});
150+
95151
it("evaluates javascript via CDP", async () => {
96152
const wsPort = await startWsServerWithMessages((msg, socket) => {
97153
if (msg.method === "Runtime.enable") {

src/browser/cdp.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
import type { SsrFPolicy } from "../infra/net/ssrf.js";
12
import { appendCdpPath, fetchJson, isLoopbackHost, withCdpSocket } from "./cdp.helpers.js";
3+
import { assertBrowserNavigationAllowed } from "./navigation-guard.js";
24

35
export { appendCdpPath, fetchJson, fetchOk, getHeadersWithAuth } from "./cdp.helpers.js";
46

@@ -85,7 +87,13 @@ export async function captureScreenshot(opts: {
8587
export async function createTargetViaCdp(opts: {
8688
cdpUrl: string;
8789
url: string;
90+
ssrfPolicy?: SsrFPolicy;
8891
}): Promise<{ targetId: string }> {
92+
await assertBrowserNavigationAllowed({
93+
url: opts.url,
94+
ssrfPolicy: opts.ssrfPolicy,
95+
});
96+
8997
const version = await fetchJson<{ webSocketDebuggerUrl?: string }>(
9098
appendCdpPath(opts.cdpUrl, "/json/version"),
9199
1500,

src/browser/config.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,4 +182,24 @@ describe("browser config", () => {
182182
});
183183
expect(resolved.extraArgs).toEqual([]);
184184
});
185+
186+
it("resolves browser SSRF policy when configured", () => {
187+
const resolved = resolveBrowserConfig({
188+
ssrfPolicy: {
189+
allowPrivateNetwork: true,
190+
allowedHostnames: [" localhost ", ""],
191+
hostnameAllowlist: [" *.trusted.example ", " "],
192+
},
193+
});
194+
expect(resolved.ssrfPolicy).toEqual({
195+
allowPrivateNetwork: true,
196+
allowedHostnames: ["localhost"],
197+
hostnameAllowlist: ["*.trusted.example"],
198+
});
199+
});
200+
201+
it("keeps browser SSRF policy undefined when not configured", () => {
202+
const resolved = resolveBrowserConfig({});
203+
expect(resolved.ssrfPolicy).toBeUndefined();
204+
});
185205
});

src/browser/config.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
DEFAULT_BROWSER_CONTROL_PORT,
77
} from "../config/port-defaults.js";
88
import { isLoopbackHost } from "../gateway/net.js";
9+
import type { SsrFPolicy } from "../infra/net/ssrf.js";
910
import {
1011
DEFAULT_OPENCLAW_BROWSER_COLOR,
1112
DEFAULT_OPENCLAW_BROWSER_ENABLED,
@@ -31,6 +32,7 @@ export type ResolvedBrowserConfig = {
3132
attachOnly: boolean;
3233
defaultProfile: string;
3334
profiles: Record<string, BrowserProfileConfig>;
35+
ssrfPolicy?: SsrFPolicy;
3436
extraArgs: string[];
3537
};
3638

@@ -61,6 +63,36 @@ function normalizeTimeoutMs(raw: number | undefined, fallback: number) {
6163
return value < 0 ? fallback : value;
6264
}
6365

66+
function normalizeStringList(raw: string[] | undefined): string[] | undefined {
67+
if (!Array.isArray(raw) || raw.length === 0) {
68+
return undefined;
69+
}
70+
const values = raw
71+
.map((value) => value.trim())
72+
.filter((value): value is string => value.length > 0);
73+
return values.length > 0 ? values : undefined;
74+
}
75+
76+
function resolveBrowserSsrFPolicy(cfg: BrowserConfig | undefined): SsrFPolicy | undefined {
77+
const allowPrivateNetwork = cfg?.ssrfPolicy?.allowPrivateNetwork;
78+
const allowedHostnames = normalizeStringList(cfg?.ssrfPolicy?.allowedHostnames);
79+
const hostnameAllowlist = normalizeStringList(cfg?.ssrfPolicy?.hostnameAllowlist);
80+
81+
if (
82+
allowPrivateNetwork === undefined &&
83+
allowedHostnames === undefined &&
84+
hostnameAllowlist === undefined
85+
) {
86+
return undefined;
87+
}
88+
89+
return {
90+
...(allowPrivateNetwork === true ? { allowPrivateNetwork: true } : {}),
91+
...(allowedHostnames ? { allowedHostnames } : {}),
92+
...(hostnameAllowlist ? { hostnameAllowlist } : {}),
93+
};
94+
}
95+
6496
export function parseHttpUrl(raw: string, label: string) {
6597
const trimmed = raw.trim();
6698
const parsed = new URL(trimmed);
@@ -200,6 +232,7 @@ export function resolveBrowserConfig(
200232
const extraArgs = Array.isArray(cfg?.extraArgs)
201233
? cfg.extraArgs.filter((a): a is string => typeof a === "string" && a.trim().length > 0)
202234
: [];
235+
const ssrfPolicy = resolveBrowserSsrFPolicy(cfg);
203236

204237
return {
205238
enabled,
@@ -217,6 +250,7 @@ export function resolveBrowserConfig(
217250
attachOnly,
218251
defaultProfile,
219252
profiles,
253+
ssrfPolicy,
220254
extraArgs,
221255
};
222256
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { describe, expect, it } from "vitest";
2+
import { SsrFBlockedError } from "../infra/net/ssrf.js";
3+
import { assertBrowserNavigationAllowed } from "./navigation-guard.js";
4+
5+
describe("browser navigation guard", () => {
6+
it("blocks private loopback URLs by default", async () => {
7+
await expect(
8+
assertBrowserNavigationAllowed({
9+
url: "http://127.0.0.1:8080",
10+
}),
11+
).rejects.toBeInstanceOf(SsrFBlockedError);
12+
});
13+
14+
it("allows non-network schemes", async () => {
15+
await expect(
16+
assertBrowserNavigationAllowed({
17+
url: "about:blank",
18+
}),
19+
).resolves.toBeUndefined();
20+
});
21+
22+
it("allows localhost when explicitly allowed", async () => {
23+
await expect(
24+
assertBrowserNavigationAllowed({
25+
url: "http://localhost:3000",
26+
ssrfPolicy: {
27+
allowedHostnames: ["localhost"],
28+
},
29+
}),
30+
).resolves.toBeUndefined();
31+
});
32+
33+
it("rejects invalid URLs", async () => {
34+
await expect(
35+
assertBrowserNavigationAllowed({
36+
url: "not a url",
37+
}),
38+
).rejects.toThrow(/Invalid URL/);
39+
});
40+
});

src/browser/navigation-guard.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { resolvePinnedHostnameWithPolicy, type SsrFPolicy } from "../infra/net/ssrf.js";
2+
3+
const NETWORK_NAVIGATION_PROTOCOLS = new Set(["http:", "https:"]);
4+
5+
export async function assertBrowserNavigationAllowed(opts: {
6+
url: string;
7+
ssrfPolicy?: SsrFPolicy;
8+
}): Promise<void> {
9+
const rawUrl = String(opts.url ?? "").trim();
10+
if (!rawUrl) {
11+
throw new Error("url is required");
12+
}
13+
14+
let parsed: URL;
15+
try {
16+
parsed = new URL(rawUrl);
17+
} catch {
18+
throw new Error(`Invalid URL: ${rawUrl}`);
19+
}
20+
21+
if (!NETWORK_NAVIGATION_PROTOCOLS.has(parsed.protocol)) {
22+
return;
23+
}
24+
25+
await resolvePinnedHostnameWithPolicy(parsed.hostname, {
26+
policy: opts.ssrfPolicy,
27+
});
28+
}

src/browser/pw-session.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ import type {
88
} from "playwright-core";
99
import { chromium } from "playwright-core";
1010
import { formatErrorMessage } from "../infra/errors.js";
11+
import type { SsrFPolicy } from "../infra/net/ssrf.js";
1112
import { appendCdpPath, fetchJson, getHeadersWithAuth, withCdpSocket } from "./cdp.helpers.js";
1213
import { normalizeCdpWsUrl } from "./cdp.js";
1314
import { getChromeWebSocketUrl } from "./chrome.js";
15+
import { assertBrowserNavigationAllowed } from "./navigation-guard.js";
1416

1517
export type BrowserConsoleMessage = {
1618
type: string;
@@ -716,7 +718,11 @@ export async function listPagesViaPlaywright(opts: { cdpUrl: string }): Promise<
716718
* Used for remote profiles where HTTP-based /json/new is ephemeral.
717719
* Returns the new page's targetId and metadata.
718720
*/
719-
export async function createPageViaPlaywright(opts: { cdpUrl: string; url: string }): Promise<{
721+
export async function createPageViaPlaywright(opts: {
722+
cdpUrl: string;
723+
url: string;
724+
ssrfPolicy?: SsrFPolicy;
725+
}): Promise<{
720726
targetId: string;
721727
title: string;
722728
url: string;
@@ -732,6 +738,10 @@ export async function createPageViaPlaywright(opts: { cdpUrl: string; url: strin
732738
// Navigate to the URL
733739
const targetUrl = opts.url.trim() || "about:blank";
734740
if (targetUrl !== "about:blank") {
741+
await assertBrowserNavigationAllowed({
742+
url: targetUrl,
743+
ssrfPolicy: opts.ssrfPolicy,
744+
});
735745
await page.goto(targetUrl, { timeout: 30_000 }).catch(() => {
736746
// Navigation might fail for some URLs, but page is still created
737747
});

src/browser/pw-tools-core.snapshot.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
import type { SsrFPolicy } from "../infra/net/ssrf.js";
12
import { type AriaSnapshotNode, formatAriaSnapshot, type RawAXNode } from "./cdp.js";
3+
import { assertBrowserNavigationAllowed } from "./navigation-guard.js";
24
import {
35
buildRoleSnapshotFromAiSnapshot,
46
buildRoleSnapshotFromAriaSnapshot,
@@ -158,11 +160,16 @@ export async function navigateViaPlaywright(opts: {
158160
targetId?: string;
159161
url: string;
160162
timeoutMs?: number;
163+
ssrfPolicy?: SsrFPolicy;
161164
}): Promise<{ url: string }> {
162165
const url = String(opts.url ?? "").trim();
163166
if (!url) {
164167
throw new Error("url is required");
165168
}
169+
await assertBrowserNavigationAllowed({
170+
url,
171+
ssrfPolicy: opts.ssrfPolicy,
172+
});
166173
const page = await getPageForTargetId(opts);
167174
ensurePageState(page);
168175
await page.goto(url, {

src/browser/routes/agent.snapshot.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,12 @@ export function registerBrowserAgentSnapshotRoutes(
6565
targetId,
6666
feature: "navigate",
6767
run: async ({ cdpUrl, tab, pw }) => {
68+
const ssrfPolicy = ctx.state().resolved.ssrfPolicy;
6869
const result = await pw.navigateViaPlaywright({
6970
cdpUrl,
7071
targetId: tab.targetId,
7172
url,
73+
...(ssrfPolicy ? { ssrfPolicy } : {}),
7274
});
7375
res.json({ ok: true, targetId: tab.targetId, ...result });
7476
},

0 commit comments

Comments
 (0)