fix: redact credentials in browser.cdpUrl config paths by Ziy1-Tan · Pull Request #67679 · openclaw/openclaw

Ziy1-Tan · 2026-04-16T13:06:07Z

What

Add browser.cdpUrl and browser.profiles.*.cdpUrl to the set of config paths recognized as sensitive URLs, so embedded credentials (query tokens like ?token=xxx and HTTP Basic auth user:pass@host) are properly redacted in config.get API responses.

Also regenerate the base schema artifact so shipped uiHints stay aligned with the updated sensitive-URL matcher and continue to expose the correct url-secret metadata for the new cdpUrl paths.

Why

Browser CDP URLs can embed authentication credentials in two documented formats:

Query token: https://chrome.browserless.io?token=<secret>
HTTP Basic auth: https://user:pass@chrome.example.com

These paths bypassed all three redaction gates:

Schema .register(sensitive) — not registered
Path-name pattern match (/token$/i, /api.?key$/i, …) — cdpUrl does not match
URL-path match (.baseUrl, .httpUrl, mcp.servers.*.url) — .cdpUrl was not listed

Fixes #67656.

Changes

src/shared/net/redact-sensitive-url.ts — Add .cdpUrl suffix check to isSensitiveUrlConfigPath() (3 lines)
src/shared/net/redact-sensitive-url.test.ts — Unit tests for isSensitiveUrlConfigPath() with cdpUrl paths
src/config/redact-snapshot.test.ts — Snapshot redact/restore round-trip tests covering:
- browser.cdpUrl with query token + HTTP Basic auth
- browser.profiles.*.cdpUrl with per-profile credentials
- Bare cdpUrl addresses without credentials remain unchanged
src/config/schema.base.generated.ts — Regenerated the base schema artifact so shipped uiHints include url-secret metadata for browser.cdpUrl and browser.profiles.*.cdpUrl

Testing

pnpm build ✅
pnpm check ✅
node --import tsx scripts/generate-base-config-schema.ts --check ✅
All 49 related tests pass (8 unit + 41 snapshot)
Tested: fully tested

Test Case	Expected	Actual	PASS
`isSensitiveUrlConfigPath()` recognizes `.cdpUrl`	Both cdpUrl paths treated as sensitive	Unit test passes	✅
Redact `browser.cdpUrl` credentials	Secrets stripped, safe parts kept	Snapshot passes (token + Basic auth)	✅
Redact `browser.profiles.*.cdpUrl` credentials	Profile secrets stripped, round-trip safe	Snapshot passes	✅
Regenerated base schema artifact	`url-secret` tag on both cdpUrl paths	`--check` passes after regeneration	✅

AI Disclosure

This PR was AI-assisted (CodeBuddy / Claude). All code has been reviewed and understood by the author. Tests were written in TDD style (failing test first, then minimal implementation).

greptile-apps · 2026-04-16T13:09:14Z

Greptile Summary

Adds .cdpUrl to the suffix-based sensitive URL config path list in isSensitiveUrlConfigPath(), closing the credential-leakage gap where query tokens and HTTP Basic auth embedded in CDP URLs passed through all three redaction gates unredacted. The implementation is a minimal 3-line addition with good unit and integration test coverage, including a "bare URL unchanged" regression guard.

Confidence Score: 5/5

Safe to merge; the fix is minimal and well-tested with one minor style issue in CHANGELOG ordering.

All findings are P2 (changelog placement); no logic, security, or correctness issues found in the implementation or tests.

CHANGELOG.md (entry ordering only)

Prompt To Fix All With AI

This is a comment left during a code review.
Path: CHANGELOG.md
Line: 13

Comment:
**Changelog entry placed at top of section**

Per the repo convention, new entries should be appended to the *end* of the target `### Fixes` section, not inserted at the top. All the existing lines below (Gateway/tools, Agents/replay, …) were already present before this PR. Move the new bullet to after the last existing entry in `### Fixes`.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "docs: add CHANGELOG entry for cdpUrl con..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b3104ed993

ℹ️ 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".

Ziy1-Tan · 2026-04-17T03:25:14Z

@codex review

chatgpt-codex-connector · 2026-04-17T03:28:05Z

Codex Review: Didn't find any major issues. Keep them coming!

ℹ️ 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".

hxy91819 · 2026-04-18T01:41:40Z

Verified that cdpUrl is actively consumed for remote browser connections — not a dormant config field:

