fix(browser): keep declared scenario secrets out of redacted evidence (#1590)

A redaction marker such as [REDACTED:password] can contain the secret it replaced, and the byte replacer never scanned the marker it inserted. Cookie names, domains, paths, and storage keys were also stored raw. Choose a marker that does not contain a declared spelling, drop the result when a spelling remains, and redact storage identity fields.

src/browser/BrowserScenario.test.ts

@@ -147,6 +147,53 @@ describe("browserScenarioSchema", () => {
     expect(secrets?.redact("prefix-suffix")).toBe("[REDACTED:login_input]");
   });
 
+  it("does not leave a declared secret inside its redaction marker", () => {
+    const secrets = BrowserScenarioSecrets.resolve(
+      browserScenarioSchema.parse({
+        ...baseScenario(),
+        secrets: [
+          ...baseScenario().secrets,
+          {
+            secret_id: "password",
+            environment_variable: "REA_TEST_SHORT_PASSWORD",
+          },
+        ],
+      }),
+      {
+        REA_TEST_SESSION: "prefix",
+        REA_TEST_PASSWORD: "prefix-suffix",
+        REA_TEST_COOKIE: "cookie",
+        REA_TEST_SHORT_PASSWORD: "pass",
+      },
+    );
+    expect(secrets?.redact("seen pass in the page")).toBe(
+      "seen [REDACTED] in the page",
+    );
+
+    const bytes = BrowserScenarioSecrets.resolve(
+      browserScenarioSchema.parse({
+        ...baseScenario(),
+        secrets: [
+          ...baseScenario().secrets,
+          {
+            secret_id: "password",
+            environment_variable: "REA_TEST_LONG_SECRET",
+          },
+          { secret_id: "pin", environment_variable: "REA_TEST_PIN" },
+        ],
+      }),
+      {
+        REA_TEST_SESSION: "prefix",
+        REA_TEST_PASSWORD: "prefix-suffix",
+        REA_TEST_COOKIE: "cookie",
+        REA_TEST_LONG_SECRET: "session-secret-value",
+        REA_TEST_PIN: "pass",
+      },
+    )?.redactBytes(Buffer.from("body session-secret-value tail"));
+    expect(bytes?.toString("utf8")).toBe("body [REDACTED] tail");
+    expect(bytes?.includes(Buffer.from("pass"))).toBe(false);
+  });
+
   it("does not resolve inherited environment properties as secret values", () => {
     const scenario = browserScenarioSchema.parse(baseScenario());
     const environment = Object.assign(

src/browser/BrowserScenarioSecrets.ts

@@ -20,6 +20,76 @@ const encodedSecretValues = (secret: string): ReadonlySet<string> =>
     new URLSearchParams([["value", secret]]).toString().slice("value=".length),
   ]);
 
+type SecretSpellings = "raw" | "url" | "bytes";
+
+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 declaredLiterals = (
+  values: ReadonlyMap<string, string>,
+  spellings: SecretSpellings,
+): readonly string[] => [
+  ...new Set(
+    [...values.values()].flatMap((secret) =>
+      secretSpellings(secret, spellings),
+    ),
+  ),
+];
+
+/** 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,
+  values: ReadonlyMap<string, string>,
+  spellings: SecretSpellings,
+  literals: readonly string[],
+): string => {
+  const replacements = [...values]
+    .flatMap(([id, secret]) =>
+      secretSpellings(secret, spellings).map((text) => ({ id, text })),
+    )
+    .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 secretNeedles = (
+  values: ReadonlyMap<string, string>,
+  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)),
+    })),
+  );
+
 /** Resolved secret values kept only for one in-memory scenario session. */
 export class BrowserScenarioSecrets {
   private constructor(private readonly values: ReadonlyMap<string, string>) {}
@@ -57,38 +127,19 @@ export class BrowserScenarioSecrets {
   }
 
   redact(value: string): string {
-    let output = value;
-    const replacements = [...this.values].sort(
-      ([leftId, left], [rightId, right]) =>
-        right.length - left.length ||
-        (leftId < rightId ? -1 : leftId > rightId ? 1 : 0),
-    );
-    for (const [id, secret] of replacements)
-      if (secret !== "")
-        output = output.replaceAll(secret, `${REDACTION_PREFIX}${id}]`);
-    return output;
+    const literals = declaredLiterals(this.values, "raw");
+    const output = replaceSecretLiterals(value, this.values, "raw", literals);
+    return literals.some((literal) => output.includes(literal)) ? "" : output;
   }
 
   /** Replace declared UTF-8, URI/form, and JSON-escaped secrets without decoding binary data. */
   redactBytes(value: Buffer): Buffer {
-    const candidates = [...this.values]
-      .flatMap(([id, secret]) => {
-        if (secret === "") return [];
-        return [
-          ...new Set([
-            ...encodedSecretValues(secret),
-            JSON.stringify(secret).slice(1, -1),
-          ]),
-        ].map((text) => {
-          const needle = Buffer.from(text);
-          return {
-            id,
-            needle,
-            marker: Buffer.from(`${REDACTION_PREFIX}${id}]`),
-            index: value.indexOf(needle),
-          };
-        });
-      })
+    const literals = declaredLiterals(this.values, "bytes");
+    const candidates = secretNeedles(this.values, literals)
+      .map((candidate) => ({
+        ...candidate,
+        index: value.indexOf(candidate.needle),
+      }))
       .sort(
         (left, right) =>
           right.needle.length - left.needle.length ||
@@ -113,23 +164,23 @@ export class BrowserScenarioSecrets {
     }
     if (parts.length === 0) return value;
     parts.push(value.subarray(offset));
-    return Buffer.concat(parts);
+    const redacted = Buffer.concat(parts);
+    return literals.some((literal) => redacted.includes(Buffer.from(literal)))
+      ? Buffer.alloc(0)
+      : redacted;
   }
 
   /** Remove only declared secret values from one URL and report that action. */
   sanitizeUrl(value: string): SanitizedBrowserUrl {
-    let output = value;
-    const replacements = [...this.values].sort(
-      ([leftId, left], [rightId, right]) =>
-        right.length - left.length ||
-        (leftId < rightId ? -1 : leftId > rightId ? 1 : 0),
-    );
-    for (const [id, secret] of replacements) {
-      if (secret === "") continue;
-      const marker = `${REDACTION_PREFIX}${id}]`;
-      for (const candidate of encodedSecretValues(secret))
-        output = output.replaceAll(candidate, marker);
-    }
+    const literals = declaredLiterals(this.values, "url");
+    const output = replaceSecretLiterals(value, this.values, "url", literals);
+    if (literals.some((literal) => output.includes(literal)))
+      return {
+        url: "",
+        origin: null,
+        query_parameter_names: [],
+        redacted: true,
+      };
     const sanitized = sanitizeBrowserUrl(output);
     return {
       ...sanitized,

src/browser/PlaywrightScenarioArtifacts.test.ts

@@ -110,3 +110,61 @@ it("redacts declared secrets from current and historical URL artifacts", async (
     },
   });
 });
+
+it("redacts a declared secret used as a cookie or storage name", async () => {
+  const secretValue = "session-token";
+  const scenarioWithSecret = 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: "storage_key", environment_variable: "REA_STORAGE_KEY" },
+    ],
+  });
+  const scenarioSecrets = BrowserScenarioSecrets.resolve(scenarioWithSecret, {
+    REA_STORAGE_KEY: secretValue,
+  });
+  if (scenarioSecrets === undefined)
+    throw new Error("Expected resolved storage secret");
+  const page = {
+    url: () => "https://app.example.test/",
+    evaluate: async () => ({
+      local_storage: [[secretValue, "kept-value"]],
+      session_storage: [["theme", secretValue]],
+    }),
+  } as unknown as Page;
+  const context = {
+    cookies: async () => [
+      {
+        name: secretValue,
+        value: "ordinary",
+        domain: "app.example.test",
+        path: "/",
+        secure: true,
+        httpOnly: false,
+        sameSite: "Lax" as const,
+      },
+    ],
+  } as unknown as BrowserContext;
+
+  const result = await capturePlaywrightStepArtifacts({
+    context,
+    page,
+    secrets: scenarioSecrets,
+    requested: new Set(["storage"]),
+  });
+
+  expect(JSON.stringify(result.storage)).not.toContain(secretValue);
+  expect(result.storage).toMatchObject({
+    state: "captured",
+    value: {
+      cookies: [{ name: "[REDACTED:storage_key]", value_state: "hashed" }],
+      local_storage: [
+        { name: "[REDACTED:storage_key]", value_state: "hashed" },
+      ],
+      session_storage: [
+        { name: "theme", value_state: "redacted-secret", value_sha256: null },
+      ],
+    },
+  });
+});

src/browser/PlaywrightScenarioArtifacts.ts

@@ -155,20 +155,20 @@ const captureStorage = async (input: {
     );
     const value = {
       cookies: cookies.map((cookie) => ({
-        name: cookie.name,
-        domain: cookie.domain,
-        path: cookie.path,
+        name: secrets.redact(cookie.name),
+        domain: secrets.redact(cookie.domain),
+        path: secrets.redact(cookie.path),
         secure: cookie.secure,
         http_only: cookie.httpOnly,
         same_site: cookie.sameSite,
         ...secrets.fingerprint(cookie.value),
       })),
       local_storage: pageStorage.local_storage.map(([name, value]) => ({
-        name,
+        name: secrets.redact(name),
         ...secrets.fingerprint(value),
       })),
       session_storage: pageStorage.session_storage.map(([name, value]) => ({
-        name,
+        name: secrets.redact(name),
         ...secrets.fingerprint(value),
       })),
     };