fix(rust): redact proxy credentials in Debug and build the proxy matcher once

EnvironmentProxies holds raw proxy URLs, which can carry user:password, and
it sits inside HttpSettings and HttpClientConfig, so any {:?} of those would
print the password. Derive veil's Redact like the auth crate does. NO_PROXY
stays readable because it holds no credentials.

The media fetcher also rebuilt the hyper-util matcher for every URL and
redirect hop. Build it once when the fetcher is created
This commit is contained in:
Yujong Lee 2026-09-19 08:30:35 -07:00
parent 0074b943a6
commit b341d21a76
4 changed files with 28 additions and 9 deletions

View file

@ -2145,6 +2145,7 @@ dependencies = [
"serde_json",
"thiserror 2.0.19",
"tokio",
"veil",
"webpki-roots",
]

View file

@ -17,6 +17,7 @@ rustls.workspace = true
serde_json.workspace = true
thiserror.workspace = true
tokio.workspace = true
veil.workspace = true
webpki-roots.workspace = true
[dev-dependencies]

View file

@ -103,8 +103,7 @@ impl MediaFetcher {
config: &HttpClientConfig,
url_policy: UrlPolicy,
) -> Result<Self, crate::Error> {
let proxies = config.proxies.clone();
let uses_proxy: ProxyMatch = Arc::new(move |url| proxies.apply_to(url));
let uses_proxy: ProxyMatch = Arc::new(config.proxies.matcher());
Self::with_resolution(
pool,
config,

View file

@ -1,10 +1,14 @@
use hyper_util::client::proxy::matcher::Matcher;
use litellm_core_utils::settings::Lookup;
use veil::Redact;
#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]
#[derive(Clone, Redact, Default, PartialEq, Eq, Hash)]
pub struct EnvironmentProxies {
#[redact]
all: String,
#[redact]
http: String,
#[redact]
https: String,
no: String,
}
@ -25,16 +29,18 @@ impl EnvironmentProxies {
}
}
pub fn apply_to(&self, url: &reqwest::Url) -> bool {
pub(crate) fn matcher(&self) -> impl Fn(&reqwest::Url) -> bool + Send + Sync + use<> {
let matcher = Matcher::builder()
.all(self.all.clone())
.http(self.http.clone())
.https(self.https.clone())
.no(self.no.clone())
.build();
url.as_str()
.parse::<http::Uri>()
.is_ok_and(|uri| matcher.intercept(&uri).is_some())
move |url| {
url.as_str()
.parse::<http::Uri>()
.is_ok_and(|uri| matcher.intercept(&uri).is_some())
}
}
pub(crate) fn reqwest_proxies(&self) -> Vec<reqwest::Proxy> {
@ -82,7 +88,7 @@ mod tests {
#[case] expected: bool,
) {
let proxies = EnvironmentProxies::from_environment(&env_of(env));
assert_eq!(proxies.apply_to(&url(target)), expected);
assert_eq!(proxies.matcher()(&url(target)), expected);
}
#[rstest]
@ -110,7 +116,19 @@ mod tests {
("REQUEST_METHOD", "GET"),
("HTTPS_PROXY", "http://proxy:3128"),
]));
assert!(proxies.apply_to(&url("https://api.test/")));
assert!(proxies.matcher()(&url("https://api.test/")));
}
#[test]
fn debug_output_hides_proxy_credentials_but_shows_which_variables_are_set() {
let proxies = EnvironmentProxies::from_environment(&env_of(&[
("HTTPS_PROXY", "http://operator:hunter2@proxy.corp:3128"),
("NO_PROXY", "internal.test"),
]));
let debug = format!("{proxies:?}");
assert!(!debug.contains("hunter2") && !debug.contains("operator"));
assert!(debug.contains("internal.test"));
assert_ne!(debug, format!("{:?}", EnvironmentProxies::default()));
}
#[test]