From f7ceb7ec5c1a6ca4a44e650a158f08ebcb30436e Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Wed, 5 Aug 2026 03:30:14 -0400 Subject: [PATCH] fix: redact secret property values in SessionContext Debug --- crates/iceberg/src/catalog/session.rs | 291 +++++++++++++++++++++++++- 1 file changed, 290 insertions(+), 1 deletion(-) diff --git a/crates/iceberg/src/catalog/session.rs b/crates/iceberg/src/catalog/session.rs index 1790f4978a..2a8fb58946 100644 --- a/crates/iceberg/src/catalog/session.rs +++ b/crates/iceberg/src/catalog/session.rs @@ -43,7 +43,7 @@ use crate::{Namespace, NamespaceIdent, Result, TableCommit, TableCreation, Table /// assert_eq!(session.identity(), Some("user123")); /// assert!(!session.session_id().is_empty()); /// ``` -#[derive(Debug, Clone, TypedBuilder)] +#[derive(Clone, TypedBuilder)] pub struct SessionContext { /// The unique identifier for this session. /// @@ -64,6 +64,146 @@ pub struct SessionContext { credentials: HashMap, } +/// Property keys whose values are secrets, or may embed them (headers, +/// connection strings, keys like `adls.account-key` or `s3.sse.key`). +fn is_sensitive_prop(key: &str) -> bool { + // Keys carry no casing or separator contract (`AWS_SECRET_ACCESS_KEY`, + // `clientSecret`), so match a lowercased, separator-free form. + let compact: String = key + .to_ascii_lowercase() + .chars() + .filter(|c| !matches!(c, '.' | '-' | '_')) + .collect(); + // `authorization`, not the broader `auth`, so credential-free endpoints + // like `oauth2-server-uri` stay visible. + compact.starts_with("header") + || compact == "auth" + || compact == "sig" + || [ + "authorization", + "token", + "credential", + "secret", + "password", + "passwd", + "pwd", + "key", + "cookie", + "signature", + "connectionstring", + ] + .iter() + .any(|pattern| compact.contains(pattern)) +} + +/// Schemes where `@` separates a container from an account rather than +/// credentials from a host (`abfss://filesystem@account.dfs.core.windows.net`). +const CONTAINER_AT_SCHEMES: &[&str] = &["abfs", "abfss", "wasb", "wasbs"]; + +/// True when the value itself carries a credential, which happens under +/// innocuous keys: a URI with userinfo (`postgres://user:pass@host/db` as the +/// SQL catalog's `uri`) or a sensitive parameter (`?password=`, a signed URL). +/// +/// Best-effort: driver-specific connection strings and URIs nested in a +/// parameter are not decomposed. +fn is_sensitive_value(value: &str) -> bool { + // URI parsers drop tab and newline characters before parsing. + let value: String = value + .chars() + .filter(|c| !matches!(c, '\t' | '\n' | '\r')) + .collect(); + let lower = value.to_ascii_lowercase(); + let scheme = lower + .trim_start() + .split_once(':') + .map_or("", |(scheme, _)| scheme); + if CONTAINER_AT_SCHEMES.contains(&scheme) { + return false; + } + + // Userinfo, via the parser (which normalizes `https:user:pass@host`) and + // via the raw authority (for opaque strings like `jdbc:postgresql://...`). + let parsed = url::Url::parse(&value).ok(); + if let Some(url) = &parsed + && (!url.username().is_empty() || url.password().is_some()) + { + return true; + } + if let Some((_, rest)) = lower.split_once("://") + && rest + .split(['/', '\\', '?', '#']) + .next() + .unwrap_or(rest) + .contains('@') + { + return true; + } + + // Credential-bearing parameters in the query or the fragment. + let sections = match &parsed { + Some(url) => [url.query(), url.fragment()] + .into_iter() + .flatten() + .map(str::to_ascii_lowercase) + .collect(), + None => Vec::new(), + }; + sections.iter().any(|section| { + section.split(['&', ';']).any(|pair| { + pair.split_once('=').is_some_and(|(name, _)| { + // Parsers accept percent-encoded names: `pass%77ord`. + is_sensitive_prop(&percent_decode(name.trim())) + }) + }) + }) +} + +/// Decodes `%XX` sequences (malformed ones pass through verbatim). +fn percent_decode(input: &str) -> String { + fn hex(byte: u8) -> Option { + (byte as char).to_digit(16).map(|digit| digit as u8) + } + let bytes = input.as_bytes(); + let mut out = Vec::with_capacity(bytes.len()); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'%' + && i + 2 < bytes.len() + && let (Some(high), Some(low)) = (hex(bytes[i + 1]), hex(bytes[i + 2])) + { + out.push(high * 16 + low); + i += 3; + continue; + } + out.push(bytes[i]); + i += 1; + } + String::from_utf8_lossy(&out).to_ascii_lowercase() +} + +impl Debug for SessionContext { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let properties: HashMap<&str, &str> = self + .properties + .iter() + .map(|(key, value)| { + let value = if is_sensitive_prop(key) || is_sensitive_value(value) { + "[REDACTED]" + } else { + value.as_str() + }; + (key.as_str(), value) + }) + .collect(); + f.debug_struct("SessionContext") + .field("session_id", &self.session_id) + .field("identity", &self.identity) + .field("properties", &properties) + .field("credentials", &self.credentials) + .finish() + } +} + impl SessionContext { /// Creates a new unique but empty session. pub fn empty() -> Self { @@ -241,6 +381,155 @@ mod tests { use crate::{Credential, SessionCatalog, SessionContext}; + #[test] + fn test_is_sensitive_prop_matching() { + for key in [ + "token", + "credential", + "clientSecret", + "password", + "s3.sse.key", + "AWS_SECRET_ACCESS_KEY", + "connection-string", + "connection_string", + "connectionString", + "adls.connection.string", + "header.authorization", + "Header_Authorization", + "Authorization", + "Proxy-Authorization", + "auth", + "Cookie", + "Set-Cookie", + ] { + assert!(super::is_sensitive_prop(key), "should match: {key}"); + } + for key in [ + "warehouse", + "uri", + "s3.region", + "prefix", + // Credential-free auth configuration stays visible. + "oauth2-server-uri", + "adls.authority-host", + ] { + assert!(!super::is_sensitive_prop(key), "should not match: {key}"); + } + } + + #[test] + fn test_is_sensitive_value_matching() { + for value in [ + "postgres://user:password@host/db", + "mysql://root@localhost:3306/iceberg", + "https:user:pass@example.com/path", + "jdbc:postgresql://user:pass@db/iceberg", + "https://bucket.s3.amazonaws.com/key?X-Amz-Signature=abc123", + "https://account.blob.core.windows.net/c?sv=2024&sig=abc", + "postgresql://localhost/db?user=u&password=qp-secret", + "postgresql://localhost/db?user=u&pass%77ord=enc-secret", + "https://client.example/cb#access_token=frag-secret", + "postgresql://localhost/db?pass\tword=tab-secret", + ] { + assert!(super::is_sensitive_value(value), "should match: {value}"); + } + for value in [ + "https://catalog.example.com/iceberg", + "https://host/path?region=us-east-1", + "sqlite::memory:", + "s3://bucket/warehouse", + "user@example.com", + "mailto:user@example.com", + "https://docs.example.com/page#section-2", + // Azure storage `@` separates container from account. + "abfss://filesystem@account.dfs.core.windows.net/warehouse", + "wasbs://container@account.blob.core.windows.net/path", + " abfss://filesystem@account.dfs.core.windows.net/path", + ] { + assert!( + !super::is_sensitive_value(value), + "should not match: {value}" + ); + } + } + + #[test] + fn test_percent_decode() { + assert_eq!(super::percent_decode("pass%77ord"), "password"); + assert_eq!(super::percent_decode("PASS%57ORD"), "password"); + // Malformed or truncated escapes pass through verbatim. + assert_eq!(super::percent_decode("pass%zzord"), "pass%zzord"); + assert_eq!(super::percent_decode("password%7"), "password%7"); + assert_eq!(super::percent_decode("password%"), "password%"); + // Invalid UTF-8 decodes lossily instead of panicking. + assert!(!super::percent_decode("token%ff").is_empty()); + } + + #[test] + fn test_session_context_debug_redacts_secret_properties() { + let context = SessionContext::builder() + .properties(HashMap::from([ + ("token".to_string(), "tok-secret".to_string()), + ("s3.secret-access-key".to_string(), "sk-secret".to_string()), + // Keys have no casing/separator contract. + ( + "AWS_SECRET_ACCESS_KEY".to_string(), + "env-secret".to_string(), + ), + ("clientSecret".to_string(), "camel-secret".to_string()), + ("connectionString".to_string(), "cs-secret".to_string()), + ( + "adls.connection.string".to_string(), + "dotted-secret".to_string(), + ), + ("Header.Authorization".to_string(), "hdr-secret".to_string()), + ( + "Header_Authorization".to_string(), + "hdr2-secret".to_string(), + ), + // Redacted by value: the keys are innocuous, the values not. + ( + "uri".to_string(), + "postgres://user:uri-secret@host/db".to_string(), + ), + ( + "endpoint".to_string(), + "https://host/o?password=query-secret".to_string(), + ), + ( + "callback".to_string(), + "https://app/cb#access_token=frag-secret".to_string(), + ), + ( + "download".to_string(), + "https://bucket/o?X-Amz-Signature=signed-secret".to_string(), + ), + ("warehouse".to_string(), "wh1".to_string()), + ])) + .credentials(HashMap::from([( + "oauth2".to_string(), + Credential::from("cred-secret".to_string()), + )])) + .build(); + + let out = format!("{context:?}"); + assert!(!out.contains("tok-secret")); + assert!(!out.contains("sk-secret")); + assert!(!out.contains("env-secret")); + assert!(!out.contains("camel-secret")); + assert!(!out.contains("cs-secret")); + assert!(!out.contains("dotted-secret")); + assert!(!out.contains("hdr-secret")); + assert!(!out.contains("hdr2-secret")); + assert!(!out.contains("uri-secret")); + assert!(!out.contains("query-secret")); + assert!(!out.contains("frag-secret")); + assert!(!out.contains("signed-secret")); + assert!(!out.contains("cred-secret")); + assert!(out.contains("[REDACTED]")); + assert!(out.contains("wh1")); + } + #[test] fn test_empty_session_context_has_uuid_session_id() { let session = SessionContext::empty();