diff --git a/src/browser/BrowserScenarioSecrets.ts b/src/browser/BrowserScenarioSecrets.ts index 18c238342c..3e0f69f85d 100644 --- a/src/browser/BrowserScenarioSecrets.ts +++ b/src/browser/BrowserScenarioSecrets.ts @@ -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; diff --git a/src/browser/BrowserScenarioSessionPort.ts b/src/browser/BrowserScenarioSessionPort.ts index fc305d46d6..72d305e416 100644 --- a/src/browser/BrowserScenarioSessionPort.ts +++ b/src/browser/BrowserScenarioSessionPort.ts @@ -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; diff --git a/src/browser/PlaywrightBrowserScenarioProvider.ts b/src/browser/PlaywrightBrowserScenarioProvider.ts index b40c4925d3..9180132df4 100644 --- a/src/browser/PlaywrightBrowserScenarioProvider.ts +++ b/src/browser/PlaywrightBrowserScenarioProvider.ts @@ -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 => { if (options.signal?.aborted) throw new BrowserObservationError(OPERATION, "cancelled", { @@ -402,7 +403,10 @@ export class PlaywrightBrowserScenarioProvider implements BrowserScenarioCapture #tail: Promise = Promise.resolve(); #closing = false; #closePromise: Promise | undefined; - readonly #pendingCleanup = new Set(); + 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 { 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); } } diff --git a/src/browser/PlaywrightScenarioSession.ts b/src/browser/PlaywrightScenarioSession.ts index ab1951c949..00c41c5f72 100644 --- a/src/browser/PlaywrightScenarioSession.ts +++ b/src/browser/PlaywrightScenarioSession.ts @@ -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>, options: { readonly signal?: AbortSignal; - readonly retainCleanup?: (lease: AnalysisCleanupLease) => void; + readonly retainCleanup?: RetainBrowserScenarioCleanup; }, ): Promise { 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>, + secrets: BrowserScenarioSecrets, + options: { + readonly signal?: AbortSignal; + readonly retainCleanup?: RetainBrowserScenarioCleanup; + }, + ): Promise { const startedAt = performance.now(); const openingCleanup = new Set(); 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 { let events: PlaywrightScenarioEvents | undefined; diff --git a/src/domain/explicitSensitiveFailure.ts b/src/domain/explicitSensitiveFailure.ts index 75127a9d18..67babfcc73 100644 --- a/src/domain/explicitSensitiveFailure.ts +++ b/src/domain/explicitSensitiveFailure.ts @@ -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 => 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, { diff --git a/tests/composition/browserScenarioStartupRedaction.test.ts b/tests/composition/browserScenarioStartupRedaction.test.ts new file mode 100644 index 0000000000..8dc80b2127 --- /dev/null +++ b/tests/composition/browserScenarioStartupRedaction.test.ts @@ -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((resolve) => { + finishOpening = resolve; + }); + let launchStarted: () => void = () => undefined; + const started = new Promise((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(); + } + }, +);