[PR #1590] [MERGED] fix(browser): keep declared scenario secrets out of redacted evidence #2991

Closed
opened 2026-10-17 21:12:39 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1590
Author: @Adolanium
Created: 10/10/2026
Status: ✅ Merged
Merged: 10/10/2026
Merged by: @morluto

Base: main ← Head: fix/scenario-secret-redaction


📝 Commits (1)

  • ed500f3 fix(browser): keep declared scenario secrets out of redacted evidence

📊 Changes

4 files changed (+202 additions, -46 deletions)

View changed files

📝 src/browser/BrowserScenario.test.ts (+47 -0)
📝 src/browser/BrowserScenarioSecrets.ts (+92 -41)
📝 src/browser/PlaywrightScenarioArtifacts.test.ts (+58 -0)
📝 src/browser/PlaywrightScenarioArtifacts.ts (+5 -5)

📄 Description

Summary

Fixes #1591.

Declared scenario secrets could survive redaction when the marker [REDACTED:<id>] contained the secret, and storage identity fields were copied unchanged.

Problem and expected behavior

docs/browser-scenario-contract.md says declared secret values are removed from observed content, including a secret declared only so matching observations are removed. redactExplicitText already refuses a replacement that still contains a declared literal.

BrowserScenarioSecrets.redact always inserted [REDACTED:<secret_id>]. For secret id password and value pass, the result seen [REDACTED:password] in the page still contains pass. redactBytes searched only the original buffer, so a longer secret's marker could reintroduce a shorter secret. Cookie name, domain, and path, and localStorage / sessionStorage keys, were stored without redaction. Header names already went through redact.

Change and scope

Text, URL, and byte replacement now pick the first of [REDACTED:<id>], [REDACTED], …, and an empty string that does not contain a declared spelling. If a spelling remains, text becomes empty, a URL is returned as redacted with an empty URL, and bytes become an empty buffer. Storage capture redacts cookie names, domains, paths, and storage keys through the same text redactor.

Ordinary long secrets still use [REDACTED:<id>]. Longest-first replacement is unchanged.

Contract and boundary impact

  • Semantic owner and earliest changed stage: browser scenario secret redaction, while projecting captured text, URLs, bodies, and storage.
  • CLI and MCP/tool-catalog contract: none. The same scenario fields are returned. A marker that would keep a secret is replaced by a marker that does not, or the field is cleared and marked redacted.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: retained scenario text, URL strings, body bytes, and storage identity fields no longer include a declared secret spelling introduced by redaction. Digests still describe the retained bytes.
  • Process execution, authorization, cleanup, or containment impact: none.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Evidence and regression coverage

  • Tests added or updated: BrowserScenario.test.ts covers the pass / password marker and a byte body whose longer marker would have contained pass. PlaywrightScenarioArtifacts.test.ts covers a secret used as a cookie name and a storage key.
  • Base reproduction or other evidence: executed the previous replacement. pass remained inside [REDACTED:password], and a body containing only session-secret-value gained pass from [REDACTED:password].
  • User-visible CLI/MCP output (if applicable): scenario evidence uses [REDACTED] when the id marker would contain a declared secret. Storage names that equal a secret are redacted.
  • Remaining proof gaps: no live browser scenario was captured. The regression uses the redaction and artifact functions directly.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:focused -- src/browser/BrowserScenario.test.ts src/browser/PlaywrightScenarioArtifacts.test.ts — 17 tests passed.
  • npx tsc --noEmit --pretty false — no errors.
  • npx oxlint on the four edited files — 0 errors.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Callers that compared an exact [REDACTED:<id>] marker still get that marker when it does not contain a declared spelling. A short secret that is a substring of its id now produces [REDACTED] instead of a marker that still contains the secret.
  • Real Hopper/Ghidra, browser, or OS coverage: not run.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: declared secret values no longer remain in scenario evidence through a redaction marker or a storage identity field.

Review checklist

  • This PR addresses a concrete problem or an agreed enhancement.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1590 **Author:** [@Adolanium](https://github.com/Adolanium) **Created:** 10/10/2026 **Status:** ✅ Merged **Merged:** 10/10/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/scenario-secret-redaction` --- ### 📝 Commits (1) - [`ed500f3`](https://github.com/morluto/rea/commit/ed500f3bd3c724b3969cc428e66463303c462931) fix(browser): keep declared scenario secrets out of redacted evidence ### 📊 Changes **4 files changed** (+202 additions, -46 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/BrowserScenario.test.ts` (+47 -0) 📝 `src/browser/BrowserScenarioSecrets.ts` (+92 -41) 📝 `src/browser/PlaywrightScenarioArtifacts.test.ts` (+58 -0) 📝 `src/browser/PlaywrightScenarioArtifacts.ts` (+5 -5) </details> ### 📄 Description ## Summary Fixes #1591. Declared scenario secrets could survive redaction when the marker `[REDACTED:<id>]` contained the secret, and storage identity fields were copied unchanged. ## Problem and expected behavior `docs/browser-scenario-contract.md` says declared secret values are removed from observed content, including a secret declared only so matching observations are removed. `redactExplicitText` already refuses a replacement that still contains a declared literal. `BrowserScenarioSecrets.redact` always inserted `[REDACTED:<secret_id>]`. For secret id `password` and value `pass`, the result `seen [REDACTED:password] in the page` still contains `pass`. `redactBytes` searched only the original buffer, so a longer secret's marker could reintroduce a shorter secret. Cookie `name`, `domain`, and `path`, and `localStorage` / `sessionStorage` keys, were stored without redaction. Header names already went through `redact`. ## Change and scope Text, URL, and byte replacement now pick the first of `[REDACTED:<id>]`, `[REDACTED]`, `…`, and an empty string that does not contain a declared spelling. If a spelling remains, text becomes empty, a URL is returned as redacted with an empty URL, and bytes become an empty buffer. Storage capture redacts cookie names, domains, paths, and storage keys through the same text redactor. Ordinary long secrets still use `[REDACTED:<id>]`. Longest-first replacement is unchanged. ## Contract and boundary impact - Semantic owner and earliest changed stage: browser scenario secret redaction, while projecting captured text, URLs, bodies, and storage. - CLI and MCP/tool-catalog contract: none. The same scenario fields are returned. A marker that would keep a secret is replaced by a marker that does not, or the field is cleared and marked redacted. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: retained scenario text, URL strings, body bytes, and storage identity fields no longer include a declared secret spelling introduced by redaction. Digests still describe the retained bytes. - Process execution, authorization, cleanup, or containment impact: none. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. ## Evidence and regression coverage - Tests added or updated: `BrowserScenario.test.ts` covers the `pass` / `password` marker and a byte body whose longer marker would have contained `pass`. `PlaywrightScenarioArtifacts.test.ts` covers a secret used as a cookie name and a storage key. - Base reproduction or other evidence: executed the previous replacement. `pass` remained inside `[REDACTED:password]`, and a body containing only `session-secret-value` gained `pass` from `[REDACTED:password]`. - User-visible CLI/MCP output (if applicable): scenario evidence uses `[REDACTED]` when the id marker would contain a declared secret. Storage names that equal a secret are redacted. - Remaining proof gaps: no live browser scenario was captured. The regression uses the redaction and artifact functions directly. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:focused -- src/browser/BrowserScenario.test.ts src/browser/PlaywrightScenarioArtifacts.test.ts` — 17 tests passed. - `npx tsc --noEmit --pretty false` — no errors. - `npx oxlint` on the four edited files — 0 errors. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers that compared an exact `[REDACTED:<id>]` marker still get that marker when it does not contain a declared spelling. A short secret that is a substring of its id now produces `[REDACTED]` instead of a marker that still contains the secret. - Real Hopper/Ghidra, browser, or OS coverage: not run. - Package or release metadata impact: none. - Security, privacy, process, or containment review: declared secret values no longer remain in scenario evidence through a redaction marker or a storage identity field. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-17 21:12:39 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#2991
No description provided.