From 575286922b08c66559fa051029c6fa7e5a76dfe5 Mon Sep 17 00:00:00 2001 From: Jack Ye Date: Tue, 15 Sep 2026 08:31:28 -0700 Subject: [PATCH] fix: redact authentication headers from debug logs (#4179) Prevent bearer tokens, API keys, cookies, and proxy credentials from appearing in request debug output through a reusable `redact_sensitive_headers` utility. The utility is applied after default/configured header construction and dynamic header merging, so it also covers retry requests. Invalid dynamic-header values are no longer echoed, and regression coverage verifies that credentials are redacted while ordinary diagnostic headers remain visible. Extracted from the OAuth discussion in #4173, following [Colin's review](https://github.com/lancedb/lancedb/pull/4173#issuecomment-5674048100). --- rust/lancedb/src/remote/client.rs | 101 +++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 3 deletions(-) diff --git a/rust/lancedb/src/remote/client.rs b/rust/lancedb/src/remote/client.rs index 57dd89890..704afe948 100644 --- a/rust/lancedb/src/remote/client.rs +++ b/rust/lancedb/src/remote/client.rs @@ -15,6 +15,25 @@ use crate::remote::retry::{ResolvedRetryConfig, RetryCounter}; const REQUEST_ID_HEADER: HeaderName = HeaderName::from_static("x-request-id"); +pub(crate) fn redact_sensitive_headers(headers: &mut HeaderMap) { + const SENSITIVE_HEADERS: [&str; 5] = [ + "authorization", + "proxy-authorization", + "cookie", + "set-cookie", + "x-api-key", + ]; + + for (name, value) in headers.iter_mut() { + if SENSITIVE_HEADERS + .iter() + .any(|sensitive| name.as_str().eq_ignore_ascii_case(sensitive)) + { + value.set_sensitive(true); + } + } +} + /// Configuration for TLS/mTLS settings. #[derive(Clone, Debug)] pub struct TlsConfig { @@ -681,6 +700,7 @@ impl RestfulLanceDbClient { ); } + redact_sensitive_headers(&mut headers); Ok(headers) } @@ -714,13 +734,14 @@ impl RestfulLanceDbClient { if let Ok(header_value) = HeaderValue::from_str(&value) { request_headers.insert(header_name, header_value); } else { - debug!("Invalid header value for key {}: {}", key, value); + debug!("Invalid header value for key {}", key); } } else { debug!("Invalid header name: {}", key); } } } + redact_sensitive_headers(request.headers_mut()); Ok(request) } @@ -1180,7 +1201,7 @@ mod tests { assert!(!headers.contains_key("x-api-key")); let headers = RestfulLanceDbClient::::default_headers( - "api-key", + "static-secret-value", "us-east-1", "db-name", false, @@ -1189,7 +1210,55 @@ mod tests { &ClientConfig::default(), ) .unwrap(); - assert_eq!(headers.get("x-api-key").unwrap(), "api-key"); + let api_key = headers.get("x-api-key").unwrap(); + assert_eq!(api_key, "static-secret-value"); + assert!(api_key.is_sensitive()); + assert!(!format!("{headers:?}").contains("static-secret-value")); + } + + #[test] + fn test_configured_authentication_headers_are_sensitive() { + let config = ClientConfig { + extra_headers: HashMap::from([ + ( + "Authorization".to_string(), + "Bearer configured-secret".to_string(), + ), + ("X-API-Key".to_string(), "configured-api-key".to_string()), + ( + "Cookie".to_string(), + "session=configured-cookie".to_string(), + ), + ( + "Proxy-Authorization".to_string(), + "Basic configured-proxy-secret".to_string(), + ), + ("X-Custom".to_string(), "visible-value".to_string()), + ]), + ..Default::default() + }; + let headers = RestfulLanceDbClient::::default_headers( + "", + "us-east-1", + "db-name", + false, + &RemoteOptions::default(), + None, + &config, + ) + .unwrap(); + + assert!(headers.get("authorization").unwrap().is_sensitive()); + assert!(headers.get("x-api-key").unwrap().is_sensitive()); + assert!(headers.get("cookie").unwrap().is_sensitive()); + assert!(headers.get("proxy-authorization").unwrap().is_sensitive()); + assert!(!headers.get("x-custom").unwrap().is_sensitive()); + let debug = format!("{headers:?}"); + assert!(!debug.contains("configured-secret")); + assert!(!debug.contains("configured-api-key")); + assert!(!debug.contains("configured-cookie")); + assert!(!debug.contains("configured-proxy-secret")); + assert!(debug.contains("visible-value")); } #[test] @@ -1303,6 +1372,7 @@ mod tests { // Test that dynamic headers override existing headers let mut headers = HashMap::new(); headers.insert("Authorization".to_string(), "Bearer new-token".to_string()); + headers.insert("X-API-Key".to_string(), "new-api-key".to_string()); headers.insert("X-Custom".to_string(), "custom-value".to_string()); let provider = TestHeaderProvider::new(headers); @@ -1338,6 +1408,31 @@ mod tests { updated_request.headers().get("X-Custom").unwrap(), "custom-value" ); + assert!( + updated_request + .headers() + .get("Authorization") + .unwrap() + .is_sensitive() + ); + assert!( + updated_request + .headers() + .get("X-API-Key") + .unwrap() + .is_sensitive() + ); + assert!( + !updated_request + .headers() + .get("X-Custom") + .unwrap() + .is_sensitive() + ); + let debug = format!("{updated_request:?}"); + assert!(!debug.contains("new-token")); + assert!(!debug.contains("new-api-key")); + assert!(debug.contains("custom-value")); // Existing headers should still be present assert_eq!( updated_request.headers().get("X-Existing").unwrap(),