From 732c16501ae30395b69e85d873e79b4b0afb3ba5 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 9 Mar 2026 20:51:48 -0500 Subject: [PATCH 1/6] Redact secrets from logs and downgrade PII/payload log levels --- AGENTS.md | 2 +- crates/common/build.rs | 3 + crates/common/src/auth.rs | 2 +- crates/common/src/http_util.rs | 8 +- .../common/src/integrations/adserver_mock.rs | 4 +- crates/common/src/integrations/aps.rs | 4 +- crates/common/src/integrations/prebid.rs | 8 +- crates/common/src/lib.rs | 1 + crates/common/src/redacted.rs | 164 ++++++++++++++++++ .../common/src/request_signing/endpoints.rs | 4 +- crates/common/src/settings.rs | 69 +++++--- crates/common/src/synthetic.rs | 12 +- crates/fastly/src/main.rs | 4 +- 13 files changed, 240 insertions(+), 45 deletions(-) create mode 100644 crates/common/src/redacted.rs diff --git a/AGENTS.md b/AGENTS.md index 17b51b88..52d00780 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,5 +21,5 @@ If you cannot read `CLAUDE.md`, follow these rules: 4. Run `cargo fmt --all -- --check` and `cargo clippy --all-targets --all-features -- -D warnings`. 5. Run JS tests with `cd crates/js/lib && npx vitest run` when touching JS/TS code. 6. Use `error-stack` (`Report`) for error handling — not anyhow, eyre, or thiserror. -7. Use `tracing` macros (not `println!`) and `expect("should ...")` (not `unwrap()`). +7. Use `log` macros (not `println!`) and `expect("should ...")` (not `unwrap()`). 8. Target is `wasm32-wasip1` — no Tokio or OS-specific dependencies in core crates. diff --git a/crates/common/build.rs b/crates/common/build.rs index 3dddd230..14e0a822 100644 --- a/crates/common/build.rs +++ b/crates/common/build.rs @@ -6,6 +6,9 @@ mod error; #[path = "src/auction_config_types.rs"] mod auction_config_types; +#[path = "src/redacted.rs"] +mod redacted; + #[path = "src/settings.rs"] mod settings; diff --git a/crates/common/src/auth.rs b/crates/common/src/auth.rs index 020f648c..707205ec 100644 --- a/crates/common/src/auth.rs +++ b/crates/common/src/auth.rs @@ -30,7 +30,7 @@ pub fn enforce_basic_auth(settings: &Settings, req: &Request) -> Option return Some(unauthorized_response()), }; - if username == handler.username && password == handler.password { + if *handler.username.expose() == username && *handler.password.expose() == password { None } else { Some(unauthorized_response()) diff --git a/crates/common/src/http_util.rs b/crates/common/src/http_util.rs index 5b3c4b8b..a53a2e4a 100644 --- a/crates/common/src/http_util.rs +++ b/crates/common/src/http_util.rs @@ -256,13 +256,13 @@ pub fn serve_static_with_etag(body: &str, req: &Request, content_type: &str) -> #[must_use] pub fn encode_url(settings: &Settings, plaintext_url: &str) -> String { // Derive a 32-byte key via SHA-256(secret) - let key_bytes = Sha256::digest(settings.publisher.proxy_secret.as_bytes()); + let key_bytes = Sha256::digest(settings.publisher.proxy_secret.expose().as_bytes()); let cipher = XChaCha20Poly1305::new(&key_bytes); // Deterministic 24-byte nonce derived from secret and plaintext (stable tokens) let mut hasher = Sha256::new(); hasher.update(b"ts-proxy-x1"); - hasher.update(settings.publisher.proxy_secret.as_bytes()); + hasher.update(settings.publisher.proxy_secret.expose().as_bytes()); hasher.update(plaintext_url.as_bytes()); let nonce_full = hasher.finalize(); let mut nonce = [0u8; 24]; @@ -294,7 +294,7 @@ pub fn decode_url(settings: &Settings, token: &str) -> Option { let nonce = XNonce::from_slice(nonce_bytes); let ciphertext = &data[2 + 24..]; - let key_bytes = Sha256::digest(settings.publisher.proxy_secret.as_bytes()); + let key_bytes = Sha256::digest(settings.publisher.proxy_secret.expose().as_bytes()); let cipher = XChaCha20Poly1305::new(&key_bytes); cipher .decrypt(nonce, ciphertext) @@ -312,7 +312,7 @@ pub fn decode_url(settings: &Settings, token: &str) -> Option { pub fn sign_clear_url(settings: &Settings, clear_url: &str) -> String { let mut hasher = Sha256::new(); hasher.update(b"ts-proxy-v2"); - hasher.update(settings.publisher.proxy_secret.as_bytes()); + hasher.update(settings.publisher.proxy_secret.expose().as_bytes()); hasher.update(clear_url.as_bytes()); let digest = hasher.finalize(); URL_SAFE_NO_PAD.encode(digest) diff --git a/crates/common/src/integrations/adserver_mock.rs b/crates/common/src/integrations/adserver_mock.rs index ea9ae587..0c704887 100644 --- a/crates/common/src/integrations/adserver_mock.rs +++ b/crates/common/src/integrations/adserver_mock.rs @@ -282,7 +282,7 @@ impl AuctionProvider for AdServerMockProvider { message: "Failed to build mediation request".to_string(), })?; - log::debug!("AdServer Mock: mediation request: {:?}", mediation_req); + log::trace!("AdServer Mock: mediation request: {:?}", mediation_req); // Build endpoint URL with context-driven query parameters let endpoint_url = self.build_endpoint_url(request); @@ -348,7 +348,7 @@ impl AuctionProvider for AdServerMockProvider { message: "Failed to parse mediation response".to_string(), })?; - log::debug!("AdServer Mock response: {:?}", response_json); + log::trace!("AdServer Mock response: {:?}", response_json); let auction_response = self.parse_mediation_response(&response_json, response_time_ms); diff --git a/crates/common/src/integrations/aps.rs b/crates/common/src/integrations/aps.rs index ae96533d..9cee6bb9 100644 --- a/crates/common/src/integrations/aps.rs +++ b/crates/common/src/integrations/aps.rs @@ -442,7 +442,7 @@ impl AuctionProvider for ApsAuctionProvider { message: "Failed to serialize APS bid request".to_string(), })?; - log::debug!("APS: sending bid request: {:?}", aps_json); + log::trace!("APS: sending bid request: {:?}", aps_json); // Create HTTP POST request let mut aps_req = Request::new(Method::POST, &self.config.endpoint); @@ -494,7 +494,7 @@ impl AuctionProvider for ApsAuctionProvider { message: "Failed to parse APS response JSON".to_string(), })?; - log::debug!("APS: received response: {:?}", response_json); + log::trace!("APS: received response: {:?}", response_json); // Transform to unified format let auction_response = self.parse_aps_response(&response_json, response_time_ms); diff --git a/crates/common/src/integrations/prebid.rs b/crates/common/src/integrations/prebid.rs index 0114aaff..a78ab3fe 100644 --- a/crates/common/src/integrations/prebid.rs +++ b/crates/common/src/integrations/prebid.rs @@ -864,9 +864,9 @@ impl AuctionProvider for PrebidAuctionProvider { ); // Log the outgoing OpenRTB request for debugging - if log::log_enabled!(log::Level::Debug) { + if log::log_enabled!(log::Level::Trace) { match serde_json::to_string_pretty(&openrtb) { - Ok(json) => log::debug!( + Ok(json) => log::trace!( "Prebid OpenRTB request to {}/openrtb2/auction:\n{}", self.config.server_url, json @@ -931,9 +931,9 @@ impl AuctionProvider for PrebidAuctionProvider { // Log the full response body when debug is enabled to surface // ext.debug.httpcalls, resolvedrequest, bidstatus, errors, etc. - if self.config.debug && log::log_enabled!(log::Level::Debug) { + if self.config.debug && log::log_enabled!(log::Level::Trace) { match serde_json::to_string_pretty(&response_json) { - Ok(json) => log::debug!("Prebid OpenRTB response:\n{json}"), + Ok(json) => log::trace!("Prebid OpenRTB response:\n{json}"), Err(e) => { log::warn!("Prebid: failed to serialize response for logging: {e}"); } diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index a01865f6..f247eb2c 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -50,6 +50,7 @@ pub mod models; pub mod openrtb; pub mod proxy; pub mod publisher; +pub mod redacted; pub mod request_signing; pub mod rsc_flight; pub mod settings; diff --git a/crates/common/src/redacted.rs b/crates/common/src/redacted.rs new file mode 100644 index 00000000..94cb17eb --- /dev/null +++ b/crates/common/src/redacted.rs @@ -0,0 +1,164 @@ +//! A wrapper type that redacts sensitive values in [`Debug`] and [`Display`] output. +//! +//! Use [`Redacted`] for secrets, passwords, API keys, and other sensitive values +//! that must never appear in logs or error messages. + +use core::fmt; + +use serde::{Deserialize, Serialize}; + +/// Wraps a value so that [`Debug`] and [`Display`] print `[REDACTED]` +/// instead of the inner contents. +/// +/// Access the real value via [`expose`](Redacted::expose). Callers must +/// never log or display the returned reference. +/// +/// # Examples +/// +/// ``` +/// use trusted_server_common::redacted::Redacted; +/// +/// let secret = Redacted::new("my-secret-key".to_string()); +/// assert_eq!(format!("{:?}", secret), "[REDACTED]"); +/// assert_eq!(secret.expose(), "my-secret-key"); +/// ``` +#[derive(Clone, Serialize, Deserialize)] +#[serde(transparent)] +pub struct Redacted(T); + +impl Redacted { + /// Creates a new [`Redacted`] value. + #[allow(dead_code)] + pub fn new(value: T) -> Self { + Self(value) + } + + /// Exposes the inner value for use in operations that need the actual secret. + /// + /// Callers should never log or display the returned reference. + pub fn expose(&self) -> &T { + &self.0 + } +} + +impl Default for Redacted { + fn default() -> Self { + Self(T::default()) + } +} + +impl fmt::Debug for Redacted { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "[REDACTED]") + } +} + +impl fmt::Display for Redacted { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "[REDACTED]") + } +} + +impl From for Redacted { + fn from(value: String) -> Self { + Self(value) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn debug_output_is_redacted() { + let secret = Redacted::new("super-secret".to_string()); + assert_eq!( + format!("{:?}", secret), + "[REDACTED]", + "should print [REDACTED] in debug output" + ); + } + + #[test] + fn display_output_is_redacted() { + let secret = Redacted::new("super-secret".to_string()); + assert_eq!( + format!("{}", secret), + "[REDACTED]", + "should print [REDACTED] in display output" + ); + } + + #[test] + fn expose_returns_inner_value() { + let secret = Redacted::new("super-secret".to_string()); + assert_eq!( + secret.expose(), + "super-secret", + "should return the inner value" + ); + } + + #[test] + fn default_creates_empty_redacted() { + let secret: Redacted = Redacted::default(); + assert_eq!(secret.expose(), "", "should default to empty string"); + } + + #[test] + fn from_string_creates_redacted() { + let secret = Redacted::from("my-key".to_string()); + assert_eq!(secret.expose(), "my-key", "should create from String"); + } + + #[test] + fn clone_preserves_inner_value() { + let secret = Redacted::new("cloneable".to_string()); + let cloned = secret.clone(); + assert_eq!( + cloned.expose(), + "cloneable", + "should preserve value after clone" + ); + } + + #[test] + fn serde_roundtrip() { + let secret = Redacted::new("serialize-me".to_string()); + let json = serde_json::to_string(&secret).expect("should serialize"); + assert_eq!(json, "\"serialize-me\"", "should serialize transparently"); + + let deserialized: Redacted = + serde_json::from_str(&json).expect("should deserialize"); + assert_eq!( + deserialized.expose(), + "serialize-me", + "should deserialize transparently" + ); + } + + #[test] + fn struct_with_redacted_field_debug() { + #[derive(Debug)] + #[allow(dead_code)] + struct Config { + name: String, + api_key: Redacted, + } + + let config = Config { + name: "test".to_string(), + api_key: Redacted::new("secret-key-123".to_string()), + }; + + let debug = format!("{:?}", config); + assert!( + debug.contains("[REDACTED]"), + "should contain [REDACTED] for the api_key field" + ); + assert!( + !debug.contains("secret-key-123"), + "should not contain the actual secret" + ); + } +} diff --git a/crates/common/src/request_signing/endpoints.rs b/crates/common/src/request_signing/endpoints.rs index 762b2692..983de8cc 100644 --- a/crates/common/src/request_signing/endpoints.rs +++ b/crates/common/src/request_signing/endpoints.rs @@ -149,7 +149,7 @@ pub fn handle_rotate_key( mut req: Request, ) -> Result> { let (config_store_id, secret_store_id) = match &settings.request_signing { - Some(setting) => (&setting.config_store_id, &setting.secret_store_id), + Some(setting) => (&setting.config_store_id, setting.secret_store_id.expose()), None => { return Err(TrustedServerError::Configuration { message: "Missing signing storage configuration.".to_string(), @@ -253,7 +253,7 @@ pub fn handle_deactivate_key( mut req: Request, ) -> Result> { let (config_store_id, secret_store_id) = match &settings.request_signing { - Some(setting) => (&setting.config_store_id, &setting.secret_store_id), + Some(setting) => (&setting.config_store_id, setting.secret_store_id.expose()), None => { return Err(TrustedServerError::Configuration { message: "Missing signing storage configuration.".to_string(), diff --git a/crates/common/src/settings.rs b/crates/common/src/settings.rs index 989d5ca0..b6a4a411 100644 --- a/crates/common/src/settings.rs +++ b/crates/common/src/settings.rs @@ -11,6 +11,7 @@ use validator::{Validate, ValidationError}; use crate::auction_config_types::AuctionConfig; use crate::error::TrustedServerError; +use crate::redacted::Redacted; pub const ENVIRONMENT_VARIABLE_PREFIX: &str = "TRUSTED_SERVER"; pub const ENVIRONMENT_VARIABLE_SEPARATOR: &str = "__"; @@ -23,8 +24,8 @@ pub struct Publisher { pub origin_url: String, /// Secret used to encrypt/decrypt proxied URLs in `/first-party/proxy`. /// Keep this secret stable to allow existing links to decode. - #[validate(length(min = 1))] - pub proxy_secret: String, + #[validate(custom(function = validate_redacted_not_empty))] + pub proxy_secret: Redacted, } impl Publisher { @@ -46,11 +47,12 @@ impl Publisher { /// /// ``` /// # use trusted_server_common::settings::Publisher; + /// # use trusted_server_common::redacted::Redacted; /// let publisher = Publisher { /// domain: "example.com".to_string(), /// cookie_domain: ".example.com".to_string(), /// origin_url: "https://origin.example.com:8080".to_string(), - /// proxy_secret: "proxy-secret".to_string(), + /// proxy_secret: Redacted::new("proxy-secret".to_string()), /// }; /// assert_eq!(publisher.origin_host(), "origin.example.com:8080"); /// ``` @@ -193,8 +195,8 @@ impl DerefMut for IntegrationSettings { pub struct Synthetic { pub counter_store: String, pub opid_store: String, - #[validate(length(min = 1))] - pub secret_key: String, + #[validate(custom(function = Synthetic::validate_secret_key))] + pub secret_key: Redacted, #[validate(length(min = 1))] pub template: String, } @@ -211,6 +213,23 @@ impl Synthetic { .iter() .any(|p| p.eq_ignore_ascii_case(secret_key)) } + + /// Validates that the secret key is not empty or a placeholder value. + /// + /// # Errors + /// + /// Returns a validation error if the secret key is empty or matches a + /// known placeholder. + pub fn validate_secret_key(secret_key: &Redacted) -> Result<(), ValidationError> { + let value = secret_key.expose().as_str(); + if value.is_empty() { + return Err(ValidationError::new("Secret key must not be empty")); + } + if Self::is_placeholder_secret_key(value) { + return Err(ValidationError::new("Secret key is not valid")); + } + Ok(()) + } } #[derive(Debug, Default, Clone, Deserialize, Serialize, Validate)] @@ -253,10 +272,10 @@ impl Rewrite { pub struct Handler { #[validate(length(min = 1), custom(function = validate_path))] pub path: String, - #[validate(length(min = 1))] - pub username: String, - #[validate(length(min = 1))] - pub password: String, + #[validate(custom(function = validate_redacted_not_empty))] + pub username: Redacted, + #[validate(custom(function = validate_redacted_not_empty))] + pub password: Redacted, #[serde(skip, default)] #[validate(skip)] regex: OnceLock, @@ -279,7 +298,7 @@ pub struct RequestSigning { #[serde(default = "default_request_signing_enabled")] pub enabled: bool, pub config_store_id: String, - pub secret_store_id: String, + pub secret_store_id: Redacted, } fn default_request_signing_enabled() -> bool { @@ -405,10 +424,10 @@ impl Settings { pub fn reject_placeholder_secrets(&self) -> Result<(), Report> { let mut insecure_fields: Vec<&str> = Vec::new(); - if Synthetic::is_placeholder_secret_key(&self.synthetic.secret_key) { + if Synthetic::is_placeholder_secret_key(self.synthetic.secret_key.expose()) { insecure_fields.push("synthetic.secret_key"); } - if Publisher::is_placeholder_proxy_secret(&self.publisher.proxy_secret) { + if Publisher::is_placeholder_proxy_secret(self.publisher.proxy_secret.expose()) { insecure_fields.push("publisher.proxy_secret"); } @@ -498,6 +517,13 @@ fn validate_no_trailing_slash(value: &str) -> Result<(), ValidationError> { Ok(()) } +fn validate_redacted_not_empty(value: &Redacted) -> Result<(), ValidationError> { + if value.expose().is_empty() { + return Err(ValidationError::new("empty_value")); + } + Ok(()) +} + fn validate_path(value: &str) -> Result<(), ValidationError> { Regex::new(value).map(|_| ()).map_err(|err| { let mut validation_error = ValidationError::new("invalid_regex"); @@ -634,6 +660,7 @@ mod tests { nextjs::NextJsIntegrationConfig, prebid::PrebidIntegrationConfig, testlight::TestlightConfig, }; + use crate::redacted::Redacted; use crate::test_support::tests::{crate_test_settings_str, create_test_settings}; #[test] @@ -677,7 +704,7 @@ mod tests { ); assert_eq!(settings.synthetic.counter_store, "test-counter-store"); assert_eq!(settings.synthetic.opid_store, "test-opid-store"); - assert_eq!(settings.synthetic.secret_key, "test-secret-key"); + assert_eq!(settings.synthetic.secret_key.expose(), "test-secret-key"); assert!(settings.synthetic.template.contains("{{client_ip}}")); settings.validate().expect("Failed to validate settings"); @@ -962,8 +989,8 @@ mod tests { assert_eq!(settings.handlers.len(), 2); let handler = &settings.handlers[0]; assert_eq!(handler.path, "^/env-handler"); - assert_eq!(handler.username, "env-user"); - assert_eq!(handler.password, "env-pass"); + assert_eq!(handler.username.expose(), "env-user"); + assert_eq!(handler.password.expose(), "env-pass"); }, ); } @@ -1056,7 +1083,7 @@ mod tests { domain: "example.com".to_string(), cookie_domain: ".example.com".to_string(), origin_url: "https://origin.example.com:8080".to_string(), - proxy_secret: "test-secret".to_string(), + proxy_secret: Redacted::new("test-secret".to_string()), }; assert_eq!(publisher.origin_host(), "origin.example.com:8080"); @@ -1065,7 +1092,7 @@ mod tests { domain: "example.com".to_string(), cookie_domain: ".example.com".to_string(), origin_url: "https://origin.example.com".to_string(), - proxy_secret: "test-secret".to_string(), + proxy_secret: Redacted::new("test-secret".to_string()), }; assert_eq!(publisher.origin_host(), "origin.example.com"); @@ -1074,7 +1101,7 @@ mod tests { domain: "example.com".to_string(), cookie_domain: ".example.com".to_string(), origin_url: "http://localhost:9090".to_string(), - proxy_secret: "test-secret".to_string(), + proxy_secret: Redacted::new("test-secret".to_string()), }; assert_eq!(publisher.origin_host(), "localhost:9090"); @@ -1083,7 +1110,7 @@ mod tests { domain: "example.com".to_string(), cookie_domain: ".example.com".to_string(), origin_url: "localhost:9090".to_string(), - proxy_secret: "test-secret".to_string(), + proxy_secret: Redacted::new("test-secret".to_string()), }; assert_eq!(publisher.origin_host(), "localhost:9090"); @@ -1092,7 +1119,7 @@ mod tests { domain: "example.com".to_string(), cookie_domain: ".example.com".to_string(), origin_url: "http://192.168.1.1:8080".to_string(), - proxy_secret: "test-secret".to_string(), + proxy_secret: Redacted::new("test-secret".to_string()), }; assert_eq!(publisher.origin_host(), "192.168.1.1:8080"); @@ -1101,7 +1128,7 @@ mod tests { domain: "example.com".to_string(), cookie_domain: ".example.com".to_string(), origin_url: "http://[::1]:8080".to_string(), - proxy_secret: "test-secret".to_string(), + proxy_secret: Redacted::new("test-secret".to_string()), }; assert_eq!(publisher.origin_host(), "[::1]:8080"); } diff --git a/crates/common/src/synthetic.rs b/crates/common/src/synthetic.rs index 853c7141..1da904e5 100644 --- a/crates/common/src/synthetic.rs +++ b/crates/common/src/synthetic.rs @@ -96,9 +96,9 @@ pub fn generate_synthetic_id( message: "Failed to render synthetic ID template".to_string(), })?; - log::info!("Input string for fresh ID: {} {}", input_string, data); + log::debug!("Input string for fresh ID: {} {}", input_string, data); - let mut mac = HmacSha256::new_from_slice(settings.synthetic.secret_key.as_bytes()) + let mut mac = HmacSha256::new_from_slice(settings.synthetic.secret_key.expose().as_bytes()) .change_context(TrustedServerError::SyntheticId { message: "Failed to create HMAC instance".to_string(), })?; @@ -109,7 +109,7 @@ pub fn generate_synthetic_id( let random_suffix = generate_random_suffix(6); let synthetic_id = format!("{}.{}", hmac_hash, random_suffix); - log::info!("Generated fresh ID: {}", synthetic_id); + log::debug!("Generated fresh ID: {}", synthetic_id); Ok(synthetic_id) } @@ -132,7 +132,7 @@ pub fn get_synthetic_id(req: &Request) -> Result, Report Result, Report { if let Some(cookie) = jar.get(COOKIE_SYNTHETIC_ID) { let id = cookie.value().to_string(); - log::info!("Using existing Trusted Server ID from cookie: {}", id); + log::debug!("Using existing Trusted Server ID from cookie: {}", id); return Ok(Some(id)); } } @@ -173,7 +173,7 @@ pub fn get_or_generate_synthetic_id( // If no existing Synthetic ID found, generate a fresh one let synthetic_id = generate_synthetic_id(settings, req)?; - log::info!("No existing synthetic_id, generated: {}", synthetic_id); + log::debug!("No existing synthetic_id, generated: {}", synthetic_id); Ok(synthetic_id) } diff --git a/crates/fastly/src/main.rs b/crates/fastly/src/main.rs index 9f4ef479..418396b8 100644 --- a/crates/fastly/src/main.rs +++ b/crates/fastly/src/main.rs @@ -46,7 +46,7 @@ fn main(req: Request) -> Result { return Ok(to_error_response(&e)); } }; - log::info!("Settings {settings:?}"); + log::debug!("Settings {settings:?}"); // Build the auction orchestrator once at startup let orchestrator = build_orchestrator(&settings); @@ -187,7 +187,7 @@ fn init_logger() { let logger = Logger::builder() .default_endpoint("tslog") .echo_stdout(true) - .max_level(log::LevelFilter::Debug) + .max_level(log::LevelFilter::Info) .build() .expect("should build Logger"); From af1a75e5afda9720e06ba915626018e318da1c5c Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 11 Mar 2026 14:12:15 -0500 Subject: [PATCH 2/6] Address PR review: consolidate sentinel checks, fix PII leaks, cleanup - Split Prebid warn log so status stays at warn! and body moves to trace! - Consolidate secret-key placeholder checks into validate_secret_key - Use proper ValidationError codes (snake_case identifiers) - Remove redundant InsecureSecretKey error variant - Downgrade synthetic template input log from debug! to trace! - Add build.rs #[path] guard comment to redacted.rs - Downgrade test log from info! to debug! --- crates/common/src/integrations/prebid.rs | 11 ++++++++--- crates/common/src/redacted.rs | 3 +++ crates/common/src/settings.rs | 4 ++-- crates/common/src/synthetic.rs | 4 ++-- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/common/src/integrations/prebid.rs b/crates/common/src/integrations/prebid.rs index a78ab3fe..d036fda2 100644 --- a/crates/common/src/integrations/prebid.rs +++ b/crates/common/src/integrations/prebid.rs @@ -915,12 +915,17 @@ impl AuctionProvider for PrebidAuctionProvider { let body_bytes = response.take_body_bytes(); if !response.get_status().is_success() { - let body_preview = String::from_utf8_lossy(&body_bytes); log::warn!( - "Prebid returned non-success status: {} — body: {}", + "Prebid returned non-success status: {}", response.get_status(), - &body_preview[..body_preview.floor_char_boundary(1000)] ); + if log::log_enabled!(log::Level::Trace) { + let body_preview = String::from_utf8_lossy(&body_bytes); + log::trace!( + "Prebid error response body: {}", + &body_preview[..body_preview.floor_char_boundary(1000)] + ); + } return Ok(AuctionResponse::error("prebid", response_time_ms)); } diff --git a/crates/common/src/redacted.rs b/crates/common/src/redacted.rs index 94cb17eb..19a0c6cb 100644 --- a/crates/common/src/redacted.rs +++ b/crates/common/src/redacted.rs @@ -1,3 +1,6 @@ +// NOTE: This file is also included in build.rs via #[path]. +// It must remain self-contained (no `crate::` imports). + //! A wrapper type that redacts sensitive values in [`Debug`] and [`Display`] output. //! //! Use [`Redacted`] for secrets, passwords, API keys, and other sensitive values diff --git a/crates/common/src/settings.rs b/crates/common/src/settings.rs index b6a4a411..a662622a 100644 --- a/crates/common/src/settings.rs +++ b/crates/common/src/settings.rs @@ -223,10 +223,10 @@ impl Synthetic { pub fn validate_secret_key(secret_key: &Redacted) -> Result<(), ValidationError> { let value = secret_key.expose().as_str(); if value.is_empty() { - return Err(ValidationError::new("Secret key must not be empty")); + return Err(ValidationError::new("empty_secret_key")); } if Self::is_placeholder_secret_key(value) { - return Err(ValidationError::new("Secret key is not valid")); + return Err(ValidationError::new("placeholder_secret_key")); } Ok(()) } diff --git a/crates/common/src/synthetic.rs b/crates/common/src/synthetic.rs index 1da904e5..5aaa511d 100644 --- a/crates/common/src/synthetic.rs +++ b/crates/common/src/synthetic.rs @@ -96,7 +96,7 @@ pub fn generate_synthetic_id( message: "Failed to render synthetic ID template".to_string(), })?; - log::debug!("Input string for fresh ID: {} {}", input_string, data); + log::trace!("Input string for fresh ID: {} {}", input_string, data); let mut mac = HmacSha256::new_from_slice(settings.synthetic.secret_key.expose().as_bytes()) .change_context(TrustedServerError::SyntheticId { @@ -263,7 +263,7 @@ mod tests { let synthetic_id = generate_synthetic_id(&settings, &req).expect("should generate synthetic ID"); - log::info!("Generated synthetic ID: {}", synthetic_id); + log::debug!("Generated synthetic ID: {}", synthetic_id); assert!( is_synthetic_id_format(&synthetic_id), "should match synthetic ID format" From 741a88a5641df9f175a39b631a5e86bfeffee5a2 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 16 Mar 2026 09:38:23 -0500 Subject: [PATCH 3/6] Address PR review feedback - Add #[validate] to proxy_secret to reject empty values - Downgrade synthetic ID logs from debug to trace for PII consistency - Revert secret_store_id to plain String (store identifier, not a secret) - Add PartialEq impls to Redacted and simplify auth comparison --- crates/common/src/auth.rs | 2 +- crates/common/src/publisher.rs | 2 +- crates/common/src/redacted.rs | 34 +++++++++++++++++++ .../common/src/request_signing/endpoints.rs | 4 +-- crates/common/src/settings.rs | 2 +- crates/common/src/synthetic.rs | 8 ++--- 6 files changed, 43 insertions(+), 9 deletions(-) diff --git a/crates/common/src/auth.rs b/crates/common/src/auth.rs index 707205ec..816891f6 100644 --- a/crates/common/src/auth.rs +++ b/crates/common/src/auth.rs @@ -30,7 +30,7 @@ pub fn enforce_basic_auth(settings: &Settings, req: &Request) -> Option return Some(unauthorized_response()), }; - if *handler.username.expose() == username && *handler.password.expose() == password { + if handler.username == username && handler.password == password { None } else { Some(unauthorized_response()) diff --git a/crates/common/src/publisher.rs b/crates/common/src/publisher.rs index 2850ab17..ff5e6e0e 100644 --- a/crates/common/src/publisher.rs +++ b/crates/common/src/publisher.rs @@ -238,7 +238,7 @@ pub fn handle_publisher_request( // Generate synthetic identifiers before the request body is consumed. let synthetic_id = get_or_generate_synthetic_id(settings, &req)?; - log::debug!("Proxy synthetic IDs - trusted: {}", synthetic_id); + log::trace!("Proxy synthetic IDs - trusted: {}", synthetic_id); let backend_name = BackendConfig::from_url( &settings.publisher.origin_url, diff --git a/crates/common/src/redacted.rs b/crates/common/src/redacted.rs index 19a0c6cb..a78eeab2 100644 --- a/crates/common/src/redacted.rs +++ b/crates/common/src/redacted.rs @@ -62,6 +62,18 @@ impl fmt::Display for Redacted { } } +impl PartialEq for Redacted { + fn eq(&self, other: &Self) -> bool { + self.0 == other.0 + } +} + +impl PartialEq for Redacted { + fn eq(&self, other: &T) -> bool { + self.0 == *other + } +} + impl From for Redacted { fn from(value: String) -> Self { Self(value) @@ -140,6 +152,28 @@ mod tests { ); } + #[test] + fn partial_eq_with_inner_type() { + let secret = Redacted::new("my-secret".to_string()); + assert!( + secret == "my-secret".to_string(), + "should equal the inner value" + ); + assert!( + secret != "other".to_string(), + "should not equal a different value" + ); + } + + #[test] + fn partial_eq_between_redacted() { + let a = Redacted::new("same".to_string()); + let b = Redacted::new("same".to_string()); + let c = Redacted::new("different".to_string()); + assert!(a == b, "should equal when inner values match"); + assert!(a != c, "should not equal when inner values differ"); + } + #[test] fn struct_with_redacted_field_debug() { #[derive(Debug)] diff --git a/crates/common/src/request_signing/endpoints.rs b/crates/common/src/request_signing/endpoints.rs index 983de8cc..762b2692 100644 --- a/crates/common/src/request_signing/endpoints.rs +++ b/crates/common/src/request_signing/endpoints.rs @@ -149,7 +149,7 @@ pub fn handle_rotate_key( mut req: Request, ) -> Result> { let (config_store_id, secret_store_id) = match &settings.request_signing { - Some(setting) => (&setting.config_store_id, setting.secret_store_id.expose()), + Some(setting) => (&setting.config_store_id, &setting.secret_store_id), None => { return Err(TrustedServerError::Configuration { message: "Missing signing storage configuration.".to_string(), @@ -253,7 +253,7 @@ pub fn handle_deactivate_key( mut req: Request, ) -> Result> { let (config_store_id, secret_store_id) = match &settings.request_signing { - Some(setting) => (&setting.config_store_id, setting.secret_store_id.expose()), + Some(setting) => (&setting.config_store_id, &setting.secret_store_id), None => { return Err(TrustedServerError::Configuration { message: "Missing signing storage configuration.".to_string(), diff --git a/crates/common/src/settings.rs b/crates/common/src/settings.rs index a662622a..a73df0e1 100644 --- a/crates/common/src/settings.rs +++ b/crates/common/src/settings.rs @@ -298,7 +298,7 @@ pub struct RequestSigning { #[serde(default = "default_request_signing_enabled")] pub enabled: bool, pub config_store_id: String, - pub secret_store_id: Redacted, + pub secret_store_id: String, } fn default_request_signing_enabled() -> bool { diff --git a/crates/common/src/synthetic.rs b/crates/common/src/synthetic.rs index 5aaa511d..4e91c0ad 100644 --- a/crates/common/src/synthetic.rs +++ b/crates/common/src/synthetic.rs @@ -109,7 +109,7 @@ pub fn generate_synthetic_id( let random_suffix = generate_random_suffix(6); let synthetic_id = format!("{}.{}", hmac_hash, random_suffix); - log::debug!("Generated fresh ID: {}", synthetic_id); + log::trace!("Generated fresh ID: {}", synthetic_id); Ok(synthetic_id) } @@ -132,7 +132,7 @@ pub fn get_synthetic_id(req: &Request) -> Result, Report Result, Report { if let Some(cookie) = jar.get(COOKIE_SYNTHETIC_ID) { let id = cookie.value().to_string(); - log::debug!("Using existing Trusted Server ID from cookie: {}", id); + log::trace!("Using existing Trusted Server ID from cookie: {}", id); return Ok(Some(id)); } } @@ -173,7 +173,7 @@ pub fn get_or_generate_synthetic_id( // If no existing Synthetic ID found, generate a fresh one let synthetic_id = generate_synthetic_id(settings, req)?; - log::debug!("No existing synthetic_id, generated: {}", synthetic_id); + log::trace!("No existing synthetic_id, generated: {}", synthetic_id); Ok(synthetic_id) } From a28d8bd265d90926925eb74b931a0aa73267b24d Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 16 Mar 2026 10:26:43 -0500 Subject: [PATCH 4/6] Add comment explaining why #[allow(dead_code)] is needed on Redacted::new --- crates/common/src/redacted.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/common/src/redacted.rs b/crates/common/src/redacted.rs index a78eeab2..24a23465 100644 --- a/crates/common/src/redacted.rs +++ b/crates/common/src/redacted.rs @@ -31,7 +31,7 @@ pub struct Redacted(T); impl Redacted { /// Creates a new [`Redacted`] value. - #[allow(dead_code)] + #[allow(dead_code)] // Used by the library crate but not by build.rs which includes this file via #[path] pub fn new(value: T) -> Self { Self(value) } From e5026c4e999701db46a1610747cdbe2a3c888822 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 16 Mar 2026 13:33:48 -0500 Subject: [PATCH 5/6] Address PR review feedback - Downgrade test synthetic ID log from debug to trace (synthetic.rs:266) for consistency with production code log levels - Replace println! with log::debug! in endpoints.rs test module per project convention to use log macros instead of println --- .../common/src/request_signing/endpoints.rs | 30 +++++++++++-------- crates/common/src/synthetic.rs | 2 +- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/crates/common/src/request_signing/endpoints.rs b/crates/common/src/request_signing/endpoints.rs index 762b2692..032a6fc5 100644 --- a/crates/common/src/request_signing/endpoints.rs +++ b/crates/common/src/request_signing/endpoints.rs @@ -437,12 +437,13 @@ mod tests { let body = resp.take_body_str(); let response: RotateKeyResponse = serde_json::from_str(&body).expect("should deserialize rotate response"); - println!( + log::debug!( "Rotation response: success={}, message={}", - response.success, response.message + response.success, + response.message ); } - Err(e) => println!("Expected error in test environment: {}", e), + Err(e) => log::debug!("Expected error in test environment: {}", e), } } @@ -464,12 +465,13 @@ mod tests { let body = resp.take_body_str(); let response: RotateKeyResponse = serde_json::from_str(&body).expect("should deserialize rotate response"); - println!( + log::debug!( "Custom KID rotation: success={}, new_kid={}", - response.success, response.new_kid + response.success, + response.new_kid ); } - Err(e) => println!("Expected error in test environment: {}", e), + Err(e) => log::debug!("Expected error in test environment: {}", e), } } @@ -503,12 +505,13 @@ mod tests { let body = resp.take_body_str(); let response: DeactivateKeyResponse = serde_json::from_str(&body).expect("should deserialize deactivate response"); - println!( + log::debug!( "Deactivate response: success={}, message={}", - response.success, response.message + response.success, + response.message ); } - Err(e) => println!("Expected error in test environment: {}", e), + Err(e) => log::debug!("Expected error in test environment: {}", e), } } @@ -532,12 +535,13 @@ mod tests { let body = resp.take_body_str(); let response: DeactivateKeyResponse = serde_json::from_str(&body).expect("should deserialize deactivate response"); - println!( + log::debug!( "Delete response: success={}, deleted={}", - response.success, response.deleted + response.success, + response.deleted ); } - Err(e) => println!("Expected error in test environment: {}", e), + Err(e) => log::debug!("Expected error in test environment: {}", e), } } @@ -594,7 +598,7 @@ mod tests { assert!(discovery.get("endpoints").is_none()); assert!(discovery.get("capabilities").is_none()); } - Err(e) => println!("Expected error in test environment: {}", e), + Err(e) => log::debug!("Expected error in test environment: {}", e), } } } diff --git a/crates/common/src/synthetic.rs b/crates/common/src/synthetic.rs index 4e91c0ad..38a3456f 100644 --- a/crates/common/src/synthetic.rs +++ b/crates/common/src/synthetic.rs @@ -263,7 +263,7 @@ mod tests { let synthetic_id = generate_synthetic_id(&settings, &req).expect("should generate synthetic ID"); - log::debug!("Generated synthetic ID: {}", synthetic_id); + log::trace!("Generated synthetic ID: {}", synthetic_id); assert!( is_synthetic_id_format(&synthetic_id), "should match synthetic ID format" From bee55c39189dc705aae87ee90807b2a43d37268f Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 18 Mar 2026 10:23:11 -0500 Subject: [PATCH 6/6] Remove placeholder check from build-time validator validate_secret_key runs at build time via from_toml_and_env where placeholder values are legitimate. Placeholder rejection belongs exclusively in reject_placeholder_secrets at runtime. --- crates/common/src/settings.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/common/src/settings.rs b/crates/common/src/settings.rs index a73df0e1..396b6eb0 100644 --- a/crates/common/src/settings.rs +++ b/crates/common/src/settings.rs @@ -214,20 +214,20 @@ impl Synthetic { .any(|p| p.eq_ignore_ascii_case(secret_key)) } - /// Validates that the secret key is not empty or a placeholder value. + /// Validates that the secret key is not empty. + /// + /// Placeholder detection is intentionally **not** performed here because + /// this validator runs at build time (via `from_toml_and_env`) when the + /// config legitimately contains placeholder values. Placeholder rejection + /// happens at runtime via [`Settings::reject_placeholder_secrets`]. /// /// # Errors /// - /// Returns a validation error if the secret key is empty or matches a - /// known placeholder. + /// Returns a validation error if the secret key is empty. pub fn validate_secret_key(secret_key: &Redacted) -> Result<(), ValidationError> { - let value = secret_key.expose().as_str(); - if value.is_empty() { + if secret_key.expose().is_empty() { return Err(ValidationError::new("empty_secret_key")); } - if Self::is_placeholder_secret_key(value) { - return Err(ValidationError::new("placeholder_secret_key")); - } Ok(()) } }