refactor(browser): unify navigation guard path and error typing

src/browser/cdp.ts

@@ -1,6 +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";
+import { assertBrowserNavigationAllowed, withBrowserNavigationPolicy } from "./navigation-guard.js";
 
 export { appendCdpPath, fetchJson, fetchOk, getHeadersWithAuth } from "./cdp.helpers.js";
 
@@ -88,11 +88,14 @@ export async function createTargetViaCdp(opts: {
   cdpUrl: string;
   url: string;
   ssrfPolicy?: SsrFPolicy;
+  navigationChecked?: boolean;
 }): Promise<{ targetId: string }> {
-  await assertBrowserNavigationAllowed({
-    url: opts.url,
-    ssrfPolicy: opts.ssrfPolicy,
-  });
+  if (!opts.navigationChecked) {
+    await assertBrowserNavigationAllowed({
+      url: opts.url,
+      ...withBrowserNavigationPolicy(opts.ssrfPolicy),
+    });
+  }
 
   const version = await fetchJson<{ webSocketDebuggerUrl?: string }>(
     appendCdpPath(opts.cdpUrl, "/json/version"),

src/browser/navigation-guard.test.ts

@@ -1,6 +1,14 @@
-import { describe, expect, it } from "vitest";
-import { SsrFBlockedError } from "../infra/net/ssrf.js";
-import { assertBrowserNavigationAllowed } from "./navigation-guard.js";
+import { describe, expect, it, vi } from "vitest";
+import { SsrFBlockedError, type LookupFn } from "../infra/net/ssrf.js";
+import {
+  assertBrowserNavigationAllowed,
+  InvalidBrowserNavigationUrlError,
+} from "./navigation-guard.js";
+
+function createLookupFn(address: string): LookupFn {
+  const family = address.includes(":") ? 6 : 4;
+  return vi.fn(async () => [{ address, family }]) as unknown as LookupFn;
+}
 
 describe("browser navigation guard", () => {
   it("blocks private loopback URLs by default", async () => {
@@ -19,15 +27,39 @@ describe("browser navigation guard", () => {
     ).resolves.toBeUndefined();
   });
 
-  it("allows localhost when explicitly allowed", async () => {
+  it("allows blocked hostnames when explicitly allowed", async () => {
+    const lookupFn = createLookupFn("127.0.0.1");
     await expect(
       assertBrowserNavigationAllowed({
-        url: "http://localhost:3000",
+        url: "http://agent.internal:3000",
         ssrfPolicy: {
-          allowedHostnames: ["localhost"],
+          allowedHostnames: ["agent.internal"],
         },
+        lookupFn,
       }),
     ).resolves.toBeUndefined();
+    expect(lookupFn).toHaveBeenCalledWith("agent.internal", { all: true });
+  });
+
+  it("blocks hostnames that resolve to private addresses by default", async () => {
+    const lookupFn = createLookupFn("127.0.0.1");
+    await expect(
+      assertBrowserNavigationAllowed({
+        url: "https://example.com",
+        lookupFn,
+      }),
+    ).rejects.toBeInstanceOf(SsrFBlockedError);
+  });
+
+  it("allows hostnames that resolve to public addresses", async () => {
+    const lookupFn = createLookupFn("93.184.216.34");
+    await expect(
+      assertBrowserNavigationAllowed({
+        url: "https://example.com",
+        lookupFn,
+      }),
+    ).resolves.toBeUndefined();
+    expect(lookupFn).toHaveBeenCalledWith("example.com", { all: true });
   });
 
   it("rejects invalid URLs", async () => {
@@ -35,6 +67,6 @@ describe("browser navigation guard", () => {
       assertBrowserNavigationAllowed({
         url: "not a url",
       }),
-    ).rejects.toThrow(/Invalid URL/);
+    ).rejects.toBeInstanceOf(InvalidBrowserNavigationUrlError);
   });
 });

src/browser/navigation-guard.ts

@@ -1,21 +1,44 @@
-import { resolvePinnedHostnameWithPolicy, type SsrFPolicy } from "../infra/net/ssrf.js";
+import {
+  resolvePinnedHostnameWithPolicy,
+  type LookupFn,
+  type SsrFPolicy,
+} from "../infra/net/ssrf.js";
 
 const NETWORK_NAVIGATION_PROTOCOLS = new Set(["http:", "https:"]);
 
-export async function assertBrowserNavigationAllowed(opts: {
-  url: string;
+export class InvalidBrowserNavigationUrlError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = "InvalidBrowserNavigationUrlError";
+  }
+}
+
+export type BrowserNavigationPolicyOptions = {
   ssrfPolicy?: SsrFPolicy;
-}): Promise<void> {
+};
+
+export function withBrowserNavigationPolicy(
+  ssrfPolicy?: SsrFPolicy,
+): BrowserNavigationPolicyOptions {
+  return ssrfPolicy ? { ssrfPolicy } : {};
+}
+
+export async function assertBrowserNavigationAllowed(
+  opts: {
+    url: string;
+    lookupFn?: LookupFn;
+  } & BrowserNavigationPolicyOptions,
+): Promise<void> {
   const rawUrl = String(opts.url ?? "").trim();
   if (!rawUrl) {
-    throw new Error("url is required");
+    throw new InvalidBrowserNavigationUrlError("url is required");
   }
 
   let parsed: URL;
   try {
     parsed = new URL(rawUrl);
   } catch {
-    throw new Error(`Invalid URL: ${rawUrl}`);
+    throw new InvalidBrowserNavigationUrlError(`Invalid URL: ${rawUrl}`);
   }
 
   if (!NETWORK_NAVIGATION_PROTOCOLS.has(parsed.protocol)) {
@@ -23,6 +46,7 @@ export async function assertBrowserNavigationAllowed(opts: {
   }
 
   await resolvePinnedHostnameWithPolicy(parsed.hostname, {
+    lookupFn: opts.lookupFn,
     policy: opts.ssrfPolicy,
   });
 }

src/browser/pw-session.ts

@@ -12,7 +12,7 @@ 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";
+import { assertBrowserNavigationAllowed, withBrowserNavigationPolicy } from "./navigation-guard.js";
 
 export type BrowserConsoleMessage = {
   type: string;
@@ -722,6 +722,7 @@ export async function createPageViaPlaywright(opts: {
   cdpUrl: string;
   url: string;
   ssrfPolicy?: SsrFPolicy;
+  navigationChecked?: boolean;
 }): Promise<{
   targetId: string;
   title: string;
@@ -738,10 +739,12 @@ export async function createPageViaPlaywright(opts: {
   // Navigate to the URL
   const targetUrl = opts.url.trim() || "about:blank";
   if (targetUrl !== "about:blank") {
-    await assertBrowserNavigationAllowed({
-      url: targetUrl,
-      ssrfPolicy: opts.ssrfPolicy,
-    });
+    if (!opts.navigationChecked) {
+      await assertBrowserNavigationAllowed({
+        url: targetUrl,
+        ...withBrowserNavigationPolicy(opts.ssrfPolicy),
+      });
+    }
     await page.goto(targetUrl, { timeout: 30_000 }).catch(() => {
       // Navigation might fail for some URLs, but page is still created
     });

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

@@ -1,6 +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 { assertBrowserNavigationAllowed, withBrowserNavigationPolicy } from "./navigation-guard.js";
 import {
   buildRoleSnapshotFromAiSnapshot,
   buildRoleSnapshotFromAriaSnapshot,
@@ -168,7 +168,7 @@ export async function navigateViaPlaywright(opts: {
   }
   await assertBrowserNavigationAllowed({
     url,
-    ssrfPolicy: opts.ssrfPolicy,
+    ...withBrowserNavigationPolicy(opts.ssrfPolicy),
   });
   const page = await getPageForTargetId(opts);
   ensurePageState(page);

src/browser/routes/agent.snapshot.ts

@@ -6,6 +6,7 @@ import {
   DEFAULT_AI_SNAPSHOT_EFFICIENT_MAX_CHARS,
   DEFAULT_AI_SNAPSHOT_MAX_CHARS,
 } from "../constants.js";
+import { withBrowserNavigationPolicy } from "../navigation-guard.js";
 import {
   DEFAULT_BROWSER_SCREENSHOT_MAX_BYTES,
   DEFAULT_BROWSER_SCREENSHOT_MAX_SIDE,
@@ -65,12 +66,11 @@ 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 } : {}),
+          ...withBrowserNavigationPolicy(ctx.state().resolved.ssrfPolicy),
         });
         res.json({ ok: true, targetId: tab.targetId, ...result });
       },

src/browser/routes/tabs.ts

@@ -121,6 +121,7 @@ export function registerBrowserTabRoutes(app: BrowserRouteRegistrar, ctx: Browse
       req,
       res,
       ctx,
+      mapTabError: true,
       run: async (profileCtx) => {
         await profileCtx.ensureBrowserAvailable();
         const tab = await profileCtx.openTab(url);

src/browser/server-context.remote-tab-ops.test.ts

@@ -90,6 +90,12 @@ describe("browser server-context remote profile tab operations", () => {
     const opened = await remote.openTab("http://127.0.0.1:3000");
     expect(opened.targetId).toBe("T2");
     expect(state.profiles.get("remote")?.lastTargetId).toBe("T2");
+    expect(createPageViaPlaywright).toHaveBeenCalledWith({
+      cdpUrl: "https://browserless.example/chrome?token=abc",
+      url: "http://127.0.0.1:3000",
+      ssrfPolicy: { allowPrivateNetwork: true },
+      navigationChecked: true,
+    });
 
     await remote.closeTab("T1");
     expect(closePageByTargetIdViaPlaywright).toHaveBeenCalledWith({
@@ -214,7 +220,9 @@ describe("browser server-context remote profile tab operations", () => {
 
 describe("browser server-context tab selection state", () => {
   it("updates lastTargetId when openTab is created via CDP", async () => {
-    vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "CREATED" });
+    const createTargetViaCdp = vi
+      .spyOn(cdpModule, "createTargetViaCdp")
+      .mockResolvedValue({ targetId: "CREATED" });
 
     const fetchMock = vi.fn(async (url: unknown) => {
       const u = String(url);
@@ -244,5 +252,11 @@ describe("browser server-context tab selection state", () => {
     const opened = await openclaw.openTab("http://127.0.0.1:8080");
     expect(opened.targetId).toBe("CREATED");
     expect(state.profiles.get("openclaw")?.lastTargetId).toBe("CREATED");
+    expect(createTargetViaCdp).toHaveBeenCalledWith({
+      cdpUrl: "http://127.0.0.1:18800",
+      url: "http://127.0.0.1:8080",
+      ssrfPolicy: { allowPrivateNetwork: true },
+      navigationChecked: true,
+    });
   });
 });

src/browser/server-context.ts

@@ -15,7 +15,11 @@ import {
   ensureChromeExtensionRelayServer,
   stopChromeExtensionRelayServer,
 } from "./extension-relay.js";
-import { assertBrowserNavigationAllowed } from "./navigation-guard.js";
+import {
+  assertBrowserNavigationAllowed,
+  InvalidBrowserNavigationUrlError,
+  withBrowserNavigationPolicy,
+} from "./navigation-guard.js";
 import type { PwAiModule } from "./pw-ai-module.js";
 import { getPwAiModule } from "./pw-ai-module.js";
 import {
@@ -132,8 +136,8 @@ function createProfileContext(
   };
 
   const openTab = async (url: string): Promise<BrowserTab> => {
-    const ssrfPolicy = state().resolved.ssrfPolicy;
-    await assertBrowserNavigationAllowed({ url, ssrfPolicy });
+    const ssrfPolicyOpts = withBrowserNavigationPolicy(state().resolved.ssrfPolicy);
+    await assertBrowserNavigationAllowed({ url, ...ssrfPolicyOpts });
 
     // For remote profiles, use Playwright's persistent connection to create tabs
     // This ensures the tab persists beyond a single request
@@ -144,7 +148,8 @@ function createProfileContext(
         const page = await createPageViaPlaywright({
           cdpUrl: profile.cdpUrl,
           url,
-          ...(ssrfPolicy ? { ssrfPolicy } : {}),
+          ...ssrfPolicyOpts,
+          navigationChecked: true,
         });
         const profileState = getProfileState();
         profileState.lastTargetId = page.targetId;
@@ -160,7 +165,8 @@ function createProfileContext(
     const createdViaCdp = await createTargetViaCdp({
       cdpUrl: profile.cdpUrl,
       url,
-      ...(ssrfPolicy ? { ssrfPolicy } : {}),
+      ...ssrfPolicyOpts,
+      navigationChecked: true,
     })
       .then((r) => r.targetId)
       .catch(() => null);
@@ -645,10 +651,10 @@ export function createBrowserRouteContext(opts: ContextOptions): BrowserRouteCon
     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 (err instanceof InvalidBrowserNavigationUrlError) {
+      return { status: 400, message: err.message };
     }
+    const msg = String(err);
     if (msg.includes("ambiguous target id prefix")) {
       return { status: 409, message: "ambiguous target id prefix" };
     }

src/browser/server.post-tabs-open-profile-unknown-returns-404.test.ts

@@ -32,6 +32,20 @@ describe("browser control server", () => {
     const body = (await result.json()) as { error: string };
     expect(body.error).toContain("not found");
   });
+
+  it("POST /tabs/open returns 400 for invalid URLs", async () => {
+    await startBrowserControlServerFromConfig();
+    const base = getBrowserControlServerBaseUrl();
+
+    const result = await realFetch(`${base}/tabs/open`, {
+      method: "POST",
+      headers: { "Content-Type": "application/json" },
+      body: JSON.stringify({ url: "not a url" }),
+    });
+    expect(result.status).toBe(400);
+    const body = (await result.json()) as { error: string };
+    expect(body.error).toContain("Invalid URL:");
+  });
 });
 
 describe("profile CRUD endpoints", () => {