From d2abcab625cb5152c30098af4a9863c082375d42 Mon Sep 17 00:00:00 2001 From: Cole MacKenzie Date: Mon, 9 Feb 2026 13:35:49 -0800 Subject: [PATCH] fix(rest): Redact sensitive headers from error logs (#2117) Sensitive headers like Set-Cookie, Authorization, and Cookie were being included in error messages, potentially exposing authentication tokens and session information in logs. This change replaces sensitive headers with `[REDACTED]` from error context rather than logging their values, preventing credential leakage. This option can be disabled using `REST_CATALOG_PROP_DISABLE_HEADER_REDACTION=true` --- crates/catalog/rest/src/catalog.rs | 109 ++++++++++++++--- crates/catalog/rest/src/client.rs | 184 ++++++++++++++++++++++++++++- 2 files changed, 277 insertions(+), 16 deletions(-) diff --git a/crates/catalog/rest/src/catalog.rs b/crates/catalog/rest/src/catalog.rs index ddbf6a4e01..eeea1f13e9 100644 --- a/crates/catalog/rest/src/catalog.rs +++ b/crates/catalog/rest/src/catalog.rs @@ -50,6 +50,8 @@ use crate::types::{ pub const REST_CATALOG_PROP_URI: &str = "uri"; /// REST catalog warehouse location pub const REST_CATALOG_PROP_WAREHOUSE: &str = "warehouse"; +/// Disable header redaction in error logs (defaults to false for security) +pub const REST_CATALOG_PROP_DISABLE_HEADER_REDACTION: &str = "disable-header-redaction"; const ICEBERG_REST_SPEC_VERSION: &str = "0.14.1"; const CARGO_PKG_VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -293,6 +295,17 @@ impl RestCatalogConfig { params } + /// Check if header redaction is disabled in error logs. + /// + /// Returns true if the `disable-header-redaction` property is set to "true". + /// Defaults to false for security (headers are redacted by default). + pub(crate) fn disable_header_redaction(&self) -> bool { + self.props + .get(REST_CATALOG_PROP_DISABLE_HEADER_REDACTION) + .map(|v| v.eq_ignore_ascii_case("true")) + .unwrap_or(false) + } + /// Merge the `RestCatalogConfig` with the a [`CatalogConfig`] (fetched from the REST server). pub(crate) fn merge_with_config(mut self, mut config: CatalogConfig) -> Self { if let Some(uri) = config.overrides.remove("uri") { @@ -378,7 +391,11 @@ impl RestCatalog { match http_response.status() { StatusCode::OK => deserialize_catalog_response(http_response).await, - _ => Err(deserialize_unexpected_catalog_error(http_response).await), + _ => Err(deserialize_unexpected_catalog_error( + http_response, + client.disable_header_redaction(), + ) + .await), } } @@ -479,7 +496,13 @@ impl Catalog for RestCatalog { "The parent parameter of the namespace provided does not exist", )); } - _ => return Err(deserialize_unexpected_catalog_error(http_response).await), + _ => { + return Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await); + } } } @@ -514,7 +537,11 @@ impl Catalog for RestCatalog { ErrorKind::Unexpected, "Tried to create a namespace that already exists", )), - _ => Err(deserialize_unexpected_catalog_error(http_response).await), + _ => Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await), } } @@ -538,7 +565,11 @@ impl Catalog for RestCatalog { ErrorKind::Unexpected, "Tried to get a namespace that does not exist", )), - _ => Err(deserialize_unexpected_catalog_error(http_response).await), + _ => Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await), } } @@ -555,7 +586,11 @@ impl Catalog for RestCatalog { match http_response.status() { StatusCode::NO_CONTENT | StatusCode::OK => Ok(true), StatusCode::NOT_FOUND => Ok(false), - _ => Err(deserialize_unexpected_catalog_error(http_response).await), + _ => Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await), } } @@ -586,7 +621,11 @@ impl Catalog for RestCatalog { ErrorKind::Unexpected, "Tried to drop a namespace that does not exist", )), - _ => Err(deserialize_unexpected_catalog_error(http_response).await), + _ => Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await), } } @@ -623,7 +662,13 @@ impl Catalog for RestCatalog { "Tried to list tables of a namespace that does not exist", )); } - _ => return Err(deserialize_unexpected_catalog_error(http_response).await), + _ => { + return Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await); + } } } @@ -677,7 +722,13 @@ impl Catalog for RestCatalog { "The table already exists", )); } - _ => return Err(deserialize_unexpected_catalog_error(http_response).await), + _ => { + return Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await); + } }; let metadata_location = response.metadata_location.as_ref().ok_or(Error::new( @@ -732,7 +783,13 @@ impl Catalog for RestCatalog { "Tried to load a table that does not exist", )); } - _ => return Err(deserialize_unexpected_catalog_error(http_response).await), + _ => { + return Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await); + } }; let config = response @@ -774,7 +831,11 @@ impl Catalog for RestCatalog { ErrorKind::Unexpected, "Tried to drop a table that does not exist", )), - _ => Err(deserialize_unexpected_catalog_error(http_response).await), + _ => Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await), } } @@ -792,7 +853,11 @@ impl Catalog for RestCatalog { match http_response.status() { StatusCode::NO_CONTENT | StatusCode::OK => Ok(true), StatusCode::NOT_FOUND => Ok(false), - _ => Err(deserialize_unexpected_catalog_error(http_response).await), + _ => Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await), } } @@ -821,7 +886,11 @@ impl Catalog for RestCatalog { ErrorKind::Unexpected, "Tried to rename a table to a name that already exists", )), - _ => Err(deserialize_unexpected_catalog_error(http_response).await), + _ => Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await), } } @@ -865,7 +934,13 @@ impl Catalog for RestCatalog { "The given table already exists.", )); } - _ => return Err(deserialize_unexpected_catalog_error(http_response).await), + _ => { + return Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await); + } }; let metadata_location = response.metadata_location.as_ref().ok_or(Error::new( @@ -934,7 +1009,13 @@ impl Catalog for RestCatalog { "A server-side gateway timeout occurred; the commit state is unknown.", )); } - _ => return Err(deserialize_unexpected_catalog_error(http_response).await), + _ => { + return Err(deserialize_unexpected_catalog_error( + http_response, + context.client.disable_header_redaction(), + ) + .await); + } }; let file_io = self diff --git a/crates/catalog/rest/src/client.rs b/crates/catalog/rest/src/client.rs index 361c036bb6..07dc0620da 100644 --- a/crates/catalog/rest/src/client.rs +++ b/crates/catalog/rest/src/client.rs @@ -43,6 +43,8 @@ pub(crate) struct HttpClient { extra_headers: HeaderMap, /// Extra oauth parameters to be added to each authentication request. extra_oauth_params: HashMap, + /// Whether to disable header redaction in error logs (defaults to false for security). + disable_header_redaction: bool, } impl Debug for HttpClient { @@ -65,6 +67,7 @@ impl HttpClient { credential: cfg.credential(), extra_headers, extra_oauth_params: cfg.extra_oauth_params(), + disable_header_redaction: cfg.disable_header_redaction(), }) } @@ -92,6 +95,7 @@ impl HttpClient { } else { self.extra_oauth_params }, + disable_header_redaction: cfg.disable_header_redaction(), }) } @@ -258,6 +262,11 @@ impl HttpClient { self.authenticate(&mut request).await?; self.execute(request).await } + + /// Returns whether header redaction is disabled for this client. + pub(crate) fn disable_header_redaction(&self) -> bool { + self.disable_header_redaction + } } /// Deserializes a catalog response into the given [`DeserializedOwned`] type. @@ -278,14 +287,64 @@ pub(crate) async fn deserialize_catalog_response( }) } +/// Headers that contain sensitive information and should be excluded from logs. +const SENSITIVE_HEADERS: &[&str] = &[ + "authorization", + "proxy-authorization", + "set-cookie", + "cookie", + "x-api-key", + "x-auth-token", +]; + +/// Returns true if the header name is considered sensitive. +fn is_sensitive_header(name: &str) -> bool { + let name_lower = name.to_lowercase(); + SENSITIVE_HEADERS.iter().any(|h| name_lower == *h) +} + +/// Redacts sensitive headers and returns a debug-formatted string. +/// +/// If `disable_redaction` is true, returns all headers without redaction. +/// Otherwise, replaces sensitive header values with "[REDACTED]". +fn format_headers_redacted(headers: &HeaderMap, disable_redaction: bool) -> String { + if disable_redaction { + // Return all headers as-is without redaction + let all: HashMap<&str, &str> = headers + .iter() + .filter_map(|(name, value)| value.to_str().ok().map(|v| (name.as_str(), v))) + .collect(); + return format!("{all:?}"); + } + + // Redact sensitive headers by replacing their values with "[REDACTED]" + let redacted: HashMap<&str, &str> = headers + .iter() + .filter_map(|(name, value)| { + if is_sensitive_header(name.as_str()) { + Some((name.as_str(), "[REDACTED]")) + } else { + value.to_str().ok().map(|v| (name.as_str(), v)) + } + }) + .collect(); + format!("{redacted:?}") +} + /// Deserializes a unexpected catalog response into an error. -pub(crate) async fn deserialize_unexpected_catalog_error(response: Response) -> Error { +pub(crate) async fn deserialize_unexpected_catalog_error( + response: Response, + disable_header_redaction: bool, +) -> Error { let err = Error::new( ErrorKind::Unexpected, "Received response with unexpected status code", ) .with_context("status", response.status().to_string()) - .with_context("headers", format!("{:?}", response.headers())); + .with_context( + "headers", + format_headers_redacted(response.headers(), disable_header_redaction), + ); let bytes = match response.bytes().await { Ok(bytes) => bytes, @@ -297,3 +356,124 @@ pub(crate) async fn deserialize_unexpected_catalog_error(response: Response) -> } err.with_context("json", String::from_utf8_lossy(&bytes)) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_format_headers_redacted_empty() { + let headers = HeaderMap::new(); + let result = format_headers_redacted(&headers, false); + assert_eq!(result, "{}"); + } + + #[test] + fn test_format_headers_redacted_non_sensitive() { + let mut headers = HeaderMap::new(); + headers.insert("content-type", "application/json".parse().unwrap()); + headers.insert("x-request-id", "abc123".parse().unwrap()); + + let result = format_headers_redacted(&headers, false); + + assert!(result.contains("content-type")); + assert!(result.contains("application/json")); + assert!(result.contains("x-request-id")); + assert!(result.contains("abc123")); + } + + #[test] + fn test_format_headers_redacted_filters_sensitive() { + let mut headers = HeaderMap::new(); + headers.insert("authorization", "Bearer secret-token".parse().unwrap()); + headers.insert("content-type", "application/json".parse().unwrap()); + + let result = format_headers_redacted(&headers, false); + + // Sensitive header should be present but with redacted value + assert!(result.contains("authorization")); + assert!(result.contains("[REDACTED]")); + // Sensitive value should NOT be present + assert!(!result.contains("secret-token")); + // Non-sensitive header should be present with actual value + assert!(result.contains("content-type")); + assert!(result.contains("application/json")); + } + + #[test] + fn test_format_headers_redacted_filters_set_cookie() { + let mut headers = HeaderMap::new(); + headers.insert( + "set-cookie", + "CF_Authorization=sensitive-session-token; Path=/; Secure;" + .parse() + .unwrap(), + ); + headers.insert("server", "cloudflare".parse().unwrap()); + + let result = format_headers_redacted(&headers, false); + + // Sensitive header should be present but with redacted value + assert!(result.contains("set-cookie")); + assert!(result.contains("[REDACTED]")); + // Sensitive value should NOT be present + assert!(!result.contains("sensitive-session-token")); + // Non-sensitive header should be present with actual value + assert!(result.contains("server")); + assert!(result.contains("cloudflare")); + } + + #[test] + fn test_format_headers_redacted_filters_all_sensitive() { + let mut headers = HeaderMap::new(); + headers.insert("authorization", "Bearer token".parse().unwrap()); + headers.insert("proxy-authorization", "Basic creds".parse().unwrap()); + headers.insert("set-cookie", "session=abc".parse().unwrap()); + headers.insert("cookie", "session=abc".parse().unwrap()); + headers.insert("x-api-key", "api-key-123".parse().unwrap()); + headers.insert("x-auth-token", "auth-token-456".parse().unwrap()); + headers.insert("x-request-id", "req-123".parse().unwrap()); + + let result = format_headers_redacted(&headers, false); + + // All sensitive headers should be present but with redacted values + assert!(result.contains("authorization")); + assert!(result.contains("proxy-authorization")); + assert!(result.contains("set-cookie")); + assert!(result.contains("cookie")); + assert!(result.contains("x-api-key")); + assert!(result.contains("x-auth-token")); + assert!(result.contains("[REDACTED]")); + + // Ensure no sensitive values leaked + assert!(!result.contains("Bearer token")); + assert!(!result.contains("Basic creds")); + assert!(!result.contains("session=abc")); + assert!(!result.contains("api-key-123")); + assert!(!result.contains("auth-token-456")); + + // Non-sensitive header should be present with actual value + assert!(result.contains("x-request-id")); + assert!(result.contains("req-123")); + } + + #[test] + fn test_format_headers_with_redaction_disabled() { + let mut headers = HeaderMap::new(); + headers.insert("authorization", "Bearer secret-token".parse().unwrap()); + headers.insert("x-api-key", "api-key-123".parse().unwrap()); + headers.insert("content-type", "application/json".parse().unwrap()); + + let result = format_headers_redacted(&headers, true); + + // When redaction is disabled, all headers and values should be present + assert!(result.contains("authorization")); + assert!(result.contains("Bearer secret-token")); + assert!(result.contains("x-api-key")); + assert!(result.contains("api-key-123")); + assert!(result.contains("content-type")); + assert!(result.contains("application/json")); + // [REDACTED] should NOT be present when redaction is disabled + assert!(!result.contains("[REDACTED]")); + } +}