diff --git a/extensions/browser/src/browser/routes/agent.act.existing-session-navigation-guard.test.ts b/extensions/browser/src/browser/routes/agent.act.existing-session-navigation-guard.test.ts index ca672b0b17e..dc1e626d5c8 100644 --- a/extensions/browser/src/browser/routes/agent.act.existing-session-navigation-guard.test.ts +++ b/extensions/browser/src/browser/routes/agent.act.existing-session-navigation-guard.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createBrowserRouteApp, createBrowserRouteResponse } from "./test-helpers.js"; import type { BrowserRequest } from "./types.js"; @@ -74,13 +74,15 @@ const DEFAULT_SSRF_POLICY = { allowPrivateNetwork: false } as const; const { registerBrowserAgentActRoutes } = await import("./agent.act.js"); -function getActPostHandler(ssrfPolicy?: { allowPrivateNetwork: false }) { +function getActPostHandler( + ssrfPolicy: { allowPrivateNetwork: false } | null = DEFAULT_SSRF_POLICY, +) { const { app, postHandlers } = createBrowserRouteApp(); registerBrowserAgentActRoutes(app, { state: () => ({ resolved: { evaluateEnabled: true, - ssrfPolicy: ssrfPolicy ?? DEFAULT_SSRF_POLICY, + ssrfPolicy: ssrfPolicy ?? undefined, }, }), } as never); @@ -91,6 +93,7 @@ function getActPostHandler(ssrfPolicy?: { allowPrivateNetwork: false }) { describe("existing-session interaction navigation guard", () => { beforeEach(() => { + vi.useFakeTimers(); for (const fn of Object.values(chromeMcpMocks)) { fn.mockClear(); } @@ -100,24 +103,30 @@ describe("existing-session interaction navigation guard", () => { chromeMcpMocks.evaluateChromeMcpScript.mockResolvedValue("https://example.com"); }); + afterEach(() => { + vi.useRealTimers(); + }); + + async function runAction( + body: Record, + ssrfPolicy: { allowPrivateNetwork: false } | null = DEFAULT_SSRF_POLICY, + ) { + const handler = getActPostHandler(ssrfPolicy); + const response = createBrowserRouteResponse(); + const pending = handler?.({ params: {}, query: {}, body }, response.res); + await vi.runAllTimersAsync(); + await pending; + return response; + } + 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, - ); + const clickResponse = await runAction({ kind: "click", ref: "btn-1" }); + const typeResponse = await runAction({ + kind: "type", + ref: "field-1", + text: "hello", + submit: true, + }); expect(clickResponse.statusCode).toBe(200); expect(typeResponse.statusCode).toBe(200); @@ -145,22 +154,13 @@ describe("existing-session interaction navigation guard", () => { }); 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, - ); + const response = await runAction({ kind: "evaluate", fn: "() => document.title" }); expect(response.statusCode).toBe(200); expect(chromeMcpMocks.evaluateChromeMcpScript).toHaveBeenCalledTimes(4); @@ -179,23 +179,32 @@ describe("existing-session interaction navigation guard", () => { }); 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); + const response = await runAction({ kind: "press", key: "Enter" }, null); expect(response.statusCode).toBe(200); expect(chromeMcpMocks.pressChromeMcpKey).toHaveBeenCalledOnce(); expect(chromeMcpMocks.evaluateChromeMcpScript).not.toHaveBeenCalled(); expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).not.toHaveBeenCalled(); }); + + it("still probes navigation when the interaction command throws", async () => { + chromeMcpMocks.clickChromeMcpElement.mockImplementationOnce(() => { + throw new Error("stale element"); + }); + + const handler = getActPostHandler(); + const response = createBrowserRouteResponse(); + const pending = + handler?.({ params: {}, query: {}, body: { kind: "click", ref: "btn-1" } }, response.res) ?? + Promise.resolve(); + void pending.catch(() => {}); + const completion = (async () => { + await vi.runAllTimersAsync(); + await pending; + })(); + + await expect(completion).rejects.toThrow("stale element"); + expect(chromeMcpMocks.evaluateChromeMcpScript).toHaveBeenCalled(); + expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenCalled(); + }); }); diff --git a/extensions/browser/src/browser/routes/agent.act.ts b/extensions/browser/src/browser/routes/agent.act.ts index 3df06319625..1ed64dd74bf 100644 --- a/extensions/browser/src/browser/routes/agent.act.ts +++ b/extensions/browser/src/browser/routes/agent.act.ts @@ -73,6 +73,7 @@ async function assertExistingSessionPostInteractionNavigationAllowed(params: { let lastUrl = params.baselineUrl ?? ""; let sawStableUrl = false; + let readSucceeded = false; for (const delayMs of EXISTING_SESSION_INTERACTION_NAVIGATION_RECHECK_DELAYS_MS) { if (delayMs > 0) { await sleep(delayMs); @@ -81,8 +82,9 @@ async function assertExistingSessionPostInteractionNavigationAllowed(params: { try { currentUrl = await readExistingSessionLocationHref(params); } catch { - return; + continue; } + readSucceeded = true; await assertBrowserNavigationResultAllowed({ url: currentUrl, ...ssrfPolicyOpts, @@ -97,6 +99,33 @@ async function assertExistingSessionPostInteractionNavigationAllowed(params: { } sawStableUrl = true; } + + if (!readSucceeded) { + throw new Error("Unable to verify post-interaction navigation"); + } +} + +async function runExistingSessionActionWithNavigationGuard(params: { + execute: () => Promise; + guard?: Parameters[0]; +}): Promise { + let actionError: unknown; + let result: T | undefined; + try { + result = await params.execute(); + } catch (error) { + actionError = error; + } + + if (params.guard) { + await assertExistingSessionPostInteractionNavigationAllowed(params.guard); + } + + if (actionError) { + throw actionError; + } + + return result as T; } function buildExistingSessionWaitPredicate(params: { @@ -322,139 +351,164 @@ export function registerBrowserAgentActRoutes( } switch (action.kind) { case "click": - await clickChromeMcpElement({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - 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({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - uid: action.ref!, - value: action.text, - }); - if (action.submit) { - await pressChromeMcpKey({ + await runExistingSessionActionWithNavigationGuard({ + execute: () => + clickChromeMcpElement({ + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + uid: action.ref!, + doubleClick: action.doubleClick ?? false, + }), + guard: { profileName, userDataDir: profileCtx.profile.userDataDir, targetId: tab.targetId, - key: "Enter", - }); - } - await assertExistingSessionPostInteractionNavigationAllowed({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - baselineUrl: tab.url, - ssrfPolicy, + baselineUrl: tab.url, + ssrfPolicy, + }, + }); + return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + case "type": + await runExistingSessionActionWithNavigationGuard({ + execute: async () => { + await fillChromeMcpElement({ + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + uid: action.ref!, + value: action.text, + }); + if (action.submit) { + await pressChromeMcpKey({ + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + key: "Enter", + }); + } + }, + guard: { + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + baselineUrl: tab.url, + ssrfPolicy, + }, }); return res.json({ ok: true, targetId: tab.targetId }); case "press": - await pressChromeMcpKey({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - key: action.key, - }); - await assertExistingSessionPostInteractionNavigationAllowed({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - baselineUrl: tab.url, - ssrfPolicy, + await runExistingSessionActionWithNavigationGuard({ + execute: () => + pressChromeMcpKey({ + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + key: action.key, + }), + guard: { + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + baselineUrl: tab.url, + ssrfPolicy, + }, }); return res.json({ ok: true, targetId: tab.targetId }); case "hover": - await hoverChromeMcpElement({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - uid: action.ref!, - }); - await assertExistingSessionPostInteractionNavigationAllowed({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - baselineUrl: tab.url, - ssrfPolicy, + await runExistingSessionActionWithNavigationGuard({ + execute: () => + hoverChromeMcpElement({ + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + uid: action.ref!, + }), + guard: { + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + baselineUrl: tab.url, + ssrfPolicy, + }, }); return res.json({ ok: true, targetId: tab.targetId }); case "scrollIntoView": - await evaluateChromeMcpScript({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - 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, + await runExistingSessionActionWithNavigationGuard({ + execute: () => + evaluateChromeMcpScript({ + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + fn: `(el) => { el.scrollIntoView({ block: "center", inline: "center" }); return true; }`, + args: [action.ref!], + }), + guard: { + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + baselineUrl: tab.url, + ssrfPolicy, + }, }); return res.json({ ok: true, targetId: tab.targetId }); case "drag": - await dragChromeMcpElement({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - fromUid: action.startRef!, - toUid: action.endRef!, - }); - await assertExistingSessionPostInteractionNavigationAllowed({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - baselineUrl: tab.url, - ssrfPolicy, + await runExistingSessionActionWithNavigationGuard({ + execute: () => + dragChromeMcpElement({ + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + fromUid: action.startRef!, + toUid: action.endRef!, + }), + guard: { + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + baselineUrl: tab.url, + ssrfPolicy, + }, }); return res.json({ ok: true, targetId: tab.targetId }); case "select": - await fillChromeMcpElement({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - uid: action.ref!, - value: action.values[0] ?? "", - }); - await assertExistingSessionPostInteractionNavigationAllowed({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - baselineUrl: tab.url, - ssrfPolicy, + await runExistingSessionActionWithNavigationGuard({ + execute: () => + fillChromeMcpElement({ + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + uid: action.ref!, + value: action.values[0] ?? "", + }), + guard: { + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + baselineUrl: tab.url, + ssrfPolicy, + }, }); return res.json({ ok: true, targetId: tab.targetId }); case "fill": - await fillChromeMcpForm({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - elements: action.fields.map((field) => ({ - uid: field.ref, - value: String(field.value ?? ""), - })), - }); - await assertExistingSessionPostInteractionNavigationAllowed({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - baselineUrl: tab.url, - ssrfPolicy, + await runExistingSessionActionWithNavigationGuard({ + execute: () => + fillChromeMcpForm({ + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + elements: action.fields.map((field) => ({ + uid: field.ref, + value: String(field.value ?? ""), + })), + }), + guard: { + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + baselineUrl: tab.url, + ssrfPolicy, + }, }); return res.json({ ok: true, targetId: tab.targetId }); case "resize": @@ -482,19 +536,22 @@ export function registerBrowserAgentActRoutes( }); return res.json({ ok: true, targetId: tab.targetId }); case "evaluate": { - const result = await evaluateChromeMcpScript({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - fn: action.fn, - args: action.ref ? [action.ref] : undefined, - }); - await assertExistingSessionPostInteractionNavigationAllowed({ - profileName, - userDataDir: profileCtx.profile.userDataDir, - targetId: tab.targetId, - baselineUrl: tab.url, - ssrfPolicy, + const result = await runExistingSessionActionWithNavigationGuard({ + execute: () => + evaluateChromeMcpScript({ + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + fn: action.fn, + args: action.ref ? [action.ref] : undefined, + }), + guard: { + profileName, + userDataDir: profileCtx.profile.userDataDir, + targetId: tab.targetId, + baselineUrl: tab.url, + ssrfPolicy, + }, }); return res.json({ ok: true,