Skip to content

fix(browser): CDP auth fails with percent-encoded credentials#97972

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
VectorPeak:fix/browser-cdp-base-url
Jul 1, 2026
Merged

fix(browser): CDP auth fails with percent-encoded credentials#97972
vincentkoc merged 1 commit into
openclaw:mainfrom
VectorPeak:fix/browser-cdp-base-url

Conversation

@VectorPeak

@VectorPeak VectorPeak commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR fixes browser CDP Basic auth failures when the configured endpoint URL contains percent-encoded username or password characters.

Related: #93680 also extends getHeadersWithAuth for extension-bridge Bearer auth. This PR is complementary: it only decodes percent-encoded URL username/password before constructing the existing Basic Authorization header, and preserves the current precedence where explicit URL credentials win over fallback auth.

What Problem This Solves

Browser CDP endpoints can be configured with URL-embedded Basic auth credentials, for example:

http://alice:p%40ss%20word@127.0.0.1:9222/json/version

In URL userinfo, reserved characters must be percent-encoded. The password above represents the real credential value p@ss word, where %40 is @ and %20 is a space.

The browser CDP helper read URL.username and URL.password directly when constructing the Authorization: Basic ... header. In Node's WHATWG URL implementation, those properties remain percent-encoded when read, so OpenClaw built the Basic auth payload from the literal string:

alice:p%40ss%20word

instead of the credential the user configured:

alice:p@ss word

That causes remote CDP services requiring Basic auth to reject otherwise valid credentials whenever the username or password contains characters such as @, spaces, :, or other URL-reserved characters that are commonly percent-encoded in URLs.

Why This Change Was Made

Decode the URL username/password components before building the Basic auth header, while preserving the existing fallback behavior if decoding fails.

This keeps the browser CDP authentication surface narrow: guarded HTTP CDP fetches, WebSocket CDP connections, and Playwright CDP attach paths continue to share the same getHeadersWithAuth helper. Explicit Authorization headers still take precedence over URL credentials, and this PR does not change URL credential stripping from outbound request URLs.

User Impact

Users can configure browser CDP endpoints whose Basic auth credentials contain reserved characters such as @, spaces, or other percent-encoded characters, and OpenClaw will send the decoded credentials expected by the remote CDP service.

Change

  • Decode URL.username and URL.password before constructing the Basic auth payload.
  • Keep the prior defensive behavior for malformed percent-encoding by falling back to the original value.
  • Add regression coverage for both direct getHeadersWithAuth() behavior and the guarded CDP fetch path.

Evidence

Before this fix, the credential-bearing URL:

http://alice:p%40ss%20word@127.0.0.1:9222/json/version

produced a Basic auth payload for:

alice:p%40ss%20word

After this fix, the same URL produces a Basic auth payload for:

alice:p@ss word

The guarded CDP fetch regression also verifies that the request URL still has credentials stripped before fetch:

http://127.0.0.1:9222/json/version

while the decoded Basic auth header is sent through request headers.

Real behavior proof after this fix:

  • Started a real local Chrome instance with --remote-debugging-port.
  • Placed Chrome's /json/version endpoint behind a loopback Basic-auth proxy.
  • Configured the proxy to accept only the decoded credential alice:p@ss word.
  • Called the proxy URL with encoded URL credentials: http://alice:p%40ss%20word@127.0.0.1:<proxy-port>/json/version.
  • Used the current branch's getHeadersWithAuth() output for the request headers.

Observed terminal output:

OpenClaw browser CDP encoded Basic credentials proof
Scope: local real Chrome CDP /json/version behind a loopback Basic-auth proxy.
No external service contacted; no file writes except a temporary Chrome profile.

Chrome CDP endpoint: http://127.0.0.1:55495/json/version
Basic-auth proxy: http://127.0.0.1:57127/json/version
configured credential URL: http://alice:p%40ss%20word@127.0.0.1:57127/json/version
expected decoded credential: alice:p@ss word

Before-fix source behavior (Node WHATWG URL read):
URL.password read value: p%40ss%20word
vulnerable Basic payload would be: alice:p%40ss%20word

After-fix request behavior:
request URL sent to proxy: http://127.0.0.1:57127/json/version
proxy observed request target: GET /json/version
proxy decoded Authorization payload: alice:p@ss word
CDP response status through proxy: 200
CDP response has webSocketDebuggerUrl: true
Chrome reported browser: Chrome/<version>

verdict: PASS - encoded URL credentials were decoded and accepted by the real CDP proxy path.

This proof covers the real Chrome CDP HTTP discovery path through the shared auth helper. The credentials in the proof are synthetic, and the only endpoint is local loopback.

Possible call chain / impact

The affected helper is shared by the browser CDP connection paths:

extensions/browser/src/browser/cdp.helpers.ts
  -> getHeadersWithAuth(...)
    -> fetchCdpChecked(...) / fetchJson(...) / fetchOk(...)
    -> openCdpWebSocket(...) / withCdpSocket(...)
    -> Playwright CDP attach headers through the shared helper

This PR only changes how URL-embedded Basic auth credentials are converted into an auth header. It does not change CDP endpoint parsing, SSRF policy, proxy handling, explicit Authorization header precedence, WebSocket connection behavior, or browser tool behavior after authentication succeeds.

Testing

  • pnpm install --frozen-lockfile
  • node scripts/run-vitest.mjs extensions/browser/src/browser/cdp.helpers.test.ts extensions/browser/src/browser/browser-utils.test.ts
    • 2 test files passed
    • 43 tests passed
  • node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.extensions.json extensions/browser/src/browser/browser-utils.test.ts extensions/browser/src/browser/cdp.helpers.test.ts extensions/browser/src/browser/cdp.helpers.ts
  • pnpm tsgo:extensions
  • pnpm tsgo:extensions:test
  • git diff --check
  • Local real-behavior proof with Chrome CDP /json/version behind a loopback Basic-auth proxy, using encoded URL credentials and the current branch's getHeadersWithAuth() output.

