mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-25 03:04:29 -04:00
fix(browser): unify SSRF guard path for navigation
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { createServer } from "node:http";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { type WebSocket, WebSocketServer } from "ws";
|
||||
import { SsrFBlockedError } from "../infra/net/ssrf.js";
|
||||
import { rawDataToString } from "../infra/ws.js";
|
||||
import { createTargetViaCdp, evaluateJavaScript, normalizeCdpWsUrl, snapshotAria } from "./cdp.js";
|
||||
|
||||
@@ -92,6 +93,61 @@ describe("cdp", () => {
|
||||
expect(created.targetId).toBe("TARGET_123");
|
||||
});
|
||||
|
||||
it("blocks private navigation targets by default", async () => {
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
try {
|
||||
await expect(
|
||||
createTargetViaCdp({
|
||||
cdpUrl: "http://127.0.0.1:9222",
|
||||
url: "http://127.0.0.1:8080",
|
||||
}),
|
||||
).rejects.toBeInstanceOf(SsrFBlockedError);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
fetchSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("allows private navigation targets when explicitly configured", async () => {
|
||||
const wsPort = await startWsServerWithMessages((msg, socket) => {
|
||||
if (msg.method !== "Target.createTarget") {
|
||||
return;
|
||||
}
|
||||
expect(msg.params?.url).toBe("http://127.0.0.1:8080");
|
||||
socket.send(
|
||||
JSON.stringify({
|
||||
id: msg.id,
|
||||
result: { targetId: "TARGET_LOCAL" },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
httpServer = createServer((req, res) => {
|
||||
if (req.url === "/json/version") {
|
||||
res.setHeader("content-type", "application/json");
|
||||
res.end(
|
||||
JSON.stringify({
|
||||
webSocketDebuggerUrl: `ws://127.0.0.1:${wsPort}/devtools/browser/TEST`,
|
||||
}),
|
||||
);
|
||||
return;
|
||||
}
|
||||
res.statusCode = 404;
|
||||
res.end("not found");
|
||||
});
|
||||
|
||||
await new Promise<void>((resolve) => httpServer?.listen(0, "127.0.0.1", resolve));
|
||||
const httpPort = (httpServer.address() as { port: number }).port;
|
||||
|
||||
const created = await createTargetViaCdp({
|
||||
cdpUrl: `http://127.0.0.1:${httpPort}`,
|
||||
url: "http://127.0.0.1:8080",
|
||||
ssrfPolicy: { allowPrivateNetwork: true },
|
||||
});
|
||||
|
||||
expect(created.targetId).toBe("TARGET_LOCAL");
|
||||
});
|
||||
|
||||
it("evaluates javascript via CDP", async () => {
|
||||
const wsPort = await startWsServerWithMessages((msg, socket) => {
|
||||
if (msg.method === "Runtime.enable") {
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import type { SsrFPolicy } from "../infra/net/ssrf.js";
|
||||
import { appendCdpPath, fetchJson, isLoopbackHost, withCdpSocket } from "./cdp.helpers.js";
|
||||
import { assertBrowserNavigationAllowed } from "./navigation-guard.js";
|
||||
|
||||
export { appendCdpPath, fetchJson, fetchOk, getHeadersWithAuth } from "./cdp.helpers.js";
|
||||
|
||||
@@ -85,7 +87,13 @@ export async function captureScreenshot(opts: {
|
||||
export async function createTargetViaCdp(opts: {
|
||||
cdpUrl: string;
|
||||
url: string;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
}): Promise<{ targetId: string }> {
|
||||
await assertBrowserNavigationAllowed({
|
||||
url: opts.url,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
});
|
||||
|
||||
const version = await fetchJson<{ webSocketDebuggerUrl?: string }>(
|
||||
appendCdpPath(opts.cdpUrl, "/json/version"),
|
||||
1500,
|
||||
|
||||
@@ -182,4 +182,24 @@ describe("browser config", () => {
|
||||
});
|
||||
expect(resolved.extraArgs).toEqual([]);
|
||||
});
|
||||
|
||||
it("resolves browser SSRF policy when configured", () => {
|
||||
const resolved = resolveBrowserConfig({
|
||||
ssrfPolicy: {
|
||||
allowPrivateNetwork: true,
|
||||
allowedHostnames: [" localhost ", ""],
|
||||
hostnameAllowlist: [" *.trusted.example ", " "],
|
||||
},
|
||||
});
|
||||
expect(resolved.ssrfPolicy).toEqual({
|
||||
allowPrivateNetwork: true,
|
||||
allowedHostnames: ["localhost"],
|
||||
hostnameAllowlist: ["*.trusted.example"],
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps browser SSRF policy undefined when not configured", () => {
|
||||
const resolved = resolveBrowserConfig({});
|
||||
expect(resolved.ssrfPolicy).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
DEFAULT_BROWSER_CONTROL_PORT,
|
||||
} from "../config/port-defaults.js";
|
||||
import { isLoopbackHost } from "../gateway/net.js";
|
||||
import type { SsrFPolicy } from "../infra/net/ssrf.js";
|
||||
import {
|
||||
DEFAULT_OPENCLAW_BROWSER_COLOR,
|
||||
DEFAULT_OPENCLAW_BROWSER_ENABLED,
|
||||
@@ -31,6 +32,7 @@ export type ResolvedBrowserConfig = {
|
||||
attachOnly: boolean;
|
||||
defaultProfile: string;
|
||||
profiles: Record<string, BrowserProfileConfig>;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
extraArgs: string[];
|
||||
};
|
||||
|
||||
@@ -61,6 +63,36 @@ function normalizeTimeoutMs(raw: number | undefined, fallback: number) {
|
||||
return value < 0 ? fallback : value;
|
||||
}
|
||||
|
||||
function normalizeStringList(raw: string[] | undefined): string[] | undefined {
|
||||
if (!Array.isArray(raw) || raw.length === 0) {
|
||||
return undefined;
|
||||
}
|
||||
const values = raw
|
||||
.map((value) => value.trim())
|
||||
.filter((value): value is string => value.length > 0);
|
||||
return values.length > 0 ? values : undefined;
|
||||
}
|
||||
|
||||
function resolveBrowserSsrFPolicy(cfg: BrowserConfig | undefined): SsrFPolicy | undefined {
|
||||
const allowPrivateNetwork = cfg?.ssrfPolicy?.allowPrivateNetwork;
|
||||
const allowedHostnames = normalizeStringList(cfg?.ssrfPolicy?.allowedHostnames);
|
||||
const hostnameAllowlist = normalizeStringList(cfg?.ssrfPolicy?.hostnameAllowlist);
|
||||
|
||||
if (
|
||||
allowPrivateNetwork === undefined &&
|
||||
allowedHostnames === undefined &&
|
||||
hostnameAllowlist === undefined
|
||||
) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return {
|
||||
...(allowPrivateNetwork === true ? { allowPrivateNetwork: true } : {}),
|
||||
...(allowedHostnames ? { allowedHostnames } : {}),
|
||||
...(hostnameAllowlist ? { hostnameAllowlist } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
export function parseHttpUrl(raw: string, label: string) {
|
||||
const trimmed = raw.trim();
|
||||
const parsed = new URL(trimmed);
|
||||
@@ -200,6 +232,7 @@ export function resolveBrowserConfig(
|
||||
const extraArgs = Array.isArray(cfg?.extraArgs)
|
||||
? cfg.extraArgs.filter((a): a is string => typeof a === "string" && a.trim().length > 0)
|
||||
: [];
|
||||
const ssrfPolicy = resolveBrowserSsrFPolicy(cfg);
|
||||
|
||||
return {
|
||||
enabled,
|
||||
@@ -217,6 +250,7 @@ export function resolveBrowserConfig(
|
||||
attachOnly,
|
||||
defaultProfile,
|
||||
profiles,
|
||||
ssrfPolicy,
|
||||
extraArgs,
|
||||
};
|
||||
}
|
||||
|
||||
40
src/browser/navigation-guard.test.ts
Normal file
40
src/browser/navigation-guard.test.ts
Normal file
@@ -0,0 +1,40 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { SsrFBlockedError } from "../infra/net/ssrf.js";
|
||||
import { assertBrowserNavigationAllowed } from "./navigation-guard.js";
|
||||
|
||||
describe("browser navigation guard", () => {
|
||||
it("blocks private loopback URLs by default", async () => {
|
||||
await expect(
|
||||
assertBrowserNavigationAllowed({
|
||||
url: "http://127.0.0.1:8080",
|
||||
}),
|
||||
).rejects.toBeInstanceOf(SsrFBlockedError);
|
||||
});
|
||||
|
||||
it("allows non-network schemes", async () => {
|
||||
await expect(
|
||||
assertBrowserNavigationAllowed({
|
||||
url: "about:blank",
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("allows localhost when explicitly allowed", async () => {
|
||||
await expect(
|
||||
assertBrowserNavigationAllowed({
|
||||
url: "http://localhost:3000",
|
||||
ssrfPolicy: {
|
||||
allowedHostnames: ["localhost"],
|
||||
},
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("rejects invalid URLs", async () => {
|
||||
await expect(
|
||||
assertBrowserNavigationAllowed({
|
||||
url: "not a url",
|
||||
}),
|
||||
).rejects.toThrow(/Invalid URL/);
|
||||
});
|
||||
});
|
||||
28
src/browser/navigation-guard.ts
Normal file
28
src/browser/navigation-guard.ts
Normal file
@@ -0,0 +1,28 @@
|
||||
import { resolvePinnedHostnameWithPolicy, type SsrFPolicy } from "../infra/net/ssrf.js";
|
||||
|
||||
const NETWORK_NAVIGATION_PROTOCOLS = new Set(["http:", "https:"]);
|
||||
|
||||
export async function assertBrowserNavigationAllowed(opts: {
|
||||
url: string;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
}): Promise<void> {
|
||||
const rawUrl = String(opts.url ?? "").trim();
|
||||
if (!rawUrl) {
|
||||
throw new Error("url is required");
|
||||
}
|
||||
|
||||
let parsed: URL;
|
||||
try {
|
||||
parsed = new URL(rawUrl);
|
||||
} catch {
|
||||
throw new Error(`Invalid URL: ${rawUrl}`);
|
||||
}
|
||||
|
||||
if (!NETWORK_NAVIGATION_PROTOCOLS.has(parsed.protocol)) {
|
||||
return;
|
||||
}
|
||||
|
||||
await resolvePinnedHostnameWithPolicy(parsed.hostname, {
|
||||
policy: opts.ssrfPolicy,
|
||||
});
|
||||
}
|
||||
@@ -8,9 +8,11 @@ import type {
|
||||
} from "playwright-core";
|
||||
import { chromium } from "playwright-core";
|
||||
import { formatErrorMessage } from "../infra/errors.js";
|
||||
import type { SsrFPolicy } from "../infra/net/ssrf.js";
|
||||
import { appendCdpPath, fetchJson, getHeadersWithAuth, withCdpSocket } from "./cdp.helpers.js";
|
||||
import { normalizeCdpWsUrl } from "./cdp.js";
|
||||
import { getChromeWebSocketUrl } from "./chrome.js";
|
||||
import { assertBrowserNavigationAllowed } from "./navigation-guard.js";
|
||||
|
||||
export type BrowserConsoleMessage = {
|
||||
type: string;
|
||||
@@ -716,7 +718,11 @@ export async function listPagesViaPlaywright(opts: { cdpUrl: string }): Promise<
|
||||
* Used for remote profiles where HTTP-based /json/new is ephemeral.
|
||||
* Returns the new page's targetId and metadata.
|
||||
*/
|
||||
export async function createPageViaPlaywright(opts: { cdpUrl: string; url: string }): Promise<{
|
||||
export async function createPageViaPlaywright(opts: {
|
||||
cdpUrl: string;
|
||||
url: string;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
}): Promise<{
|
||||
targetId: string;
|
||||
title: string;
|
||||
url: string;
|
||||
@@ -732,6 +738,10 @@ export async function createPageViaPlaywright(opts: { cdpUrl: string; url: strin
|
||||
// Navigate to the URL
|
||||
const targetUrl = opts.url.trim() || "about:blank";
|
||||
if (targetUrl !== "about:blank") {
|
||||
await assertBrowserNavigationAllowed({
|
||||
url: targetUrl,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
});
|
||||
await page.goto(targetUrl, { timeout: 30_000 }).catch(() => {
|
||||
// Navigation might fail for some URLs, but page is still created
|
||||
});
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import type { SsrFPolicy } from "../infra/net/ssrf.js";
|
||||
import { type AriaSnapshotNode, formatAriaSnapshot, type RawAXNode } from "./cdp.js";
|
||||
import { assertBrowserNavigationAllowed } from "./navigation-guard.js";
|
||||
import {
|
||||
buildRoleSnapshotFromAiSnapshot,
|
||||
buildRoleSnapshotFromAriaSnapshot,
|
||||
@@ -158,11 +160,16 @@ export async function navigateViaPlaywright(opts: {
|
||||
targetId?: string;
|
||||
url: string;
|
||||
timeoutMs?: number;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
}): Promise<{ url: string }> {
|
||||
const url = String(opts.url ?? "").trim();
|
||||
if (!url) {
|
||||
throw new Error("url is required");
|
||||
}
|
||||
await assertBrowserNavigationAllowed({
|
||||
url,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
});
|
||||
const page = await getPageForTargetId(opts);
|
||||
ensurePageState(page);
|
||||
await page.goto(url, {
|
||||
|
||||
@@ -65,10 +65,12 @@ export function registerBrowserAgentSnapshotRoutes(
|
||||
targetId,
|
||||
feature: "navigate",
|
||||
run: async ({ cdpUrl, tab, pw }) => {
|
||||
const ssrfPolicy = ctx.state().resolved.ssrfPolicy;
|
||||
const result = await pw.navigateViaPlaywright({
|
||||
cdpUrl,
|
||||
targetId: tab.targetId,
|
||||
url,
|
||||
...(ssrfPolicy ? { ssrfPolicy } : {}),
|
||||
});
|
||||
res.json({ ok: true, targetId: tab.targetId, ...result });
|
||||
},
|
||||
|
||||
@@ -34,6 +34,7 @@ function makeState(
|
||||
headless: true,
|
||||
noSandbox: false,
|
||||
attachOnly: false,
|
||||
ssrfPolicy: { allowPrivateNetwork: true },
|
||||
defaultProfile: profile,
|
||||
profiles: {
|
||||
remote: {
|
||||
@@ -65,12 +66,12 @@ function createRemoteRouteHarness(fetchMock?: ReturnType<typeof vi.fn>) {
|
||||
describe("browser server-context remote profile tab operations", () => {
|
||||
it("uses Playwright tab operations when available", async () => {
|
||||
const listPagesViaPlaywright = vi.fn(async () => [
|
||||
{ targetId: "T1", title: "Tab 1", url: "https://a.example", type: "page" },
|
||||
{ targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" },
|
||||
]);
|
||||
const createPageViaPlaywright = vi.fn(async () => ({
|
||||
targetId: "T2",
|
||||
title: "Tab 2",
|
||||
url: "https://b.example",
|
||||
url: "http://127.0.0.1:3000",
|
||||
type: "page",
|
||||
}));
|
||||
const closePageByTargetIdViaPlaywright = vi.fn(async () => {});
|
||||
@@ -86,7 +87,7 @@ describe("browser server-context remote profile tab operations", () => {
|
||||
const tabs = await remote.listTabs();
|
||||
expect(tabs.map((t) => t.targetId)).toEqual(["T1"]);
|
||||
|
||||
const opened = await remote.openTab("https://b.example");
|
||||
const opened = await remote.openTab("http://127.0.0.1:3000");
|
||||
expect(opened.targetId).toBe("T2");
|
||||
expect(state.profiles.get("remote")?.lastTargetId).toBe("T2");
|
||||
|
||||
@@ -102,21 +103,21 @@ describe("browser server-context remote profile tab operations", () => {
|
||||
const responses = [
|
||||
// ensureTabAvailable() calls listTabs twice
|
||||
[
|
||||
{ targetId: "A", title: "A", url: "https://a.example", type: "page" },
|
||||
{ targetId: "B", title: "B", url: "https://b.example", type: "page" },
|
||||
{ targetId: "A", title: "A", url: "https://example.com", type: "page" },
|
||||
{ targetId: "B", title: "B", url: "https://www.example.com", type: "page" },
|
||||
],
|
||||
[
|
||||
{ targetId: "A", title: "A", url: "https://a.example", type: "page" },
|
||||
{ targetId: "B", title: "B", url: "https://b.example", type: "page" },
|
||||
{ targetId: "A", title: "A", url: "https://example.com", type: "page" },
|
||||
{ targetId: "B", title: "B", url: "https://www.example.com", type: "page" },
|
||||
],
|
||||
// second ensureTabAvailable() calls listTabs twice, order flips
|
||||
[
|
||||
{ targetId: "B", title: "B", url: "https://b.example", type: "page" },
|
||||
{ targetId: "A", title: "A", url: "https://a.example", type: "page" },
|
||||
{ targetId: "B", title: "B", url: "https://www.example.com", type: "page" },
|
||||
{ targetId: "A", title: "A", url: "https://example.com", type: "page" },
|
||||
],
|
||||
[
|
||||
{ targetId: "B", title: "B", url: "https://b.example", type: "page" },
|
||||
{ targetId: "A", title: "A", url: "https://a.example", type: "page" },
|
||||
{ targetId: "B", title: "B", url: "https://www.example.com", type: "page" },
|
||||
{ targetId: "A", title: "A", url: "https://example.com", type: "page" },
|
||||
],
|
||||
];
|
||||
|
||||
@@ -148,7 +149,7 @@ describe("browser server-context remote profile tab operations", () => {
|
||||
|
||||
it("uses Playwright focus for remote profiles when available", async () => {
|
||||
const listPagesViaPlaywright = vi.fn(async () => [
|
||||
{ targetId: "T1", title: "Tab 1", url: "https://a.example", type: "page" },
|
||||
{ targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" },
|
||||
]);
|
||||
const focusPageByTargetIdViaPlaywright = vi.fn(async () => {});
|
||||
|
||||
@@ -195,7 +196,7 @@ describe("browser server-context remote profile tab operations", () => {
|
||||
{
|
||||
id: "T1",
|
||||
title: "Tab 1",
|
||||
url: "https://a.example",
|
||||
url: "https://example.com",
|
||||
webSocketDebuggerUrl: "wss://browserless.example/devtools/page/T1",
|
||||
type: "page",
|
||||
},
|
||||
@@ -226,7 +227,7 @@ describe("browser server-context tab selection state", () => {
|
||||
{
|
||||
id: "CREATED",
|
||||
title: "New Tab",
|
||||
url: "https://created.example",
|
||||
url: "http://127.0.0.1:8080",
|
||||
webSocketDebuggerUrl: "ws://127.0.0.1/devtools/page/CREATED",
|
||||
type: "page",
|
||||
},
|
||||
@@ -240,7 +241,7 @@ describe("browser server-context tab selection state", () => {
|
||||
const ctx = createBrowserRouteContext({ getState: () => state });
|
||||
const openclaw = ctx.forProfile("openclaw");
|
||||
|
||||
const opened = await openclaw.openTab("https://created.example");
|
||||
const opened = await openclaw.openTab("http://127.0.0.1:8080");
|
||||
expect(opened.targetId).toBe("CREATED");
|
||||
expect(state.profiles.get("openclaw")?.lastTargetId).toBe("CREATED");
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import fs from "node:fs";
|
||||
import { SsrFBlockedError } from "../infra/net/ssrf.js";
|
||||
import { fetchJson, fetchOk } from "./cdp.helpers.js";
|
||||
import { appendCdpPath, createTargetViaCdp, normalizeCdpWsUrl } from "./cdp.js";
|
||||
import {
|
||||
@@ -14,6 +15,7 @@ import {
|
||||
ensureChromeExtensionRelayServer,
|
||||
stopChromeExtensionRelayServer,
|
||||
} from "./extension-relay.js";
|
||||
import { assertBrowserNavigationAllowed } from "./navigation-guard.js";
|
||||
import type { PwAiModule } from "./pw-ai-module.js";
|
||||
import { getPwAiModule } from "./pw-ai-module.js";
|
||||
import {
|
||||
@@ -130,13 +132,20 @@ function createProfileContext(
|
||||
};
|
||||
|
||||
const openTab = async (url: string): Promise<BrowserTab> => {
|
||||
const ssrfPolicy = state().resolved.ssrfPolicy;
|
||||
await assertBrowserNavigationAllowed({ url, ssrfPolicy });
|
||||
|
||||
// For remote profiles, use Playwright's persistent connection to create tabs
|
||||
// This ensures the tab persists beyond a single request
|
||||
if (!profile.cdpIsLoopback) {
|
||||
const mod = await getPwAiModule({ mode: "strict" });
|
||||
const createPageViaPlaywright = (mod as Partial<PwAiModule> | null)?.createPageViaPlaywright;
|
||||
if (typeof createPageViaPlaywright === "function") {
|
||||
const page = await createPageViaPlaywright({ cdpUrl: profile.cdpUrl, url });
|
||||
const page = await createPageViaPlaywright({
|
||||
cdpUrl: profile.cdpUrl,
|
||||
url,
|
||||
...(ssrfPolicy ? { ssrfPolicy } : {}),
|
||||
});
|
||||
const profileState = getProfileState();
|
||||
profileState.lastTargetId = page.targetId;
|
||||
return {
|
||||
@@ -151,6 +160,7 @@ function createProfileContext(
|
||||
const createdViaCdp = await createTargetViaCdp({
|
||||
cdpUrl: profile.cdpUrl,
|
||||
url,
|
||||
...(ssrfPolicy ? { ssrfPolicy } : {}),
|
||||
})
|
||||
.then((r) => r.targetId)
|
||||
.catch(() => null);
|
||||
@@ -632,7 +642,13 @@ export function createBrowserRouteContext(opts: ContextOptions): BrowserRouteCon
|
||||
const getDefaultContext = () => forProfile();
|
||||
|
||||
const mapTabError = (err: unknown) => {
|
||||
if (err instanceof SsrFBlockedError) {
|
||||
return { status: 400, message: err.message };
|
||||
}
|
||||
const msg = String(err);
|
||||
if (msg.includes("Invalid URL:")) {
|
||||
return { status: 400, message: msg };
|
||||
}
|
||||
if (msg.includes("ambiguous target id prefix")) {
|
||||
return { status: 409, message: "ambiguous target id prefix" };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user