From 90dbede0565080a270941fbb63275b1ece4cbb07 Mon Sep 17 00:00:00 2001 From: wanyongstar Date: Wed, 12 Aug 2026 01:38:29 +0800 Subject: [PATCH] fix(browser): bound snapshot tree rendering depth (#119217) Both snapshot renderers walked accessibility trees recursively with no default depth bound: the chrome-mcp builder only honored maxDepth when callers passed one, and the CDP renderRoleTree had no limit at all. A pathologically nested page could overflow the call stack and grow indent output quadratically before any output truncation ran. Add a generous hard depth bound (100) to both traversal paths. --- .../browser/src/browser/cdp.contract.test.ts | 126 +++++++++++++ extensions/browser/src/browser/cdp.test.ts | 168 +++--------------- extensions/browser/src/browser/cdp.ts | 42 +++-- .../browser/src/browser/chrome-mcp-routing.ts | 41 ++++- .../src/browser/chrome-mcp.snapshot-result.ts | 30 ++++ .../src/browser/chrome-mcp.snapshot.test.ts | 80 ++++++--- .../src/browser/chrome-mcp.snapshot.ts | 38 ++-- .../browser/src/browser/chrome-mcp.test.ts | 33 ++++ .../src/browser/client-actions-types.ts | 1 + extensions/browser/src/browser/client.ts | 1 + .../routes/agent.existing-session.test.ts | 50 +++++- .../src/browser/routes/agent.snapshot.ts | 24 ++- .../src/browser/snapshot-depth-limit.ts | 13 ++ 13 files changed, 446 insertions(+), 201 deletions(-) create mode 100644 extensions/browser/src/browser/cdp.contract.test.ts create mode 100644 extensions/browser/src/browser/chrome-mcp.snapshot-result.ts create mode 100644 extensions/browser/src/browser/snapshot-depth-limit.ts diff --git a/extensions/browser/src/browser/cdp.contract.test.ts b/extensions/browser/src/browser/cdp.contract.test.ts new file mode 100644 index 000000000000..50f44c73807a --- /dev/null +++ b/extensions/browser/src/browser/cdp.contract.test.ts @@ -0,0 +1,126 @@ +// Browser tests cover CDP URL and error contracts. +import { describe, expect, it } from "vitest"; +import { SsrFBlockedError } from "../infra/net/ssrf.js"; +import { + isDirectCdpWebSocketEndpoint, + isWebSocketUrl, + parseBrowserHttpUrl, +} from "./cdp.helpers.js"; +import { + BrowserCdpEndpointBlockedError, + BrowserValidationError, + toBrowserErrorResponse, +} from "./errors.js"; + +describe("browser error mapping", () => { + it("maps blocked browser targets to conflict responses", () => { + const err = new Error( + "Browser target is unavailable after SSRF policy blocked its navigation.", + ); + err.name = "BlockedBrowserTargetError"; + + expect(toBrowserErrorResponse(err)).toEqual({ + status: 409, + message: "Browser target is unavailable after SSRF policy blocked its navigation.", + }); + }); + + it("preserves BrowserError mappings", () => { + expect(toBrowserErrorResponse(new BrowserValidationError("bad input"))).toEqual({ + status: 400, + message: "bad input", + }); + }); + + it("sanitizes navigation-target SSRF policy errors without leaking raw policy details", () => { + expect( + toBrowserErrorResponse( + new SsrFBlockedError("Blocked hostname or private/internal/special-use IP address"), + ), + ).toEqual({ status: 400, message: "browser navigation blocked by policy" }); + }); + + it("maps CDP endpoint policy blocks to a distinct endpoint-scoped message", () => { + expect(toBrowserErrorResponse(new BrowserCdpEndpointBlockedError())).toEqual({ + status: 400, + message: "browser endpoint blocked by policy", + }); + }); +}); + +describe("isWebSocketUrl", () => { + it("recognizes ws and wss URLs", () => { + expect(isWebSocketUrl("ws://127.0.0.1:9222")).toBe(true); + expect(isWebSocketUrl("ws://example.com/devtools/browser/ABC")).toBe(true); + expect(isWebSocketUrl("wss://connect.example.com")).toBe(true); + expect(isWebSocketUrl("wss://connect.example.com?apiKey=abc")).toBe(true); + }); + + it("rejects other protocols and invalid input", () => { + expect(isWebSocketUrl("http://127.0.0.1:9222")).toBe(false); + expect(isWebSocketUrl("https://production-sfo.browserless.io?token=abc")).toBe(false); + expect(isWebSocketUrl("not-a-url")).toBe(false); + expect(isWebSocketUrl("")).toBe(false); + expect(isWebSocketUrl("ftp://example.com")).toBe(false); + }); +}); + +describe("isDirectCdpWebSocketEndpoint", () => { + it("recognizes ws/wss URLs with a /devtools// path", () => { + expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/devtools/browser/ABC")).toBe(true); + expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/devtools/page/42")).toBe(true); + expect(isDirectCdpWebSocketEndpoint("wss://connect.example.com/devtools/browser/xyz")).toBe( + true, + ); + expect( + isDirectCdpWebSocketEndpoint("wss://connect.example.com/devtools/browser/xyz?token=secret"), + ).toBe(true); + }); + + it("rejects bare ws/wss URLs that need discovery", () => { + // Reproduces the configuration shape reported in #68027. + expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222")).toBe(false); + expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/")).toBe(false); + expect(isDirectCdpWebSocketEndpoint("wss://browserless.example")).toBe(false); + expect(isDirectCdpWebSocketEndpoint("wss://browserless.example/?token=abc")).toBe(false); + }); + + it("rejects non-CDP paths, other protocols, and invalid input", () => { + expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/json/version")).toBe(false); + expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/devtools")).toBe(false); + expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/devtools/")).toBe(false); + expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/other/path")).toBe(false); + expect(isDirectCdpWebSocketEndpoint("http://127.0.0.1:9222/devtools/browser/ABC")).toBe(false); + expect(isDirectCdpWebSocketEndpoint("https://host/devtools/browser/ABC")).toBe(false); + expect(isDirectCdpWebSocketEndpoint("not-a-url")).toBe(false); + expect(isDirectCdpWebSocketEndpoint("")).toBe(false); + }); +}); + +describe("parseBrowserHttpUrl with WebSocket protocols", () => { + it("applies default ports", () => { + const secure = parseBrowserHttpUrl("wss://connect.example.com?apiKey=abc", "test"); + expect(secure.parsed.protocol).toBe("wss:"); + expect(secure.port).toBe(443); + expect(secure.normalized).toContain("wss://connect.example.com"); + + const insecure = parseBrowserHttpUrl("ws://127.0.0.1/devtools", "test"); + expect(insecure.parsed.protocol).toBe("ws:"); + expect(insecure.port).toBe(80); + }); + + it("preserves explicit and HTTP ports", () => { + expect(parseBrowserHttpUrl("wss://connect.example.com:8443/path", "test").port).toBe(8443); + expect(parseBrowserHttpUrl("http://127.0.0.1:9222", "test").port).toBe(9222); + expect(parseBrowserHttpUrl("https://browserless.example?token=abc", "test").port).toBe(443); + }); + + it("rejects unsupported protocols", () => { + expect(() => parseBrowserHttpUrl("ftp://example.com", "test")).toThrow( + "must be http(s) or ws(s)", + ); + expect(() => parseBrowserHttpUrl("file:///etc/passwd", "test")).toThrow( + "must be http(s) or ws(s)", + ); + }); +}); diff --git a/extensions/browser/src/browser/cdp.test.ts b/extensions/browser/src/browser/cdp.test.ts index ec968c496865..d3ea694d475e 100644 --- a/extensions/browser/src/browser/cdp.test.ts +++ b/extensions/browser/src/browser/cdp.test.ts @@ -7,28 +7,17 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { type WebSocket, WebSocketServer } from "ws"; import { SsrFBlockedError } from "../infra/net/ssrf.js"; import "../test-support/browser-security.mock.js"; -import { - closeTrackedCdpTarget, - isDirectCdpWebSocketEndpoint, - isWebSocketUrl, - parseBrowserHttpUrl as parseHttpUrl, - resolveCdpTabOwnership, -} from "./cdp.helpers.js"; +import { closeTrackedCdpTarget, resolveCdpTabOwnership } from "./cdp.helpers.js"; import { createTargetViaCdp, normalizeCdpWsUrl, snapshotAria, + snapshotRoleViaCdp, waitForCdpCommittedNavigationUrl, } from "./cdp.js"; -import { - BrowserCdpEndpointBlockedError, - BrowserValidationError, - toBrowserErrorResponse, -} from "./errors.js"; +import { BrowserCdpEndpointBlockedError } from "./errors.js"; import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js"; -const BROWSER_ENDPOINT_BLOCKED_MESSAGE = "browser endpoint blocked by policy"; -const BROWSER_NAVIGATION_BLOCKED_MESSAGE = "browser navigation blocked by policy"; const CDP_TEST_WS_MAX_PAYLOAD_BYTES = 1024 * 1024; describe("cdp", () => { @@ -860,6 +849,29 @@ describe("cdp", () => { expect(snap.nodes[1]?.depth).toBe(1); }); + it("hard-bounds CDP role rendering above a requested depth", async () => { + const nodes = Array.from({ length: 1_000 }, (_value, index) => ({ + nodeId: String(index), + role: { value: index === 0 ? "RootWebArea" : "generic" }, + name: { value: `n${index}` }, + childIds: index + 1 < 1_000 ? [String(index + 1)] : [], + })); + const wsPort = await startWsServerWithMessages((msg, socket) => { + if (msg.method === "Accessibility.getFullAXTree") { + socket.send(JSON.stringify({ id: msg.id, result: { nodes } })); + } + }); + + const snap = await snapshotRoleViaCdp({ + wsUrl: `ws://127.0.0.1:${wsPort}`, + options: { maxDepth: 50_000 }, + }); + expect(snap.snapshot).toContain("[...TRUNCATED - accessibility tree too deep]"); + const roleLines = snap.snapshot.split("\n").filter((line) => line.trimStart().startsWith("-")); + expect(roleLines).toHaveLength(101); + expect(snap.truncated).toBe(true); + }); + it("normalizes loopback websocket URLs for remote CDP hosts", () => { const normalized = normalizeCdpWsUrl( "ws://127.0.0.1:9222/devtools/browser/ABC", @@ -937,134 +949,6 @@ describe("cdp", () => { }); }); -describe("browser error mapping", () => { - it("maps blocked browser targets to conflict responses", () => { - const err = new Error( - "Browser target is unavailable after SSRF policy blocked its navigation.", - ); - err.name = "BlockedBrowserTargetError"; - - expect(toBrowserErrorResponse(err)).toEqual({ - status: 409, - message: "Browser target is unavailable after SSRF policy blocked its navigation.", - }); - }); - - it("preserves BrowserError mappings", () => { - expect(toBrowserErrorResponse(new BrowserValidationError("bad input"))).toEqual({ - status: 400, - message: "bad input", - }); - }); - - it("sanitizes navigation-target SSRF policy errors without leaking raw policy details", () => { - expect( - toBrowserErrorResponse( - new SsrFBlockedError("Blocked hostname or private/internal/special-use IP address"), - ), - ).toEqual({ - status: 400, - message: BROWSER_NAVIGATION_BLOCKED_MESSAGE, - }); - }); - - it("maps CDP endpoint policy blocks to a distinct endpoint-scoped message", () => { - expect(toBrowserErrorResponse(new BrowserCdpEndpointBlockedError())).toEqual({ - status: 400, - message: BROWSER_ENDPOINT_BLOCKED_MESSAGE, - }); - }); -}); - -describe("isWebSocketUrl", () => { - it("returns true for ws:// URLs", () => { - expect(isWebSocketUrl("ws://127.0.0.1:9222")).toBe(true); - expect(isWebSocketUrl("ws://example.com/devtools/browser/ABC")).toBe(true); - }); - - it("returns true for wss:// URLs", () => { - expect(isWebSocketUrl("wss://connect.example.com")).toBe(true); - expect(isWebSocketUrl("wss://connect.example.com?apiKey=abc")).toBe(true); - }); - - it("returns false for http:// and https:// URLs", () => { - expect(isWebSocketUrl("http://127.0.0.1:9222")).toBe(false); - expect(isWebSocketUrl("https://production-sfo.browserless.io?token=abc")).toBe(false); - }); - - it("returns false for invalid or non-URL strings", () => { - expect(isWebSocketUrl("not-a-url")).toBe(false); - expect(isWebSocketUrl("")).toBe(false); - expect(isWebSocketUrl("ftp://example.com")).toBe(false); - }); -}); - -describe("isDirectCdpWebSocketEndpoint", () => { - it("returns true for ws/wss URLs with a /devtools// path", () => { - expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/devtools/browser/ABC")).toBe(true); - expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/devtools/page/42")).toBe(true); - expect(isDirectCdpWebSocketEndpoint("wss://connect.example.com/devtools/browser/xyz")).toBe( - true, - ); - expect( - isDirectCdpWebSocketEndpoint("wss://connect.example.com/devtools/browser/xyz?token=secret"), - ).toBe(true); - }); - - it("returns false for bare ws/wss URLs without a /devtools/ path (needs discovery)", () => { - // Reproduces the configuration shape reported in #68027. - expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222")).toBe(false); - expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/")).toBe(false); - expect(isDirectCdpWebSocketEndpoint("wss://browserless.example")).toBe(false); - expect(isDirectCdpWebSocketEndpoint("wss://browserless.example/?token=abc")).toBe(false); - }); - - it("returns false for ws URLs whose path is not /devtools/*", () => { - expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/json/version")).toBe(false); - expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/devtools")).toBe(false); - expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/devtools/")).toBe(false); - expect(isDirectCdpWebSocketEndpoint("ws://127.0.0.1:9222/other/path")).toBe(false); - }); - - it("returns false for http/https URLs, invalid URLs, and empty strings", () => { - expect(isDirectCdpWebSocketEndpoint("http://127.0.0.1:9222/devtools/browser/ABC")).toBe(false); - expect(isDirectCdpWebSocketEndpoint("https://host/devtools/browser/ABC")).toBe(false); - expect(isDirectCdpWebSocketEndpoint("not-a-url")).toBe(false); - expect(isDirectCdpWebSocketEndpoint("")).toBe(false); - }); -}); - -describe("parseHttpUrl with WebSocket protocols", () => { - it("accepts wss:// URLs and defaults to port 443", () => { - const result = parseHttpUrl("wss://connect.example.com?apiKey=abc", "test"); - expect(result.parsed.protocol).toBe("wss:"); - expect(result.port).toBe(443); - expect(result.normalized).toContain("wss://connect.example.com"); - }); - - it("accepts ws:// URLs and defaults to port 80", () => { - const result = parseHttpUrl("ws://127.0.0.1/devtools", "test"); - expect(result.parsed.protocol).toBe("ws:"); - expect(result.port).toBe(80); - }); - - it("preserves explicit ports in wss:// URLs", () => { - const result = parseHttpUrl("wss://connect.example.com:8443/path", "test"); - expect(result.port).toBe(8443); - }); - - it("still accepts http:// and https:// URLs", () => { - const http = parseHttpUrl("http://127.0.0.1:9222", "test"); - expect(http.port).toBe(9222); - const https = parseHttpUrl("https://browserless.example?token=abc", "test"); - expect(https.port).toBe(443); - }); - - it("rejects unsupported protocols", () => { - expect(() => parseHttpUrl("ftp://example.com", "test")).toThrow("must be http(s) or ws(s)"); - expect(() => parseHttpUrl("file:///etc/passwd", "test")).toThrow("must be http(s) or ws(s)"); - }); -}); const proxyEnvKeys = [ "ALL_PROXY", "all_proxy", diff --git a/extensions/browser/src/browser/cdp.ts b/extensions/browser/src/browser/cdp.ts index 0892302ad6b4..bdef8c5be6be 100644 --- a/extensions/browser/src/browser/cdp.ts +++ b/extensions/browser/src/browser/cdp.ts @@ -28,6 +28,10 @@ import { } from "./cdp.helpers.js"; import { assertBrowserNavigationAllowed, withBrowserNavigationPolicy } from "./navigation-guard.js"; import { finalizeRoleSnapshot, type RoleSnapshotIdentityMode } from "./pw-role-snapshot.js"; +import { + appendRoleSnapshotDepthTruncationMarker, + ROLE_SNAPSHOT_MAX_DEPTH, +} from "./snapshot-depth-limit.js"; import { CONTENT_ROLES, INTERACTIVE_ROLES, STRUCTURAL_ROLES } from "./snapshot-roles.js"; export { appendCdpPath } from "./cdp.helpers.js"; @@ -533,9 +537,6 @@ function buildRoleTree(nodes: RawAXNode[]): { tree: RoleTreeNode[]; roots: numbe function shouldIncludeRoleNode(node: RoleTreeNode, options: CdpRoleSnapshotOptions): boolean { const role = node.role.toLowerCase(); - if (options.maxDepth !== undefined && node.depth > options.maxDepth) { - return false; - } if (options.interactive) { return INTERACTIVE_ROLES.has(role) || role === "iframe" || Boolean(node.cursorInfo); } @@ -572,14 +573,23 @@ function renderRoleTree( index: number, output: string[], options: CdpRoleSnapshotOptions, + state: { truncated: boolean }, indentOffset = 0, ): void { const node = tree[index]; if (!node) { return; } + if (options.maxDepth !== undefined && node.depth > options.maxDepth) { + return; + } + const effectiveDepth = Math.max(0, node.depth + indentOffset); + if (effectiveDepth > ROLE_SNAPSHOT_MAX_DEPTH) { + state.truncated = true; + return; + } if (shouldIncludeRoleNode(node, options)) { - const indent = " ".repeat(Math.max(0, node.depth + indentOffset)); + const indent = " ".repeat(effectiveDepth); const name = node.name ? ` "${escapeRoleSnapshotValue(node.name)}"` : ""; const ref = node.ref ? ` [ref=${node.ref}]` : ""; const nth = node.nth !== undefined && node.nth > 0 ? ` [nth=${node.nth}]` : ""; @@ -590,7 +600,7 @@ function renderRoleTree( ); } for (const child of node.children) { - renderRoleTree(tree, child, output, options, indentOffset); + renderRoleTree(tree, child, output, options, state, indentOffset); } } @@ -778,6 +788,7 @@ async function buildCdpRoleSnapshot(params: { }): Promise<{ lines: string[]; refs: Record; + truncated: boolean; }> { const res = (await params.send( "Accessibility.getFullAXTree", @@ -866,8 +877,9 @@ async function buildCdpRoleSnapshot(params: { } const lines: string[] = []; + const renderState = { truncated: false }; for (const root of roots) { - renderRoleTree(tree, root, lines, params.options); + renderRoleTree(tree, root, lines, params.options, renderState); } if (params.recurseIframes) { @@ -883,7 +895,11 @@ async function buildCdpRoleSnapshot(params: { frameId: iframe.frameId, recurseIframes: false, }).catch(() => null); - if (!child?.lines.length) { + if (!child) { + continue; + } + renderState.truncated ||= child.truncated; + if (!child.lines.length) { continue; } Object.assign(refs, child.refs); @@ -894,6 +910,7 @@ async function buildCdpRoleSnapshot(params: { return { lines, refs, + truncated: renderState.truncated, }; } @@ -923,15 +940,20 @@ export async function snapshotRoleViaCdp(opts: { recurseIframes: true, nextRef: { value: 1 }, }); - const snapshot = + const renderedSnapshot = built.lines.join("\n").trim() || (opts.options?.interactive ? "(no interactive elements)" : "(empty page)"); - return finalizeRoleSnapshot({ - snapshot, + const finalized = finalizeRoleSnapshot({ + snapshot: built.truncated + ? appendRoleSnapshotDepthTruncationMarker(renderedSnapshot) + : renderedSnapshot, refs: built.refs, maxChars: opts.maxChars, delta: opts.delta, }); + return built.truncated && !finalized.truncated + ? { ...finalized, truncated: true } + : finalized; }, { commandTimeoutMs: opts.timeoutMs ?? 5000 }, ); diff --git a/extensions/browser/src/browser/chrome-mcp-routing.ts b/extensions/browser/src/browser/chrome-mcp-routing.ts index a0a94826f88c..109dd91538a3 100644 --- a/extensions/browser/src/browser/chrome-mcp-routing.ts +++ b/extensions/browser/src/browser/chrome-mcp-routing.ts @@ -158,7 +158,7 @@ export function wrapChromeMcpSnapshotRefs( clearChromeMcpSnapshotRefsForTarget(routing, targetId); const wrappedByUid = new Map(); - const visit = (node: ChromeMcpSnapshotNode): ChromeMcpSnapshotNode => { + const wrapNode = (node: ChromeMcpSnapshotNode): ChromeMcpSnapshotNode => { const rawUid = normalizeOptionalString(node.id); let id: string | undefined; if (rawUid) { @@ -173,11 +173,46 @@ export function wrapChromeMcpSnapshotRefs( return { ...node, ...(id ? { id } : {}), - ...(node.children ? { children: node.children.map(visit) } : {}), + ...(node.children ? { children: [] } : {}), }; }; - return visit(root); + // Ref rewriting is the first traversal of external MCP output. Keep it + // iterative so the renderer can own depth truncation and report that fact. + let wrappedRoot: ChromeMcpSnapshotNode | undefined; + const stack: Array<{ + source: ChromeMcpSnapshotNode; + parent?: ChromeMcpSnapshotNode[]; + index?: number; + }> = [{ source: root }]; + while (stack.length > 0) { + const current = stack.pop(); + if (!current) { + break; + } + const wrapped = wrapNode(current.source); + if (current.parent && current.index !== undefined) { + current.parent[current.index] = wrapped; + } else { + wrappedRoot = wrapped; + } + const sourceChildren = current.source.children; + if (!sourceChildren) { + continue; + } + const wrappedChildren: ChromeMcpSnapshotNode[] = []; + wrapped.children = wrappedChildren; + for (let index = sourceChildren.length - 1; index >= 0; index -= 1) { + const child = sourceChildren[index]; + if (child) { + stack.push({ source: child, parent: wrappedChildren, index }); + } + } + } + if (!wrappedRoot) { + throw new Error("Chrome MCP snapshot did not contain a root node"); + } + return wrappedRoot; } export function resolveChromeMcpSnapshotRef( diff --git a/extensions/browser/src/browser/chrome-mcp.snapshot-result.ts b/extensions/browser/src/browser/chrome-mcp.snapshot-result.ts new file mode 100644 index 000000000000..f06c399596e4 --- /dev/null +++ b/extensions/browser/src/browser/chrome-mcp.snapshot-result.ts @@ -0,0 +1,30 @@ +import { + buildAiSnapshotFromChromeMcpSnapshot, + type ChromeMcpSnapshotNode, + flattenChromeMcpSnapshotToAriaResult, +} from "./chrome-mcp.snapshot.js"; +// Route-facing Chrome MCP snapshot results preserve automatic truncation facts. +import type { SnapshotAriaNode } from "./client.types.js"; +import type { RoleRefMap, RoleSnapshotOptions } from "./pw-role-snapshot.js"; +import { appendRoleSnapshotDepthTruncationMarker } from "./snapshot-depth-limit.js"; + +export function buildChromeMcpRouteSnapshot(params: { + root: ChromeMcpSnapshotNode; + options?: RoleSnapshotOptions; +}): { snapshot: string; refs: RoleRefMap; truncated?: true } { + const built = buildAiSnapshotFromChromeMcpSnapshot(params); + return built.truncated + ? { + ...built, + snapshot: appendRoleSnapshotDepthTruncationMarker(built.snapshot), + truncated: true as const, + } + : built; +} + +export function flattenChromeMcpRouteSnapshot( + root: ChromeMcpSnapshotNode, + limit = 500, +): { nodes: SnapshotAriaNode[]; truncated?: true } { + return flattenChromeMcpSnapshotToAriaResult(root, limit); +} diff --git a/extensions/browser/src/browser/chrome-mcp.snapshot.test.ts b/extensions/browser/src/browser/chrome-mcp.snapshot.test.ts index ade98b1a8730..37eae9674a73 100644 --- a/extensions/browser/src/browser/chrome-mcp.snapshot.test.ts +++ b/extensions/browser/src/browser/chrome-mcp.snapshot.test.ts @@ -2,8 +2,9 @@ import { describe, expect, it } from "vitest"; import { buildAiSnapshotFromChromeMcpSnapshot, - flattenChromeMcpSnapshotToAriaNodes, + flattenChromeMcpSnapshotToAriaResult, } from "./chrome-mcp.snapshot.js"; +import type { ChromeMcpSnapshotNode } from "./chrome-mcp.snapshot.js"; import { finalizeRoleSnapshot } from "./pw-role-snapshot.js"; import { appendSnapshotUrls } from "./snapshot-urls.js"; @@ -28,33 +29,35 @@ const snapshot = { describe("chrome MCP snapshot conversion", () => { it("flattens structured snapshots into aria-style nodes", () => { - const nodes = flattenChromeMcpSnapshotToAriaNodes(snapshot, 10); - expect(nodes).toEqual([ - { - ref: "root", - role: "document", - name: "Example", - value: undefined, - description: undefined, - depth: 0, - }, - { - ref: "btn-1", - role: "button", - name: "Continue", - value: undefined, - description: undefined, - depth: 1, - }, - { - ref: "txt-1", - role: "textbox", - name: "Email", - value: "peter@example.com", - description: undefined, - depth: 1, - }, - ]); + const result = flattenChromeMcpSnapshotToAriaResult(snapshot, 10); + expect(result).toEqual({ + nodes: [ + { + ref: "root", + role: "document", + name: "Example", + value: undefined, + description: undefined, + depth: 0, + }, + { + ref: "btn-1", + role: "button", + name: "Continue", + value: undefined, + description: undefined, + depth: 1, + }, + { + ref: "txt-1", + role: "textbox", + name: "Email", + value: "peter@example.com", + description: undefined, + depth: 1, + }, + ], + }); }); it("builds AI snapshots that preserve Chrome MCP uids as refs", () => { @@ -112,4 +115,25 @@ describe("chrome MCP snapshot conversion", () => { visible: { role: "button", name: "Visible\n- button [ref=hidden]" }, }); }); + + it("bounds traversal of pathologically deep snapshot trees", () => { + // A page can nest DOM tens of thousands of levels deep; traversal must hit + // the depth bound instead of overflowing the stack or exploding indents. + let root: ChromeMcpSnapshotNode = { id: "leaf", role: "text", name: "leaf" }; + for (let index = 0; index < 50_000; index += 1) { + root = { id: `n${index}`, role: "generic", name: `n${index}`, children: [root] }; + } + + for (const options of [undefined, { maxDepth: 50_000 }]) { + const built = buildAiSnapshotFromChromeMcpSnapshot({ root, options }); + expect(built.snapshot.length).toBeGreaterThan(0); + expect(built.snapshot.split("\n").length).toBeLessThanOrEqual(101); + expect(built.truncated).toBe(true); + } + + const flattened = flattenChromeMcpSnapshotToAriaResult(root); + expect(flattened.nodes.length).toBeGreaterThan(0); + expect(Math.max(...flattened.nodes.map((node) => node.depth))).toBeLessThanOrEqual(100); + expect(flattened.truncated).toBe(true); + }); }); diff --git a/extensions/browser/src/browser/chrome-mcp.snapshot.ts b/extensions/browser/src/browser/chrome-mcp.snapshot.ts index 72e79b9e1bf3..a084fd197a5d 100644 --- a/extensions/browser/src/browser/chrome-mcp.snapshot.ts +++ b/extensions/browser/src/browser/chrome-mcp.snapshot.ts @@ -7,6 +7,7 @@ import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; import type { SnapshotAriaNode } from "./client.types.js"; import type { RoleRefMap, RoleSnapshotOptions } from "./pw-role-snapshot.js"; +import { ROLE_SNAPSHOT_MAX_DEPTH } from "./snapshot-depth-limit.js"; import { CONTENT_ROLES, INTERACTIVE_ROLES, STRUCTURAL_ROLES } from "./snapshot-roles.js"; /** Structured snapshot node shape returned by chrome-devtools-mcp. */ @@ -88,16 +89,22 @@ function registerRef( return undefined; } -/** Flatten a Chrome MCP snapshot tree into OpenClaw ARIA-style nodes. */ -export function flattenChromeMcpSnapshotToAriaNodes( +/** Build ARIA nodes while preserving whether a traversal ceiling omitted input. */ +export function flattenChromeMcpSnapshotToAriaResult( root: ChromeMcpSnapshotNode, limit = 500, -): SnapshotAriaNode[] { +): { nodes: SnapshotAriaNode[]; truncated?: true } { const boundedLimit = Math.max(1, Math.min(2000, Math.floor(limit))); const out: SnapshotAriaNode[] = []; + let truncated = false; const visit = (node: ChromeMcpSnapshotNode, depth: number) => { if (out.length >= boundedLimit) { + truncated = true; + return; + } + if (depth > ROLE_SNAPSHOT_MAX_DEPTH) { + truncated = true; return; } const ref = normalizeSnapshotString(node.id); @@ -111,16 +118,18 @@ export function flattenChromeMcpSnapshotToAriaNodes( depth, }); } - for (const child of node.children ?? []) { + const children = node.children ?? []; + for (const [index, child] of children.entries()) { visit(child, depth + 1); if (out.length >= boundedLimit) { + truncated ||= index + 1 < children.length; return; } } }; visit(root, 0); - return out; + return truncated ? { nodes: out, truncated: true } : { nodes: out }; } /** Build a compact text snapshot and ref map from a Chrome MCP snapshot tree. */ @@ -130,20 +139,28 @@ export function buildAiSnapshotFromChromeMcpSnapshot(params: { }): { snapshot: string; refs: RoleRefMap; + truncated?: true; } { const refs: RoleRefMap = {}; const tracker = createDuplicateTracker(); const lines: string[] = []; + const maxDepth = Math.min( + params.options?.maxDepth ?? ROLE_SNAPSHOT_MAX_DEPTH, + ROLE_SNAPSHOT_MAX_DEPTH, + ); + const hardLimitApplied = + params.options?.maxDepth === undefined || params.options.maxDepth >= ROLE_SNAPSHOT_MAX_DEPTH; + let truncated = false; const visit = (node: ChromeMcpSnapshotNode, depth: number) => { + if (depth > maxDepth) { + truncated ||= hardLimitApplied; + return; + } const role = normalizeRole(node); const name = normalizeSnapshotString(node.name); const value = normalizeSnapshotString(node.value); const description = normalizeSnapshotString(node.description); - const maxDepth = params.options?.maxDepth; - if (maxDepth !== undefined && depth > maxDepth) { - return; - } const includeNode = shouldIncludeNode({ role, name, options: params.options }); if (includeNode) { @@ -180,5 +197,6 @@ export function buildAiSnapshotFromChromeMcpSnapshot(params: { } } - return { snapshot: lines.join("\n"), refs }; + const result = { snapshot: lines.join("\n"), refs }; + return truncated ? { ...result, truncated: true } : result; } diff --git a/extensions/browser/src/browser/chrome-mcp.test.ts b/extensions/browser/src/browser/chrome-mcp.test.ts index 4d80ec9f63ca..ea9fba037185 100644 --- a/extensions/browser/src/browser/chrome-mcp.test.ts +++ b/extensions/browser/src/browser/chrome-mcp.test.ts @@ -33,6 +33,7 @@ import { uploadChromeMcpFile, withChromeMcpDocument, } from "./chrome-mcp.js"; +import type { ChromeMcpSnapshotNode } from "./chrome-mcp.snapshot.js"; type ToolCall = { name: string; @@ -973,6 +974,38 @@ describe("chrome MCP page parsing", () => { expect(clickedUids).toEqual(["1_2", "1_2"]); }); + it("wraps deeply nested snapshot refs without recursive traversal", async () => { + let root: ChromeMcpSnapshotNode = { id: "leaf", role: "text", name: "leaf" }; + for (let index = 0; index < 50_000; index += 1) { + root = { + id: `n${index}`, + role: "generic", + name: `n${index}`, + children: [root], + }; + } + const session = createPageSession({ + pid: 141, + pages: [{ id: 1, url: "https://a.example" }], + onTool: (call) => + call.name === "take_snapshot" ? { structuredContent: { snapshot: root } } : undefined, + }); + setChromeMcpSessionFactoryForTest(async () => session); + + const [target] = await listChromeMcpTabs("chrome-live"); + let node = await takeChromeMcpSnapshot({ + profileName: "chrome-live", + targetId: target?.targetId ?? "", + }); + let depth = 0; + while (node.children?.[0]) { + node = node.children[0]; + depth += 1; + } + expect(depth).toBe(50_000); + expect(node.id).toMatch(/^mcp-ref:/); + }); + it("unwraps current snapshot refs for every ref-scoped MCP adapter", async () => { const session = createPageSession({ pid: 141, diff --git a/extensions/browser/src/browser/client-actions-types.ts b/extensions/browser/src/browser/client-actions-types.ts index 54aa8c0a1ca3..119e909ec58b 100644 --- a/extensions/browser/src/browser/client-actions-types.ts +++ b/extensions/browser/src/browser/client-actions-types.ts @@ -40,6 +40,7 @@ export type BrowserActionPathResult = { labels?: boolean; labelsCount?: number; labelsSkipped?: number; + truncated?: boolean; /** * Per-ref bounding boxes when labels=true. Coordinates are in the * captured image's space (viewport / fullpage / element-relative). diff --git a/extensions/browser/src/browser/client.ts b/extensions/browser/src/browser/client.ts index 496300f7cece..8011d01ad9b6 100644 --- a/extensions/browser/src/browser/client.ts +++ b/extensions/browser/src/browser/client.ts @@ -130,6 +130,7 @@ export type SnapshotResult = targetId: string; url: string; nodes: SnapshotAriaNode[]; + truncated?: boolean; blockedByDialog?: boolean; browserState?: unknown; } diff --git a/extensions/browser/src/browser/routes/agent.existing-session.test.ts b/extensions/browser/src/browser/routes/agent.existing-session.test.ts index 98413aad88b3..f2ceb2b4c932 100644 --- a/extensions/browser/src/browser/routes/agent.existing-session.test.ts +++ b/extensions/browser/src/browser/routes/agent.existing-session.test.ts @@ -1,6 +1,7 @@ // Browser tests cover agent.existing session plugin behavior. import { createRequireRecord } from "openclaw/plugin-sdk/test-fixtures"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { ChromeMcpSnapshotNode } from "../chrome-mcp.snapshot.js"; import { EXISTING_SESSION_LIMITS } from "./existing-session-limits.js"; import { createExistingSessionAgentSharedModule, @@ -20,7 +21,7 @@ const chromeMcpMocks = vi.hoisted(() => ({ fillChromeMcpElement: vi.fn(async () => {}), navigateChromeMcpPage: vi.fn(async ({ url }: { url: string }) => ({ url })), takeChromeMcpScreenshot: vi.fn(async () => Buffer.from("png")), - takeChromeMcpSnapshot: vi.fn(async () => ({ + takeChromeMcpSnapshot: vi.fn<() => Promise>(async () => ({ id: "root", role: "document", name: "Example", @@ -270,6 +271,53 @@ describe("existing-session browser routes", () => { expect(renderParams.fn).not.toContain('"btn-2"'); }); + it("reports automatic Chrome MCP depth truncation through AI and ARIA routes", async () => { + let root: ChromeMcpSnapshotNode = { id: "leaf", role: "text", name: "leaf" }; + for (let index = 0; index < 1_000; index += 1) { + root = { id: `n${index}`, role: "generic", name: `n${index}`, children: [root] }; + } + chromeMcpMocks.takeChromeMcpSnapshot + .mockResolvedValueOnce(root) + .mockResolvedValueOnce(root) + .mockResolvedValueOnce(root); + const handler = getSnapshotGetHandler(); + + const ai = createBrowserRouteResponse(); + await handler?.({ params: {}, query: { format: "ai" } }, ai.res); + const aiBody = requireRecord(ai.body, "AI snapshot body"); + expect(aiBody.truncated).toBe(true); + expect(aiBody.snapshot).toContain("[...TRUNCATED - accessibility tree too deep]"); + + const aria = createBrowserRouteResponse(); + await handler?.({ params: {}, query: { format: "aria" } }, aria.res); + const ariaBody = requireRecord(aria.body, "ARIA snapshot body"); + expect(ariaBody.truncated).toBe(true); + expect(ariaBody.nodes).toHaveLength(101); + + const requestedDepth = createBrowserRouteResponse(); + await handler?.({ params: {}, query: { format: "ai", depth: "5" } }, requestedDepth.res); + const requestedDepthBody = requireRecord(requestedDepth.body, "requested-depth snapshot body"); + expect(requestedDepthBody.truncated).toBeUndefined(); + expect(requestedDepthBody.snapshot).not.toContain("TRUNCATED"); + }); + + it("reports automatic Chrome MCP depth truncation on labeled screenshots", async () => { + let root: ChromeMcpSnapshotNode = { id: "leaf", role: "text", name: "leaf" }; + for (let index = 0; index < 1_000; index += 1) { + root = { id: `n${index}`, role: "generic", name: `n${index}`, children: [root] }; + } + chromeMcpMocks.takeChromeMcpSnapshot.mockResolvedValueOnce(root); + const handler = getSnapshotPostHandler(); + const response = createBrowserRouteResponse(); + + await handler?.({ params: {}, query: {}, body: { labels: true } }, response.res); + + expect(response.statusCode).toBe(200); + const body = requireRecord(response.body, "labeled screenshot body"); + expect(body.labels).toBe(true); + expect(body.truncated).toBe(true); + }); + it("allows ref screenshots for existing-session profiles", async () => { const handler = getSnapshotPostHandler(); const response = createBrowserRouteResponse(); diff --git a/extensions/browser/src/browser/routes/agent.snapshot.ts b/extensions/browser/src/browser/routes/agent.snapshot.ts index 91784b91bfff..df53ce925192 100644 --- a/extensions/browser/src/browser/routes/agent.snapshot.ts +++ b/extensions/browser/src/browser/routes/agent.snapshot.ts @@ -24,9 +24,9 @@ import { type ChromeMcpProfileOptions, } from "../chrome-mcp.js"; import { - buildAiSnapshotFromChromeMcpSnapshot, - flattenChromeMcpSnapshotToAriaNodes, -} from "../chrome-mcp.snapshot.js"; + buildChromeMcpRouteSnapshot, + flattenChromeMcpRouteSnapshot, +} from "../chrome-mcp.snapshot-result.js"; import { DEFAULT_BROWSER_SCREENSHOT_TIMEOUT_MS } from "../constants.js"; import { assertBrowserNavigationAllowed, @@ -204,6 +204,7 @@ async function saveNormalizedScreenshotResponse(params: { labels?: boolean; labelsCount?: number; labelsSkipped?: number; + truncated?: boolean; annotations?: AnnotationItem[]; }) { // Measure original dimensions BEFORE normalization so we can rescale @@ -232,6 +233,7 @@ async function saveNormalizedScreenshotResponse(params: { labels: params.labels, labelsCount: params.labelsCount, labelsSkipped: params.labelsSkipped, + truncated: params.truncated, annotations, }); } @@ -275,6 +277,7 @@ async function saveBrowserMediaResponse(params: { labels?: boolean; labelsCount?: number; labelsSkipped?: number; + truncated?: boolean; annotations?: AnnotationItem[]; }) { await ensureMediaDir(); @@ -292,6 +295,7 @@ async function saveBrowserMediaResponse(params: { ...(params.labels ? { labels: true } : {}), ...(typeof params.labelsCount === "number" ? { labelsCount: params.labelsCount } : {}), ...(typeof params.labelsSkipped === "number" ? { labelsSkipped: params.labelsSkipped } : {}), + ...(params.truncated ? { truncated: true } : {}), ...(params.annotations && params.annotations.length > 0 ? { annotations: params.annotations } : {}), @@ -466,7 +470,7 @@ export function registerBrowserAgentSnapshotRoutes( } if (labels) { const snapshot = await takeChromeMcpSnapshot(operation); - const built = buildAiSnapshotFromChromeMcpSnapshot({ root: snapshot }); + const built = buildChromeMcpRouteSnapshot({ root: snapshot }); const labelResult = await renderChromeMcpLabels({ ...operation, refs: Object.keys(built.refs), @@ -486,6 +490,7 @@ export function registerBrowserAgentSnapshotRoutes( labels: true, labelsCount: labelResult.labels, labelsSkipped: labelResult.skipped, + truncated: built.truncated, }); } finally { await clearChromeMcpOverlay(operation); @@ -680,16 +685,17 @@ export function registerBrowserAgentSnapshotRoutes( }; const snapshot = await takeChromeMcpSnapshot(operation); if (plan.format === "aria") { + const flattened = flattenChromeMcpRouteSnapshot(snapshot, plan.limit); return res.json({ ok: true, format: "aria", targetId: tab.targetId, url: tab.url, - nodes: flattenChromeMcpSnapshotToAriaNodes(snapshot, plan.limit), + ...flattened, }); } const deltaState = createDeltaState(); - const built = buildAiSnapshotFromChromeMcpSnapshot({ + const built = buildChromeMcpRouteSnapshot({ root: snapshot, options: { interactive: plan.interactive ?? undefined, @@ -706,11 +712,15 @@ export function registerBrowserAgentSnapshotRoutes( ), } : built; - const finalized = finalizeRoleSnapshot({ + const finalizedBase = finalizeRoleSnapshot({ ...builtWithUrls, maxChars: plan.resolvedMaxChars, delta: deltaState.delta, }); + const finalized = + built.truncated && !finalizedBase.truncated + ? { ...finalizedBase, truncated: true } + : finalizedBase; if (plan.labels) { const refs = Object.keys(finalized.refs); const labelResult = await renderChromeMcpLabels({ diff --git a/extensions/browser/src/browser/snapshot-depth-limit.ts b/extensions/browser/src/browser/snapshot-depth-limit.ts new file mode 100644 index 000000000000..7b91cd2e5a60 --- /dev/null +++ b/extensions/browser/src/browser/snapshot-depth-limit.ts @@ -0,0 +1,13 @@ +/** + * Hard ceiling for recursive accessibility-tree rendering. This bounds both + * call-stack use and quadratic indentation growth on external tree input. + */ +export const ROLE_SNAPSHOT_MAX_DEPTH = 100; + +const ROLE_SNAPSHOT_DEPTH_TRUNCATION_MARKER = "[...TRUNCATED - accessibility tree too deep]"; + +export function appendRoleSnapshotDepthTruncationMarker(snapshot: string): string { + return snapshot + ? `${snapshot}\n\n${ROLE_SNAPSHOT_DEPTH_TRUNCATION_MARKER}` + : ROLE_SNAPSHOT_DEPTH_TRUNCATION_MARKER; +}