mirror/rea
1
0
Fork 0
mirror of https://github.com/morluto/rea synced 2026-10-28 06:46:47 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix(browser): preserve discovery identity ambiguity

Browse Source
This commit is contained in:
morluto
2026-10-27 11:44:58 +08:00
parent a486cfe518
commit 4e33bd3e08
10 changed files with 502 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/browser/CdpAuthorizedTarget.ts
+6 -2
View File
@@ -1,3 +1,4 @@
import { selectCdpDiscoveryTarget } from "./CdpDiscoveryTargets.js";
import { sanitizeBrowserUrl } from "../domain/browserObservation.js";
import { BrowserObservationError } from "../domain/browserObservationError.js";
import type { BrowserObservationOperation } from "../domain/browserObservationErrors.js";
@@ -15,8 +16,11 @@ export const authorizeCdpTarget = (
readonly target: CdpEndpointTarget;
readonly allowedOrigins: ReadonlySet<string>;
} => {
const target = discovery.targets.find(
(candidate) => candidate.id === input.target_id,
const target = selectCdpDiscoveryTarget(
discovery.targets,
input.target_id,
operation,
discovery.ambiguousTargetIds,
);
if (target === undefined)
throw new BrowserObservationError(operation, "target_not_found", {
src/browser/CdpDiscoveryTargets.test.ts
+221
View File
@@ -0,0 +1,221 @@
import { once } from "node:events";
import { createServer } from "node:http";
import { expect, it } from "vitest";
import { BrowserObservationError } from "../domain/browserObservationError.js";
import { inspectWebPageInputSchema } from "../domain/browserObservation.js";
import { inspectElectronPageInputSchema } from "../domain/javascript/electronObservation.js";
import { V8InspectorProvider } from "../inspector/V8InspectorProvider.js";
import { CdpBrowserProvider } from "./CdpBrowserProvider.js";
import { CdpElectronProvider } from "./CdpElectronProvider.js";
import { parseCdpDiscoveryTargets } from "./CdpDiscoveryTargets.js";
import { discoverCdpBrowserSocket } from "./CdpEndpoint.js";
import { authorizeCdpTarget } from "./CdpAuthorizedTarget.js";
it.each(["CDP /json/list", "CDP Target.getTargets"] as const)(
"%s retains sibling identities and marks duplicates before malformed rows are excluded",
(source) => {
const idField = source === "CDP /json/list" ? "id" : "targetId";
const result = parseCdpDiscoveryTargets(
[
{ [idField]: "duplicate", type: "page", url: "https://allowed.test/" },
{ [idField]: "duplicate", type: "page" },
{
[idField]: "sibling",
type: "page",
url: "https://allowed.test/other",
},
],
"list_browser_targets",
source,
);
expect(result.targets).toMatchObject([
{ id: "duplicate" },
{ id: "sibling" },
]);
expect(result.ambiguousTargetIds).toEqual(["duplicate"]);
expect(result.limitations).toEqual(
expect.arrayContaining([
expect.stringContaining("identity is ambiguous"),
expect.stringContaining("identity is malformed"),
]),
);
},
);
it.each([0, 1, 2])(
"browser-socket selection rejects duplicate IDs before origin policy (malformed duplicate rows: %s)",
async (malformedRows) => {
const discovery = await discoverCdpBrowserSocket(
{
send: async (method) =>
method === "Browser.getVersion"
? {
protocolVersion: "1.3",
product: "Chrome",
revision: "r",
userAgent: "ua",
jsVersion: "v8",
}
: {
targetInfos: [
{
targetId: "duplicate",
type: "page",
...(malformedRows === 2
? {}
: { url: "https://allowed.test/" }),
},
{
targetId: "duplicate",
type: "page",
...(malformedRows > 0
? {}
: { url: "https://foreign.test/" }),
},
{
targetId: "sibling",
type: "page",
url: "https://allowed.test/other",
},
],
},
},
"ws://127.0.0.1:9222/devtools/browser/fixture",
"inspect_web_page",
);
expect(() =>
authorizeCdpTarget(
discovery,
{
target_id: "duplicate",
allowed_origins: ["https://allowed.test"],
},
"inspect_web_page",
),
).toThrow(
expect.objectContaining({
reason: "invalid_endpoint_response",
detail: expect.stringContaining("ambiguous"),
}),
);
expect(
authorizeCdpTarget(
discovery,
{
target_id: "sibling",
allowed_origins: ["https://allowed.test"],
},
"inspect_web_page",
).target.id,
).toBe("sibling");
},
);
it.each(
(["browser", "electron", "inspector"] as const).flatMap((providerKind) => [
{ providerKind, malformedRows: 0 },
{ providerKind, malformedRows: 1 },
{ providerKind, malformedRows: 2 },
]),
)(
"$providerKind rejects ambiguous HTTP target identity before authorizing its location or opening a socket (malformed duplicate rows: $malformedRows)",
async ({ providerKind, malformedRows }) => {
let upgrades = 0;
const server = createServer((request, response) => {
const address = server.address();
if (address === null || typeof address === "string")
throw new Error("Missing TCP address");
response.setHeader("Content-Type", "application/json");
response.end(
JSON.stringify(
request.url === "/json/version"
? {
Browser: "Node.js/v24",
"Protocol-Version": "1.3",
"User-Agent": "fixture",
"V8-Version": "v8",
"WebKit-Version": "r",
webSocketDebuggerUrl: `ws://127.0.0.1:${String(address.port)}/devtools/browser/fixture`,
}
: [
{
id: "duplicate",
type: "page",
...(malformedRows === 2
? {}
: { url: "https://allowed.test/" }),
},
{
id: "duplicate",
type: "node",
...(malformedRows > 0
? {}
: { url: "file:///missing/foreign.js" }),
},
{
id: "sibling",
type: "page",
url: "https://allowed.test/other",
},
],
),
);
});
server.on("upgrade", (_request, socket) => {
upgrades += 1;
socket.destroy();
});
server.listen(0, "127.0.0.1");
await once(server, "listening");
const address = server.address();
if (address === null || typeof address === "string")
throw new Error("Missing TCP address");
const endpoint = `http://127.0.0.1:${String(address.port)}`;
const provider =
providerKind === "browser"
? new CdpBrowserProvider()
: providerKind === "electron"
? new CdpElectronProvider()
: new V8InspectorProvider();
try {
const result =
provider instanceof CdpBrowserProvider
? await provider.inspectPage(
inspectWebPageInputSchema.parse({
cdp_endpoint: endpoint,
target_id: "duplicate",
allowed_origins: ["https://allowed.test"],
observation_ms: 0,
}),
)
: provider instanceof CdpElectronProvider
? await provider.inspectPage(
inspectElectronPageInputSchema.parse({
cdp_endpoint: endpoint,
target_id: "duplicate",
observation_ms: 0,
}),
)
: await provider.observe({
inspector_endpoint: endpoint,
target_id: "duplicate",
observation_ms: 0,
});
expect(result.ok).toBe(false);
if (result.ok) throw new Error("Expected ambiguous target rejection");
expect(result.error).toBeInstanceOf(BrowserObservationError);
expect(result.error.userMessage).toContain("ambiguous");
expect(upgrades).toBe(0);
} finally {
await provider.close();
server.closeAllConnections();
await new Promise<void>((resolve, reject) =>
server.close((error) =>
error === undefined ? resolve() : reject(error),
),
);
}
},
);
src/browser/CdpDiscoveryTargets.ts
+55 -7
View File
@@ -12,7 +12,7 @@ const identitySchema = z.object({
webSocketDebuggerUrl: z.unknown().optional(),
});
/** Shared HTTP /json/list facts; socket authority stays with each transport owner. */
/** Shared discovery facts; socket authority stays with each transport owner. */
export interface CdpDiscoveryTarget {
readonly id: string;
readonly type: string;
@@ -26,21 +26,49 @@ export interface CdpDiscoveryTarget {
export const parseCdpDiscoveryTargets = (
input: unknown,
operation: BrowserObservationOperation,
source: "CDP /json/list" | "CDP Target.getTargets" = "CDP /json/list",
): {
readonly targets: readonly CdpDiscoveryTarget[];
readonly ambiguousTargetIds: readonly string[];
readonly limitations: readonly string[];
} => {
if (!Array.isArray(input))
throw new BrowserObservationError(operation, "invalid_endpoint_response", {
detail: "CDP /json/list must return an array of target rows.",
detail: `${source} must return an array of target rows.`,
});
const targets: CdpDiscoveryTarget[] = [];
const limitations: string[] = [];
const idCounts = new Map<string, number>();
for (const row of input) {
if (typeof row !== "object" || row === null || Array.isArray(row)) continue;
const id: unknown = Reflect.get(
row,
source === "CDP /json/list" ? "id" : "targetId",
);
if (typeof id === "string" && id !== "")
idCounts.set(id, (idCounts.get(id) ?? 0) + 1);
}
const ambiguousTargetIds: string[] = [];
for (const [id, count] of idCounts) {
if (count > 1) {
ambiguousTargetIds.push(id);
limitations.push(
`${source} reported target ID ${id} in ${String(count)} rows; its identity is ambiguous and cannot be selected.`,
);
}
}
for (const [index, row] of input.entries()) {
const parsed = identitySchema.safeParse(row);
const parsed = identitySchema.safeParse(
source === "CDP Target.getTargets" &&
typeof row === "object" &&
row !== null &&
!Array.isArray(row)
? { ...row, id: Reflect.get(row, "targetId") }
: row,
);
if (!parsed.success) {
limitations.push(
`CDP /json/list row ${String(index)} was excluded because its target identity is malformed: ${JSON.stringify(parsed.error.issues)}.`,
`${source} row ${String(index)} was excluded because its target identity is malformed: ${JSON.stringify(parsed.error.issues)}.`,
);
continue;
}
@@ -51,11 +79,11 @@ export const parseCdpDiscoveryTargets = (
typeof target.attached !== "boolean"
)
limitations.push(
`CDP /json/list row ${String(index)} target ${target.id} reported malformed attached metadata ${JSON.stringify(target.attached)}; attachment state is unknown.`,
`${source} row ${String(index)} target ${target.id} reported malformed attached metadata ${JSON.stringify(target.attached)}; attachment state is unknown.`,
);
if (target.title !== undefined && typeof target.title !== "string")
limitations.push(
`CDP /json/list row ${String(index)} target ${target.id} reported malformed title metadata ${JSON.stringify(target.title)}; title is unavailable.`,
`${source} row ${String(index)} target ${target.id} reported malformed title metadata ${JSON.stringify(target.title)}; title is unavailable.`,
);
targets.push({
id: target.id,
@@ -66,5 +94,25 @@ export const parseCdpDiscoveryTargets = (
webSocketDebuggerUrl: target.webSocketDebuggerUrl,
});
}
return { targets, limitations };
return { targets, limitations, ambiguousTargetIds };
};
/** Select only an unambiguous identity before applying target-location policy. */
export const selectCdpDiscoveryTarget = <
Target extends { readonly id: string },
>(
targets: readonly Target[],
targetId: string,
operation: BrowserObservationOperation,
ambiguousTargetIds: readonly string[] = [],
): Target | undefined => {
const matches = targets.filter((target) => target.id === targetId);
if (matches.length > 1 || ambiguousTargetIds.includes(targetId))
throw new BrowserObservationError(operation, "invalid_endpoint_response", {
detail: `Target ID ${targetId} is ambiguous in endpoint discovery; REA cannot select its identity or location.`,
diagnostics: { target_id: targetId },
remediationAction:
"Refresh discovery after the endpoint reports a unique row for this target ID, or select a different target.",
});
return matches[0];
};
src/browser/CdpElectronProvider.ts
+9 -3
View File
@@ -1,3 +1,4 @@
import { selectCdpDiscoveryTarget } from "./CdpDiscoveryTargets.js";
import {
DEFAULT_ASAR_HEADER_ADMISSION,
type AsarHeaderAdmission,
@@ -283,7 +284,7 @@ export class CdpElectronProvider implements ElectronObservationPort {
options.signal,
);
const target = await authorizeTarget(
discovery.targets,
discovery,
input.target_id,
this.#asarHeaderAdmission,
options.signal,
@@ -431,13 +432,18 @@ export class CdpElectronProvider implements ElectronObservationPort {
}
const authorizeTarget = async (
targets: readonly CdpEndpointTarget[],
discovery: CdpEndpointDiscovery,
targetId: string,
asarHeaderAdmission: AsarHeaderAdmission,
signal?: AbortSignal,
): Promise<CdpEndpointTarget> => {
throwIfElectronOperationCancelled(signal, "inspect_electron_page");
const target = targets.find(({ id }) => id === targetId);
const target = selectCdpDiscoveryTarget(
discovery.targets,
targetId,
"inspect_electron_page",
discovery.ambiguousTargetIds,
);
if (target === undefined)
throw new BrowserObservationError("inspect_web_page", "target_not_found");
if (target.type !== "page")
src/browser/CdpEndpoint.test.ts
+65
View File
@@ -9,6 +9,7 @@ import { afterEach, describe, expect, it } from "vitest";
import {
discoverCdpEndpoint,
discoverCdpBrowserSocket,
readCdpJson,
withCdpDiscoveryDeadline,
} from "./CdpEndpoint.js";
@@ -338,3 +339,67 @@ it("preserves an independent typed cancellation when the discovery deadline also
).catch((cause: unknown) => cause);
expect(failure).toBe(primary);
});
it("preserves browser-socket sibling identities and reports malformed optional metadata", async () => {
const discovery = await discoverCdpBrowserSocket(
{
send: async (method) =>
method === "Browser.getVersion"
? {
protocolVersion: "1.3",
product: "Chrome/fixture",
revision: "r",
userAgent: "fixture",
jsVersion: "v8",
}
: {
targetInfos: [
{
targetId: "valid",
type: "page",
url: "https://example.test/",
title: "Example",
attached: false,
},
{
targetId: "partial",
type: "page",
url: "https://example.test/other",
title: 42,
attached: "yes",
},
{ targetId: "broken", type: "page" },
{
id: "wrong-identity-field",
type: "page",
url: "https://example.test/",
},
],
},
},
"ws://127.0.0.1:9222/devtools/browser/fixture",
"list_browser_targets",
);
expect(discovery.targets).toEqual([
{
id: "valid",
type: "page",
url: "https://example.test/",
title: "Example",
attached: false,
},
{
id: "partial",
type: "page",
url: "https://example.test/other",
title: "",
attached: null,
},
]);
expect(discovery.limitations).toHaveLength(4);
expect(
discovery.limitations?.every((limitation) =>
limitation.startsWith("CDP Target.getTargets row"),
),
).toBe(true);
});
src/browser/CdpEndpoint.ts
+14 -13
View File
@@ -74,15 +74,7 @@ const browserSocketVersionSchema = z.object({
});
const browserSocketTargetsSchema = z.object({
targetInfos: z.array(
z.object({
targetId: z.string().min(1),
type: z.string().min(1),
title: z.string().default(""),
url: z.string(),
attached: z.boolean().optional(),
}),
),
targetInfos: z.array(z.unknown()),
});
/** Validated direct CDP WebSocket bound to one discovered page target. */
@@ -109,6 +101,7 @@ export interface CdpEndpointTarget {
export interface CdpEndpointDiscovery {
readonly limitations?: readonly string[];
readonly ambiguousTargetIds?: readonly string[];
readonly webSocket: CdpWebSocketEndpoint;
readonly version: {
readonly product: string;
@@ -141,12 +134,13 @@ export const discoverCdpEndpoint = async (
versionInput,
operation,
);
const { targets, limitations } = parseCdpDiscoveryTargets(
const { targets, limitations, ambiguousTargetIds } = parseCdpDiscoveryTargets(
targetsInput,
operation,
);
return {
limitations,
ambiguousTargetIds,
webSocket: safeCdpWebSocketEndpoint(
endpoint,
version.webSocketDebuggerUrl,
@@ -231,7 +225,7 @@ export const withCdpDiscoveryDeadline = async <T>(
* apply the discovery deadline through `signal`.
*/
export const discoverCdpBrowserSocket = async (
connection: CdpConnection,
connection: Pick<CdpConnection, "send">,
url: string,
operation: BrowserObservationOperation,
signal?: AbortSignal,
@@ -258,7 +252,14 @@ export const discoverCdpBrowserSocket = async (
targetsInput,
operation,
);
const parsedTargets = parseCdpDiscoveryTargets(
targets.targetInfos,
operation,
"CDP Target.getTargets",
);
return {
limitations: parsedTargets.limitations,
ambiguousTargetIds: parsedTargets.ambiguousTargetIds,
webSocket: { scope: "browser", url },
version: {
product: version.product,
@@ -267,8 +268,8 @@ export const discoverCdpBrowserSocket = async (
user_agent: version.userAgent,
js_version: version.jsVersion,
},
targets: targets.targetInfos.map((target) => ({
id: target.targetId,
targets: parsedTargets.targets.map((target) => ({
id: target.id,
type: target.type,
title: sanitizeTargetTitle(target.title, target.url),
url: target.url,
src/browser/PlaywrightScenarioServiceWorkers.test.ts
+100
View File
@@ -0,0 +1,100 @@
import { expect, it, vi } from "vitest";
import { startFakeCdpBrowser } from "../../tests/fixtures/fakeCdpBrowser.js";
import { CdpConnection } from "./CdpConnection.js";
import { PlaywrightScenarioServiceWorkers } from "./PlaywrightScenarioServiceWorkers.js";
it.each(["duplicate", "malformed duplicates", "missing"])(
"releases an opened browser socket before rejecting service-worker target discovery: %s",
async (kind) => {
const browser = await startFakeCdpBrowser({
commandResult: ({ method }) =>
method === "Target.getTargets"
? {
targetInfos:
kind === "missing"
? []
: [
{
targetId: "target",
type: "page",
...(kind === "duplicate"
? { url: "https://allowed.test/" }
: {}),
},
{ targetId: "target", type: "page", url: 42 },
],
}
: undefined,
});
const policy = new PlaywrightScenarioServiceWorkers(
{
frames: () => [],
isClosed: () => false,
mainFrame: () => {
throw new Error("No frame mutation expected");
},
},
browser.browserWebSocketUrl,
"target",
);
try {
await expect(policy.initialize()).rejects.toMatchObject({
reason:
kind === "missing" ? "target_not_found" : "invalid_endpoint_response",
...(kind === "missing"
? {}
: { detail: expect.stringContaining("ambiguous") }),
});
await expect.poll(() => browser.activeConnectionCount()).toBe(0);
await policy.close();
expect(browser.commands.map(({ method }) => method)).toEqual([
"Browser.getVersion",
"Target.getTargets",
]);
} finally {
await policy.close();
await browser.close();
}
},
);
it("retains ownership and retries socket cleanup after ambiguous service-worker discovery", async () => {
const browser = await startFakeCdpBrowser({
commandResult: ({ method }) =>
method === "Target.getTargets"
? { targetInfos: [{ targetId: "target" }, { targetId: "target" }] }
: undefined,
});
const policy = new PlaywrightScenarioServiceWorkers(
{
frames: () => [],
isClosed: () => false,
mainFrame: () => {
throw new Error("No frame mutation expected");
},
},
browser.browserWebSocketUrl,
"target",
);
const cleanupFailure = new Error("socket close failed");
const close = vi
.spyOn(CdpConnection.prototype, "close")
.mockRejectedValueOnce(cleanupFailure);
try {
await expect(policy.initialize()).rejects.toMatchObject({
cleanupIncomplete: true,
});
await policy.close();
expect(close).toHaveBeenCalledTimes(2);
await expect.poll(() => browser.activeConnectionCount()).toBe(0);
expect(browser.commands.map(({ method }) => method)).toEqual([
"Browser.getVersion",
"Target.getTargets",
]);
} finally {
close.mockRestore();
await policy.close();
await browser.close();
}
});
src/browser/PlaywrightScenarioServiceWorkers.ts
+18 -10
View File
@@ -12,7 +12,11 @@ import { AnalysisTimeoutError } from "../domain/analysisErrorCore.js";
import { browserObservationEndpointSchema } from "../domain/browserObservation.js";
import { BrowserObservationError } from "../domain/browserObservationError.js";
import { CdpTargetSessionOwner } from "./CdpTargetSessionOwner.js";
import { withCdpDiscoveryDeadline } from "./CdpEndpoint.js";
import {
withCdpDiscoveryDeadline,
type CdpEndpointTarget,
} from "./CdpEndpoint.js";
import { selectCdpDiscoveryTarget } from "./CdpDiscoveryTargets.js";
import {
attachCdpTargetSession,
failCdpOpenAfterCleanup,
@@ -81,7 +85,7 @@ export class PlaywrightScenarioServiceWorkers {
#setup = new AbortController();
constructor(
private readonly page: Page,
private readonly page: Pick<Page, "frames" | "isClosed" | "mainFrame">,
private readonly endpoint: string,
private readonly targetId: string,
private readonly commandTimeoutMs: number = selectedCdpCommandTimeoutMs(
@@ -115,22 +119,26 @@ export class PlaywrightScenarioServiceWorkers {
this.commandTimeoutMs,
),
);
const target = opened.discovery.targets.find(
({ id }) => id === this.targetId,
);
if (target === undefined) {
const failure = new BrowserObservationError(
let target: CdpEndpointTarget;
try {
const selected = selectCdpDiscoveryTarget(
opened.discovery.targets,
this.targetId,
OPERATION,
"target_not_found",
opened.discovery.ambiguousTargetIds,
);
if (selected === undefined)
throw new BrowserObservationError(OPERATION, "target_not_found");
target = selected;
} catch (cause: unknown) {
if (opened.connection !== undefined)
return await failCdpOpenAfterCleanup(
opened.connection,
failure,
cause,
OPERATION,
this.#transportOwner,
);
throw failure;
throw cause;
}
const transport =
opened.connection ??
src/inspector/V8InspectorEndpoint.ts
+3 -1
View File
@@ -28,6 +28,7 @@ export interface V8InspectorTarget {
export interface V8InspectorDiscovery {
readonly limitations: readonly string[];
readonly ambiguousTargetIds?: readonly string[];
readonly runtime: JavaScriptRuntimeTargetList["runtime"];
readonly targets: readonly V8InspectorTarget[];
}
@@ -43,12 +44,13 @@ export const discoverV8Inspector = async (
readCdpJson(new URL("/json/list", endpoint), operation, signal),
]);
const version = parseCdpEndpointValue(versionSchema, versionInput, operation);
const { targets, limitations } = parseCdpDiscoveryTargets(
const { targets, limitations, ambiguousTargetIds } = parseCdpDiscoveryTargets(
targetsInput,
operation,
);
return {
limitations,
ambiguousTargetIds,
runtime: {
product: version.Browser,
protocol_version: version["Protocol-Version"],
src/inspector/V8InspectorProvider.ts
+11 -7
View File
@@ -1,3 +1,4 @@
import { selectCdpDiscoveryTarget } from "../browser/CdpDiscoveryTargets.js";
import {
DEFAULT_ASAR_HEADER_ADMISSION,
type AsarHeaderAdmission,
@@ -52,7 +53,7 @@ import {
import {
discoverV8Inspector,
type AuthorizedV8InspectorTarget,
type V8InspectorTarget,
type V8InspectorDiscovery,
} from "./V8InspectorEndpoint.js";
import { V8_INSPECTOR_PROVIDER_IDENTITY } from "./providerIdentity.js";
@@ -364,9 +365,8 @@ export class V8InspectorProvider implements JavaScriptRuntimeObservationPort {
options.signal,
);
target = await authorizedTarget(
discovery.targets,
discovery,
input,
discovery.runtime.product,
this.#asarHeaderAdmission,
options.signal,
);
@@ -506,14 +506,18 @@ const projectTarget = (target: AuthorizedV8InspectorTarget) => ({
});
const authorizedTarget = async (
targets: readonly V8InspectorTarget[],
discovery: V8InspectorDiscovery,
input: ObserveJavaScriptRuntimeInput,
product: string,
asarHeaderAdmission: AsarHeaderAdmission,
signal?: AbortSignal,
): Promise<AuthorizedV8InspectorTarget> => {
throwIfRuntimeObservationCancelled(signal);
const target = targets.find(({ id }) => id === input.target_id);
const target = selectCdpDiscoveryTarget(
discovery.targets,
input.target_id,
"observe_javascript_runtime",
discovery.ambiguousTargetIds,
);
if (target === undefined)
throw new BrowserObservationError(
"observe_javascript_runtime",
@@ -521,7 +525,7 @@ const authorizedTarget = async (
);
const decision = await authorizeRuntimeTargetLocation(
target.url,
{ type: target.type, product },
{ type: target.type, product: discovery.runtime.product },
signal,
"observe_javascript_runtime",
asarHeaderAdmission,