From 197302dc6154dab233a691364704c257a73947ad Mon Sep 17 00:00:00 2001 From: zach schuermann Date: Wed, 20 May 2026 14:56:07 -0700 Subject: [PATCH 1/4] feat(client): impl Debug for Client --- src/lib.rs | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ed59a305..df29ecac 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -71,7 +71,7 @@ pub struct Client { mocked: bool, } -#[derive(Clone)] +#[derive(Clone, Debug)] struct ProductInfo { name: String, version: String, @@ -109,6 +109,32 @@ impl Default for Client { } } +/// Manual `Debug` implementation in order to (1) remove http/insert metadata cache and (2) redact +/// sensitive information. +/// +/// Redactions: +/// - `authentication` is variant tag only (`"credentials"` or `"jwt"`) +/// - `headers` is keys-only +impl std::fmt::Debug for Client { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let authentication_redacted = match &self.authentication { + Authentication::Credentials { .. } => "credentials", + Authentication::Jwt { .. } => "jwt", + }; + f.debug_struct("Client") + .field("url", &self.url) + .field("database", &self.database) + .field("authentication", &authentication_redacted) + .field("compression", &self.compression) + .field("roles", &self.roles) + .field("settings", &self.settings) + .field("headers", &self.headers.keys()) + .field("products_info", &self.products_info) + .field("validation", &self.validation) + .finish_non_exhaustive() + } +} + /// Cache for [`RowMetadata`] to avoid allocating it for the same struct more than once /// during the application lifecycle. Key: fully qualified table name (e.g. `database.table`). #[derive(Default)] @@ -707,7 +733,7 @@ pub mod _priv { mod client_tests { use crate::_priv::RowKind; use crate::row_metadata::{AccessType, RowMetadata}; - use crate::{Authentication, Client, Row}; + use crate::{Authentication, Client, Compression, Row}; use clickhouse_types::{Column, DataTypeNode}; #[test] @@ -896,6 +922,81 @@ mod client_tests { assert_ne!(client.settings, client_clone.settings,); } + #[test] + fn client_debug() { + let client = Client::default() + .with_url("http://localhost:8123") + .with_database("mydb") + .with_user("zach") + .with_password("verysecretpassword") + .with_compression(Compression::None) + .with_roles(["reader"]) + .with_setting("async_insert", "1") + .with_header("X-Trace-Id", "abc") + .with_product_info("MyApp", "0.0.1") + .with_validation(false); + + let dbg = format!("{client:#?}"); + let expected = "\ +Client { + url: \"http://localhost:8123\", + database: Some( + \"mydb\", + ), + authentication: \"credentials\", + compression: None, + roles: { + \"reader\", + }, + settings: { + \"async_insert\": \"1\", + }, + headers: [ + \"X-Trace-Id\", + ], + products_info: [ + ProductInfo { + name: \"MyApp\", + version: \"0.0.1\", + }, + ], + validation: false, + .. +}"; + assert_eq!(dbg, expected); + } + + #[test] + fn client_debug_redaction() { + let client = Client::default() + .with_url("http://localhost:8123") + .with_database("mydb") + .with_user("zach") + .with_password("verysecretpassword") + .with_header("Authorization", "Bearer super-secret") + .with_header("X-Trace-Id", "abc"); + + let dbg = format!("{client:?}"); + assert!( + !dbg.contains("verysecretpassword"), + "password leaked: {dbg}" + ); + assert!(!dbg.contains("zach"), "user leaked: {dbg}"); + assert!(!dbg.contains("super-secret"), "header value leaked: {dbg}"); + assert!(dbg.contains("\"credentials\""), "missing auth tag: {dbg}"); + assert!(dbg.contains("http://localhost:8123"), "missing url: {dbg}"); + assert!(dbg.contains("mydb"), "missing database: {dbg}"); + // header names visible + assert!(dbg.contains("Authorization"), "missing header name: {dbg}"); + assert!(dbg.contains("X-Trace-Id"), "missing header name: {dbg}"); + + // JWT: access token must not appear + let client = Client::default().with_access_token("eyJhbGciOi.payload.signature"); + let dbg = format!("{client:?}"); + assert!(!dbg.contains("eyJhbGciOi"), "access token leaked: {dbg}"); + assert!(dbg.contains("\"jwt\""), "missing auth tag: {dbg}"); + } + #[test] fn it_gets_and_sets_settings() { let mut client = Client::default(); From f5eaae1980c3788cdda83a4d42dcdb1000f8f3b1 Mon Sep 17 00:00:00 2001 From: zach schuermann Date: Thu, 21 May 2026 10:58:12 -0700 Subject: [PATCH 2/4] address feedback, redact from url --- src/lib.rs | 52 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index df29ecac..97e7b7de 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -109,26 +109,27 @@ impl Default for Client { } } -/// Manual `Debug` implementation in order to (1) remove http/insert metadata cache and (2) redact -/// sensitive information. -/// -/// Redactions: -/// - `authentication` is variant tag only (`"credentials"` or `"jwt"`) -/// - `headers` is keys-only +/// Manual `Debug` implementation to redact sensitive information and omit internal implementation +/// details from the output. impl std::fmt::Debug for Client { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // redact auth let authentication_redacted = match &self.authentication { Authentication::Credentials { .. } => "credentials", Authentication::Jwt { .. } => "jwt", }; + // redact user/pass in Url + let origin = url::Url::parse(&self.url) + .map(|url| url.origin().ascii_serialization()) + .unwrap_or_else(|_| "".to_owned()); f.debug_struct("Client") - .field("url", &self.url) + .field("origin", &origin) .field("database", &self.database) .field("authentication", &authentication_redacted) .field("compression", &self.compression) .field("roles", &self.roles) .field("settings", &self.settings) - .field("headers", &self.headers.keys()) + .field("headers", &self.headers.keys()) // redact values .field("products_info", &self.products_info) .field("validation", &self.validation) .finish_non_exhaustive() @@ -939,7 +940,7 @@ mod client_tests { let dbg = format!("{client:#?}"); let expected = "\ Client { - url: \"http://localhost:8123\", + origin: \"http://localhost:8123\", database: Some( \"mydb\", ), @@ -969,7 +970,7 @@ Client { #[test] fn client_debug_redaction() { let client = Client::default() - .with_url("http://localhost:8123") + .with_url("http://urluser:urlsecret@localhost:8123") .with_database("mydb") .with_user("zach") .with_password("verysecretpassword") @@ -982,9 +983,15 @@ Client { "password leaked: {dbg}" ); assert!(!dbg.contains("zach"), "user leaked: {dbg}"); + // credentials embedded in the URL are redacted down to the origin + assert!(!dbg.contains("urluser"), "url user leaked: {dbg}"); + assert!(!dbg.contains("urlsecret"), "url password leaked: {dbg}"); assert!(!dbg.contains("super-secret"), "header value leaked: {dbg}"); assert!(dbg.contains("\"credentials\""), "missing auth tag: {dbg}"); - assert!(dbg.contains("http://localhost:8123"), "missing url: {dbg}"); + assert!( + dbg.contains("http://localhost:8123"), + "missing origin: {dbg}" + ); assert!(dbg.contains("mydb"), "missing database: {dbg}"); // header names visible assert!(dbg.contains("Authorization"), "missing header name: {dbg}"); @@ -997,6 +1004,29 @@ Client { assert!(dbg.contains("\"jwt\""), "missing auth tag: {dbg}"); } + #[test] + fn client_debug_invalid_url() { + // An empty URL (the default) cannot be parsed; `origin` falls back to a + // placeholder rather than echoing the raw string. + let dbg = format!("{:?}", Client::default()); + assert!( + dbg.contains("origin: \"\""), + "unexpected: {dbg}" + ); + + // A malformed URL must not be echoed verbatim even on the fallback path, + // since it may still contain credentials. + let dbg = format!( + "{:?}", + Client::default().with_url("not a url but has a secret hunter2") + ); + assert!( + dbg.contains("origin: \"\""), + "unexpected: {dbg}" + ); + assert!(!dbg.contains("hunter2"), "raw url leaked: {dbg}"); + } + #[test] fn it_gets_and_sets_settings() { let mut client = Client::default(); From 0afe8281f983c3aa0904b40b42a0ed248b8fb4c5 Mon Sep 17 00:00:00 2001 From: zach schuermann Date: Thu, 21 May 2026 11:14:34 -0700 Subject: [PATCH 3/4] redact settings values --- src/lib.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 97e7b7de..b0f0cb64 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -123,12 +123,12 @@ impl std::fmt::Debug for Client { .map(|url| url.origin().ascii_serialization()) .unwrap_or_else(|_| "".to_owned()); f.debug_struct("Client") - .field("origin", &origin) + .field("url", &origin) .field("database", &self.database) .field("authentication", &authentication_redacted) .field("compression", &self.compression) .field("roles", &self.roles) - .field("settings", &self.settings) + .field("settings", &self.settings.keys()) // redact values .field("headers", &self.headers.keys()) // redact values .field("products_info", &self.products_info) .field("validation", &self.validation) @@ -940,7 +940,7 @@ mod client_tests { let dbg = format!("{client:#?}"); let expected = "\ Client { - origin: \"http://localhost:8123\", + url: \"http://localhost:8123\", database: Some( \"mydb\", ), @@ -949,9 +949,9 @@ Client { roles: { \"reader\", }, - settings: { - \"async_insert\": \"1\", - }, + settings: [ + \"async_insert\", + ], headers: [ \"X-Trace-Id\", ], @@ -1009,10 +1009,7 @@ Client { // An empty URL (the default) cannot be parsed; `origin` falls back to a // placeholder rather than echoing the raw string. let dbg = format!("{:?}", Client::default()); - assert!( - dbg.contains("origin: \"\""), - "unexpected: {dbg}" - ); + assert!(dbg.contains("url: \"\""), "unexpected: {dbg}"); // A malformed URL must not be echoed verbatim even on the fallback path, // since it may still contain credentials. @@ -1020,10 +1017,7 @@ Client { "{:?}", Client::default().with_url("not a url but has a secret hunter2") ); - assert!( - dbg.contains("origin: \"\""), - "unexpected: {dbg}" - ); + assert!(dbg.contains("url: \"\""), "unexpected: {dbg}"); assert!(!dbg.contains("hunter2"), "raw url leaked: {dbg}"); } From 63f5effa6dcf4752697387e452c5c1e0a4f73ed1 Mon Sep 17 00:00:00 2001 From: zach schuermann Date: Thu, 21 May 2026 11:29:34 -0700 Subject: [PATCH 4/4] revert settings --- src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b0f0cb64..19bfba4d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -128,7 +128,7 @@ impl std::fmt::Debug for Client { .field("authentication", &authentication_redacted) .field("compression", &self.compression) .field("roles", &self.roles) - .field("settings", &self.settings.keys()) // redact values + .field("settings", &self.settings) .field("headers", &self.headers.keys()) // redact values .field("products_info", &self.products_info) .field("validation", &self.validation) @@ -949,9 +949,9 @@ Client { roles: { \"reader\", }, - settings: [ - \"async_insert\", - ], + settings: { + \"async_insert\": \"1\", + }, headers: [ \"X-Trace-Id\", ],