fix(browser): guard interaction-driven navigations

This commit is contained in:
Agustin Rivera
2026-04-07 10:03:12 -07:00
committed by GitHub
parent df881d5c18
commit 049acf23cb
7 changed files with 292 additions and 1 deletions

View File

@@ -0,0 +1,180 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
const pageState = vi.hoisted(() => ({
page: null as Record<string, unknown> | null,
locator: null as Record<string, unknown> | null,
}));
const sessionMocks = vi.hoisted(() => ({
assertPageNavigationCompletedSafely: vi.fn(async () => {}),
ensurePageState: vi.fn(() => ({})),
forceDisconnectPlaywrightForTarget: vi.fn(async () => {}),
getPageForTargetId: vi.fn(async () => {
if (!pageState.page) {
throw new Error("missing page");
}
return pageState.page;
}),
gotoPageWithNavigationGuard: vi.fn(async () => null),
refLocator: vi.fn(() => {
if (!pageState.locator) {
throw new Error("missing locator");
}
return pageState.locator;
}),
restoreRoleRefsForTarget: vi.fn(() => {}),
storeRoleRefsForTarget: vi.fn(() => {}),
}));
const pageCdpMocks = vi.hoisted(() => ({
withPageScopedCdpClient: vi.fn(
async ({ fn }: { fn: (send: () => Promise<unknown>) => unknown }) =>
await fn(async () => ({ nodes: [] })),
),
}));
vi.mock("./pw-session.js", () => sessionMocks);
vi.mock("./pw-session.page-cdp.js", () => pageCdpMocks);
const interactions = await import("./pw-tools-core.interactions.js");
const snapshots = await import("./pw-tools-core.snapshot.js");
describe("pw-tools-core browser SSRF guards", () => {
beforeEach(() => {
pageState.page = null;
pageState.locator = null;
for (const fn of Object.values(sessionMocks)) {
fn.mockClear();
}
for (const fn of Object.values(pageCdpMocks)) {
fn.mockClear();
}
});
it("re-checks click-triggered navigations with the session safety helper", async () => {
pageState.page = { url: vi.fn(() => "https://example.com") };
pageState.locator = { click: vi.fn(async () => {}) };
await interactions.clickViaPlaywright({
cdpUrl: "http://127.0.0.1:18792",
targetId: "tab-1",
ref: "1",
ssrfPolicy: { allowPrivateNetwork: false },
});
expect(sessionMocks.assertPageNavigationCompletedSafely).toHaveBeenCalledWith({
cdpUrl: "http://127.0.0.1:18792",
page: pageState.page,
response: null,
ssrfPolicy: { allowPrivateNetwork: false },
targetId: "tab-1",
});
});
it("preserves helper compatibility when no ssrfPolicy is provided", async () => {
pageState.page = { url: vi.fn(() => "https://example.com") };
pageState.locator = { click: vi.fn(async () => {}) };
await interactions.clickViaPlaywright({
cdpUrl: "http://127.0.0.1:18792",
targetId: "tab-1",
ref: "1",
// no ssrfPolicy: direct helper callers keep previous compatibility semantics
});
expect(sessionMocks.assertPageNavigationCompletedSafely).not.toHaveBeenCalled();
});
it("re-checks batched click-triggered navigations with the session safety helper", async () => {
pageState.page = { url: vi.fn(() => "https://example.com") };
pageState.locator = { click: vi.fn(async () => {}) };
await interactions.batchViaPlaywright({
cdpUrl: "http://127.0.0.1:18792",
targetId: "tab-1",
actions: [{ kind: "click", ref: "1" }],
ssrfPolicy: { allowPrivateNetwork: false },
});
expect(sessionMocks.assertPageNavigationCompletedSafely).toHaveBeenCalledWith({
cdpUrl: "http://127.0.0.1:18792",
page: pageState.page,
response: null,
ssrfPolicy: { allowPrivateNetwork: false },
targetId: "tab-1",
});
});
it("re-checks current page URL before snapshotting AI content", async () => {
const snapshotForAI = vi.fn(async () => ({ full: 'button "Save"' }));
pageState.page = {
_snapshotForAI: snapshotForAI,
url: vi.fn(() => "https://example.com"),
};
await snapshots.snapshotAiViaPlaywright({
cdpUrl: "http://127.0.0.1:18792",
targetId: "tab-1",
ssrfPolicy: { allowPrivateNetwork: false },
});
expect(sessionMocks.assertPageNavigationCompletedSafely).toHaveBeenCalledWith({
cdpUrl: "http://127.0.0.1:18792",
page: pageState.page,
response: null,
ssrfPolicy: { allowPrivateNetwork: false },
targetId: "tab-1",
});
expect(
sessionMocks.assertPageNavigationCompletedSafely.mock.invocationCallOrder[0],
).toBeLessThan(snapshotForAI.mock.invocationCallOrder[0]);
});
it("re-checks current page URL before role snapshots", async () => {
const ariaSnapshot = vi.fn(async () => "");
pageState.page = {
locator: vi.fn(() => ({ ariaSnapshot })),
url: vi.fn(() => "https://example.com"),
};
await snapshots.snapshotRoleViaPlaywright({
cdpUrl: "http://127.0.0.1:18792",
targetId: "tab-1",
ssrfPolicy: { allowPrivateNetwork: false },
});
expect(sessionMocks.assertPageNavigationCompletedSafely).toHaveBeenCalledWith({
cdpUrl: "http://127.0.0.1:18792",
page: pageState.page,
response: null,
ssrfPolicy: { allowPrivateNetwork: false },
targetId: "tab-1",
});
expect(
sessionMocks.assertPageNavigationCompletedSafely.mock.invocationCallOrder[0],
).toBeLessThan(ariaSnapshot.mock.invocationCallOrder[0]);
});
it("re-checks current page URL before aria snapshots", async () => {
pageState.page = {
url: vi.fn(() => "https://example.com"),
};
await snapshots.snapshotAriaViaPlaywright({
cdpUrl: "http://127.0.0.1:18792",
targetId: "tab-1",
ssrfPolicy: { allowPrivateNetwork: false },
});
expect(sessionMocks.assertPageNavigationCompletedSafely).toHaveBeenCalledWith({
cdpUrl: "http://127.0.0.1:18792",
page: pageState.page,
response: null,
ssrfPolicy: { allowPrivateNetwork: false },
targetId: "tab-1",
});
expect(
sessionMocks.assertPageNavigationCompletedSafely.mock.invocationCallOrder[0],
).toBeLessThan(pageCdpMocks.withPageScopedCdpClient.mock.invocationCallOrder[0]);
});
});

