From f09bb8a99773f79c7306b8799c86f856fe2c0d4b Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 13 Oct 2026 07:52:47 +0800 Subject: [PATCH] fix(browser): redact encoded declared secrets consistently --- src/browser/BrowserScenario.test.ts | 43 +++ src/browser/BrowserScenarioSecrets.ts | 328 ++++++++++++++----- src/browser/PlaywrightScenarioEvents.test.ts | 75 ++++- 3 files changed, 370 insertions(+), 76 deletions(-) diff --git a/src/browser/BrowserScenario.test.ts b/src/browser/BrowserScenario.test.ts index 2262f410a..ee736ee5e 100644 --- a/src/browser/BrowserScenario.test.ts +++ b/src/browser/BrowserScenario.test.ts @@ -80,6 +80,49 @@ const baseScenario = () => ({ }, }); +describe("browser scenario secret redaction", () => { + it("matches percent-escape hex case without case-folding raw secrets", () => { + const secrets = BrowserScenarioSecrets.resolve( + browserScenarioSchema.parse(baseScenario()), + { + REA_TEST_SESSION: "a/b?", + REA_TEST_PASSWORD: "password", + REA_TEST_COOKIE: "cookie", + }, + ); + if (secrets === undefined) throw new Error("Expected resolved secrets"); + + expect(secrets.redact("logged a%2fb%3f, a%2Fb%3F and a%2fb%3F")).toBe( + "logged [REDACTED:session], [REDACTED:session] and [REDACTED:session]", + ); + expect(secrets.redact("raw A/B? remains case-sensitive")).toBe( + "raw A/B? remains case-sensitive", + ); + expect( + secrets.redactBytes( + Buffer.concat([ + Buffer.from([0xff, 0]), + Buffer.from("a%2fb%3F"), + Buffer.from([0x80]), + ]), + ), + ).toEqual( + Buffer.concat([ + Buffer.from([0xff, 0]), + Buffer.from("[REDACTED:session]"), + Buffer.from([0x80]), + ]), + ); + expect( + secrets.sanitizeUrl("https://app.example.test/?token=a%2fb%3F").url, + ).toBe("https://app.example.test/?token=[REDACTED:session]"); + expect(secrets.fingerprint("stored a%2fb%3F")).toEqual({ + value_state: "redacted-secret", + value_sha256: null, + }); + }); +}); + describe("browserScenarioSchema", () => { it("accepts and normalizes a declared scenario", () => { const parsed = browserScenarioSchema.parse(baseScenario()); diff --git a/src/browser/BrowserScenarioSecrets.ts b/src/browser/BrowserScenarioSecrets.ts index 05fb99047..aa583c016 100644 --- a/src/browser/BrowserScenarioSecrets.ts +++ b/src/browser/BrowserScenarioSecrets.ts @@ -15,84 +15,229 @@ const REDACTION_PREFIX = "[REDACTED:"; const encodedSecretValues = (secret: string): ReadonlySet => new Set([ - secret, encodeURIComponent(secret), new URLSearchParams([["value", secret]]).toString().slice("value=".length), ]); -type SecretSpellings = "raw" | "url" | "bytes"; +type SecretSpellings = "url" | "text"; +interface SecretPattern { + readonly text: string; + readonly searchText: string; + readonly percentInsensitive: boolean; +} +interface SecretReplacement extends SecretPattern { + readonly id: string; +} +interface SecretNeedle { + readonly id: string; + readonly needle: Buffer; + readonly percentInsensitive: boolean; + readonly marker: Buffer; +} -const secretSpellings = ( - secret: string, - spellings: SecretSpellings, -): readonly string[] => { - if (secret === "") return []; - if (spellings === "raw") return [secret]; - const values = [...encodedSecretValues(secret)]; - if (spellings === "bytes") values.push(JSON.stringify(secret).slice(1, -1)); - return [...new Set(values)].filter((value) => value !== ""); +const normalizePercentEscapes = (value: string): string => { + if (!value.includes("%")) return value; + return value.replace(/%[0-9a-f]{2}/giu, (escape) => escape.toLowerCase()); }; -const declaredLiterals = ( +const secretPatterns = ( + secret: string, + spellings: SecretSpellings, +): readonly SecretPattern[] => { + if (secret === "") return []; + const patterns: SecretPattern[] = [ + { text: secret, searchText: secret, percentInsensitive: false }, + ...[...encodedSecretValues(secret)].map((text) => ({ + text, + searchText: normalizePercentEscapes(text), + percentInsensitive: true, + })), + ]; + if (spellings === "text") + patterns.push({ + text: JSON.stringify(secret).slice(1, -1), + searchText: JSON.stringify(secret).slice(1, -1), + percentInsensitive: false, + }); + const seen = new Set(); + return patterns.filter(({ text, percentInsensitive }) => { + if (text === "") return false; + const key = `${percentInsensitive ? "percent" : "literal"}:${text}`; + if (seen.has(key)) return false; + seen.add(key); + return true; + }); +}; + +const declaredPatterns = ( values: ReadonlyMap, spellings: SecretSpellings, -): readonly string[] => [ - ...new Set( - [...values.values()].flatMap((secret) => - secretSpellings(secret, spellings), - ), - ), -]; +): readonly SecretPattern[] => { + const seen = new Set(); + return [...values.values()] + .flatMap((secret) => secretPatterns(secret, spellings)) + .filter(({ text, percentInsensitive }) => { + const key = `${percentInsensitive ? "percent" : "literal"}:${text}`; + if (seen.has(key)) return false; + seen.add(key); + return true; + }); +}; -/** Pick a marker that does not itself contain a declared secret spelling. */ -const redactionMarker = (id: string, literals: readonly string[]): string => - [`${REDACTION_PREFIX}${id}]`, "[REDACTED]", "…", ""].find((candidate) => - literals.every((literal) => !candidate.includes(literal)), - ) ?? ""; - -const replaceSecretLiterals = ( - value: string, +const secretReplacements = ( values: ReadonlyMap, spellings: SecretSpellings, - literals: readonly string[], -): string => { - const replacements = [...values] +): readonly SecretReplacement[] => + [...values] .flatMap(([id, secret]) => - secretSpellings(secret, spellings).map((text) => ({ id, text })), + secretPatterns(secret, spellings).map((pattern) => ({ id, ...pattern })), ) .sort( (left, right) => right.text.length - left.text.length || (left.id < right.id ? -1 : left.id > right.id ? 1 : 0), ); - let output = value; - for (const replacement of replacements) - output = output.replaceAll( - replacement.text, - redactionMarker(replacement.id, literals), - ); - return output; + +const normalizePercentEscapesBytes = (value: Buffer): Buffer => { + if (value.indexOf(37) < 0) return value; + const normalized = Buffer.from(value); + for (let index = 0; index + 2 < normalized.length; index += 1) + if ( + normalized[index] === 37 && + isHexByte(normalized[index + 1]) && + isHexByte(normalized[index + 2]) + ) { + normalized[index + 1] = lowercaseHexByte(normalized[index + 1]!); + normalized[index + 2] = lowercaseHexByte(normalized[index + 2]!); + index += 2; + } + return normalized; +}; + +const isHexByte = (value: number | undefined): boolean => + value !== undefined && + ((value >= 48 && value <= 57) || + (value >= 65 && value <= 70) || + (value >= 97 && value <= 102)); + +const lowercaseHexByte = (value: number): number => + value >= 65 && value <= 70 ? value + 32 : value; + +const patternMatches = ( + value: string, + normalizedValue: string, + pattern: SecretPattern, +): boolean => + (pattern.percentInsensitive ? normalizedValue : value).includes( + pattern.searchText, + ); + +const containsSecretPattern = ( + value: string, + normalizedValue: string, + patterns: readonly SecretPattern[], +): boolean => { + if (patterns.length === 0) return false; + return patterns.some((pattern) => + patternMatches(value, normalizedValue, pattern), + ); +}; + +const containsSecretBytePattern = ( + value: Buffer, + normalizedValue: Buffer, + needles: readonly SecretNeedle[], +): boolean => + needles.some((needle) => + (needle.percentInsensitive ? normalizedValue : value).includes( + needle.needle, + ), + ); + +/** Pick a marker that does not itself contain a declared secret spelling. */ +const redactionMarker = ( + id: string, + patterns: readonly SecretPattern[], +): string => { + const candidates = [`${REDACTION_PREFIX}${id}]`, "[REDACTED]", "…", ""]; + return ( + candidates.find((candidate) => + patterns.every( + (pattern) => + !patternMatches( + candidate, + normalizePercentEscapes(candidate), + pattern, + ), + ), + ) ?? "" + ); +}; + +const replaceSecretLiterals = ( + value: string, + replacements: readonly SecretReplacement[], + patterns: readonly SecretPattern[], +): string => { + const normalizedValue = replacements.some( + ({ percentInsensitive }) => percentInsensitive, + ) + ? normalizePercentEscapes(value) + : value; + const parts: string[] = []; + let offset = 0; + while (offset < value.length) { + let next: + | ((typeof replacements)[number] & { readonly index: number }) + | undefined; + for (const replacement of replacements) { + const search = replacement.searchText; + const view = replacement.percentInsensitive ? normalizedValue : value; + const index = view.indexOf(search, offset); + if ( + index >= 0 && + (next === undefined || + index < next.index || + (index === next.index && replacement.text.length > next.text.length)) + ) + next = { ...replacement, index }; + } + if (next === undefined) break; + const marker = redactionMarker(next.id, patterns); + parts.push(value.slice(offset, next.index), marker); + offset = next.index + next.text.length; + } + if (parts.length === 0) return value; + parts.push(value.slice(offset)); + return parts.join(""); }; const secretNeedles = ( - values: ReadonlyMap, - literals: readonly string[], -): Array<{ - readonly id: string; - readonly needle: Buffer; - readonly marker: Buffer; -}> => - [...values].flatMap(([id, secret]) => - secretSpellings(secret, "bytes").map((text) => ({ - id, - needle: Buffer.from(text), - marker: Buffer.from(redactionMarker(id, literals)), - })), - ); + replacements: readonly SecretReplacement[], + patterns: readonly SecretPattern[], +): SecretNeedle[] => + replacements.map((pattern) => ({ + id: pattern.id, + needle: Buffer.from(pattern.searchText), + percentInsensitive: pattern.percentInsensitive, + marker: Buffer.from(redactionMarker(pattern.id, patterns)), + })); /** Resolved secret values kept only for one in-memory scenario session. */ export class BrowserScenarioSecrets { - private constructor(private readonly values: ReadonlyMap) {} + private readonly urlPatterns: readonly SecretPattern[]; + private readonly textPatterns: readonly SecretPattern[]; + private readonly urlReplacements: readonly SecretReplacement[]; + private readonly textReplacements: readonly SecretReplacement[]; + private readonly textNeedles: readonly SecretNeedle[]; + + private constructor(private readonly values: ReadonlyMap) { + this.urlPatterns = declaredPatterns(values, "url"); + this.textPatterns = declaredPatterns(values, "text"); + this.urlReplacements = secretReplacements(values, "url"); + this.textReplacements = secretReplacements(values, "text"); + this.textNeedles = secretNeedles(this.textReplacements, this.textPatterns); + } static resolve( scenario: BrowserScenario, @@ -127,18 +272,40 @@ export class BrowserScenarioSecrets { } redact(value: string): string { - const literals = declaredLiterals(this.values, "raw"); - const output = replaceSecretLiterals(value, this.values, "raw", literals); - return literals.some((literal) => output.includes(literal)) ? "" : output; + const patterns = this.textPatterns; + if (patterns.length === 0) return value; + const output = replaceSecretLiterals( + value, + this.textReplacements, + patterns, + ); + return containsSecretPattern( + output, + normalizePercentEscapes(output), + patterns, + ) + ? "" + : output; } - /** Replace declared UTF-8, URI/form, and JSON-escaped secrets without decoding binary data. */ + /** Replace declared UTF-8, URI/form, and JSON-escaped secret spellings. */ redactBytes(value: Buffer): Buffer { - const literals = declaredLiterals(this.values, "bytes"); - const candidates = secretNeedles(this.values, literals) + const patterns = this.textPatterns; + if (patterns.length === 0) return value; + const candidates = this.textNeedles; + const hasPercentPatterns = candidates.some( + ({ percentInsensitive }) => percentInsensitive, + ); + const normalizedValue = + hasPercentPatterns && value.indexOf(37) >= 0 + ? normalizePercentEscapesBytes(value) + : value; + const sortedCandidates = candidates .map((candidate) => ({ ...candidate, - index: value.indexOf(candidate.needle), + index: (candidate.percentInsensitive ? normalizedValue : value).indexOf( + candidate.needle, + ), })) .sort( (left, right) => @@ -148,13 +315,18 @@ export class BrowserScenarioSecrets { const parts: Buffer[] = []; let offset = 0; while (true) { - let next: (typeof candidates)[number] | undefined; - for (const candidate of candidates) { + let next: (typeof sortedCandidates)[number] | undefined; + for (const candidate of sortedCandidates) { if (candidate.index >= 0 && candidate.index < offset) - candidate.index = value.indexOf(candidate.needle, offset); + candidate.index = ( + candidate.percentInsensitive ? normalizedValue : value + ).indexOf(candidate.needle, offset); if ( candidate.index >= 0 && - (next === undefined || candidate.index < next.index) + (next === undefined || + candidate.index < next.index || + (candidate.index === next.index && + candidate.needle.length > next.needle.length)) ) next = candidate; } @@ -165,16 +337,23 @@ export class BrowserScenarioSecrets { if (parts.length === 0) return value; parts.push(value.subarray(offset)); const redacted = Buffer.concat(parts); - return literals.some((literal) => redacted.includes(Buffer.from(literal))) + const normalizedRedacted = + hasPercentPatterns && redacted.indexOf(37) >= 0 + ? normalizePercentEscapesBytes(redacted) + : redacted; + return containsSecretBytePattern(redacted, normalizedRedacted, candidates) ? Buffer.alloc(0) : redacted; } /** Remove only declared secret values from one URL and report that action. */ sanitizeUrl(value: string): SanitizedBrowserUrl { - const literals = declaredLiterals(this.values, "url"); - const output = replaceSecretLiterals(value, this.values, "url", literals); - if (literals.some((literal) => output.includes(literal))) + const patterns = this.urlPatterns; + if (patterns.length === 0) return sanitizeBrowserUrl(value); + const output = replaceSecretLiterals(value, this.urlReplacements, patterns); + if ( + containsSecretPattern(output, normalizePercentEscapes(output), patterns) + ) return { url: "", origin: null, @@ -189,14 +368,15 @@ export class BrowserScenarioSecrets { } fingerprint(value: string): BrowserStorageValueFingerprint { - for (const secret of this.values.values()) - if (secret !== "" && value.includes(secret)) - return { value_state: "redacted-secret", value_sha256: null }; + const patterns = this.textPatterns; + if ( + patterns.length > 0 && + containsSecretPattern(value, normalizePercentEscapes(value), patterns) + ) + return { value_state: "redacted-secret", value_sha256: null }; return { value_state: "hashed", - value_sha256: createHash("sha256") - .update(this.redact(value)) - .digest("hex"), + value_sha256: createHash("sha256").update(value).digest("hex"), }; } } diff --git a/src/browser/PlaywrightScenarioEvents.test.ts b/src/browser/PlaywrightScenarioEvents.test.ts index 4c96e8d3d..4e7841365 100644 --- a/src/browser/PlaywrightScenarioEvents.test.ts +++ b/src/browser/PlaywrightScenarioEvents.test.ts @@ -5,6 +5,77 @@ import { browserScenarioSchema } from "../domain/browserScenario.js"; import { BrowserScenarioSecrets } from "./BrowserScenarioSecrets.js"; import { PlaywrightScenarioEvents } from "./PlaywrightScenarioEvents.js"; +describe("declared secrets in scenario events", () => { + it("redacts encoded declared secrets from text evidence and fingerprints", () => { + const listeners = new Map void>(); + const socketListeners = new Map void>(); + const page = { + on: (name: string, listener: (value: unknown) => void) => { + listeners.set(name, listener); + }, + } as unknown as Page; + const secret = "a/b&c\n d"; + const scenario = browserScenarioSchema.parse({ + browser: { mode: "launch", executable_path: "/opt/chromium" }, + start_url: { url: "https://app.example.test/" }, + actions: [ + { step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }, + ], + secrets: [ + { secret_id: "login_input", environment_variable: "REA_TEST_TOKEN" }, + ], + capture: { events: ["console", "page-errors", "websockets"] }, + }); + const secrets = BrowserScenarioSecrets.resolve(scenario, { + REA_TEST_TOKEN: secret, + }); + if (secrets === undefined) throw new Error("Expected resolved secrets"); + const events = new PlaywrightScenarioEvents({ + page, + enabled: new Set(["console", "page-errors", "websockets"]), + secrets, + }); + + listeners.get("console")?.({ + type: () => "log", + text: () => encodeURIComponent(secret), + location: () => ({ url: "https://app.example.test/app.js" }), + }); + const escapedSecret = JSON.stringify(secret).slice(1, -1); + const error = new Error(`failure ${escapedSecret}`); + error.stack = `failure ${escapedSecret}`; + listeners.get("pageerror")?.(error); + listeners.get("websocket")?.({ + url: () => "wss://app.example.test/socket", + on: (name: string, listener: (value: unknown) => void) => { + socketListeners.set(name, listener); + }, + }); + const formEncodedSecret = new URLSearchParams([["value", secret]]) + .toString() + .slice("value=".length); + socketListeners.get("framesent")?.({ payload: formEncodedSecret }); + + const items = events.result().items; + expect(items).toMatchObject([ + { kind: "console", text: "[REDACTED:login_input]" }, + { + kind: "page-error", + message: "failure [REDACTED:login_input]", + stack: "failure [REDACTED:login_input]", + }, + { kind: "websocket-opened" }, + { + kind: "websocket-frame-sent", + payload_text: "[REDACTED:login_input]", + }, + ]); + expect(secrets.fingerprint(`stored ${encodeURIComponent(secret)}`)).toEqual( + { value_state: "redacted-secret", value_sha256: null }, + ); + }); +}); + describe("PlaywrightScenarioEvents", () => { it("observes requested popup page errors independently of popup lifecycle events", () => { const listeners = new Map void>(); @@ -42,7 +113,7 @@ describe("PlaywrightScenarioEvents", () => { ]); }); - it("bounds oversized page errors before validating captured events", () => { + it("retains oversized page errors without truncating their text", () => { const listeners = new Map void>(); const page = { url: () => "https://app.example.test/", @@ -88,7 +159,7 @@ describe("PlaywrightScenarioEvents", () => { const error = new Error("m".repeat(70_000)); error.stack = "s".repeat(300_000); - expect(() => listeners.get("pageerror")?.(error)).not.toThrow(); + listeners.get("pageerror")?.(error); expect(events.result().items[0]).toMatchObject({ kind: "page-error", message: "m".repeat(70_000),