Repository navigation
Embedded-URL redaction splits a sensitive query value that itself contains http(s):// #2907
Description
The problem
The embedded-URL matcher in core/mcp/fetchTracking.ts (EMBEDDED_URL_PATTERN, shared through mapUrlsInText / redactUrlsInText) stops a match wherever a second http(s):// begins. It does this on purpose, so that two comma-joined URLs get redacted separately. The side effect is that a sensitive query value which is itself a URL gets cut short, and its tail is printed verbatim:
redactUrlsInText("https://as.example/token?client_secret=https://secret.example/private")
// -> https://as.example/token?client_secret=%5Bredacted%5Dhttps://secret.example/private
Every caller is affected the same way: the CLI error envelope, the web and TUI error text, and the --oauth-debug trace (#2844).
Expected
When a nested scheme sits inside the value of a sensitive query parameter, it belongs to that value, and the whole value is redacted. Truly separate URLs (comma-joined, space-separated) should still be split apart.
Notes
Found by Copilot while reviewing #2902 (#2902 (comment)). It was declined there as out of scope: the matcher predates that PR, and changing it affects every redaction caller. A credential that is literally an http URL is unusual, which is why this is Low priority.
Activity
cliffhall commentedon Nov 5, 2026
Triage: Priority Low. Severity is low: credentials leak only when a credential is itself an http(s) URL, which is unusual. Urgency is low: nothing is blocked on this, and it was found in review of #2902.
media-ctrl commentedon Nov 5, 2026
Confirmed on main:
core/mcp/fetchTracking.tsstill hasEMBEDDED_URL_PATTERN = /\bhttps?:\/\/(?:(?!https?:\/\/)[^\s"<>])+/gi, so the negative lookahead cuts the match at the second scheme andredactUrlsInTextreplaces only the part before it. The tail is emitted verbatim, and in your example the tail is the secret.Splitting the problem in two is cheaper than making the URL pattern smarter, because the two cases have different boundaries. A comma-joined pair is separated by punctuation outside any value. A nested scheme sits inside a query value. One regex cannot tell them apart, but two passes can:
[?&](client_secret|client_id|code|access_token|refresh_token|id_token|assertion|key|token|signature|sig)=and redact up to the next&, whitespace, or fragment. That captures a value which contains a scheme, and it removes the leak without touching the matcher. It also covers the plain case where a query value is not a URL at all, which the URL matcher never sees today.redactUrlsInTextfor everything else, so two comma-joined URLs still split exactly as they do now.The boundary decision is the part worth writing down. A nested URL in a query value is usually not percent-encoded and can itself contain
&, so a value-only boundary still truncates a signed URL that carries its own query string. Keeping&as the terminator and emitting%5Bredacted%5D, which is already the output format, is the usual compromise, but it does mean the redacted text is slightly shorter than the input value and a reader cannot tell whether truncation happened.On priority: Low is fair for the matcher in general, but not for this shape of secret. A presigned URL or a proxy token that is itself a URL carries its own credential in its query string, so the leaked tail alone can be usable, and the callers you listed include the OAuth debug trace, which is the path most likely to be pasted into an issue. A test with exactly your example string in each caller, plus one comma-joined pair as the control, would pin the behaviour before the matcher changes.
fix(core): join a URL-valued secret without a lookbehind; empty set j…
fix(core): join a nested URL only at a real sensitive query parameter (…
fix(core): scan joins incrementally, keep a literal ? in keys, fall b…
cliffhall commentedon Nov 5, 2026
Fixed by #2920 (merged to v2/main in 9608809).