fix(browser): redact startup cleanup diagnostics

This commit is contained in:
morluto
2026-10-27 04:59:22 +08:00
parent 855eb703af
commit 3233992733
6 changed files with 301 additions and 46 deletions
+7
View File
@@ -1,3 +1,5 @@
import type { AnalysisError } from "../domain/analysisErrorBase.js";
import { redactSensitiveFailure } from "../domain/explicitSensitiveFailure.js";
import { AnalysisInputError } from "../domain/analysisErrorCore.js";
import { err, ok, type Result } from "../domain/result.js";
import { createHash } from "node:crypto";
@@ -341,6 +343,11 @@ export class BrowserScenarioSecrets {
return Math.ceil(length * this.redactionCostPerCharacter);
}
redactFailure(error: AnalysisError): AnalysisError {
if (this.textPatterns.length === 0) return error;
return redactSensitiveFailure(error, (value) => this.redact(value));
}
redact(value: string): string {
const patterns = this.textPatterns;
if (patterns.length === 0) return value;
+7 -1
View File
@@ -41,11 +41,17 @@ export interface BrowserScenarioSessionPort {
redactError(error: unknown): string;
}
/** Retain the exact release authority and its producer-owned diagnostic projection. */
export type RetainBrowserScenarioCleanup = (
lease: AnalysisCleanupLease,
redactText?: (value: string) => string,
) => void;
/** Acquire one scenario session whose lifecycle the caller owns. */
export type BrowserScenarioSessionOpener = (
scenario: BrowserScenario,
options: {
readonly signal?: AbortSignal;
readonly retainCleanup?: (lease: AnalysisCleanupLease) => void;
readonly retainCleanup?: RetainBrowserScenarioCleanup;
},
) => Promise<BrowserScenarioSessionPort>;
@@ -42,6 +42,7 @@ import {
import { ProviderAdapterError } from "../domain/providerAdapterError.js";
import { err, ok, type Result } from "../domain/result.js";
import type {
RetainBrowserScenarioCleanup,
BrowserScenarioSessionOpener,
BrowserScenarioSessionPort,
} from "./BrowserScenarioSessionPort.js";
@@ -252,7 +253,7 @@ const runScenario = async (
openSession: BrowserScenarioSessionOpener,
scenario: BrowserScenario,
options: ExecutionOptions,
retainCleanup: (lease: AnalysisCleanupLease) => void,
retainCleanup: RetainBrowserScenarioCleanup,
): Promise<BrowserScenarioCapture> => {
if (options.signal?.aborted)
throw new BrowserObservationError(OPERATION, "cancelled", {
@@ -402,7 +403,10 @@ export class PlaywrightBrowserScenarioProvider implements BrowserScenarioCapture
#tail: Promise<void> = Promise.resolve();
#closing = false;
#closePromise: Promise<void> | undefined;
readonly #pendingCleanup = new Set<AnalysisCleanupLease>();
readonly #pendingCleanup = new Map<
AnalysisCleanupLease,
(value: string) => string
>();
constructor(private readonly openSession: BrowserScenarioSessionOpener) {}
@@ -436,10 +440,15 @@ export class PlaywrightBrowserScenarioProvider implements BrowserScenarioCapture
const retained: AnalysisCleanupLease[] = [];
try {
return ok(
await runScenario(this.openSession, scenario, options, (lease) => {
this.#pendingCleanup.add(lease);
retained.push(lease);
}),
await runScenario(
this.openSession,
scenario,
options,
(lease, redactText = (value) => value) => {
this.#pendingCleanup.set(lease, redactText);
retained.push(lease);
},
),
);
} catch (cause: unknown) {
const primary =
@@ -453,11 +462,15 @@ export class PlaywrightBrowserScenarioProvider implements BrowserScenarioCapture
if (retained.length === 0) return err(primary);
const cleanup = new ProviderCleanupError(
PLAYWRIGHT_BROWSER_SCENARIO_PROVIDER_IDENTITY.id,
retained.flatMap((lease) => lease.cleanupResources ?? []),
retained.flatMap((lease) =>
(lease.cleanupResources ?? []).map(
this.#pendingCleanup.get(lease) ?? ((value) => value),
),
),
{ reason: "Browser scenario cleanup capabilities remain retained." },
{ operation: OPERATION, cause },
);
new AnalysisCleanupLeaseGroup(cleanup, retained);
this.#bindCleanup(cleanup, retained);
return err(
analysisErrorWithCleanupFailure(
primary,
@@ -489,9 +502,33 @@ export class PlaywrightBrowserScenarioProvider implements BrowserScenarioCapture
});
}
#bindCleanup(
error: AnalysisError,
leases: readonly AnalysisCleanupLease[],
): void {
new AnalysisCleanupLeaseGroup(error, leases);
const projections = this.#pendingCleanup;
error.bindCleanupResourceOwner({
get cleanupResources() {
const pending = leases.filter((lease) => !lease.released);
if (pending.some((lease) => lease.cleanupResources === undefined))
return undefined;
return [
...new Set(
pending.flatMap((lease) =>
(lease.cleanupResources ?? []).map(
projections.get(lease) ?? ((value) => value),
),
),
),
];
},
});
}
async #retryCleanup(): Promise<AnalysisError | undefined> {
const failures: AnalysisError[] = [];
for (const pending of this.#pendingCleanup) {
for (const [pending, redactText] of this.#pendingCleanup) {
try {
await withPlaywrightExecutionBoundary(
() => pending.release(),
@@ -501,11 +538,15 @@ export class PlaywrightBrowserScenarioProvider implements BrowserScenarioCapture
} catch (cause: unknown) {
const failure = new ProviderCleanupError(
PLAYWRIGHT_BROWSER_SCENARIO_PROVIDER_IDENTITY.id,
pending.cleanupResources ?? [],
{ reason: cause instanceof Error ? cause.message : String(cause) },
(pending.cleanupResources ?? []).map(redactText),
{
reason: redactText(
cause instanceof Error ? cause.message : String(cause),
),
},
{ operation: OPERATION, cause },
);
new AnalysisCleanupLeaseGroup(failure, [pending]);
this.#bindCleanup(failure, [pending]);
failures.push(failure);
}
}
+34 -3
View File
@@ -11,7 +11,10 @@ import { PLAYWRIGHT_BROWSER_SCENARIO_PROVIDER_IDENTITY } from "./providerIdentit
import { ProviderAdapterError } from "../domain/providerAdapterError.js";
import { AnalysisError } from "../domain/analysisErrorBase.js";
import { BrowserObservationError } from "../domain/browserObservationError.js";
import type { BrowserScenarioSessionPort } from "./BrowserScenarioSessionPort.js";
import type {
RetainBrowserScenarioCleanup,
BrowserScenarioSessionPort,
} from "./BrowserScenarioSessionPort.js";
import { BrowserScenarioSecrets } from "./BrowserScenarioSecrets.js";
import {
failBrowserScenarioOperation,
@@ -158,13 +161,41 @@ export class PlaywrightScenarioSession implements BrowserScenarioSessionPort {
environment: Readonly<Record<string, string | undefined>>,
options: {
readonly signal?: AbortSignal;
readonly retainCleanup?: (lease: AnalysisCleanupLease) => void;
readonly retainCleanup?: RetainBrowserScenarioCleanup;
},
): Promise<PlaywrightScenarioSession> {
throwIfScenarioCancelled(options.signal);
const resolved = BrowserScenarioSecrets.resolve(scenario, environment);
if (!resolved.ok) throw resolved.error;
const secrets = resolved.value;
const projectedOptions = {
...options,
retainCleanup: (lease: AnalysisCleanupLease) =>
options.retainCleanup?.(lease, (value) => secrets.redact(value)),
};
try {
return await this.openResolved(
scenario,
environment,
secrets,
projectedOptions,
);
} catch (cause: unknown) {
throw cause instanceof AnalysisError
? secrets.redactFailure(cause)
: cause;
}
}
private static async openResolved(
scenario: BrowserScenario,
environment: Readonly<Record<string, string | undefined>>,
secrets: BrowserScenarioSecrets,
options: {
readonly signal?: AbortSignal;
readonly retainCleanup?: RetainBrowserScenarioCleanup;
},
): Promise<PlaywrightScenarioSession> {
const startedAt = performance.now();
const openingCleanup = new Set<AnalysisCleanupLease>();
const opening = openPlaywrightScenarioBrowser(scenario, environment, {
@@ -292,7 +323,7 @@ export class PlaywrightScenarioSession implements BrowserScenarioSessionPort {
secrets: BrowserScenarioSecrets,
options: {
readonly signal?: AbortSignal;
readonly retainCleanup?: (lease: AnalysisCleanupLease) => void;
readonly retainCleanup?: RetainBrowserScenarioCleanup;
},
): Promise<PlaywrightScenarioSession> {
let events: PlaywrightScenarioEvents | undefined;
+38 -30
View File
@@ -1,3 +1,4 @@
import { BrowserObservationError } from "./browserObservationError.js";
import { EvidenceFileError } from "./evidenceErrors.js";
import type { AnalysisError } from "./analysisErrorBase.js";
import {
@@ -24,7 +25,7 @@ import {
/** Exclude whole identity values; redacted text is no longer a path or digest. */
const excludeSnapshotIdentity = (
value: unknown,
values: readonly string[],
text: (value: string) => string,
): FileSnapshotPartialObservation | undefined => {
if (
value === null ||
@@ -37,10 +38,7 @@ const excludeSnapshotIdentity = (
if (!parsed.success) return undefined;
const snapshot = parsed.data;
const exclude = (identity: string | null): string | null =>
identity !== null &&
values.some((literal) => literal !== "" && identity.includes(literal))
? null
: identity;
identity !== null && text(identity) !== identity ? null : identity;
const path = exclude(snapshot.path);
if (snapshot.kind === "file-snapshot-hash") {
const sha256 = exclude(snapshot.sha256);
@@ -54,17 +52,18 @@ const excludeSnapshotIdentity = (
export const redactExplicitPointer = (
pointer: string,
values: readonly string[],
): string =>
redactSensitivePointer(pointer, (value) => redactExplicitText(value, values));
const redactSensitivePointer = (
pointer: string,
text: (value: string) => string,
): string => {
let parent = "";
for (const segment of pointer.split("/").slice(1)) {
const decoded = segment.replaceAll("~1", "/").replaceAll("~0", "~");
const child = `${parent}/${segment}`;
if (
values.some(
(literal) => decoded.includes(literal) || child.includes(literal),
)
)
return parent;
if (text(decoded) !== decoded || text(child) !== child) return parent;
parent = child;
}
return parent;
@@ -76,25 +75,29 @@ export const redactExplicitFailure = (
values: readonly string[],
): AnalysisError => {
if (values.length === 0) return error;
const snapshot = excludeSnapshotIdentity(error.partialObservation, values);
return redactSensitiveFailure(error, (value) =>
redactExplicitText(value, values),
);
};
/** Apply the producer's parsed sensitivity rules without exporting resolved secrets. */
export const redactSensitiveFailure = (
error: AnalysisError,
text: (value: string) => string,
): AnalysisError => {
const snapshot = excludeSnapshotIdentity(error.partialObservation, text);
if (snapshot !== undefined) error.partialObservation = snapshot;
const redacted = redactFailure(error, values);
const redacted = redactFailure(error, text);
// Other observation owners already apply their format-aware exclusions.
// Keep those facts across diagnostic replacements, including input/capacity.
if (redacted !== error && error.partialObservation !== undefined)
redacted.retainPartialObservation(error.partialObservation);
return redacted === error
? error
: mirrorCleanup(error, redacted, (value) =>
redactExplicitText(value, values),
);
return redacted === error ? error : mirrorCleanup(error, redacted, text);
};
const redactFailure = (
error: AnalysisError,
values: readonly string[],
text: (value: string) => string,
): AnalysisError => {
const text = (value: string): string => redactExplicitText(value, values);
const path = (
parts: readonly (string | number)[],
ordinaryNames: boolean,
@@ -104,11 +107,11 @@ const redactFailure = (
if (typeof part !== "string") {
retained.push(part);
} else if (!ordinaryNames && part.startsWith("/")) {
const pointer = redactExplicitPointer(part, values);
const pointer = redactSensitivePointer(part, text);
retained.push(pointer);
if (pointer !== part) break;
} else {
if (values.some((literal) => part.includes(literal))) break;
if (text(part) !== part) break;
retained.push(part);
}
}
@@ -119,11 +122,11 @@ const redactFailure = (
): Record<string, JsonValue> =>
Object.fromEntries(
Object.entries(value)
.filter(([key]) => !values.some((literal) => key.includes(literal)))
.filter(([key]) => text(key) === key)
.map(([key, item]) => [key, visit(item)]),
);
const visit = (value: JsonValue): JsonValue => {
const snapshot = excludeSnapshotIdentity(value, values);
const snapshot = excludeSnapshotIdentity(value, text);
if (snapshot !== undefined) return snapshot;
if (typeof value === "string") return text(value);
if (Array.isArray(value)) return value.map(visit);
@@ -148,7 +151,7 @@ const redactFailure = (
...(error.cleanupPrimary === undefined
? {}
: {
cleanupPrimary: redactExplicitFailure(error.cleanupPrimary, values),
cleanupPrimary: redactSensitiveFailure(error.cleanupPrimary, text),
}),
...(error.diagnostics === undefined
? {}
@@ -171,12 +174,17 @@ const redactFailure = (
},
}),
};
if (error instanceof BrowserObservationError)
return new BrowserObservationError(error.operation, error.reason, {
...context,
...(error.detail === undefined ? {} : { detail: text(error.detail) }),
...(error.remediationAction === undefined
? {}
: { remediationAction: text(error.remediationAction) }),
});
if (error instanceof EvidenceFileError) {
const identity = (value: string | undefined): string | undefined =>
value !== undefined &&
values.some((literal) => literal !== "" && value.includes(literal))
? undefined
: value;
value !== undefined && text(value) !== value ? undefined : value;
const retainedPath = identity(error.path);
const publishedPath = identity(error.published?.path);
return new EvidenceFileError(error.operation, error.reason, {
@@ -0,0 +1,162 @@
import { BrowserObservationError } from "../../src/domain/browserObservationError.js";
import { PlaywrightScenarioSession } from "../../src/browser/PlaywrightScenarioSession.js";
import { chromium, type BrowserContext } from "playwright-core";
import { expect, it, vi } from "vitest";
import { captureBrowserScenario } from "../../src/application/BrowserScenarioCaptureService.js";
import { createBrowserScenarioProvider } from "../../src/composition/browserScenario.js";
import { browserScenarioSchema } from "../../src/domain/browserScenario.js";
import { projectAnalysisError } from "../../src/domain/analysisErrorProjection.js";
import {
AnalysisError,
primaryAnalysisError,
} from "../../src/domain/analysisErrorBase.js";
it.each([
{ late: false, secret: "declared-startup-private-value" },
{ late: false, secret: "browser_profile" },
{ late: false, secret: "startup private/value" },
{ late: true, secret: "declared-startup-private-value" },
{ late: true, secret: "browser_profile" },
])(
"excludes declared startup secrets across opening cleanup and retries: %j",
async ({ late, secret }) => {
const scenario = browserScenarioSchema.parse({
browser: { mode: "launch", executable_path: process.execPath },
start_url: { url: "https://app.test/" },
actions: [
{ step_id: "wait", action: "wait_for_timeout", duration_ms: 1 },
],
secrets: [
{ secret_id: "credential", environment_variable: "REA_STARTUP_SECRET" },
],
});
const diagnosticSecret = encodeURIComponent(secret).replace(
/%[0-9A-F]{2}/gu,
(escape) => escape.toLowerCase(),
);
let blocked = true;
let closeCalls = 0;
const context = {
// An unusable launch reply fails before a PlaywrightScenarioSession exists.
browser: () => null,
close: async () => {
closeCalls += 1;
if (blocked)
throw new Error(`startup cleanup failed ${diagnosticSecret}`);
},
} as unknown as BrowserContext;
let finishOpening: () => void = () => undefined;
const opening = new Promise<void>((resolve) => {
finishOpening = resolve;
});
let launchStarted: () => void = () => undefined;
const started = new Promise<void>((resolve) => {
launchStarted = resolve;
});
const launch = vi
.spyOn(chromium, "launchPersistentContext")
.mockImplementation(async () => {
launchStarted();
if (late) await opening;
return context;
});
const provider = createBrowserScenarioProvider({
REA_STARTUP_SECRET: secret,
});
if (provider.close === undefined)
throw new Error("Production scenario provider must expose cleanup");
const closeProvider = provider.close.bind(provider);
const cancellation = new AbortController();
const reason = { caller: "cancelled opening" };
try {
const capturing = captureBrowserScenario(provider, scenario, {
signal: cancellation.signal,
});
await started;
if (late) cancellation.abort(reason);
const result = await capturing;
if (result.ok) throw new Error("Expected startup failure");
expect(result.error.cleanupIncomplete).toBe(true);
const initialProjection = JSON.stringify(
projectAnalysisError(result.error),
);
expect(initialProjection).not.toContain(secret);
expect(initialProjection).not.toContain(diagnosticSecret);
if (!late && secret === "browser_profile")
expect(result.error.cleanupResources).toContain(
"[REDACTED:credential]",
);
if (late) {
expect(primaryAnalysisError(result.error).cause).toBe(reason);
expect(projectAnalysisError(result.error).code).toBe(
"cleanup_incomplete",
);
finishOpening();
}
let retry: unknown;
try {
await closeProvider();
} catch (error) {
retry = error;
}
if (!(retry instanceof AnalysisError))
throw new Error("Expected retained startup cleanup failure");
expect(retry.cleanupIncomplete).toBe(true);
const retryProjection = JSON.stringify(projectAnalysisError(retry));
expect(retryProjection).not.toContain(secret);
expect(retryProjection).not.toContain(diagnosticSecret);
if (secret === "browser_profile")
expect(retry.cleanupResources).toContain("[REDACTED:credential]");
expect(closeCalls).toBeGreaterThan(0);
blocked = false;
await closeProvider();
expect(result.error.cleanupIncomplete).toBe(false);
expect(retry.cleanupIncomplete).toBe(false);
expect(result.error.cleanupResources).toEqual([]);
} finally {
blocked = false;
finishOpening();
await closeProvider();
launch.mockRestore();
}
},
);
it.each([false, true])(
"preserves the original startup failure when no effective secret is declared: %s",
async (emptyDeclaration) => {
const scenario = browserScenarioSchema.parse({
browser: { mode: "launch", executable_path: process.execPath },
start_url: { url: "https://app.test/" },
actions: [
{ step_id: "wait", action: "wait_for_timeout", duration_ms: 1 },
],
secrets: emptyDeclaration
? [{ secret_id: "empty", environment_variable: "REA_EMPTY_SECRET" }]
: [],
});
const original = new BrowserObservationError(
"capture_browser_scenario",
"protocol_error",
{ detail: "ordinary observed startup failure" },
);
const close = vi.fn(async () => undefined);
const launch = vi
.spyOn(chromium, "launchPersistentContext")
.mockResolvedValue({
browser: () => {
throw original;
},
close,
} as unknown as BrowserContext);
try {
await expect(
PlaywrightScenarioSession.open(scenario, { REA_EMPTY_SECRET: "" }, {}),
).rejects.toBe(original);
expect(close).toHaveBeenCalledOnce();
expect(original.cleanupIncomplete).toBe(false);
} finally {
launch.mockRestore();
}
},
);