From 4cc044be68f813e96907e6cedcf422d0a3170de3 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 16 Oct 2026 05:17:50 +0800 Subject: [PATCH] fix(browser): retain malformed inventory uncertainty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Distinguish missing CDP inventories from observed empty arrays, retain valid accessibility and worker siblings, and admit required fields before scope inference. Stop interpreting DOM documents without a usable string table. Validated producer semantics and simulated CDP provider boundaries; this does not establish real Chrome or Electron engine behavior. Refs #3376, #3379, #3850. Co-authored-by: 李云周 <154221037+YunzhouLi-hub@users.noreply.github.com> Co-authored-by: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> --- src/browser/CdpCaptureDocuments.test.ts | 88 +++++++++++- src/browser/CdpCaptureDocuments.ts | 58 +++++++- src/browser/CdpCaptureValues.ts | 4 + src/browser/CdpElectronInspection.ts | 11 +- src/browser/CdpPageCapture.ts | 4 +- src/browser/CdpPageCaptureWorkers.test.ts | 127 ++++++++++++++++++ src/browser/CdpPageCaptureWorkers.ts | 65 +++++++-- .../browser/cdpElectronProvider.test.ts | 17 +++ 8 files changed, 349 insertions(+), 25 deletions(-) create mode 100644 src/browser/CdpPageCaptureWorkers.test.ts diff --git a/src/browser/CdpCaptureDocuments.test.ts b/src/browser/CdpCaptureDocuments.test.ts index 5993e7e27..124ceb176 100644 --- a/src/browser/CdpCaptureDocuments.test.ts +++ b/src/browser/CdpCaptureDocuments.test.ts @@ -299,6 +299,90 @@ describe("CDP frame capture", () => { }); describe("CDP accessibility capture", () => { + it("keeps an explicit empty accessibility inventory complete", () => { + const completeness = new CdpCaptureCompleteness(); + const capture = captureAccessibility([{ nodes: [] }], { + includeText: true, + completeness, + }); + + expect(capture).toMatchObject({ + total: 0, + nodes: [], + hasUnobservedChildren: false, + }); + expect(completeness.snapshot()).toMatchObject({ + unavailable_sections: [], + excluded: [], + }); + }); + + it.each([undefined, { length: 1 }])( + "keeps an omitted or non-array node inventory unknown: %s", + (nodes) => { + const completeness = new CdpCaptureCompleteness(); + const capture = captureAccessibility([{ nodes }], { + includeText: true, + completeness, + }); + + expect(capture).toMatchObject({ + total: 0, + nodes: [], + hasUnobservedChildren: false, + }); + expect(completeness.snapshot()).toMatchObject({ + unavailable_sections: ["accessibility"], + excluded: [ + { + section: "accessibility", + reason: "invalid_protocol_value", + count: null, + }, + ], + }); + }, + ); + + it("retains valid nodes and counts malformed inventory entries", () => { + const completeness = new CdpCaptureCompleteness(); + const capture = captureAccessibility( + [ + { + nodes: [ + null, + {}, + { nodeId: "missing-ignored" }, + { nodeId: "invalid-ignored", ignored: "false" }, + { nodeId: "invalid-parent", ignored: false, parentId: 3 }, + { nodeId: "invalid-children", ignored: false, childIds: "bad" }, + { nodeId: "node-1", ignored: false, role: { value: "button" } }, + "malformed", + ], + }, + ], + { includeText: true, completeness }, + ); + + expect(capture.total).toBe(3); + expect(capture.nodes.map((node) => node.node_id)).toEqual([ + "invalid-parent", + "invalid-children", + "node-1", + ]); + expect(capture.nodes[2]).toMatchObject({ role: "button", ignored: false }); + expect(completeness.snapshot()).toMatchObject({ + unavailable_sections: ["accessibility"], + excluded: [ + { + section: "accessibility", + reason: "invalid_protocol_value", + count: 7, + }, + ], + }); + }); + it("retains full approved accessibility text", () => { const capture = captureAccessibility( [ @@ -306,6 +390,7 @@ describe("CDP accessibility capture", () => { nodes: [ { nodeId: "node-1", + ignored: false, role: { value: "button" }, name: { value: "😀a" }, description: { value: "éé" }, @@ -328,7 +413,7 @@ describe("CDP accessibility capture", () => { description: "éé", states: [{ name: "disabled", value: true }], }); - expect(capture.treeIncomplete).toBe(true); + expect(capture.hasUnobservedChildren).toBe(true); expect(capture.textCapture).toEqual({ status: "included", retained_bytes: 9, @@ -343,6 +428,7 @@ describe("CDP accessibility capture", () => { nodes: [ { nodeId: "node-1", + ignored: false, name: { value: "private name" }, description: { value: "private description" }, }, diff --git a/src/browser/CdpCaptureDocuments.ts b/src/browser/CdpCaptureDocuments.ts index 5dcbfd4a4..f6f8b5545 100644 --- a/src/browser/CdpCaptureDocuments.ts +++ b/src/browser/CdpCaptureDocuments.ts @@ -2,6 +2,7 @@ import type { InspectWebPageInput } from "../domain/browserObservation.js"; import type { WebPageInspection } from "../domain/browserObservationSchemas.js"; import { allowedSanitizedUrl, + admitRecords, numberValue, recordValue, recordsValue, @@ -505,16 +506,63 @@ export const captureAccessibility = ( options: { readonly includeText: boolean; readonly unavailable?: boolean; + readonly completeness?: CdpCaptureCompleteness; }, ): { readonly total: number; readonly nodes: WebPageInspection["accessibility"]["nodes"]; readonly textCapture: WebPageInspection["accessibility"]["text_capture"]; - readonly treeIncomplete: boolean; + readonly hasUnobservedChildren: boolean; } => { - const all = results.flatMap((result) => - recordsValue(requiredRecord(result).nodes), - ); + const all = results.flatMap((result) => { + const inventory = admitRecords(requiredRecord(result).nodes); + if (inventory === undefined) { + options.completeness?.exclude( + "accessibility", + "invalid_protocol_value", + null, + ); + return []; + } + const nodes: UnknownRecord[] = []; + let malformedCount = 0; + for (const entry of inventory) { + const node = recordValue(entry); + if ( + node === undefined || + cdpStringValue(node.nodeId) === undefined || + cdpStringValue(node.nodeId)?.length === 0 || + typeof node.ignored !== "boolean" + ) { + malformedCount += 1; + continue; + } + const parentId = cdpStringValue(node.parentId); + if ( + node.parentId !== undefined && + (parentId === undefined || parentId.length === 0) + ) + malformedCount += 1; + if ( + node.childIds !== undefined && + (!Array.isArray(node.childIds) || + node.childIds.some( + (childId) => + cdpStringValue(childId) === undefined || + cdpStringValue(childId)?.length === 0, + )) + ) + malformedCount += 1; + nodes.push(node); + } + if (malformedCount > 0) + options.completeness?.exclude( + "accessibility", + "invalid_protocol_value", + malformedCount, + ); + return nodes; + }); const nodeIds = new Set( all.flatMap((node) => { const nodeId = cdpStringValue(node.nodeId); @@ -545,7 +593,7 @@ export const captureAccessibility = ( return { total: all.length, nodes, - treeIncomplete: all.some( + hasUnobservedChildren: all.some( (node) => Array.isArray(node.childIds) && node.childIds.some((childId) => { diff --git a/src/browser/CdpCaptureValues.ts b/src/browser/CdpCaptureValues.ts index 6d639a454..b0f8f4d4f 100644 --- a/src/browser/CdpCaptureValues.ts +++ b/src/browser/CdpCaptureValues.ts @@ -12,6 +12,10 @@ const isRecord = (value: unknown): value is UnknownRecord => export const recordValue = (value: unknown): UnknownRecord | undefined => isRecord(value) ? value : undefined; +/** Preserve an omitted producer inventory as unknown rather than empty. */ +export const admitRecords = (value: unknown): readonly unknown[] | undefined => + Array.isArray(value) ? value : undefined; + export const recordsValue = (value: unknown): readonly UnknownRecord[] => Array.isArray(value) ? value.flatMap((item) => { diff --git a/src/browser/CdpElectronInspection.ts b/src/browser/CdpElectronInspection.ts index 43e208428..8d64baec7 100644 --- a/src/browser/CdpElectronInspection.ts +++ b/src/browser/CdpElectronInspection.ts @@ -328,10 +328,13 @@ const captureDom = async ( ); const invalid = (count: number | null) => completeness.exclude("dom", "invalid_protocol_value", count); - if (!Array.isArray(result.strings)) invalid(null); - const strings = Array.isArray(result.strings) - ? result.strings.map((value) => (typeof value === "string" ? value : "")) - : []; + if (!Array.isArray(result.strings)) { + invalid(null); + return { total_nodes: 0, nodes: [] }; + } + const strings = result.strings.map((value) => + typeof value === "string" ? value : "", + ); const nodes: ElectronPageInspection["dom"]["nodes"] = []; let total = 0; for (const document of snapshotDocuments(result.documents, invalid)) { diff --git a/src/browser/CdpPageCapture.ts b/src/browser/CdpPageCapture.ts index 337f99dfa..a986366cb 100644 --- a/src/browser/CdpPageCapture.ts +++ b/src/browser/CdpPageCapture.ts @@ -414,6 +414,7 @@ const accessibilityForFrames = async ( const capture = captureAccessibility(results, { includeText: context.input.include_accessibility_text, ...(results.length === 0 && unavailable ? { unavailable: true } : {}), + completeness: events.completeness, }); if (!context.input.include_accessibility_text) events.completeness.exclude( @@ -421,7 +422,8 @@ const accessibilityForFrames = async ( "not_approved", capture.textCapture.excluded_fields, ); - if (capture.treeIncomplete) events.completeness.truncate("accessibility"); + if (capture.hasUnobservedChildren) + events.completeness.truncate("accessibility"); return capture; }; diff --git a/src/browser/CdpPageCaptureWorkers.test.ts b/src/browser/CdpPageCaptureWorkers.test.ts new file mode 100644 index 000000000..77768ad99 --- /dev/null +++ b/src/browser/CdpPageCaptureWorkers.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it } from "vitest"; + +import { inspectWebPageInputSchema } from "../domain/browserObservation.js"; +import { CdpCaptureEvents } from "./CdpCaptureEvents.js"; +import type { CdpConnection } from "./CdpConnection.js"; +import type { CdpEndpointTarget } from "./CdpEndpoint.js"; +import { captureWorkers } from "./CdpPageCaptureWorkers.js"; +import type { CaptureContext } from "./CdpPageCapture.js"; + +const origin = "http://127.0.0.1:43127"; + +const capture = async (inventory: unknown) => { + const input = inspectWebPageInputSchema.parse({ + cdp_endpoint: origin, + allowed_origins: [origin], + target_id: "page", + }); + const events = new CdpCaptureEvents(input, new Set([origin])); + const context = { + connection: { + async send() { + return inventory; + }, + } as unknown as CdpConnection, + sessionId: undefined, + target: { + id: "page", + type: "page", + title: "Fixture", + url: `${origin}/index.html`, + attached: false, + } satisfies CdpEndpointTarget, + } as unknown as CaptureContext; + const workers = await captureWorkers( + { + context, + allowedOrigins: new Set([origin]), + limitations: [], + events, + }, + new Set(), + ); + return { workers, completeness: events.completeness.snapshot() }; +}; + +describe("web worker target inventory", () => { + it("preserves an explicit empty inventory", async () => { + const result = await capture({ targetInfos: [] }); + + expect(result.workers).toEqual([]); + expect(result.completeness.unavailable_sections).not.toContain("workers"); + }); + + it.each([undefined, { length: 1 }])( + "reports a missing or non-array target inventory as unknown: %s", + async (targetInfos) => { + const result = await capture({ targetInfos }); + + expect(result.workers).toEqual([]); + expect(result.completeness).toMatchObject({ + unavailable_sections: ["workers"], + excluded: [ + { + section: "workers", + reason: "invalid_protocol_value", + count: null, + }, + ], + }); + }, + ); + + it("retains valid worker records beside malformed entries", async () => { + const result = await capture({ + targetInfos: [ + null, + { + targetId: "worker-1", + type: "service_worker", + url: `${origin}/worker.js`, + attached: true, + openerId: "page", + }, + { + targetId: "worker-missing-url", + type: "worker", + attached: true, + openerId: "another-page", + }, + { + type: "worker", + url: `${origin}/missing-id.js`, + attached: true, + openerId: "page", + }, + { + targetId: "worker-invalid-attached", + type: "worker", + url: `${origin}/invalid-attached.js`, + attached: "true", + openerId: "page", + }, + "malformed", + ], + }); + + expect(result.workers.map((worker) => worker.target_id)).toEqual([ + "worker-1", + ]); + expect(result.completeness).toMatchObject({ + unavailable_sections: ["workers"], + excluded: [ + { + section: "workers", + reason: "invalid_protocol_value", + count: 5, + }, + ], + }); + expect(result.completeness.excluded).not.toContainEqual( + expect.objectContaining({ + section: "workers", + reason: "out_of_target_scope", + }), + ); + }); +}); diff --git a/src/browser/CdpPageCaptureWorkers.ts b/src/browser/CdpPageCaptureWorkers.ts index 3a564c888..1ff14ad9d 100644 --- a/src/browser/CdpPageCaptureWorkers.ts +++ b/src/browser/CdpPageCaptureWorkers.ts @@ -1,10 +1,10 @@ import type { WebPageInspection } from "../domain/browserObservationSchemas.js"; import type { CdpCaptureEvents } from "./CdpCaptureEvents.js"; import { + admitRecords, allowedSanitizedUrl, isHttpUrl, recordValue, - recordsValue, cdpStringValue, } from "./CdpCaptureValues.js"; import type { CaptureContext } from "./CdpPageCapture.js"; @@ -31,14 +31,53 @@ export const captureWorkers = async ( ); const items: WebPageInspection["workers"] = []; if (result === undefined) completeness.unavailable("workers"); - for (const target of recordsValue(recordValue(result)?.targetInfos)) { - const type = cdpStringValue(target.type) ?? ""; + const targets = admitRecords(recordValue(result)?.targetInfos); + if (result !== undefined && targets === undefined) { + completeness.exclude("workers", "invalid_protocol_value", null); + return items; + } + let malformedEntries = 0; + for (const entry of targets ?? []) { + const target = recordValue(entry); + if (target === undefined) { + malformedEntries += 1; + continue; + } + const type = cdpStringValue(target.type); + if (type === undefined || type.length === 0) { + malformedEntries += 1; + continue; + } + if (!type.includes("worker")) continue; + const targetId = cdpStringValue(target.targetId); + if ( + targetId === undefined || + targetId.length === 0 || + typeof target.attached !== "boolean" || + typeof target.url !== "string" + ) { + malformedEntries += 1; + continue; + } + const openerId = + target.openerId === undefined + ? undefined + : cdpStringValue(target.openerId); + const parentFrameId = + target.parentFrameId === undefined + ? undefined + : cdpStringValue(target.parentFrameId); + if ( + (target.openerId !== undefined && openerId === undefined) || + (target.parentFrameId !== undefined && parentFrameId === undefined) + ) { + malformedEntries += 1; + continue; + } const url = allowedSanitizedUrl(target.url, allowedOrigins); const relatedToPage = - cdpStringValue(target.openerId) === context.target.id || - (cdpStringValue(target.parentFrameId) !== undefined && - frameIds.has(cdpStringValue(target.parentFrameId) ?? "")); - if (!type.includes("worker")) continue; + openerId === context.target.id || + (parentFrameId !== undefined && frameIds.has(parentFrameId)); if (!relatedToPage) { completeness.exclude("workers", "out_of_target_scope"); continue; @@ -55,19 +94,17 @@ export const captureWorkers = async ( ); continue; } - if (typeof target.attached !== "boolean") { - completeness.exclude("workers", "invalid_protocol_value"); - continue; - } items.push({ - target_id: cdpStringValue(target.targetId) ?? "", + target_id: targetId, type, url: url.url, origin: url.origin, attached: target.attached, - opener_target_id: cdpStringValue(target.openerId) ?? null, - parent_frame_id: cdpStringValue(target.parentFrameId) ?? null, + opener_target_id: openerId ?? null, + parent_frame_id: parentFrameId ?? null, }); } + if (malformedEntries > 0) + completeness.exclude("workers", "invalid_protocol_value", malformedEntries); return items; }; diff --git a/tests/boundary/browser/cdpElectronProvider.test.ts b/tests/boundary/browser/cdpElectronProvider.test.ts index c8a3241f4..71a8e49fb 100644 --- a/tests/boundary/browser/cdpElectronProvider.test.ts +++ b/tests/boundary/browser/cdpElectronProvider.test.ts @@ -372,6 +372,23 @@ describe("CdpElectronProvider malformed inventory replies", () => { return result.value; }; + it("keeps documents unknown when the DOM string table is missing", async () => { + const inspected = await inspectWith("DOMSnapshot.captureSnapshot", () => ({ + documents: [{ documentURL: 0, nodes: { nodeType: [] } }], + })); + + expect(inspected.dom).toEqual({ total_nodes: 0, nodes: [] }); + expect(inspected.completeness.unavailable_sections).toContain("dom"); + expect(inspected.completeness.excluded).toContainEqual({ + section: "dom", + reason: "invalid_protocol_value", + count: null, + }); + expect(inspected.completeness.policy_filtered_sections).not.toContain( + "dom", + ); + }); + it.each([ ["dom", "DOMSnapshot.captureSnapshot", () => ({ strings: [] })], [