fix(security): redact URLs in provider errors (#11230)
This commit is contained in:
@@ -96,6 +96,31 @@ fn is_network_error(err: &reqwest::Error) -> bool {
|
||||
err.is_connect() || err.is_timeout() || (err.status().is_none() && err.is_request())
|
||||
}
|
||||
|
||||
fn sanitized_reqwest_url(error: &reqwest::Error) -> Option<String> {
|
||||
let mut url = error.url()?.clone();
|
||||
let _ = url.set_password(None);
|
||||
let _ = url.set_username("");
|
||||
url.set_query(None);
|
||||
url.set_fragment(None);
|
||||
Some(url.to_string())
|
||||
}
|
||||
|
||||
fn reqwest_error_category(error: &reqwest::Error) -> &'static str {
|
||||
if error.is_builder() {
|
||||
"Request builder error"
|
||||
} else if error.is_redirect() {
|
||||
"Redirect error"
|
||||
} else if error.is_status() {
|
||||
"HTTP status error"
|
||||
} else if error.is_body() {
|
||||
"Request body error"
|
||||
} else if error.is_decode() {
|
||||
"Response decode error"
|
||||
} else {
|
||||
"Request error"
|
||||
}
|
||||
}
|
||||
|
||||
fn provider_error_from_reqwest(error: &reqwest::Error) -> ProviderError {
|
||||
if is_network_error(error) {
|
||||
let msg = if error.is_timeout() {
|
||||
@@ -121,14 +146,19 @@ fn provider_error_from_reqwest(error: &reqwest::Error) -> ProviderError {
|
||||
return ProviderError::NetworkError(msg);
|
||||
}
|
||||
|
||||
let mut details = vec![];
|
||||
let mut details = Vec::new();
|
||||
if let Some(status) = error.status() {
|
||||
details.push(format!("status: {}", status));
|
||||
}
|
||||
if let Some(url) = sanitized_reqwest_url(error) {
|
||||
details.push(format!("url: {url}"));
|
||||
}
|
||||
|
||||
let category = reqwest_error_category(error);
|
||||
let msg = if details.is_empty() {
|
||||
error.to_string()
|
||||
category.to_string()
|
||||
} else {
|
||||
format!("{} ({})", error, details.join(", "))
|
||||
format!("{category} ({})", details.join(", "))
|
||||
};
|
||||
ProviderError::RequestFailed(msg)
|
||||
}
|
||||
@@ -172,6 +202,68 @@ impl From<LogError> for ProviderError {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::io::{Read, Write};
|
||||
use std::net::TcpListener;
|
||||
|
||||
#[tokio::test]
|
||||
async fn direct_reqwest_error_redacts_unsupported_scheme_url() {
|
||||
let error = reqwest::Client::new()
|
||||
.get("ftp://url-user:url-password@provider.invalid/chat?api_key=query-secret")
|
||||
.send()
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert!(error.to_string().contains("query-secret"));
|
||||
assert!(!is_network_error(&error));
|
||||
|
||||
let message = ProviderError::from(error).to_string();
|
||||
assert!(!message.contains("query-secret"));
|
||||
assert!(!message.contains("url-user"));
|
||||
assert!(!message.contains("url-password"));
|
||||
assert!(message.contains("ftp://provider.invalid/chat"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn anyhow_wrapped_status_error_redacts_url() {
|
||||
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
|
||||
let address = listener.local_addr().unwrap();
|
||||
let server = std::thread::spawn(move || {
|
||||
let (mut stream, _) = listener.accept().unwrap();
|
||||
let mut request = [0; 1024];
|
||||
assert!(stream.read(&mut request).unwrap() > 0);
|
||||
stream
|
||||
.write_all(
|
||||
b"HTTP/1.1 401 Unauthorized\r\nContent-Length: 0\r\nConnection: close\r\n\r\n",
|
||||
)
|
||||
.unwrap();
|
||||
});
|
||||
|
||||
let error = reqwest::Client::new()
|
||||
.get(format!(
|
||||
"http://url-user:url-password@{address}/chat?api_key=query-secret"
|
||||
))
|
||||
.send()
|
||||
.await
|
||||
.unwrap()
|
||||
.error_for_status()
|
||||
.unwrap_err();
|
||||
server.join().unwrap();
|
||||
|
||||
assert!(error.to_string().contains("query-secret"));
|
||||
assert_eq!(error.status(), Some(StatusCode::UNAUTHORIZED));
|
||||
|
||||
let message = ProviderError::from(anyhow::Error::new(error)).to_string();
|
||||
assert!(!message.contains("query-secret"));
|
||||
assert!(!message.contains("url-user"));
|
||||
assert!(!message.contains("url-password"));
|
||||
assert!(message.contains("status: 401 Unauthorized"));
|
||||
assert!(message.contains(&format!("http://{address}/chat")));
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub enum GoogleErrorCode {
|
||||
BadRequest = 400,
|
||||
|
||||
Reference in New Issue
Block a user