diff --git a/src/browser/CdpElectronWorkers.ts b/src/browser/CdpElectronWorkers.ts index 51a6b1e9e2..18f1bd3304 100644 --- a/src/browser/CdpElectronWorkers.ts +++ b/src/browser/CdpElectronWorkers.ts @@ -5,13 +5,16 @@ import type { } from "../domain/javascript/electronObservation.js"; import type { CdpConnection } from "./CdpConnection.js"; import { CdpCaptureCompleteness } from "./CdpCaptureCompleteness.js"; -import { recordValue, cdpStringValue } from "./CdpCaptureValues.js"; +import { recordValue } from "./CdpCaptureValues.js"; import type { CdpEndpointTarget } from "./CdpEndpoint.js"; import { optionalCdpCommand } from "./CdpOptionalCommand.js"; import { authorizedElectronFile } from "./ElectronFileScope.js"; import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; -import { workerTargetOwnership } from "./CdpWorkerTargetOwnership.js"; +import { + parseCdpWorkerTarget, + workerTargetOwnership, +} from "./CdpWorkerTarget.js"; /** Inventory local worker targets without attaching to them. */ export const captureElectronWorkers = async (input: { @@ -47,33 +50,27 @@ export const captureElectronWorkers = async (input: { } for (const entry of targetInfos) { throwIfElectronWorkerCaptureCancelled(input.signal); - const target = recordValue(entry); - if (target === undefined) { + const parsed = parseCdpWorkerTarget(entry); + if (!parsed.ok) { input.completeness.exclude("workers", "invalid_protocol_value"); continue; } - const type = cdpStringValue(target.type) ?? ""; - if (!type.includes("worker")) continue; + const target = parsed.value; + if (target.kind === "non_worker") continue; const ownership = workerTargetOwnership( target, input.target.id, input.frameIds, ); - if (!ownership.ok) { - input.completeness.exclude("workers", "invalid_protocol_value"); - continue; - } - if (ownership.value.scope !== "selected") { + if (ownership !== "selected") { input.completeness.exclude( "workers", - ownership.value.scope === "unknown" - ? "unattributed_target" - : "out_of_target_scope", + ownership === "unknown" ? "unattributed_target" : "out_of_target_scope", ); continue; } const path = await authorizedElectronFile( - cdpStringValue(target.url) ?? "", + target.url, input.signal, "inspect_electron_page", input.asarHeaderAdmission, @@ -82,22 +79,13 @@ export const captureElectronWorkers = async (input: { input.completeness.exclude("workers", "out_of_target_scope"); continue; } - const targetId = boundedTargetField(target.targetId); - if (targetId === null) { - input.completeness.exclude("workers", "invalid_protocol_value"); - continue; - } - if (typeof target.attached !== "boolean") { - input.completeness.exclude("workers", "invalid_protocol_value"); - continue; - } workers.push({ - target_id: targetId, - type, + target_id: target.targetId, + type: target.type, file_path: path, attached: target.attached, - opener_target_id: ownership.value.openerTargetId, - parent_frame_id: ownership.value.parentFrameId, + opener_target_id: target.openerTargetId, + parent_frame_id: target.parentFrameId, }); } return workers.sort((left, right) => @@ -111,8 +99,3 @@ const throwIfElectronWorkerCaptureCancelled = (signal?: AbortSignal): void => { cause: signal.reason, }); }; - -const boundedTargetField = (value: unknown): string | null => { - const field = cdpStringValue(value); - return field === undefined || field === "" ? null : field; -}; diff --git a/src/browser/CdpPageCaptureWorkers.test.ts b/src/browser/CdpPageCaptureWorkers.test.ts index 8a3f9e5e04..cc9ec57d91 100644 --- a/src/browser/CdpPageCaptureWorkers.test.ts +++ b/src/browser/CdpPageCaptureWorkers.test.ts @@ -9,7 +9,7 @@ import type { CaptureContext } from "./CdpPageCapture.js"; const origin = "http://127.0.0.1:43127"; -const capture = async (inventory: unknown) => { +const capture = async (inventory: unknown, frameIds = new Set()) => { const input = inspectWebPageInputSchema.parse({ cdp_endpoint: origin, allowed_origins: [origin], @@ -38,7 +38,7 @@ const capture = async (inventory: unknown) => { limitations: [], events, }, - new Set(), + frameIds, ); return { workers, completeness: events.completeness.snapshot() }; }; @@ -199,3 +199,74 @@ it.each([{ openerId: "" }, { parentFrameId: null }])( }); }, ); + +it.each([ + { type: undefined }, + { type: 7 }, + { type: "" }, + { targetId: "" }, + { url: undefined }, + { attached: null }, + { openerId: null }, + { parentFrameId: "" }, +])( + "reports malformed worker facts before ownership and URL policy: %j", + async (override) => { + const result = await capture({ + targetInfos: [ + { + targetId: "worker", + type: "worker", + url: "https://outside.example/worker.js", + attached: false, + openerId: "foreign-page", + ...override, + }, + ], + }); + expect(result.workers).toEqual([]); + expect(result.completeness.excluded).toEqual([ + { section: "workers", reason: "invalid_protocol_value", count: 1 }, + ]); + }, +); + +it("uses parsed frame relationships without treating valid non-workers as malformed", async () => { + const result = await capture( + { + targetInfos: [ + { type: "page" }, + { + targetId: "worker", + type: "shared_worker", + url: `${origin}/worker.js`, + attached: false, + openerId: "foreign-page", + parentFrameId: "selected-frame", + }, + { + targetId: "foreign", + type: "worker", + url: `${origin}/foreign.js`, + attached: true, + parentFrameId: "foreign-frame", + }, + ], + }, + new Set(["selected-frame"]), + ); + expect(result.workers).toEqual([ + { + target_id: "worker", + type: "shared_worker", + url: `${origin}/worker.js`, + origin, + attached: false, + opener_target_id: "foreign-page", + parent_frame_id: "selected-frame", + }, + ]); + expect(result.completeness.excluded).toEqual([ + { section: "workers", reason: "out_of_target_scope", count: 1 }, + ]); +}); diff --git a/src/browser/CdpPageCaptureWorkers.ts b/src/browser/CdpPageCaptureWorkers.ts index 7415feef19..28f066f8e1 100644 --- a/src/browser/CdpPageCaptureWorkers.ts +++ b/src/browser/CdpPageCaptureWorkers.ts @@ -4,11 +4,13 @@ import { admitRecords, allowedSanitizedUrl, recordValue, - cdpStringValue, } from "./CdpCaptureValues.js"; import type { CaptureContext } from "./CdpPageCapture.js"; import { optionalCdpCommand } from "./CdpOptionalCommand.js"; -import { workerTargetOwnership } from "./CdpWorkerTargetOwnership.js"; +import { + parseCdpWorkerTarget, + workerTargetOwnership, +} from "./CdpWorkerTarget.js"; import { exclusionReasonForUrl } from "./CdpCaptureEventHelpers.js"; interface CaptureWorkersInput { @@ -42,59 +44,38 @@ export const captureWorkers = async ( } 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" - ) { + const parsed = parseCdpWorkerTarget(entry); + if (!parsed.ok) { malformedEntries += 1; continue; } + const target = parsed.value; + if (target.kind === "non_worker") continue; const ownership = workerTargetOwnership( target, context.target.id, frameIds, ); - if (!ownership.ok) { - malformedEntries += 1; - continue; - } - if (ownership.value.scope !== "selected") { + if (ownership !== "selected") { completeness.exclude( "workers", - ownership.value.scope === "unknown" - ? "unattributed_target" - : "out_of_target_scope", + ownership === "unknown" ? "unattributed_target" : "out_of_target_scope", ); continue; } const url = allowedSanitizedUrl(target.url, allowedOrigins); if (url === undefined) { - const rawUrl = cdpStringValue(target.url); - completeness.exclude("workers", exclusionReasonForUrl(rawUrl)); + completeness.exclude("workers", exclusionReasonForUrl(target.url)); continue; } items.push({ - target_id: targetId, - type, + target_id: target.targetId, + type: target.type, url: url.url, origin: url.origin, attached: target.attached, - opener_target_id: ownership.value.openerTargetId, - parent_frame_id: ownership.value.parentFrameId, + opener_target_id: target.openerTargetId, + parent_frame_id: target.parentFrameId, }); } if (malformedEntries > 0) diff --git a/src/browser/CdpWorkerTarget.ts b/src/browser/CdpWorkerTarget.ts new file mode 100644 index 0000000000..a90e6a5d55 --- /dev/null +++ b/src/browser/CdpWorkerTarget.ts @@ -0,0 +1,70 @@ +import { err, ok, type Result } from "../domain/result.js"; +import { cdpStringValue, recordValue } from "./CdpCaptureValues.js"; + +export interface CdpWorkerRelationships { + readonly openerTargetId: string | null; + readonly parentFrameId: string | null; +} + +export interface CdpWorkerTarget extends CdpWorkerRelationships { + readonly kind: "worker"; + readonly targetId: string; + readonly type: string; + readonly url: string; + readonly attached: boolean; +} + +/** Admit protocol facts before either consumer applies target or URL policy. */ +export const parseCdpWorkerTarget = ( + input: unknown, +): Result< + CdpWorkerTarget | { readonly kind: "non_worker" }, + { readonly _tag: "InvalidWorkerTarget" } +> => { + const target = recordValue(input); + const type = cdpStringValue(target?.type); + if (target === undefined || type === undefined || type === "") + return err({ _tag: "InvalidWorkerTarget" }); + if (!type.includes("worker")) return ok({ kind: "non_worker" }); + const targetId = cdpStringValue(target.targetId); + const url = cdpStringValue(target.url); + const openerTargetId = cdpStringValue(target.openerId); + const parentFrameId = cdpStringValue(target.parentFrameId); + if ( + targetId === undefined || + targetId === "" || + url === undefined || + typeof target.attached !== "boolean" || + (target.openerId !== undefined && + (openerTargetId === undefined || openerTargetId === "")) || + (target.parentFrameId !== undefined && + (parentFrameId === undefined || parentFrameId === "")) + ) + return err({ _tag: "InvalidWorkerTarget" }); + return ok({ + kind: "worker", + targetId, + type, + url, + attached: target.attached, + openerTargetId: openerTargetId ?? null, + parentFrameId: parentFrameId ?? null, + }); +}; + +/** Optional CDP relationships establish ownership only when supplied. */ +export const workerTargetOwnership = ( + relationships: CdpWorkerRelationships, + selectedTargetId: string, + frameIds: ReadonlySet, +): "selected" | "foreign" | "unknown" => { + const { openerTargetId, parentFrameId } = relationships; + if ( + openerTargetId === selectedTargetId || + (parentFrameId !== null && frameIds.has(parentFrameId)) + ) + return "selected"; + return openerTargetId === null && parentFrameId === null + ? "unknown" + : "foreign"; +}; diff --git a/src/browser/CdpWorkerTargetOwnership.ts b/src/browser/CdpWorkerTargetOwnership.ts deleted file mode 100644 index 27611119c0..0000000000 --- a/src/browser/CdpWorkerTargetOwnership.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { err, ok, type Result } from "../domain/result.js"; -import { cdpStringValue } from "./CdpCaptureValues.js"; - -/** Optional CDP relationships establish ownership only when supplied. */ -export const workerTargetOwnership = ( - target: Readonly>, - selectedTargetId: string, - frameIds: ReadonlySet, -): Result< - { - readonly scope: "selected" | "foreign" | "unknown"; - readonly openerTargetId: string | null; - readonly parentFrameId: string | null; - }, - { readonly _tag: "InvalidWorkerRelationship" } -> => { - const openerTargetId = cdpStringValue(target.openerId); - const parentFrameId = cdpStringValue(target.parentFrameId); - if ( - (target.openerId !== undefined && - (openerTargetId === undefined || openerTargetId === "")) || - (target.parentFrameId !== undefined && - (parentFrameId === undefined || parentFrameId === "")) - ) - return err({ _tag: "InvalidWorkerRelationship" }); - const selected = - openerTargetId === selectedTargetId || - (parentFrameId !== undefined && frameIds.has(parentFrameId)); - return ok({ - scope: selected - ? "selected" - : openerTargetId === undefined && parentFrameId === undefined - ? "unknown" - : "foreign", - openerTargetId: openerTargetId ?? null, - parentFrameId: parentFrameId ?? null, - }); -}; diff --git a/tests/boundary/browser/cdpElectronProvider.test.ts b/tests/boundary/browser/cdpElectronProvider.test.ts index e4ca0f6886..f18b45ec17 100644 --- a/tests/boundary/browser/cdpElectronProvider.test.ts +++ b/tests/boundary/browser/cdpElectronProvider.test.ts @@ -849,6 +849,85 @@ describe("CdpElectronProvider malformed inventory replies", () => { ); }); + it.each([ + ["missing type", { type: undefined }], + ["non-string type", { type: 7 }], + ["empty type", { type: "" }], + ["missing target identity", { targetId: undefined }], + ["non-string target identity", { targetId: 7 }], + ["empty target identity", { targetId: "" }], + ["missing URL", { url: undefined }], + ["non-string URL", { url: 7 }], + ["missing attached state", { attached: undefined }], + ["non-boolean attached state", { attached: "false" }], + ["non-string opener relationship", { openerId: 7 }], + ["empty opener relationship", { openerId: "" }], + ["non-string frame relationship", { parentFrameId: 7 }], + ["empty frame relationship", { parentFrameId: "" }], + ] as const)( + "retains valid workers while reporting %s before target policy", + async (_kind, malformed) => { + const inspected = await inspectWith("Target.getTargets", (fileUrl) => ({ + targetInfos: [ + { + targetId: "selected-worker", + type: "worker", + url: fileUrl, + attached: false, + openerId: "electron-page", + }, + { + targetId: "malformed-worker", + type: "worker", + url: fileUrl, + attached: false, + openerId: "foreign-page", + ...malformed, + }, + { + targetId: "foreign-worker", + type: "shared_worker", + url: fileUrl, + attached: true, + openerId: "foreign-page", + }, + { + targetId: "unattributed-worker", + type: "service_worker", + url: fileUrl, + attached: false, + }, + // Non-worker rows do not need worker identity or relationship fields. + { type: "page", targetId: 7, openerId: null }, + ], + })); + expect(inspected.workers).toEqual([ + expect.objectContaining({ + target_id: "selected-worker", + type: "worker", + attached: false, + opener_target_id: "electron-page", + parent_frame_id: null, + }), + ]); + const workerExclusions = inspected.completeness.excluded.filter( + ({ section }) => section === "workers", + ); + expect(workerExclusions).toHaveLength(3); + expect(workerExclusions).toEqual( + expect.arrayContaining([ + { section: "workers", reason: "invalid_protocol_value", count: 1 }, + { section: "workers", reason: "out_of_target_scope", count: 1 }, + { section: "workers", reason: "unattributed_target", count: 1 }, + ]), + ); + expect(inspected.completeness.unavailable_sections).toContain("workers"); + expect(inspected.completeness.policy_filtered_sections).toContain( + "workers", + ); + }, + ); + it.each([ ["dom", "DOMSnapshot.captureSnapshot", () => ({ strings: [] })], [