Skip to content

fix(security): redact credentials embedded in generic URLs - #6454

Closed
phpoh wants to merge 1 commit into
NousResearch:mainfrom
phpoh:fix/url-credential-redaction
Closed

phpoh wants to merge 1 commit into
NousResearch:mainfrom
phpoh:fix/url-credential-redaction

Conversation

@phpoh

@phpoh phpoh commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #6396

The secret redaction engine in agent/redact.py previously only handled database-specific protocol URLs (postgres, mysql, mongodb, redis, amqp) for credential redaction. This meant that passwords and tokens embedded in generic URLs like https://PASSWORD@github.com/user/repo.git were displayed in plain text in agent output.

This is exactly what happened in the reported issue — the agent suggested a git remote set-url command with the user's password embedded in the URL, and the redaction engine failed to mask it.

Changes

  • Extended URL credential regex (_URL_CRED_RE): Now covers https, http, git, ssh, ftp, ftps, sftp protocols in addition to the existing database protocols for user:password@host format. Replaces the old DB-only _DB_CONNSTR_RE.

  • Added bare token regex (_URL_BARE_TOKEN_RE): Catches protocol://TOKEN@host URLs where a password/token is embedded without a username prefix (the exact pattern from the bug report). Uses an 8-character threshold to avoid false positives on short usernames like "git" or "admin".

  • Reordered redaction pipeline: URL credential patterns now run before prefix patterns (sk-, ghp_, etc.) so that tokens inside URLs are redacted holistically without partial leakage.

  • 13 new test cases covering the reported scenario and various edge cases (bare passwords, user:password, GitHub PATs in URLs, DB URLs still working, false positive avoidance).

Test Plan

  • All 56 existing + new tests pass (tests/agent/test_redact.py)
  • Verified exact bug scenario is fixed: https://mysecretpassword123@github.com/user/repo.githttps://***@github.com/user/repo.git
  • Verified existing DB connection string redaction still works (postgres, mysql, etc.)
  • Verified no false positives on plain URLs without credentials
  • Verified short usernames (<8 chars) in URLs are not falsely redacted

…rch#6396)

The secret redaction engine only handled database-specific protocol URLs
(postgres, mysql, etc.) but missed generic https/git/ssh URLs with
embedded credentials like `https://PASSWORD@github.com/user/repo.git`.

This caused passwords and tokens to be displayed in plain text when the
agent suggested git commands or displayed URL output containing credentials.

Changes:
- Extend URL credential regex to cover https, http, git, ssh, ftp protocols
  for user:password@host format (replaces DB-only _DB_CONNSTR_RE)
- Add bare token pattern for protocol://TOKEN@host URLs (8+ char threshold
  to avoid false positives on short usernames like "git" or "admin")
- Run URL credential redaction before prefix patterns so tokens in URLs
  are redacted holistically without partial leakage
- Add 13 test cases covering the reported scenario and edge cases
@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround labels Apr 30, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

Competing PR with #6469 — both fix #6396 (generic URL credential redaction). This PR is narrower (redact.py only); #6469 also covers process output replay and session search.

@egilewski

Copy link
Copy Markdown
Contributor

merge conflicts

This PR does not merge cleanly with the base branch. Please rebase or merge current main and resolve the conflicts if it's still relevant.

Signed: GPT-5.5-medium in Codex

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for this, and for the clean tests — the bug in #6396 is real and your reproduction was spot-on.

We're closing this one as intentional-design, not because the work is wrong. After this PR was opened, commit acd05c6 ("fix(redact): pass web URLs through unchanged", #34029) deliberately removed userinfo + query-param URL redaction: https://user:pass@host and ?token=/?code= were getting masked by name, which corrupts any skill that round-trips a URL through history (the live call fires with the real URL, the next turn sees ***). A real consumer hit this with a magic-link checkout skill. This PR re-adds exactly what we removed there, so merging it would reintroduce that regression.

The underlying leak is still worth fixing, though, just more narrowly. We shipped #54475, which redacts only the colon-less bare-token userinfo form (scheme://TOKEN@host — the exact git remote set-url https://PASSWORD@github.com/... shape from the report) while leaving user:pass@ and query-string tokens passing through. A bare credential in userinfo is never a workflow token (those live in the query string), so masking it can't break a round-trip skill.

Credited your report and repro in that PR. Appreciate the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Security

4 participants