fix(browser): guard existing-session navigation

Co-authored-by: zsx <git@zsxsoft.com>

USER.md

@@ -0,0 +1,19 @@
+WORK LOG
+
+Add your findings and worklogs by appending to the end of this file. Do not overwrite anything that is existing in this file. Write with the format being used.
+
+[CODEX]
+
+I've brought work into the workstream.
+
+[CLAUDE]
+
+I've assigned the work to eleqtrizit.
+
+[CODEX SECURITY FIXER]
+
+- Reviewed NVIDIA-dev/openclaw-tracking#403, GHSA-527m-976r-jf79, and SECURITY.md. Determined the report is in scope: the existing-session Chrome MCP interaction path bypassed an operator-configured SSRF policy boundary rather than relying on an out-of-scope trust-model assumption.
+- Reviewed the linked private GHSA fix PR and incorporated the fix shape locally with a compatibility-safe adjustment: existing-session interaction routes now re-read and validate `window.location.href` after interaction, including a short grace-window recheck for delayed navigations.
+- Added focused regression coverage for click, submit/key-press, delayed evaluate-driven navigation, and the no-policy path.
+- Validation completed locally with `pnpm test extensions/browser/src/browser/routes/agent.act.existing-session-navigation-guard.test.ts`, `pnpm test extensions/browser/src/browser/routes/agent.existing-session.test.ts`, `pnpm check`, and `pnpm build`.
+- Attempted the required local `claude -p "/review"` step, but the command produced no review output in this environment and had to be bounded with `timeout`.

extensions/browser/src/browser/routes/agent.act.existing-session-navigation-guard.test.ts

