From b955ece6dede38573251cc626f9f365be3adfa83 Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 19 Mar 2026 14:56:10 -0500 Subject: [PATCH 1/2] Return generic error messages to clients for server-side errors Replace user_message() passthrough of Display output with a match that returns generic messages for 5xx/502/503 errors while keeping brief descriptions for 4xx client errors. Full error details are already logged via log::error! and never lost. Closes #437 --- crates/trusted-server-core/src/error.rs | 97 ++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 3 deletions(-) diff --git a/crates/trusted-server-core/src/error.rs b/crates/trusted-server-core/src/error.rs index 0e4a7456..85c413cb 100644 --- a/crates/trusted-server-core/src/error.rs +++ b/crates/trusted-server-core/src/error.rs @@ -16,6 +16,9 @@ use http::StatusCode; #[derive(Debug, Display)] pub enum TrustedServerError { /// Client-side input/validation error resulting in a 400 Bad Request. + /// + /// **Note:** `message` is returned to clients in the HTTP response body. + /// Keep it free of internal implementation details. #[display("Bad request: {message}")] BadRequest { message: String }, /// Configuration errors that prevent the server from starting. @@ -87,7 +90,10 @@ pub trait IntoHttpResponse { /// Convert the error into an HTTP status code. fn status_code(&self) -> StatusCode; - /// Get the error message to show to users (uses the Display implementation). + /// Get a safe, user-facing error message. + /// + /// Client errors (4xx) return a brief description; server/integration errors + /// return a generic message. Full error details are preserved in server logs. fn user_message(&self) -> String; } @@ -112,7 +118,92 @@ impl IntoHttpResponse for TrustedServerError { } fn user_message(&self) -> String { - // Use the Display implementation which already has the specific error message - self.to_string() + match self { + // Client errors (4xx) — safe to surface a brief description + Self::BadRequest { message } => format!("Bad request: {message}"), + // Consent strings may contain user data; return category only. + Self::GdprConsent { .. } => "GDPR consent error".to_string(), + Self::InvalidHeaderValue { .. } => "Invalid header value".to_string(), + Self::InvalidUtf8 { .. } => "Invalid request data".to_string(), + // Server/integration errors (5xx/502/503) — generic message only. + // Full details are already logged via log::error! in to_error_response. + _ => "An internal error occurred".to_string(), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn server_errors_return_generic_message() { + let cases = [ + TrustedServerError::Configuration { + message: "secret db path".into(), + }, + TrustedServerError::KvStore { + store_name: "users".into(), + message: "timeout".into(), + }, + TrustedServerError::Proxy { + message: "upstream 10.0.0.1 refused".into(), + }, + TrustedServerError::SyntheticId { + message: "seed file missing".into(), + }, + TrustedServerError::Template { + message: "render failed".into(), + }, + TrustedServerError::Auction { + message: "bid timeout".into(), + }, + TrustedServerError::Gam { + message: "api key invalid".into(), + }, + TrustedServerError::Prebid { + message: "adapter error".into(), + }, + TrustedServerError::Integration { + integration: "foo".into(), + message: "connection refused".into(), + }, + TrustedServerError::Settings { + message: "parse failed".into(), + }, + TrustedServerError::InsecureDefault { + field: "api_key".into(), + }, + ]; + for error in &cases { + assert_eq!( + error.user_message(), + "An internal error occurred", + "should not leak details for {error:?}", + ); + } + } + + #[test] + fn client_errors_return_safe_descriptions() { + let error = TrustedServerError::BadRequest { + message: "missing field".into(), + }; + assert_eq!(error.user_message(), "Bad request: missing field"); + + let error = TrustedServerError::GdprConsent { + message: "no consent string".into(), + }; + assert_eq!(error.user_message(), "GDPR consent error"); + + let error = TrustedServerError::InvalidHeaderValue { + message: "non-ascii".into(), + }; + assert_eq!(error.user_message(), "Invalid header value"); + + let error = TrustedServerError::InvalidUtf8 { + message: "byte 0xff".into(), + }; + assert_eq!(error.user_message(), "Invalid request data"); } } From 763ee4afd615fef1fd3174f5e0470e927d24ff29 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 23 Mar 2026 10:03:34 -0500 Subject: [PATCH 2/2] Address PR review feedback: reclassify InvalidUtf8 as server error, add doc comments --- crates/trusted-server-core/src/error.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/crates/trusted-server-core/src/error.rs b/crates/trusted-server-core/src/error.rs index 85c413cb..d6bd347d 100644 --- a/crates/trusted-server-core/src/error.rs +++ b/crates/trusted-server-core/src/error.rs @@ -17,8 +17,9 @@ use http::StatusCode; pub enum TrustedServerError { /// Client-side input/validation error resulting in a 400 Bad Request. /// - /// **Note:** `message` is returned to clients in the HTTP response body. - /// Keep it free of internal implementation details. + /// **Note:** The `message` field is included in client-facing HTTP responses + /// via [`IntoHttpResponse::user_message()`]. Keep it free of internal + /// implementation details. #[display("Bad request: {message}")] BadRequest { message: String }, /// Configuration errors that prevent the server from starting. @@ -33,6 +34,10 @@ pub enum TrustedServerError { #[display("GAM error: {message}")] Gam { message: String }, /// GDPR consent handling error. + /// + /// **Note:** Unlike [`BadRequest`](Self::BadRequest), the detail `message` + /// is intentionally suppressed in client-facing responses because consent + /// strings may contain user data. Only the category name is returned. #[display("GDPR consent error: {message}")] GdprConsent { message: String }, @@ -107,7 +112,7 @@ impl IntoHttpResponse for TrustedServerError { Self::GdprConsent { .. } => StatusCode::BAD_REQUEST, Self::InsecureDefault { .. } => StatusCode::INTERNAL_SERVER_ERROR, Self::InvalidHeaderValue { .. } => StatusCode::BAD_REQUEST, - Self::InvalidUtf8 { .. } => StatusCode::BAD_REQUEST, + Self::InvalidUtf8 { .. } => StatusCode::INTERNAL_SERVER_ERROR, Self::KvStore { .. } => StatusCode::SERVICE_UNAVAILABLE, Self::Prebid { .. } => StatusCode::BAD_GATEWAY, Self::Integration { .. } => StatusCode::BAD_GATEWAY, @@ -124,7 +129,6 @@ impl IntoHttpResponse for TrustedServerError { // Consent strings may contain user data; return category only. Self::GdprConsent { .. } => "GDPR consent error".to_string(), Self::InvalidHeaderValue { .. } => "Invalid header value".to_string(), - Self::InvalidUtf8 { .. } => "Invalid request data".to_string(), // Server/integration errors (5xx/502/503) — generic message only. // Full details are already logged via log::error! in to_error_response. _ => "An internal error occurred".to_string(), @@ -174,6 +178,9 @@ mod tests { TrustedServerError::InsecureDefault { field: "api_key".into(), }, + TrustedServerError::InvalidUtf8 { + message: "byte 0xff".into(), + }, ]; for error in &cases { assert_eq!( @@ -200,10 +207,5 @@ mod tests { message: "non-ascii".into(), }; assert_eq!(error.user_message(), "Invalid header value"); - - let error = TrustedServerError::InvalidUtf8 { - message: "byte 0xff".into(), - }; - assert_eq!(error.user_message(), "Invalid request data"); } }