View File

@@ -1,8 +1,10 @@
import { formatErrorMessage } from "../infra/errors.js";
import type { SsrFPolicy } from "../infra/net/ssrf.js";
import type { BrowserActRequest, BrowserFormField } from "./client-actions-core.js";
import { DEFAULT_FILL_FIELD_TYPE } from "./form-fields.js";
import { DEFAULT_UPLOAD_DIR, resolveStrictExistingPathsWithinRoot } from "./paths.js";
import {
assertPageNavigationCompletedSafely,
ensurePageState,
forceDisconnectPlaywrightForTarget,
getPageForTargetId,
@@ -64,6 +66,24 @@ async function awaitEvalWithAbort<T>(
}
}
async function assertPostInteractionNavigationSafe(opts: {
cdpUrl: string;
page: Awaited<ReturnType<typeof getPageForTargetId>>;
ssrfPolicy?: SsrFPolicy;
targetId?: string;
}): Promise<void> {
if (!opts.ssrfPolicy) {
return;
}
await assertPageNavigationCompletedSafely({
cdpUrl: opts.cdpUrl,
page: opts.page,
response: null,
ssrfPolicy: opts.ssrfPolicy,
targetId: opts.targetId,
});
}
export async function highlightViaPlaywright(opts: {
cdpUrl: string;
targetId?: string;
@@ -88,6 +108,7 @@ export async function clickViaPlaywright(opts: {
modifiers?: Array<"Alt" | "Control" | "ControlOrMeta" | "Meta" | "Shift">;
delayMs?: number;
timeoutMs?: number;
ssrfPolicy?: SsrFPolicy;
}): Promise<void> {
const resolved = requireRefOrSelector(opts.ref, opts.selector);
const page = await getRestoredPageForTarget(opts);
@@ -115,6 +136,12 @@ export async function clickViaPlaywright(opts: {
modifiers: opts.modifiers,
});
}
await assertPostInteractionNavigationSafe({
cdpUrl: opts.cdpUrl,
page,
ssrfPolicy: opts.ssrfPolicy,
targetId: opts.targetId,
});
} catch (err) {
throw toAIFriendlyError(err, label);
}
@@ -202,6 +229,7 @@ export async function pressKeyViaPlaywright(opts: {
targetId?: string;
key: string;
delayMs?: number;
ssrfPolicy?: SsrFPolicy;
}): Promise<void> {
const key = String(opts.key ?? "").trim();
if (!key) {
@@ -212,6 +240,12 @@ export async function pressKeyViaPlaywright(opts: {
await page.keyboard.press(key, {
delay: Math.max(0, Math.floor(opts.delayMs ?? 0)),
});
await assertPostInteractionNavigationSafe({
cdpUrl: opts.cdpUrl,
page,
ssrfPolicy: opts.ssrfPolicy,
targetId: opts.targetId,
});
}
export async function typeViaPlaywright(opts: {
@@ -223,6 +257,7 @@ export async function typeViaPlaywright(opts: {
submit?: boolean;
slowly?: boolean;
timeoutMs?: number;
ssrfPolicy?: SsrFPolicy;
}): Promise<void> {
const resolved = requireRefOrSelector(opts.ref, opts.selector);
const text = String(opts.text ?? "");
@@ -241,6 +276,12 @@ export async function typeViaPlaywright(opts: {
}
if (opts.submit) {
await locator.press("Enter", { timeout });
await assertPostInteractionNavigationSafe({
cdpUrl: opts.cdpUrl,
page,
ssrfPolicy: opts.ssrfPolicy,
targetId: opts.targetId,
});
}
} catch (err) {
throw toAIFriendlyError(err, label);
@@ -713,6 +754,7 @@ async function executeSingleAction(
cdpUrl: string,
targetId?: string,
evaluateEnabled?: boolean,
ssrfPolicy?: SsrFPolicy,
depth = 0,
): Promise<void> {
if (depth > MAX_BATCH_DEPTH) {
@@ -733,6 +775,7 @@ async function executeSingleAction(
>,
delayMs: action.delayMs,
timeoutMs: action.timeoutMs,
ssrfPolicy,
});
break;
case "type":
@@ -745,6 +788,7 @@ async function executeSingleAction(
submit: action.submit,
slowly: action.slowly,
timeoutMs: action.timeoutMs,
ssrfPolicy,
});
break;
case "press":
@@ -753,6 +797,7 @@ async function executeSingleAction(
targetId: effectiveTargetId,
key: action.key,
delayMs: action.delayMs,
ssrfPolicy,
});
break;
case "hover":
@@ -852,6 +897,7 @@ async function executeSingleAction(
actions: action.actions,
stopOnError: action.stopOnError,
evaluateEnabled,
ssrfPolicy,
depth: depth + 1,
});
break;
@@ -866,6 +912,7 @@ export async function batchViaPlaywright(opts: {
actions: BrowserActRequest[];
stopOnError?: boolean;
evaluateEnabled?: boolean;
ssrfPolicy?: SsrFPolicy;
depth?: number;
}): Promise<{ results: Array<{ ok: boolean; error?: string }> }> {
const depth = opts.depth ?? 0;
@@ -878,7 +925,14 @@ export async function batchViaPlaywright(opts: {
const results: Array<{ ok: boolean; error?: string }> = [];
for (const action of opts.actions) {
try {
await executeSingleAction(action, opts.cdpUrl, opts.targetId, opts.evaluateEnabled, depth);
await executeSingleAction(
action,
opts.cdpUrl,
opts.targetId,
opts.evaluateEnabled,
opts.ssrfPolicy,
depth,
);
results.push({ ok: true });
} catch (err) {
const message = formatErrorMessage(err);

View File

@@ -23,6 +23,7 @@ export async function snapshotAriaViaPlaywright(opts: {
cdpUrl: string;
targetId?: string;
limit?: number;
ssrfPolicy?: SsrFPolicy;
}): Promise<{ nodes: AriaSnapshotNode[] }> {
const limit = Math.max(1, Math.min(2000, Math.floor(opts.limit ?? 500)));
const page = await getPageForTargetId({
@@ -30,6 +31,15 @@ export async function snapshotAriaViaPlaywright(opts: {
targetId: opts.targetId,
});
ensurePageState(page);
if (opts.ssrfPolicy) {
await assertPageNavigationCompletedSafely({
cdpUrl: opts.cdpUrl,
page,
response: null,
ssrfPolicy: opts.ssrfPolicy,
targetId: opts.targetId,
});
}
const res = (await withPageScopedCdpClient({
cdpUrl: opts.cdpUrl,
page,
@@ -52,12 +62,22 @@ export async function snapshotAiViaPlaywright(opts: {
targetId?: string;
timeoutMs?: number;
maxChars?: number;
ssrfPolicy?: SsrFPolicy;
}): Promise<{ snapshot: string; truncated?: boolean; refs: RoleRefMap }> {
const page = await getPageForTargetId({
cdpUrl: opts.cdpUrl,
targetId: opts.targetId,
});
ensurePageState(page);
if (opts.ssrfPolicy) {
await assertPageNavigationCompletedSafely({
cdpUrl: opts.cdpUrl,
page,
response: null,
ssrfPolicy: opts.ssrfPolicy,
targetId: opts.targetId,
});
}
const maybe = page as unknown as WithSnapshotForAI;
if (!maybe._snapshotForAI) {
@@ -98,6 +118,7 @@ export async function snapshotRoleViaPlaywright(opts: {
frameSelector?: string;
refsMode?: "role" | "aria";
options?: RoleSnapshotOptions;
ssrfPolicy?: SsrFPolicy;
}): Promise<{
snapshot: string;
refs: Record<string, { role: string; name?: string; nth?: number }>;
@@ -108,6 +129,15 @@ export async function snapshotRoleViaPlaywright(opts: {
targetId: opts.targetId,
});
ensurePageState(page);
if (opts.ssrfPolicy) {
await assertPageNavigationCompletedSafely({
cdpUrl: opts.cdpUrl,
page,
response: null,
ssrfPolicy: opts.ssrfPolicy,
targetId: opts.targetId,
});
}
if (opts.refsMode === "aria") {
if (opts.selector?.trim() || opts.frameSelector?.trim()) {

View File

@@ -101,6 +101,7 @@ export function registerBrowserAgentActHookRoutes(
cdpUrl,
targetId: tab.targetId,
ref,
ssrfPolicy: ctx.state().resolved.ssrfPolicy,
});
}
}

View File

@@ -482,6 +482,7 @@ export function registerBrowserAgentActRoutes(
targetId,
run: async ({ profileCtx, cdpUrl, tab }) => {
const evaluateEnabled = ctx.state().resolved.evaluateEnabled;
const ssrfPolicy = ctx.state().resolved.ssrfPolicy;
const isExistingSession = getBrowserProfileCapabilities(profileCtx.profile).usesChromeMcp;
const profileName = profileCtx.profile.name;
@@ -539,6 +540,7 @@ export function registerBrowserAgentActRoutes(
cdpUrl,
targetId: tab.targetId,
doubleClick,
ssrfPolicy,
};
if (ref) {
clickRequest.ref = ref;
@@ -616,6 +618,7 @@ export function registerBrowserAgentActRoutes(
text,
submit,
slowly,
ssrfPolicy,
};
if (ref) {
typeRequest.ref = ref;
@@ -656,6 +659,7 @@ export function registerBrowserAgentActRoutes(
targetId: tab.targetId,
key,
delayMs: delayMs ?? undefined,
ssrfPolicy,
});
return res.json({ ok: true, targetId: tab.targetId });
}
@@ -1105,6 +1109,7 @@ export function registerBrowserAgentActRoutes(
actions,
stopOnError,
evaluateEnabled,
ssrfPolicy,
});
return res.json({ ok: true, targetId: tab.targetId, results: result.results });
}

View File

@@ -498,6 +498,7 @@ export function registerBrowserAgentSnapshotRoutes(
selector: plan.selectorValue,
frameSelector: plan.frameSelectorValue,
refsMode: plan.refsMode,
ssrfPolicy: ctx.state().resolved.ssrfPolicy,
options: {
interactive: plan.interactive ?? undefined,
compact: plan.compact ?? undefined,
@@ -511,6 +512,7 @@ export function registerBrowserAgentSnapshotRoutes(
.snapshotAiViaPlaywright({
cdpUrl: profileCtx.profile.cdpUrl,
targetId: tab.targetId,
ssrfPolicy: ctx.state().resolved.ssrfPolicy,
...(typeof plan.resolvedMaxChars === "number"
? { maxChars: plan.resolvedMaxChars }
: {}),
@@ -579,6 +581,7 @@ export function registerBrowserAgentSnapshotRoutes(
cdpUrl: profileCtx.profile.cdpUrl,
targetId: tab.targetId,
limit: plan.limit,
ssrfPolicy: ctx.state().resolved.ssrfPolicy,
});
});
})()

View File

@@ -43,6 +43,9 @@ describe("browser control server", () => {
cdpUrl: state.cdpBaseUrl,
targetId: "abcd1234",
maxChars: DEFAULT_AI_SNAPSHOT_MAX_CHARS,
ssrfPolicy: {
dangerouslyAllowPrivateNetwork: true,
},
});
const snapAiZero = (await realFetch(`${base}/snapshot?format=ai&maxChars=0`).then((r) =>
@@ -54,6 +57,9 @@ describe("browser control server", () => {
expect(lastCall).toEqual({
cdpUrl: state.cdpBaseUrl,
targetId: "abcd1234",
ssrfPolicy: {
dangerouslyAllowPrivateNetwork: true,
},
});
});
@@ -91,6 +97,9 @@ describe("browser control server", () => {
doubleClick: false,
button: "left",
modifiers: ["Shift"],
ssrfPolicy: {
dangerouslyAllowPrivateNetwork: true,
},
});
const clickSelector = await realFetch(`${base}/act`, {
@@ -105,6 +114,9 @@ describe("browser control server", () => {
targetId: "abcd1234",
selector: "button.save",
doubleClick: false,
ssrfPolicy: {
dangerouslyAllowPrivateNetwork: true,
},
});
const type = await postJson<{ ok: boolean }>(`${base}/act`, {
@@ -120,6 +132,9 @@ describe("browser control server", () => {
text: "",
submit: false,
slowly: false,
ssrfPolicy: {
dangerouslyAllowPrivateNetwork: true,
},
});
const press = await postJson<{ ok: boolean }>(`${base}/act`, {
@@ -131,6 +146,9 @@ describe("browser control server", () => {
cdpUrl: state.cdpBaseUrl,
targetId: "abcd1234",
key: "Enter",
ssrfPolicy: {
dangerouslyAllowPrivateNetwork: true,
},
});
const hover = await postJson<{ ok: boolean }>(`${base}/act`, {