@@ -0,0 +1,201 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { createBrowserRouteApp, createBrowserRouteResponse } from "./test-helpers.js";
+import type { BrowserRequest } from "./types.js";
+
+const routeState = vi.hoisted(() => ({
+  profileCtx: {
+    profile: {
+      driver: "existing-session" as const,
+      name: "chrome-live",
+    },
+    ensureTabAvailable: vi.fn(async () => ({
+      targetId: "7",
+      url: "https://example.com",
+    })),
+  },
+  tab: {
+    targetId: "7",
+    url: "https://example.com",
+  },
+}));
+
+const chromeMcpMocks = vi.hoisted(() => ({
+  clickChromeMcpElement: vi.fn(async () => {}),
+  dragChromeMcpElement: vi.fn(async () => {}),
+  evaluateChromeMcpScript: vi.fn(async () => "https://example.com"),
+  fillChromeMcpElement: vi.fn(async () => {}),
+  fillChromeMcpForm: vi.fn(async () => {}),
+  hoverChromeMcpElement: vi.fn(async () => {}),
+  pressChromeMcpKey: vi.fn(async () => {}),
+}));
+
+const navigationGuardMocks = vi.hoisted(() => ({
+  assertBrowserNavigationAllowed: vi.fn(async () => {}),
+  assertBrowserNavigationResultAllowed: vi.fn(async () => {}),
+  withBrowserNavigationPolicy: vi.fn((ssrfPolicy?: unknown) => (ssrfPolicy ? { ssrfPolicy } : {})),
+}));
+
+vi.mock("../chrome-mcp.js", () => ({
+  clickChromeMcpElement: chromeMcpMocks.clickChromeMcpElement,
+  closeChromeMcpTab: vi.fn(async () => {}),
+  dragChromeMcpElement: chromeMcpMocks.dragChromeMcpElement,
+  evaluateChromeMcpScript: chromeMcpMocks.evaluateChromeMcpScript,
+  fillChromeMcpElement: chromeMcpMocks.fillChromeMcpElement,
+  fillChromeMcpForm: chromeMcpMocks.fillChromeMcpForm,
+  hoverChromeMcpElement: chromeMcpMocks.hoverChromeMcpElement,
+  pressChromeMcpKey: chromeMcpMocks.pressChromeMcpKey,
+  resizeChromeMcpPage: vi.fn(async () => {}),
+}));
+
+vi.mock("../navigation-guard.js", () => navigationGuardMocks);
+
+vi.mock("./agent.shared.js", () => ({
+  getPwAiModule: vi.fn(async () => null),
+  handleRouteError: vi.fn(),
+  readBody: vi.fn((req: BrowserRequest) => req.body ?? {}),
+  requirePwAi: vi.fn(async () => {
+    throw new Error("Playwright should not be used for existing-session tests");
+  }),
+  resolveProfileContext: vi.fn(() => routeState.profileCtx),
+  resolveTargetIdFromBody: vi.fn((body: Record<string, unknown>) =>
+    typeof body.targetId === "string" ? body.targetId : undefined,
+  ),
+  withPlaywrightRouteContext: vi.fn(),
+  withRouteTabContext: vi.fn(async ({ run }: { run: (args: unknown) => Promise<void> }) => {
+    await run({
+      profileCtx: routeState.profileCtx,
+      cdpUrl: "http://127.0.0.1:18800",
+      tab: routeState.tab,
+    });
+  }),
+}));
+
+const DEFAULT_SSRF_POLICY = { allowPrivateNetwork: false } as const;
+
+const { registerBrowserAgentActRoutes } = await import("./agent.act.js");
+
+function getActPostHandler(ssrfPolicy?: { allowPrivateNetwork: false }) {
+  const { app, postHandlers } = createBrowserRouteApp();
+  registerBrowserAgentActRoutes(app, {
+    state: () => ({
+      resolved: {
+        evaluateEnabled: true,
+        ssrfPolicy: ssrfPolicy ?? DEFAULT_SSRF_POLICY,
+      },
+    }),
+  } as never);
+  const handler = postHandlers.get("/act");
+  expect(handler).toBeTypeOf("function");
+  return handler;
+}
+
+describe("existing-session interaction navigation guard", () => {
+  beforeEach(() => {
+    for (const fn of Object.values(chromeMcpMocks)) {
+      fn.mockClear();
+    }
+    for (const fn of Object.values(navigationGuardMocks)) {
+      fn.mockClear();
+    }
+    chromeMcpMocks.evaluateChromeMcpScript.mockResolvedValue("https://example.com");
+  });
+
+  it("checks navigation after click and key-driven submit paths", async () => {
+    const handler = getActPostHandler();
+
+    const clickResponse = createBrowserRouteResponse();
+    await handler?.(
+      { params: {}, query: {}, body: { kind: "click", ref: "btn-1" } },
+      clickResponse.res,
+    );
+
+    const typeResponse = createBrowserRouteResponse();
+    await handler?.(
+      {
+        params: {},
+        query: {},
+        body: { kind: "type", ref: "field-1", text: "hello", submit: true },
+      },
+      typeResponse.res,
+    );
+
+    expect(clickResponse.statusCode).toBe(200);
+    expect(typeResponse.statusCode).toBe(200);
+    expect(chromeMcpMocks.clickChromeMcpElement).toHaveBeenCalledOnce();
+    expect(chromeMcpMocks.pressChromeMcpKey).toHaveBeenCalledWith(
+      expect.objectContaining({ key: "Enter" }),
+    );
+    expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenCalledTimes(4);
+    expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenNthCalledWith(
+      1,
+      expect.objectContaining({ url: "https://example.com" }),
+    );
+    expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenNthCalledWith(
+      2,
+      expect.objectContaining({ url: "https://example.com" }),
+    );
+    expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenNthCalledWith(
+      3,
+      expect.objectContaining({ url: "https://example.com" }),
+    );
+    expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenNthCalledWith(
+      4,
+      expect.objectContaining({ url: "https://example.com" }),
+    );
+  });
+
+  it("rechecks the page url after delayed navigation-triggering interactions", async () => {
+    const handler = getActPostHandler();
+    chromeMcpMocks.evaluateChromeMcpScript
+      .mockResolvedValueOnce(42 as never)
+      .mockResolvedValueOnce("https://example.com" as never)
+      .mockResolvedValueOnce("http://169.254.169.254/latest/meta-data/" as never)
+      .mockResolvedValueOnce("http://169.254.169.254/latest/meta-data/" as never);
+
+    const response = createBrowserRouteResponse();
+    await handler?.(
+      {
+        params: {},
+        query: {},
+        body: { kind: "evaluate", fn: "() => document.title" },
+      },
+      response.res,
+    );
+
+    expect(response.statusCode).toBe(200);
+    expect(chromeMcpMocks.evaluateChromeMcpScript).toHaveBeenCalledTimes(4);
+    expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenNthCalledWith(
+      1,
+      expect.objectContaining({ url: "https://example.com" }),
+    );
+    expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenNthCalledWith(
+      2,
+      expect.objectContaining({ url: "http://169.254.169.254/latest/meta-data/" }),
+    );
+    expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenNthCalledWith(
+      3,
+      expect.objectContaining({ url: "http://169.254.169.254/latest/meta-data/" }),
+    );
+  });
+
+  it("skips the guard when no SSRF policy is configured", async () => {
+    const { app, postHandlers } = createBrowserRouteApp();
+    registerBrowserAgentActRoutes(app, {
+      state: () => ({
+        resolved: {
+          evaluateEnabled: true,
+          ssrfPolicy: undefined,
+        },
+      }),
+    } as never);
+    const handler = postHandlers.get("/act");
+    const response = createBrowserRouteResponse();
+
+    await handler?.({ params: {}, query: {}, body: { kind: "press", key: "Enter" } }, response.res);
+
+    expect(response.statusCode).toBe(200);
+    expect(chromeMcpMocks.pressChromeMcpKey).toHaveBeenCalledOnce();
+    expect(chromeMcpMocks.evaluateChromeMcpScript).not.toHaveBeenCalled();
+    expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).not.toHaveBeenCalled();
+  });
+});

