morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4731] [MERGED] fix(browser): preserve URL paths when removing credentials #6377

New issue

Closed

opened 2026-10-31 01:14:00 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:14:00 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: codex/fix-browser-url-credential-span

📝 Commits (3)

75963a4 fix(browser): preserve URL paths when removing credentials
f888dee test(browser): verify URL userinfo through real V8 captures
c942f64 chore: integrate upstream CI compatibility fixes

📊 Changes

6 files changed (+264 additions, -45 deletions)

View changed files

➕ scripts/lib/browser-url-userinfo-e2e.mjs (+186 -0)
📝 scripts/verify-real-browser.mjs (+3 -0)
📝 src/domain/browserObservation.test.ts (+36 -0)
📝 src/domain/browserObservation.ts (+8 -27)
📝 tests/boundary/browser/cdpBrowserProvider.targetInventory.test.ts (+27 -17)
📝 tests/boundary/cli/keyedArchivePrototypeKeyCli.test.ts (+4 -1)

📄 Description

Problem and outcome

Removing userinfo from https://user:pass@allowed.example\path@tail/../keep?x=1#frag changed its host to tail, while the reported origin still named allowed.example. The duplicate authority parser mistook a path @ after a special-scheme backslash for userinfo.

Reuse the shared transport parser's credential spans and remove only those original bytes. Ambiguous spellings keep the existing credential-free parsed fallback. Relative/unparseable URL behavior is unchanged; this complements #2878's proposed relative-reference handling. The superseded authority parser is removed.

Validation

Head c942f642a937fc539f08a4fe5fae445c5a81d1e7, normally merged upstream 78cf6100; Windows x64, pinned Node 24.18.0/npm 11.16.0.

Original production baseline: 2 failed / 6 passed, including compiled CLI output with the wrong host.
Final integration: 58/58 passed across the original seven focused files plus the upstream keyed-archive CLI/MCP file; npm run check 6/6 plus architecture guards passed; git diff --check passed. The upstream formatting-only call(...) correction unblocks the shared static gate without changing assertions. Original test discovery: 1,498 files exactly once; this follow-up adds verifier scripts, not Vitest files.
Real installed Edge 155.0.4283.45 completed npm run verify:browser with an isolated owned headless profile, covering public CLI/stdio MCP observation and bundle analysis, screenshots, network, script export, module tracing and scenario workflows. After the final load barrier was added, its exact helper was verified again in a fresh real Edge process. The browser production/verifier files are byte-identical to this f888dee8 validation stage after the final upstream merge.
The new verifier confirms real V8 Debugger.scriptParsed retains both raw credential-bearing and credential-free sourceURL labels. Public CLI and stdio MCP preserve the original path/query/fragment while removing userinfo, and independently verify selected source bytes and SHA-256. Independent reviews found no actionable issue.

An earlier expanded Windows selection reported 16 passed / 1 failed: the unchanged browser-startup SIGTERM fixture expected Unix signal termination, but Windows reported exit code 1 without a signal. That failure remains recorded; no assertion or production behavior was weakened to hide it. Windows process-family lineage proof also remains unavailable in the verifier report; resource-close completion is not claimed as whole-family proof.

Browser navigation normalizes backslashes and dot segments; the new regression explicitly exercises real V8 script labels. It does not claim raw navigation/HTTP URL preservation. Existing 1,101-target capacity and attachment assertions remain intact.

The current PR real Chrome job passed on Chrome/155.0.8059.39 with verified: true. The new urlUserInfo report confirms CLI, stdio MCP, raw V8 spelling, the credential-free control and original-source digest checks all passed. Its Linux point-in-time lineage check succeeded; this does not replace the unavailable Windows lineage proof. The current-head required CI passed. Earlier static-gate failures and their skipped Chrome jobs are not reported as executed provider evidence. Prepared with AI assistance.

Author readiness

