fix: keep internal URLs and proxy credentials out of error strings - #512
Merged
Conversation
reqwest's Display appends " for url (...)" to every error, and those strings reach API callers verbatim: the scrape error field, a crawl or batch document's block reason, the /v2 error envelopes, and the MCP tool responses. The URL was frequently internal infrastructure (the CDP endpoint, a sidecar host, the managed LLM endpoint) or carried credentials. The search route additionally named the search backend's host in its unreachable error and passed up to 200 characters of the backend's own error page through to the caller. crw_core::error::reqwest_message strips the URL at every site that formats a reqwest error, and crw_core::redact_proxy_url masks userinfo in every message that quotes a proxy value, including ProxyEntry::parse's own errors. The search backend's sanitized origin now goes to the log instead of the response, an upstream error page is reduced to its status, and /v1/search/research/github maps its failures through the same mapper as /v1/search, so its status codes now match that route. is_cert_error starts its walk at the error's source rather than at the reqwest error itself, whose Display carried the request URL: a target path mentioning certificates could previously classify a plain connect failure as a TLS failure and retry it with certificate verification disabled.
The docs guard rejects the bare backend name in source strings, and the assertion carried it as a literal. Checking that no origin survives says the same thing without the word.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
crw_core::error::reqwest_message(e)returns the error without reqwest's" for url (...)"tail, and is used at every site that formats a reqwest error into a returned string: CDP discovery, the camoufox and cloak sidecar clients, the HTTP tier, robots, the crawl proxy client, every LLM provider call incrw-extract, the search client's body read, and both MCP transports.crw_core::redact_proxy_url(raw)masksuser:pass@in every message that quotes a proxy value, includingProxyEntry::parse's own errors, whichcrw-rendererturned into a customer-visibleConfigError./v1/search/research/githubmaps its failures through the same mapper as/v1/search, so a backend timeout is 504 and an unreachable backend is 422 there too, where both were 502.is_cert_errorstarts its walk at the error's source. The reqwest error's own text carried the request URL, so a target path containing "certificate" could classify a connect failure as a TLS failure and retry it with certificate verification disabled.tracingat the sites where it matters.Verification
Live before the change, with a deliberately named sidecar host:
"Renderer error: CDP discovery failed: error sending request for url (http://internal-secret-chrome-host.crw-internal:9222/json/version)". After:"Renderer error: CDP discovery failed: error sending request".Unit: crw-core 43, crw-renderer http_only 115 plus camoufox and cloak, crw-search 255, crw-server search 107; workspace clippy clean.
Caveat
Two tests that asserted the search backend host is present in the transport error (from issue #90) now assert the opposite. The host is in the warn log instead.