[PR #1612] [CLOSED] fix(browser): redact serialized accessibility secret names #3003

New issue

Closed

opened 2026-10-17 21:12:47 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:12:47 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1612
Author: @Y0UW1SH
Created: 10/10/2026
Status: ❌ Closed

Base: main ← Head: fix/browser-scenario-accessibility-secrets

📝 Commits (5)

02cb219 fix(browser): redact serialized accessibility secret names
14094b8 test(android): await verified shutdown cleanup recovery
faa8d82 fix(browser): reconcile accessibility redaction with main
7502009 test(ci): align Electron fixtures and scope MCP schema validation
4e564ae test(ci): use coverage deadline and ignore generated test reports

📊 Changes

14 files changed (+379 additions, -23 deletions)

View changed files

📝 .gitignore (+1 -0)
📝 docs/browser-scenario-contract.md (+6 -0)
📝 scripts/fixtures/browser-verifier-site.mjs (+17 -0)
➕ scripts/lib/browser-scenario-accessibility-redaction-e2e.mjs (+154 -0)
📝 scripts/lib/browser-scenario-verifier.mjs (+2 -2)
📝 scripts/verify-real-browser.mjs (+4 -0)
📝 src/application/javascript/JavaScriptArtifactPathResolution.test.ts (+1 -0)
📝 src/browser/BrowserScenarioSecrets.ts (+35 -0)
📝 src/browser/PlaywrightScenarioArtifacts.test.ts (+116 -0)
📝 src/browser/PlaywrightScenarioArtifacts.ts (+1 -1)
📝 tests/acceptance/applications/electronPreloadOverridesCli.test.ts (+7 -4)
📝 tests/acceptance/applications/runtime.test.ts (+21 -11)
📝 tests/boundary/process/jadxIntegration.test.ts (+10 -4)
📝 tests/composition/javascript-applications/electronOptionKeys.test.ts (+4 -1)

📄 Description

Status

Closed by the maintainer on 2026-10-11 at 01:28 UTC following the decision not to adopt accessibility-normalization redaction; see the closure explanation. The branch and verification history are retained. No merge to upstream main is claimed.

Summary

Refs #1608. Complements #1611's DOM and URI/form artifact escaping.

Declared secrets can survive Playwright accessibility serialization and accessible-name normalization. Redact those known spellings before calculating retained artifact bytes and SHA-256, while preserving ordinary evidence.

Changes

Add accessibility-specific spelling candidates using the shared literal redactor's safe-marker and residual-secret guards.
Cover quotes, backslashes, control characters, whitespace normalization, overlapping declarations, empty normalized values, artifact identity and isolation from DOM normalization.
Verify the behavior through real Chrome CLI/MCP workflows, including ordinary evidence and external-target retention.
Retain the bounded-retry correction for the existing JADX shutdown-recovery test without changing production ownership checks.
Validate the MCP runtime test's named, discovered tool definitions instead of eagerly compiling all 170 output schemas. Preserve full inventory, output validation, lifecycle, fatal-record and cleanup assertions.
Let that catalog test inherit the existing bounded 30-second local / 60-second coverage deadline, with retries disabled. Ignore generated Vitest blob reports alongside other test outputs.

Main integration

Retained fork head: 2a1b2b885849b0151aa802e428a4d81fb59f0e12
Integrated upstream: 044aa34ce6e32708df70e5d5bed9d56a420a9e29
Verified tree: 45723e97add0cb972bcf626f4b8925894ead271a

Preserve both accessibility redaction and upstream's safe-marker, raw/URL/byte redaction and cookie/storage sanitization.

Upstream advanced during verification and added overlapping Electron fixture and runtime timeout corrections. The latest merge adopts all three Electron fixture files exactly from upstream, including ESM API identity and their original source ranges. The runtime test retains our reviewed named-schema optimization and coverage-aware deadline. Upstream's runtime change added no new assertions, so no regression coverage was lost. History is preserved without force-pushing.

Validation

Fresh checks on the merged tree, Linux / Node 24.19.0 / npm 11.9.0:

114 source tests passed, including browser redaction and Electron path/option cases.
7 compiled CLI/MCP tests passed, with 2 macOS-only skips, after rebuilding the runtime.
Build, npm run check:ci (8 tasks), npm run docs:check (6 tasks), whitespace checks and commit hooks passed.
Independent review found no merge blockers and confirmed the Electron files match upstream exactly and all runtime assertions remain.
Earlier independent accessibility probes passed 1,048 serialized spellings and three collision/reconstructed-boundary cases.

The previous head's complete CI run passed 7,198 tests / 89 named skips, all coverage gates, real-browser verification, Windows checks, Linux x64/ARM64 and macOS x64/ARM64 package checks. Its catalog test passed in 12.652s. That result predates the latest main merge; fresh CI is required for the current head.

The previous real Chrome receipt reports mocked:false, CLI/MCP accessibility redaction, retained hashes/sizes, ordinary evidence and external-target retention all true. The latest merge leaves the accessibility code unchanged.

The maintainer closed this PR before the latest fork update, so GitHub has not attached the new head or scheduled PR CI for it. The closed PR retains head 4e564aea; the latest fork merge has the local verification listed above, but no fresh full-CI result is claimed.

Local Chrome is unavailable, and local Electron CLI/composition execution is blocked by the host's incompatible process inspection. The real browser and Electron workflows therefore require GitHub CI. Local analyzer probes alone do not establish their success.

Contract and scope

No PR-specific CLI/MCP schema, tool catalog, browser lifecycle, version or generated-output changes. Only explicitly declared values and known Playwright spellings are matched. Retained bytes and digests identify the redacted text, including empty text if replacement boundaries reconstruct a declared spelling. DOM/URI artifact escaping remains #1611's complementary scope.

_{🔄 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/1612 **Author:** [@Y0UW1SH](https://github.com/Y0UW1SH) **Created:** 10/10/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/browser-scenario-accessibility-secrets` --- ### 📝 Commits (5) - [`02cb219`](https://github.com/morluto/rea/commit/02cb2196472163e49ccaa33f68bed776bc970b71) fix(browser): redact serialized accessibility secret names - [`14094b8`](https://github.com/morluto/rea/commit/14094b87c463ccc6078dee21c66186f7be00ff07) test(android): await verified shutdown cleanup recovery - [`faa8d82`](https://github.com/morluto/rea/commit/faa8d8201c86dadd9c432f76599e27ac0cea4662) fix(browser): reconcile accessibility redaction with main - [`7502009`](https://github.com/morluto/rea/commit/7502009651ad828f3cefec6dfc23808ca5ec2caf) test(ci): align Electron fixtures and scope MCP schema validation - [`4e564ae`](https://github.com/morluto/rea/commit/4e564aea4b9605d2de0ddaf0e34ed6e4f93288d1) test(ci): use coverage deadline and ignore generated test reports ### 📊 Changes **14 files changed** (+379 additions, -23 deletions) <details> <summary>View changed files</summary> 📝 `.gitignore` (+1 -0) 📝 `docs/browser-scenario-contract.md` (+6 -0) 📝 `scripts/fixtures/browser-verifier-site.mjs` (+17 -0) ➕ `scripts/lib/browser-scenario-accessibility-redaction-e2e.mjs` (+154 -0) 📝 `scripts/lib/browser-scenario-verifier.mjs` (+2 -2) 📝 `scripts/verify-real-browser.mjs` (+4 -0) 📝 `src/application/javascript/JavaScriptArtifactPathResolution.test.ts` (+1 -0) 📝 `src/browser/BrowserScenarioSecrets.ts` (+35 -0) 📝 `src/browser/PlaywrightScenarioArtifacts.test.ts` (+116 -0) 📝 `src/browser/PlaywrightScenarioArtifacts.ts` (+1 -1) 📝 `tests/acceptance/applications/electronPreloadOverridesCli.test.ts` (+7 -4) 📝 `tests/acceptance/applications/runtime.test.ts` (+21 -11) 📝 `tests/boundary/process/jadxIntegration.test.ts` (+10 -4) 📝 `tests/composition/javascript-applications/electronOptionKeys.test.ts` (+4 -1) </details> ### 📄 Description ## Status Closed by the maintainer on 2026-10-11 at 01:28 UTC following the decision not to adopt accessibility-normalization redaction; see [the closure explanation](https://github.com/morluto/rea/pull/1612#issuecomment-6104199019). The branch and verification history are retained. No merge to upstream main is claimed. ## Summary Refs #1608. Complements #1611's DOM and URI/form artifact escaping. Declared secrets can survive Playwright accessibility serialization and accessible-name normalization. Redact those known spellings before calculating retained artifact bytes and SHA-256, while preserving ordinary evidence. ## Changes - Add accessibility-specific spelling candidates using the shared literal redactor's safe-marker and residual-secret guards. - Cover quotes, backslashes, control characters, whitespace normalization, overlapping declarations, empty normalized values, artifact identity and isolation from DOM normalization. - Verify the behavior through real Chrome CLI/MCP workflows, including ordinary evidence and external-target retention. - Retain the bounded-retry correction for the existing JADX shutdown-recovery test without changing production ownership checks. - Validate the MCP runtime test's named, discovered tool definitions instead of eagerly compiling all 170 output schemas. Preserve full inventory, output validation, lifecycle, fatal-record and cleanup assertions. - Let that catalog test inherit the existing bounded 30-second local / 60-second coverage deadline, with retries disabled. Ignore generated Vitest blob reports alongside other test outputs. ## Main integration Retained fork head: `2a1b2b885849b0151aa802e428a4d81fb59f0e12` Integrated upstream: `044aa34ce6e32708df70e5d5bed9d56a420a9e29` Verified tree: `45723e97add0cb972bcf626f4b8925894ead271a` Preserve both accessibility redaction and upstream's safe-marker, raw/URL/byte redaction and cookie/storage sanitization. Upstream advanced during verification and added overlapping Electron fixture and runtime timeout corrections. The latest merge adopts all three Electron fixture files exactly from upstream, including ESM API identity and their original source ranges. The runtime test retains our reviewed named-schema optimization and coverage-aware deadline. Upstream's runtime change added no new assertions, so no regression coverage was lost. History is preserved without force-pushing. ## Validation Fresh checks on the merged tree, Linux / Node 24.19.0 / npm 11.9.0: - **114 source tests passed**, including browser redaction and Electron path/option cases. - **7 compiled CLI/MCP tests passed**, with **2 macOS-only skips**, after rebuilding the runtime. - Build, `npm run check:ci` (**8 tasks**), `npm run docs:check` (**6 tasks**), whitespace checks and commit hooks passed. - Independent review found no merge blockers and confirmed the Electron files match upstream exactly and all runtime assertions remain. - Earlier independent accessibility probes passed **1,048 serialized spellings and three collision/reconstructed-boundary cases**. The previous head's [complete CI run](https://github.com/morluto/rea/actions/runs/38101260461) passed **7,198 tests / 89 named skips**, all coverage gates, real-browser verification, Windows checks, Linux x64/ARM64 and macOS x64/ARM64 package checks. Its catalog test passed in 12.652s. That result predates the latest main merge; fresh CI is required for the current head. The previous [real Chrome receipt](https://github.com/morluto/rea/actions/runs/38101260461/job/114357537061) reports `mocked:false`, CLI/MCP accessibility redaction, retained hashes/sizes, ordinary evidence and external-target retention all true. The latest merge leaves the accessibility code unchanged. The maintainer closed this PR before the latest fork update, so GitHub has not attached the new head or scheduled PR CI for it. The closed PR retains head `4e564aea`; the latest fork merge has the local verification listed above, but no fresh full-CI result is claimed. Local Chrome is unavailable, and local Electron CLI/composition execution is blocked by the host's incompatible process inspection. The real browser and Electron workflows therefore require GitHub CI. Local analyzer probes alone do not establish their success. ## Contract and scope No PR-specific CLI/MCP schema, tool catalog, browser lifecycle, version or generated-output changes. Only explicitly declared values and known Playwright spellings are matched. Retained bytes and digests identify the redacted text, including empty text if replacement boundaries reconstruct a declared spelling. DOM/URI artifact escaping remains #1611's complementary scope. --- <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:47 +00:00