Limitations:

  • pnpm test:extension browser started the browser extension test lane but the Windows process exited with 3221226505 (0xc0000409) before reporting test assertions.
  • codex review --base origin/main could not run on this machine because Windows returned Access is denied for codex.exe.
  • The real-behavior proof covers local Chrome CDP HTTP discovery through a Basic-auth proxy. WebSocket and Playwright attach paths inherit the same shared helper behavior but are not separately integration-tested in this PR.

@clawsweeper

clawsweeper Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 10:09 PM ET / 02:09 UTC.

Summary
The PR decodes percent-encoded browser CDP URL username/password values before constructing Basic Authorization headers and adds regression tests for direct helper and guarded fetch paths.

PR surface: Source +10, Tests +29. Total +39 across 3 files.

Reproducibility: yes. Current main constructs Basic auth from encoded URL userinfo, and a focused Node URL check confirms the reported password is read as p%40ss%20word before decoding.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Wait for the two in-progress current-head CI shards to complete before merge.

Risk before merge

  • [P1] Two current-head CI shards were still in progress during this review, so maintainers should wait for normal completion before merge.

Maintainer options:

  1. Decide the mitigation before merge
    Land the shared helper fix after ordinary maintainer review and current-head CI completion.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No automated repair is needed; the remaining action is maintainer review plus normal current-head CI completion.

Security
Cleared: The diff only changes in-process CDP Basic auth header construction and tests; it adds no dependency, workflow, secret storage, or broader execution surface.

Review details

Best possible solution:

Land the shared helper fix after ordinary maintainer review and current-head CI completion.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main constructs Basic auth from encoded URL userinfo, and a focused Node URL check confirms the reported password is read as p%40ss%20word before decoding.

Is this the best way to solve the issue?

Yes. Decoding inside getHeadersWithAuth is the narrow shared fix for guarded HTTP CDP fetches, raw CDP WebSockets, and Playwright attach headers without changing config, endpoint parsing, or SSRF policy.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 54b09580f61b.

Label changes

Label justifications:

  • P2: The PR fixes a scoped browser plugin authentication failure for CDP endpoints whose Basic auth credentials contain URL-reserved characters.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from a real local Chrome CDP /json/version endpoint behind a loopback Basic-auth proxy that accepted only the decoded credential payload.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a real local Chrome CDP /json/version endpoint behind a loopback Basic-auth proxy that accepted only the decoded credential payload.
Evidence reviewed

PR surface:

Source +10, Tests +29. Total +39 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 11 1 +10
Tests 2 29 0 +29
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 40 1 +39

What I checked:

Likely related people:

  • steipete: Recent GitHub path history shows repeated browser CDP helper, runtime, timeout, and documentation maintenance across cdp.helpers.ts, cdp.ts, pw-session.ts, and chrome.ts. (role: recent area contributor; confidence: high; commits: 5dcb072f7f7a, c32748bc28d6, 44ceccd2beb3; files: extensions/browser/src/browser/cdp.helpers.ts, extensions/browser/src/browser/cdp.ts, extensions/browser/src/browser/pw-session.ts)
  • visionik: Merged CDP WebSocket discovery work touched cdp.helpers.ts and cdp.ts in the same URL normalization and attach surface. (role: browser CDP URL helper contributor; confidence: medium; commits: 4cfc8cd5beb2; files: extensions/browser/src/browser/cdp.helpers.ts, extensions/browser/src/browser/cdp.ts)
  • lightcap: Merged loopback CDP proxy-bypass work changed cdp.helpers.ts and chrome.ts around the same request, proxy, and credential-stripping surfaces. (role: recent browser CDP transport contributor; confidence: medium; commits: a37ebb2d496c; files: extensions/browser/src/browser/cdp.helpers.ts, extensions/browser/src/browser/chrome.ts)
  • eleqtrizit: Recent merged work validated discovered and initial CDP endpoints, adjacent to the same remote CDP auth and transport boundary. (role: adjacent CDP security/runtime contributor; confidence: medium; commits: ade5ac03506e, c3f84279739d; files: extensions/browser/src/browser/pw-session.ts, extensions/browser/src/browser/cdp.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 30, 2026
@VectorPeak
VectorPeak marked this pull request as ready for review June 30, 2026 02:01
@VectorPeak

VectorPeak commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

I have updated the PR body with the requested real behavior proof.

The new Evidence section now includes an after-fix local run using a real Chrome CDP /json/version endpoint behind a loopback Basic-auth proxy. The configured URL uses encoded credentials (alice:p%40ss%20word), while the proxy accepts only the decoded credential payload (alice:p@ss word). The observed request succeeded with HTTP 200, the proxy decoded the Authorization payload as alice:p@ss word, and the CDP response included webSocketDebuggerUrl: true.

This proof is scoped to the real Chrome CDP HTTP discovery path through the shared getHeadersWithAuth() helper, and the PR limitations now explicitly state that WebSocket / Playwright attach paths inherit the same helper behavior but are not separately integration-tested here.

@clawsweeper

clawsweeper Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 30, 2026
@vincentkoc vincentkoc self-assigned this Jul 1, 2026
@vincentkoc
vincentkoc merged commit 1cf6ff3 into openclaw:main Jul 1, 2026
210 of 225 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@VectorPeak @vincentkoc