fix(browser): reject excessive viewport resizes

This commit is contained in:
Vincent Koc
2026-05-29 07:18:35 +02:00
parent cdeafd1895
commit 2e042fbca8
12 changed files with 104 additions and 6 deletions

View File

@@ -0,0 +1,17 @@
import { describe, expect, it } from "vitest";
import { ACT_MAX_VIEWPORT_DIMENSION } from "./browser/act-policy.js";
import { BrowserToolSchema } from "./browser-tool.schema.js";
type SchemaRecord = Record<string, { maximum?: number; properties?: SchemaRecord }>;
describe("browser tool schema", () => {
it("advertises the viewport resize maximum on nested and flattened act params", () => {
const properties = BrowserToolSchema.properties as SchemaRecord;
const requestProperties = properties.request.properties ?? {};
expect(properties.width.maximum).toBe(ACT_MAX_VIEWPORT_DIMENSION);
expect(properties.height.maximum).toBe(ACT_MAX_VIEWPORT_DIMENSION);
expect(requestProperties.width.maximum).toBe(ACT_MAX_VIEWPORT_DIMENSION);
expect(requestProperties.height.maximum).toBe(ACT_MAX_VIEWPORT_DIMENSION);
});
});

View File

@@ -6,6 +6,7 @@ import {
stringEnum,
} from "openclaw/plugin-sdk/channel-actions";
import { Type } from "typebox";
import { ACT_MAX_VIEWPORT_DIMENSION } from "./browser/act-policy.js";
const BROWSER_ACT_KINDS = [
"click",
@@ -79,8 +80,8 @@ const BrowserActSchema = Type.Object({
// fill - use permissive array of objects
fields: Type.Optional(Type.Array(Type.Object({}, { additionalProperties: true }))),
// resize
width: optionalPositiveIntegerSchema(),
height: optionalPositiveIntegerSchema(),
width: optionalPositiveIntegerSchema({ maximum: ACT_MAX_VIEWPORT_DIMENSION }),
height: optionalPositiveIntegerSchema({ maximum: ACT_MAX_VIEWPORT_DIMENSION }),
// wait
timeMs: optionalNonNegativeIntegerSchema(),
selector: Type.Optional(Type.String()),
@@ -143,8 +144,8 @@ export const BrowserToolSchema = Type.Object({
endRef: Type.Optional(Type.String()),
values: Type.Optional(Type.Array(Type.String())),
fields: Type.Optional(Type.Array(Type.Object({}, { additionalProperties: true }))),
width: optionalPositiveIntegerSchema(),
height: optionalPositiveIntegerSchema(),
width: optionalPositiveIntegerSchema({ maximum: ACT_MAX_VIEWPORT_DIMENSION }),
height: optionalPositiveIntegerSchema({ maximum: ACT_MAX_VIEWPORT_DIMENSION }),
timeMs: optionalNonNegativeIntegerSchema(),
textGone: Type.Optional(Type.String()),
loadState: Type.Optional(Type.String()),

View File

@@ -2,6 +2,7 @@ export const ACT_MAX_BATCH_ACTIONS = 100;
export const ACT_MAX_BATCH_DEPTH = 5;
export const ACT_MAX_CLICK_DELAY_MS = 5_000;
export const ACT_MAX_WAIT_TIME_MS = 30_000;
export const ACT_MAX_VIEWPORT_DIMENSION = 8192;
const ACT_MIN_TIMEOUT_MS = 500;
const ACT_MAX_INTERACTION_TIMEOUT_MS = 60_000;

View File

@@ -218,6 +218,23 @@ describe("pw-tools-core aria snapshot storage", () => {
expect(page.setViewportSize).toHaveBeenCalledWith({ width: 1, height: 1 });
});
it("rejects excessive viewport dimensions before calling Playwright", async () => {
const page = { setViewportSize: vi.fn(async () => {}) };
getPageForTargetId.mockResolvedValue(page);
const mod = await import("./pw-tools-core.snapshot.js");
await expect(
mod.resizeViewportViaPlaywright({
cdpUrl: "http://127.0.0.1:9222",
targetId: "tab-1",
width: Number.MAX_SAFE_INTEGER,
height: 768,
}),
).rejects.toThrow("viewport width exceeds maximum of 8192");
expect(page.setViewportSize).not.toHaveBeenCalled();
});
it("stores role fallback metadata when backend markers are unavailable", async () => {
const page = { id: "page-1" };
const mod = await import("./pw-tools-core.snapshot.js");

View File

@@ -4,6 +4,7 @@ import {
normalizeOptionalString,
} from "openclaw/plugin-sdk/string-coerce-runtime";
import type { Page } from "playwright-core";
import { ACT_MAX_VIEWPORT_DIMENSION } from "./act-policy.js";
import type { SsrFPolicy } from "../infra/net/ssrf.js";
import { type AriaSnapshotNode, formatAriaSnapshot, type RawAXNode } from "./cdp.js";
import {
@@ -53,6 +54,14 @@ function resolveNavigationTimeoutMs(timeoutMs: number | undefined): number {
return resolveBoundedTimeoutMs(timeoutMs, 20_000, 1000, 120_000);
}
function resolveViewportDimension(value: unknown, label: "width" | "height"): number {
const dimension = resolveIntegerOption(value, 1, { min: 1 });
if (dimension > ACT_MAX_VIEWPORT_DIMENSION) {
throw new Error(`viewport ${label} exceeds maximum of ${ACT_MAX_VIEWPORT_DIMENSION}`);
}
return dimension;
}
async function collectSnapshotUrls(page: Page): Promise<SnapshotUrlEntry[]> {
const urls = await page
.evaluate(() => {
@@ -455,8 +464,8 @@ export async function resizeViewportViaPlaywright(opts: {
const page = await getPageForTargetId(opts);
ensurePageState(page);
await page.setViewportSize({
width: resolveIntegerOption(opts.width, 1, { min: 1 }),
height: resolveIntegerOption(opts.height, 1, { min: 1 }),
width: resolveViewportDimension(opts.width, "width"),
height: resolveViewportDimension(opts.height, "height"),
});
}

View File

@@ -1,6 +1,7 @@
import {
ACT_MAX_BATCH_ACTIONS,
ACT_MAX_CLICK_DELAY_MS,
ACT_MAX_VIEWPORT_DIMENSION,
ACT_MAX_WAIT_TIME_MS,
normalizeActBoundedNonNegativeMs,
} from "../act-policy.js";
@@ -261,6 +262,9 @@ export function normalizeActRequest(
if (width === undefined || height === undefined || width <= 0 || height <= 0) {
throw new Error("resize requires positive width and height");
}
if (width > ACT_MAX_VIEWPORT_DIMENSION || height > ACT_MAX_VIEWPORT_DIMENSION) {
throw new Error(`resize width and height must not exceed ${ACT_MAX_VIEWPORT_DIMENSION}`);
}
const targetId = toStringOrEmpty(body.targetId) || undefined;
return {
kind,

View File

@@ -278,6 +278,15 @@ describe("browser control server", () => {
expect(resizeNegative.error).toContain("resize requires positive width and height");
expect(pwMocks.resizeViewportViaPlaywright).toHaveBeenCalledTimes(1);
const resizeTooLarge = await postJson<{ error?: string; code?: string }>(`${base}/act`, {
kind: "resize",
width: 8193,
height: 600,
});
expect(resizeTooLarge.code).toBe("ACT_INVALID_REQUEST");
expect(resizeTooLarge.error).toContain("resize width and height must not exceed 8192");
expect(pwMocks.resizeViewportViaPlaywright).toHaveBeenCalledTimes(1);
const wait = await postJson<{ ok: boolean }>(`${base}/act`, {
kind: "wait",
timeMs: 5,

View File

@@ -48,4 +48,16 @@ describe("browser navigation commands", () => {
expect(capture.runtimeErrors.join("\n")).toContain("Invalid width: must be a positive integer");
expect(mocks.runBrowserResizeWithOutput).not.toHaveBeenCalled();
});
it("rejects excessive resize dimensions before dispatch", async () => {
const program = createNavigationProgram();
await expect(
program.parseAsync(["browser", "resize", "8193", "768"], { from: "user" }),
).rejects.toThrow("__exit__:1");
const capture = getBrowserCliRuntimeCapture();
expect(capture.runtimeErrors.join("\n")).toContain("Invalid width: maximum is 8192");
expect(mocks.runBrowserResizeWithOutput).not.toHaveBeenCalled();
});
});

View File

@@ -1,5 +1,6 @@
import type { Command } from "commander";
import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime";
import { ACT_MAX_VIEWPORT_DIMENSION } from "../../browser/act-policy.js";
import { runBrowserResizeWithOutput } from "../browser-cli-resize.js";
import { callBrowserRequest, type BrowserParentOpts } from "../browser-cli-shared.js";
import { danger, defaultRuntime } from "../core-api.js";
@@ -17,6 +18,11 @@ export function registerBrowserNavigationCommands(
defaultRuntime.exit(1);
return undefined;
}
if (parsed > ACT_MAX_VIEWPORT_DIMENSION) {
defaultRuntime.error(danger(`Invalid ${label}: maximum is ${ACT_MAX_VIEWPORT_DIMENSION}`));
defaultRuntime.exit(1);
return undefined;
}
return parsed;
};

View File

@@ -1,3 +1,4 @@
import { ACT_MAX_VIEWPORT_DIMENSION } from "../browser/act-policy.js";
import { callBrowserResize, type BrowserParentOpts } from "./browser-cli-shared.js";
import { danger, defaultRuntime } from "./core-api.js";
@@ -16,6 +17,13 @@ export async function runBrowserResizeWithOutput(params: {
defaultRuntime.exit(1);
return;
}
if (width > ACT_MAX_VIEWPORT_DIMENSION || height > ACT_MAX_VIEWPORT_DIMENSION) {
defaultRuntime.error(
danger(`width and height must not exceed ${ACT_MAX_VIEWPORT_DIMENSION}`),
);
defaultRuntime.exit(1);
return;
}
const result = await callBrowserResize(
params.parent,

View File

@@ -164,6 +164,14 @@ describe("browser state option collisions", () => {
expect(getBrowserCliRuntime().exit).toHaveBeenCalledWith(1);
});
it("rejects excessive viewport dimensions before resize dispatch", async () => {
await runBrowserCommand(["set", "viewport", "8193", "768"]);
expect(mocks.runBrowserResizeWithOutput).not.toHaveBeenCalled();
expectErrorMessage("Invalid width: maximum is 8192");
expect(getBrowserCliRuntime().exit).toHaveBeenCalledWith(1);
});
it("errors when set media receives an invalid value", async () => {
await runBrowserCommand(["set", "media", "sepia"]);

View File

@@ -4,6 +4,7 @@ import {
normalizeOptionalString,
} from "openclaw/plugin-sdk/string-coerce-runtime";
import { runCommandWithRuntime } from "../core-api.js";
import { ACT_MAX_VIEWPORT_DIMENSION } from "../browser/act-policy.js";
import { runBrowserResizeWithOutput } from "./browser-cli-resize.js";
import { callBrowserRequest, type BrowserParentOpts } from "./browser-cli-shared.js";
import { registerBrowserCookiesAndStorageCommands } from "./browser-cli-state.cookies-storage.js";
@@ -22,6 +23,11 @@ function parsePositiveInteger(value: unknown, label: string): number | undefined
defaultRuntime.exit(1);
return undefined;
}
if (parsed > ACT_MAX_VIEWPORT_DIMENSION) {
defaultRuntime.error(danger(`Invalid ${label}: maximum is ${ACT_MAX_VIEWPORT_DIMENSION}`));
defaultRuntime.exit(1);
return undefined;
}
return parsed;
}