fix(browser): reject credentialed page URLs safely#102952
Conversation
Co-authored-by: bitkyc08 <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b34a20f4b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const diagnostic = rawUrl.includes("@") ? "[redacted credential-bearing URL]" : rawUrl; | ||
| throw new InvalidBrowserNavigationUrlError(`Invalid URL: ${diagnostic}`); |
There was a problem hiding this comment.
When a malformed Basic-auth URL omits or encodes the @ delimiter (for example https://user:secret or https://user:secret%40example.com), new URL throws and this rawUrl.includes("@") check takes the raw branch, so the Browser tool/server error includes secret. That leaves credential-like parse failures unredacted even though this path is intended to stop credentialed navigations before dispatch; redact authority strings that look like userinfo before echoing invalid URLs.
Useful? React with 👍 / 👎.
|
Land-ready verification for exact PR head
Known proof gaps: none for the changed Browser navigation/authentication surface. |
|
Merged via squash.
|
|
Post-merge integration verification on latest
The Testbox was stopped after the successful command. Current |
Co-authored-by: bitkyc08 <[email protected]>
Co-authored-by: bitkyc08 <[email protected]>
Co-authored-by: bitkyc08 <[email protected]> (cherry picked from commit d5fb490)
1 participant
What Problem This Solves
Browser
openandnavigateaccepted page URLs containing userinfo. Chromium/Playwright can reject those URLs after the credential-bearing string has crossed tool or node-proxy boundaries, and malformed credential-like URLs could also reach dispatch before validation.Closes #55365. This carries forward and improves #82490 by @lidge-jun after the nominally maintainer-editable fork branch could not be updated.
Why This Change Was Made
Use one canonical navigation URL parser at both boundaries that matter:
Parsed URL userinfo is rejected with guidance to supported authentication flows. Malformed credential-bearing input is fully redacted in diagnostics. The refactor removes the separate one-use helper from #82490 and keeps the steady-state path small.
The supported alternatives remain intact: explicit Browser HTTP credentials, authenticated browser profiles, and credentials on configured CDP endpoint URLs.
User Impact
Credential-bearing page URLs now fail fast without exposing the credential in the error. Ordinary navigation still works, and users who need authentication retain the dedicated Browser credential/profile/CDP flows.
Evidence
navigation-guard, Browser tool host/node dispatch, Playwright navigation guard, authenticated CDP helpers).oxfmt --check, targetedoxlint, andgit diff --checkpassed.Contributor credit is preserved in the commit trailer.