pw-session.ts:428 — connectBrowser() passes cdpUrl to chromium.connectOverCDP()
chrome.ts:197 — getChromeWebSocketUrl() resolves HTTP cdpUrls to WebSocket endpoints via /json/version
cdp.ts:20 — normalizeCdpWsUrl() inherits credentials (user:pass) and query params (?token=) from the cdpUrl into the WebSocket URL
cdp.helpers.ts:320 — getHeadersWithAuth() extracts user:pass from the URL and converts to Basic Auth headers

So credentials embedded in cdpUrl are transmitted on every connection attempt, making the redact fix genuinely necessary.

hxy91819 · 2026-04-18T02:30:07Z

@codex review

chatgpt-codex-connector · 2026-04-18T02:33:34Z

Codex Review: Didn't find any major issues. Hooray!

ℹ️ 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".

Browser CDP URLs (browser.cdpUrl, browser.profiles.*.cdpUrl) can embed credentials via query tokens (?token=xxx) or HTTP Basic auth (user:pass@host). Add .cdpUrl suffix to isSensitiveUrlConfigPath() so these paths are correctly redacted in config.get responses. Refs openclaw#67656, openclaw#53433

Cover three scenarios: - browser.cdpUrl with query token and HTTP Basic auth credentials - browser.profiles.*.cdpUrl with per-profile credentials - bare cdpUrl addresses without credentials remain unchanged Refs openclaw#67656, openclaw#53433

hxy91819 · 2026-04-18T06:23:06Z

Merged via squash.

Prepared head SHA: 77bc2c50ce4bbd10c925fcd9162dc139e05e8a9f
Merge commit: 4b5987829d0f82ea44ae50f2f418ffe5ea445e7f

Thanks @Ziy1-Tan!

PR openclaw#67679 landed a duplicate line under ### Changes in the Unreleased block in addition to the detailed entry that was already present under ### Fixes. The short ### Changes line (auto-generated from the PR title during merge) is a duplicate of the same PR's ### Fixes line and also mis-categorizes a security redaction fix as a feature change. Remove the duplicate and keep the ### Fixes entry, which is the right section and carries the descriptive text.

PR #67679 landed a duplicate line under ### Changes in the Unreleased block in addition to the detailed entry that was already present under ### Fixes. The short ### Changes line (auto-generated from the PR title during merge) is a duplicate of the same PR's ### Fixes line and also mis-categorizes a security redaction fix as a feature change. Remove the duplicate and keep the ### Fixes entry, which is the right section and carries the descriptive text.

Two related gaps in the CHANGELOG write path both caused PRs to land in the wrong place: 1) `resolve_merge_changelog_section` only consulted the environment override and PR labels. If a PR was only tagged with something like `size: S` (no `bug`/`fix`/`feature` label), the default fell through to Changes even when the PR title made the category obvious (e.g. `fix(cron): clean up deleteAfterRun direct deliveries` on PR openclaw#67807). That Conventional-Commits prefix is the repo's actual classification signal for most PRs and was being ignored. Add a title-prefix fallback after the label check: a title starting with `fix`, `bugfix`, or `hotfix` (followed by `(`, `:`, `!`, or whitespace) routes to Fixes; `feat`, `feature`, `enhance` route to Changes. Labels still win when present. Deliberately strict so `fixup!`, `fixing ...`, `fixture updates`, `featured posts` are not misread as `fix:`/`feat:`. 2) `appendUnreleasedChangelogEntry` deduped only on full-text equivalence. When a PR had a detailed entry written during prepare and then the merge step called `ensure_pr_changelog_entry` again with the short PR-title form, the two text bodies differed and the same PR got a duplicate line — that is what happened to PR openclaw#67679, which ended up with lines under both `### Changes` and `### Fixes`. Add a stronger precheck: scan the `## Unreleased` block for any existing bullet whose first PR reference matches the new entry's PR number. If one exists anywhere in the block (any sub-section), skip insertion. Dedup is scoped to Unreleased so the same PR number in a released block does not suppress a new Unreleased entry, and uses integer-equality on the PR number so `openclaw#67` is not shadowed by `openclaw#6767`. Together these two changes cover both of the recently observed failure modes: - Missing section signal from labels (PR openclaw#67807) is now picked up from the title. - Second ensure at merge time (PR openclaw#67679) no longer plants a duplicate in a different sub-section. Covered by shell smoke (27 resolver scenarios) and vitest (12 cases incl. the openclaw#67679 cross-section regression, released-block false positive, and PR-number prefix collision).

@hxy91819