Human owner: @ShuaixinHuang

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 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/4731 **Author:** [@ShuaixinHuang](https://github.com/ShuaixinHuang) **Created:** 10/17/2026 **Status:** ✅ Merged **Merged:** 10/17/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/fix-browser-url-credential-span` --- ### 📝 Commits (3) - [`75963a4`](https://github.com/morluto/rea/commit/75963a47ecb84d2d291bf3a5fb3094388202754f) fix(browser): preserve URL paths when removing credentials - [`f888dee`](https://github.com/morluto/rea/commit/f888dee8ee7ee960d5c142a73411693243584acd) test(browser): verify URL userinfo through real V8 captures - [`c942f64`](https://github.com/morluto/rea/commit/c942f642a937fc539f08a4fe5fae445c5a81d1e7) chore: integrate upstream CI compatibility fixes ### 📊 Changes **6 files changed** (+264 additions, -45 deletions) <details> <summary>View changed files</summary> ➕ `scripts/lib/browser-url-userinfo-e2e.mjs` (+186 -0) 📝 `scripts/verify-real-browser.mjs` (+3 -0) 📝 `src/domain/browserObservation.test.ts` (+36 -0) 📝 `src/domain/browserObservation.ts` (+8 -27) 📝 `tests/boundary/browser/cdpBrowserProvider.targetInventory.test.ts` (+27 -17) 📝 `tests/boundary/cli/keyedArchivePrototypeKeyCli.test.ts` (+4 -1) </details> ### 📄 Description ## Problem and outcome Removing userinfo from `https://user:pass@allowed.example\path@tail/../keep?x=1#frag` changed its host to `tail`, while the reported origin still named `allowed.example`. The duplicate authority parser mistook a path `@` after a special-scheme backslash for userinfo. Reuse the shared transport parser's credential spans and remove only those original bytes. Ambiguous spellings keep the existing credential-free parsed fallback. Relative/unparseable URL behavior is unchanged; this complements #2878's proposed relative-reference handling. The superseded authority parser is removed. ## Validation Head `c942f642a937fc539f08a4fe5fae445c5a81d1e7`, normally merged upstream `78cf6100`; Windows x64, pinned Node 24.18.0/npm 11.16.0. - Original production baseline: **2 failed / 6 passed**, including compiled CLI output with the wrong host. - Final integration: **58/58 passed** across the original seven focused files plus the upstream keyed-archive CLI/MCP file; `npm run check` **6/6 plus architecture guards passed**; `git diff --check` passed. The upstream formatting-only `call(...)` correction unblocks the shared static gate without changing assertions. Original test discovery: **1,498 files exactly once**; this follow-up adds verifier scripts, not Vitest files. - Real installed Edge **155.0.4283.45** completed `npm run verify:browser` with an isolated owned headless profile, covering public CLI/stdio MCP observation and bundle analysis, screenshots, network, script export, module tracing and scenario workflows. After the final load barrier was added, its exact helper was verified again in a fresh real Edge process. The browser production/verifier files are byte-identical to this `f888dee8` validation stage after the final upstream merge. - The new verifier confirms real V8 `Debugger.scriptParsed` retains both raw credential-bearing and credential-free `sourceURL` labels. Public CLI and stdio MCP preserve the original path/query/fragment while removing userinfo, and independently verify selected source bytes and SHA-256. Independent reviews found no actionable issue. An earlier expanded Windows selection reported **16 passed / 1 failed**: the unchanged browser-startup SIGTERM fixture expected Unix signal termination, but Windows reported exit code 1 without a signal. That failure remains recorded; no assertion or production behavior was weakened to hide it. Windows process-family lineage proof also remains unavailable in the verifier report; resource-close completion is not claimed as whole-family proof. Browser navigation normalizes backslashes and dot segments; the new regression explicitly exercises real V8 script labels. It does not claim raw navigation/HTTP URL preservation. Existing 1,101-target capacity and attachment assertions remain intact. The current PR [real Chrome job](https://github.com/morluto/rea/actions/runs/39033277200/job/117398305238) passed on **Chrome/155.0.8059.39** with `verified: true`. The new `urlUserInfo` report confirms CLI, stdio MCP, raw V8 spelling, the credential-free control and original-source digest checks all passed. Its Linux point-in-time lineage check succeeded; this does not replace the unavailable Windows lineage proof. The [current-head required CI](https://github.com/morluto/rea/actions/runs/39033277200/job/117400383130) passed. Earlier static-gate failures and their skipped Chrome jobs are not reported as executed provider evidence. Prepared with AI assistance. ## Author readiness Human owner: @ShuaixinHuang - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:14:00 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:47:01 +00:00

[PR #6377] [CLOSED] fix(discovery): report unsupported_host for Linux-only tools off Linux (#5730) #7320

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#6377

No description provided.

Rows
Columns