extensions/browser/src/browser/routes/agent.act.ts

@@ -11,6 +11,11 @@ import {
   resizeChromeMcpPage,
 } from "../chrome-mcp.js";
 import type { BrowserActRequest } from "../client-actions.types.js";
+import {
+  assertBrowserNavigationResultAllowed,
+  type BrowserNavigationPolicyOptions,
+  withBrowserNavigationPolicy,
+} from "../navigation-guard.js";
 import { getBrowserProfileCapabilities } from "../profile-capabilities.js";
 import type { BrowserRouteContext } from "../server-context.js";
 import { matchBrowserUrlPattern } from "../url-pattern.js";
@@ -38,6 +43,62 @@ function sleep(ms: number): Promise<void> {
   return new Promise((resolve) => setTimeout(resolve, ms));
 }
 
+const EXISTING_SESSION_INTERACTION_NAVIGATION_RECHECK_DELAYS_MS = [0, 250, 500] as const;
+
+async function readExistingSessionLocationHref(params: {
+  profileName: string;
+  userDataDir?: string;
+  targetId: string;
+}): Promise<string> {
+  const currentUrl = await evaluateChromeMcpScript({
+    profileName: params.profileName,
+    userDataDir: params.userDataDir,
+    targetId: params.targetId,
+    fn: "() => window.location.href",
+  });
+  return typeof currentUrl === "string" ? currentUrl : "";
+}
+
+async function assertExistingSessionPostInteractionNavigationAllowed(params: {
+  profileName: string;
+  userDataDir?: string;
+  targetId: string;
+  baselineUrl?: string;
+  ssrfPolicy?: BrowserNavigationPolicyOptions["ssrfPolicy"];
+}): Promise<void> {
+  const ssrfPolicyOpts = withBrowserNavigationPolicy(params.ssrfPolicy);
+  if (!ssrfPolicyOpts.ssrfPolicy) {
+    return;
+  }
+
+  let lastUrl = params.baselineUrl ?? "";
+  let sawStableUrl = false;
+  for (const delayMs of EXISTING_SESSION_INTERACTION_NAVIGATION_RECHECK_DELAYS_MS) {
+    if (delayMs > 0) {
+      await sleep(delayMs);
+    }
+    let currentUrl: string;
+    try {
+      currentUrl = await readExistingSessionLocationHref(params);
+    } catch {
+      return;
+    }
+    await assertBrowserNavigationResultAllowed({
+      url: currentUrl,
+      ...ssrfPolicyOpts,
+    });
+    if (currentUrl !== lastUrl) {
+      lastUrl = currentUrl;
+      sawStableUrl = false;
+      continue;
+    }
+    if (sawStableUrl) {
+      return;
+    }
+    sawStableUrl = true;
+  }
+}
+
 function buildExistingSessionWaitPredicate(params: {
   text?: string;
   textGone?: string;
@@ -268,6 +329,13 @@ export function registerBrowserAgentActRoutes(
                 uid: action.ref!,
                 doubleClick: action.doubleClick ?? false,
               });
+              await assertExistingSessionPostInteractionNavigationAllowed({
+                profileName,
+                userDataDir: profileCtx.profile.userDataDir,
+                targetId: tab.targetId,
+                baselineUrl: tab.url,
+                ssrfPolicy,
+              });
               return res.json({ ok: true, targetId: tab.targetId, url: tab.url });
             case "type":
               await fillChromeMcpElement({
@@ -285,6 +353,13 @@ export function registerBrowserAgentActRoutes(
                   key: "Enter",
                 });
               }
+              await assertExistingSessionPostInteractionNavigationAllowed({
+                profileName,
+                userDataDir: profileCtx.profile.userDataDir,
+                targetId: tab.targetId,
+                baselineUrl: tab.url,
+                ssrfPolicy,
+              });
               return res.json({ ok: true, targetId: tab.targetId });
             case "press":
               await pressChromeMcpKey({
@@ -293,6 +368,13 @@ export function registerBrowserAgentActRoutes(
                 targetId: tab.targetId,
                 key: action.key,
               });
+              await assertExistingSessionPostInteractionNavigationAllowed({
+                profileName,
+                userDataDir: profileCtx.profile.userDataDir,
+                targetId: tab.targetId,
+                baselineUrl: tab.url,
+                ssrfPolicy,
+              });
               return res.json({ ok: true, targetId: tab.targetId });
             case "hover":
               await hoverChromeMcpElement({
@@ -301,6 +383,13 @@ export function registerBrowserAgentActRoutes(
                 targetId: tab.targetId,
                 uid: action.ref!,
               });
+              await assertExistingSessionPostInteractionNavigationAllowed({
+                profileName,
+                userDataDir: profileCtx.profile.userDataDir,
+                targetId: tab.targetId,
+                baselineUrl: tab.url,
+                ssrfPolicy,
+              });
               return res.json({ ok: true, targetId: tab.targetId });
             case "scrollIntoView":
               await evaluateChromeMcpScript({
@@ -310,6 +399,13 @@ export function registerBrowserAgentActRoutes(
                 fn: `(el) => { el.scrollIntoView({ block: "center", inline: "center" }); return true; }`,
                 args: [action.ref!],
               });
+              await assertExistingSessionPostInteractionNavigationAllowed({
+                profileName,
+                userDataDir: profileCtx.profile.userDataDir,
+                targetId: tab.targetId,
+                baselineUrl: tab.url,
+                ssrfPolicy,
+              });
               return res.json({ ok: true, targetId: tab.targetId });
             case "drag":
               await dragChromeMcpElement({
@@ -319,6 +415,13 @@ export function registerBrowserAgentActRoutes(
                 fromUid: action.startRef!,
                 toUid: action.endRef!,
               });
+              await assertExistingSessionPostInteractionNavigationAllowed({
+                profileName,
+                userDataDir: profileCtx.profile.userDataDir,
+                targetId: tab.targetId,
+                baselineUrl: tab.url,
+                ssrfPolicy,
+              });
               return res.json({ ok: true, targetId: tab.targetId });
             case "select":
               await fillChromeMcpElement({
@@ -328,6 +431,13 @@ export function registerBrowserAgentActRoutes(
                 uid: action.ref!,
                 value: action.values[0] ?? "",
               });
+              await assertExistingSessionPostInteractionNavigationAllowed({
+                profileName,
+                userDataDir: profileCtx.profile.userDataDir,
+                targetId: tab.targetId,
+                baselineUrl: tab.url,
+                ssrfPolicy,
+              });
               return res.json({ ok: true, targetId: tab.targetId });
             case "fill":
               await fillChromeMcpForm({
@@ -339,6 +449,13 @@ export function registerBrowserAgentActRoutes(
                   value: String(field.value ?? ""),
                 })),
               });
+              await assertExistingSessionPostInteractionNavigationAllowed({
+                profileName,
+                userDataDir: profileCtx.profile.userDataDir,
+                targetId: tab.targetId,
+                baselineUrl: tab.url,
+                ssrfPolicy,
+              });
               return res.json({ ok: true, targetId: tab.targetId });
             case "resize":
               await resizeChromeMcpPage({
@@ -372,6 +489,13 @@ export function registerBrowserAgentActRoutes(
                 fn: action.fn,
                 args: action.ref ? [action.ref] : undefined,
               });
+              await assertExistingSessionPostInteractionNavigationAllowed({
+                profileName,
+                userDataDir: profileCtx.profile.userDataDir,
+                targetId: tab.targetId,
+                baselineUrl: tab.url,
+                ssrfPolicy,
+              });
               return res.json({
                 ok: true,
                 targetId: tab.targetId,