refactor(browser): centralize navigation guard enforcement

src/browser/cdp.test.ts

@@ -4,6 +4,7 @@ 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";
+import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js";
 
 describe("cdp", () => {
   let httpServer: ReturnType<typeof createServer> | null = null;
@@ -109,6 +110,21 @@ describe("cdp", () => {
     }
   });
 
+  it("blocks unsupported non-network navigation URLs", async () => {
+    const fetchSpy = vi.spyOn(globalThis, "fetch");
+    try {
+      await expect(
+        createTargetViaCdp({
+          cdpUrl: "http://127.0.0.1:9222",
+          url: "file:///etc/passwd",
+        }),
+      ).rejects.toBeInstanceOf(InvalidBrowserNavigationUrlError);
+      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") {

src/browser/cdp.ts

@@ -88,14 +88,11 @@ export async function createTargetViaCdp(opts: {
   cdpUrl: string;
   url: string;
   ssrfPolicy?: SsrFPolicy;
-  navigationChecked?: boolean;
 }): Promise<{ targetId: string }> {
-  if (!opts.navigationChecked) {
+  await assertBrowserNavigationAllowed({
-    await assertBrowserNavigationAllowed({
+    url: opts.url,
-      url: opts.url,
+    ...withBrowserNavigationPolicy(opts.ssrfPolicy),
-      ...withBrowserNavigationPolicy(opts.ssrfPolicy),
+  });
-    });
-  }
 
   const version = await fetchJson<{ webSocketDebuggerUrl?: string }>(
     appendCdpPath(opts.cdpUrl, "/json/version"),

src/browser/navigation-guard.ts

@@ -7,6 +7,11 @@ import {
 const NETWORK_NAVIGATION_PROTOCOLS = new Set(["http:", "https:"]);
 const SAFE_NON_NETWORK_URLS = new Set(["about:blank"]);
 
+function isAllowedNonNetworkNavigationUrl(parsed: URL): boolean {
+  // Keep non-network navigation explicit; about:blank is the only allowed bootstrap URL.
+  return SAFE_NON_NETWORK_URLS.has(parsed.href);
+}
+
 export class InvalidBrowserNavigationUrlError extends Error {
   constructor(message: string) {
     super(message);
@@ -43,7 +48,7 @@ export async function assertBrowserNavigationAllowed(
   }
 
   if (!NETWORK_NAVIGATION_PROTOCOLS.has(parsed.protocol)) {
-    if (SAFE_NON_NETWORK_URLS.has(parsed.href)) {
+    if (isAllowedNonNetworkNavigationUrl(parsed)) {
       return;
     }
     throw new InvalidBrowserNavigationUrlError(

src/browser/pw-session.create-page.navigation-guard.test.ts

@@ -0,0 +1,95 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js";
+import { closePlaywrightBrowserConnection, createPageViaPlaywright } from "./pw-session.js";
+
+const connectOverCdpMock = vi.fn();
+const getChromeWebSocketUrlMock = vi.fn();
+
+vi.mock("playwright-core", () => ({
+  chromium: {
+    connectOverCDP: (...args: unknown[]) => connectOverCdpMock(...args),
+  },
+}));
+
+vi.mock("./chrome.js", () => ({
+  getChromeWebSocketUrl: (...args: unknown[]) => getChromeWebSocketUrlMock(...args),
+}));
+
+function installBrowserMocks() {
+  const pageOn = vi.fn();
+  const pageGoto = vi.fn(async () => {});
+  const pageTitle = vi.fn(async () => "");
+  const pageUrl = vi.fn(() => "about:blank");
+  const contextOn = vi.fn();
+  const browserOn = vi.fn();
+  const browserClose = vi.fn(async () => {});
+  const sessionSend = vi.fn(async (method: string) => {
+    if (method === "Target.getTargetInfo") {
+      return { targetInfo: { targetId: "TARGET_1" } };
+    }
+    return {};
+  });
+  const sessionDetach = vi.fn(async () => {});
+
+  const context = {
+    pages: () => [],
+    on: contextOn,
+    newPage: vi.fn(async () => page),
+    newCDPSession: vi.fn(async () => ({
+      send: sessionSend,
+      detach: sessionDetach,
+    })),
+  } as unknown as import("playwright-core").BrowserContext;
+
+  const page = {
+    on: pageOn,
+    context: () => context,
+    goto: pageGoto,
+    title: pageTitle,
+    url: pageUrl,
+  } as unknown as import("playwright-core").Page;
+
+  const browser = {
+    contexts: () => [context],
+    on: browserOn,
+    close: browserClose,
+  } as unknown as import("playwright-core").Browser;
+
+  connectOverCdpMock.mockResolvedValue(browser);
+  getChromeWebSocketUrlMock.mockResolvedValue(null);
+
+  return { pageGoto, browserClose };
+}
+
+afterEach(async () => {
+  connectOverCdpMock.mockReset();
+  getChromeWebSocketUrlMock.mockReset();
+  await closePlaywrightBrowserConnection().catch(() => {});
+});
+
+describe("pw-session createPageViaPlaywright navigation guard", () => {
+  it("blocks unsupported non-network URLs", async () => {
+    const { pageGoto } = installBrowserMocks();
+
+    await expect(
+      createPageViaPlaywright({
+        cdpUrl: "http://127.0.0.1:18792",
+        url: "file:///etc/passwd",
+      }),
+    ).rejects.toBeInstanceOf(InvalidBrowserNavigationUrlError);
+
+    expect(pageGoto).not.toHaveBeenCalled();
+  });
+
+  it("allows about:blank without network navigation", async () => {
+    const { pageGoto } = installBrowserMocks();
+
+    const created = await createPageViaPlaywright({
+      cdpUrl: "http://127.0.0.1:18792",
+      url: "about:blank",
+    });
+
+    expect(created.targetId).toBe("TARGET_1");
+    expect(pageGoto).not.toHaveBeenCalled();
+  });
+});

src/browser/pw-session.ts

@@ -7,8 +7,8 @@ import type {
   Response,
 } from "playwright-core";
 import { chromium } from "playwright-core";
-import { formatErrorMessage } from "../infra/errors.js";
 import type { SsrFPolicy } from "../infra/net/ssrf.js";
+import { formatErrorMessage } from "../infra/errors.js";
 import { appendCdpPath, fetchJson, getHeadersWithAuth, withCdpSocket } from "./cdp.helpers.js";
 import { normalizeCdpWsUrl } from "./cdp.js";
 import { getChromeWebSocketUrl } from "./chrome.js";
@@ -722,7 +722,6 @@ export async function createPageViaPlaywright(opts: {
   cdpUrl: string;
   url: string;
   ssrfPolicy?: SsrFPolicy;
-  navigationChecked?: boolean;
 }): Promise<{
   targetId: string;
   title: string;
@@ -739,12 +738,10 @@ export async function createPageViaPlaywright(opts: {
   // Navigate to the URL
   const targetUrl = opts.url.trim() || "about:blank";
   if (targetUrl !== "about:blank") {
-    if (!opts.navigationChecked) {
+    await assertBrowserNavigationAllowed({
-      await assertBrowserNavigationAllowed({
+      url: targetUrl,
-        url: targetUrl,
+      ...withBrowserNavigationPolicy(opts.ssrfPolicy),
-        ...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.navigate-guard.test.ts

@@ -0,0 +1,47 @@
+import { describe, expect, it, vi } from "vitest";
+import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js";
+import {
+  getPwToolsCoreSessionMocks,
+  installPwToolsCoreTestHooks,
+  setPwToolsCoreCurrentPage,
+} from "./pw-tools-core.test-harness.js";
+
+installPwToolsCoreTestHooks();
+const mod = await import("./pw-tools-core.snapshot.js");
+
+describe("pw-tools-core.snapshot navigate guard", () => {
+  it("blocks unsupported non-network URLs before page lookup", async () => {
+    const goto = vi.fn(async () => {});
+    setPwToolsCoreCurrentPage({
+      goto,
+      url: vi.fn(() => "about:blank"),
+    });
+
+    await expect(
+      mod.navigateViaPlaywright({
+        cdpUrl: "http://127.0.0.1:18792",
+        url: "file:///etc/passwd",
+      }),
+    ).rejects.toBeInstanceOf(InvalidBrowserNavigationUrlError);
+
+    expect(getPwToolsCoreSessionMocks().getPageForTargetId).not.toHaveBeenCalled();
+    expect(goto).not.toHaveBeenCalled();
+  });
+
+  it("navigates valid network URLs with clamped timeout", async () => {
+    const goto = vi.fn(async () => {});
+    setPwToolsCoreCurrentPage({
+      goto,
+      url: vi.fn(() => "https://example.com"),
+    });
+
+    const result = await mod.navigateViaPlaywright({
+      cdpUrl: "http://127.0.0.1:18792",
+      url: "https://example.com",
+      timeoutMs: 10,
+    });
+
+    expect(goto).toHaveBeenCalledWith("https://example.com", { timeout: 1000 });
+    expect(result.url).toBe("https://example.com");
+  });
+});

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

@@ -1,8 +1,9 @@
 import { afterEach, describe, expect, it, vi } from "vitest";
+import type { BrowserServerState } from "./server-context.js";
 import { withFetchPreconnect } from "../test-utils/fetch-mock.js";
 import * as cdpModule from "./cdp.js";
+import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js";
 import * as pwAiModule from "./pw-ai-module.js";
-import type { BrowserServerState } from "./server-context.js";
 import "./server-context.chrome-test-harness.js";
 import { createBrowserRouteContext } from "./server-context.js";
 
@@ -94,7 +95,6 @@ describe("browser server-context remote profile tab operations", () => {
       cdpUrl: "https://browserless.example/chrome?token=abc",
       url: "http://127.0.0.1:3000",
       ssrfPolicy: { allowPrivateNetwork: true },
-      navigationChecked: true,
     });
 
     await remote.closeTab("T1");
@@ -256,7 +256,22 @@ describe("browser server-context tab selection state", () => {
       cdpUrl: "http://127.0.0.1:18800",
       url: "http://127.0.0.1:8080",
       ssrfPolicy: { allowPrivateNetwork: true },
-      navigationChecked: true,
     });
   });
+
+  it("blocks unsupported non-network URLs before any HTTP tab-open fallback", async () => {
+    const fetchMock = vi.fn(async () => {
+      throw new Error("unexpected fetch");
+    });
+
+    global.fetch = withFetchPreconnect(fetchMock);
+    const state = makeState("openclaw");
+    const ctx = createBrowserRouteContext({ getState: () => state });
+    const openclaw = ctx.forProfile("openclaw");
+
+    await expect(openclaw.openTab("file:///etc/passwd")).rejects.toBeInstanceOf(
+      InvalidBrowserNavigationUrlError,
+    );
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
 });

src/browser/server-context.ts

@@ -1,4 +1,15 @@
 import fs from "node:fs";
+import type { ResolvedBrowserProfile } from "./config.js";
+import type { PwAiModule } from "./pw-ai-module.js";
+import type {
+  BrowserServerState,
+  BrowserRouteContext,
+  BrowserTab,
+  ContextOptions,
+  ProfileContext,
+  ProfileRuntimeState,
+  ProfileStatus,
+} from "./server-context.types.js";
 import { SsrFBlockedError } from "../infra/net/ssrf.js";
 import { fetchJson, fetchOk } from "./cdp.helpers.js";
 import { appendCdpPath, createTargetViaCdp, normalizeCdpWsUrl } from "./cdp.js";
@@ -9,7 +20,6 @@ import {
   resolveOpenClawUserDataDir,
   stopOpenClawChrome,
 } from "./chrome.js";
-import type { ResolvedBrowserProfile } from "./config.js";
 import { resolveProfile } from "./config.js";
 import {
   ensureChromeExtensionRelayServer,
@@ -20,21 +30,11 @@ import {
   InvalidBrowserNavigationUrlError,
   withBrowserNavigationPolicy,
 } from "./navigation-guard.js";
-import type { PwAiModule } from "./pw-ai-module.js";
 import { getPwAiModule } from "./pw-ai-module.js";
 import {
   refreshResolvedBrowserConfigFromDisk,
   resolveBrowserProfileWithHotReload,
 } from "./resolved-config-refresh.js";
-import type {
-  BrowserServerState,
-  BrowserRouteContext,
-  BrowserTab,
-  ContextOptions,
-  ProfileContext,
-  ProfileRuntimeState,
-  ProfileStatus,
-} from "./server-context.types.js";
 import { resolveTargetIdFromTabs } from "./target-id.js";
 import { movePathToTrash } from "./trash.js";
 
@@ -137,7 +137,6 @@ function createProfileContext(
 
   const openTab = async (url: string): Promise<BrowserTab> => {
     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
@@ -149,7 +148,6 @@ function createProfileContext(
           cdpUrl: profile.cdpUrl,
           url,
           ...ssrfPolicyOpts,
-          navigationChecked: true,
         });
         const profileState = getProfileState();
         profileState.lastTargetId = page.targetId;
@@ -166,7 +164,6 @@ function createProfileContext(
       cdpUrl: profile.cdpUrl,
       url,
       ...ssrfPolicyOpts,
-      navigationChecked: true,
     })
       .then((r) => r.targetId)
       .catch(() => null);
@@ -196,6 +193,7 @@ function createProfileContext(
     };
 
     const endpointUrl = new URL(appendCdpPath(profile.cdpUrl, "/json/new"));
+    await assertBrowserNavigationAllowed({ url, ...ssrfPolicyOpts });
     const endpoint = endpointUrl.search
       ? (() => {
           endpointUrl.searchParams.set("url", url);