diff --git a/crates/common/build.rs b/crates/common/build.rs index cb1e60ae..ef10716b 100644 --- a/crates/common/build.rs +++ b/crates/common/build.rs @@ -27,7 +27,12 @@ fn main() { let toml_content = fs::read_to_string(init_config_path) .unwrap_or_else(|_| panic!("Failed to read {init_config_path:?}")); - // Merge base TOML with environment variable overrides and write output + // Merge base TOML with environment variable overrides and write output. + // Note: placeholder secret rejection is intentionally NOT done here. + // The base trusted-server.toml ships with placeholder secrets that + // production deployments override via TRUSTED_SERVER__* env vars at + // build time. Runtime startup (get_settings) rejects any remaining + // placeholders so a misconfigured deployment fails fast. let settings = settings::Settings::from_toml_and_env(&toml_content) .expect("Failed to parse settings at build time"); diff --git a/crates/common/src/error.rs b/crates/common/src/error.rs index c5964cfe..0e4a7456 100644 --- a/crates/common/src/error.rs +++ b/crates/common/src/error.rs @@ -33,10 +33,11 @@ pub enum TrustedServerError { #[display("GDPR consent error: {message}")] GdprConsent { message: String }, - /// The synthetic secret key is using the insecure default value. - - #[display("Synthetic secret key is set to the default value - this is insecure")] - InsecureSecretKey, + /// A configuration secret is still set to a known placeholder value. + #[display( + "Configuration field '{field}' is set to a known placeholder value - this is insecure" + )] + InsecureDefault { field: String }, /// Invalid UTF-8 data encountered. #[display("Invalid UTF-8 data: {message}")] @@ -98,7 +99,7 @@ impl IntoHttpResponse for TrustedServerError { Self::Configuration { .. } | Self::Settings { .. } => StatusCode::INTERNAL_SERVER_ERROR, Self::Gam { .. } => StatusCode::BAD_GATEWAY, Self::GdprConsent { .. } => StatusCode::BAD_REQUEST, - Self::InsecureSecretKey => StatusCode::INTERNAL_SERVER_ERROR, + Self::InsecureDefault { .. } => StatusCode::INTERNAL_SERVER_ERROR, Self::InvalidHeaderValue { .. } => StatusCode::BAD_REQUEST, Self::InvalidUtf8 { .. } => StatusCode::BAD_REQUEST, Self::KvStore { .. } => StatusCode::SERVICE_UNAVAILABLE, diff --git a/crates/common/src/settings.rs b/crates/common/src/settings.rs index eff23004..4737a728 100644 --- a/crates/common/src/settings.rs +++ b/crates/common/src/settings.rs @@ -27,6 +27,17 @@ pub struct Publisher { } impl Publisher { + /// Known placeholder values that must not be used in production. + #[allow(dead_code)] + pub const PROXY_SECRET_PLACEHOLDERS: &[&str] = &["change-me-proxy-secret"]; + + /// Returns `true` if `proxy_secret` matches a known placeholder value. + #[allow(dead_code)] + #[must_use] + pub fn is_placeholder_proxy_secret(proxy_secret: &str) -> bool { + Self::PROXY_SECRET_PLACEHOLDERS.contains(&proxy_secret) + } + /// Extracts the host (including port if present) from the `origin_url`. /// /// # Examples @@ -180,23 +191,22 @@ impl DerefMut for IntegrationSettings { pub struct Synthetic { pub counter_store: String, pub opid_store: String, - #[validate(length(min = 1), custom(function = Synthetic::validate_secret_key))] + #[validate(length(min = 1))] pub secret_key: String, #[validate(length(min = 1))] pub template: String, } impl Synthetic { - /// Validates that the secret key is not the placeholder value. - /// - /// # Errors - /// - /// Returns a validation error if the secret key is `"secret_key"` (the placeholder). - pub fn validate_secret_key(secret_key: &str) -> Result<(), ValidationError> { - match secret_key { - "secret_key" => Err(ValidationError::new("Secret key is not valid")), - _ => Ok(()), - } + /// Known placeholder values that must not be used in production. + #[allow(dead_code)] + pub const SECRET_KEY_PLACEHOLDERS: &[&str] = &["secret-key", "secret_key", "trusted-server"]; + + /// Returns `true` if `secret_key` matches a known placeholder value. + #[allow(dead_code)] + #[must_use] + pub fn is_placeholder_secret_key(secret_key: &str) -> bool { + Self::SECRET_KEY_PLACEHOLDERS.contains(&secret_key) } } @@ -613,6 +623,7 @@ mod tests { #[test] fn test_settings_missing_required_fields() { let re = Regex::new(r"origin_url = .*").expect("regex should compile"); + let toml_str = crate_test_settings_str(); let toml_str = re.replace(&toml_str, ""); @@ -623,6 +634,42 @@ mod tests { ); } + #[test] + fn is_placeholder_secret_key_rejects_all_known_placeholders() { + for placeholder in Synthetic::SECRET_KEY_PLACEHOLDERS { + assert!( + Synthetic::is_placeholder_secret_key(placeholder), + "should detect placeholder secret_key '{placeholder}'" + ); + } + } + + #[test] + fn is_placeholder_secret_key_accepts_non_placeholder() { + assert!( + !Synthetic::is_placeholder_secret_key("test-secret-key"), + "should accept non-placeholder secret_key" + ); + } + + #[test] + fn is_placeholder_proxy_secret_rejects_all_known_placeholders() { + for placeholder in Publisher::PROXY_SECRET_PLACEHOLDERS { + assert!( + Publisher::is_placeholder_proxy_secret(placeholder), + "should detect placeholder proxy_secret '{placeholder}'" + ); + } + } + + #[test] + fn is_placeholder_proxy_secret_accepts_non_placeholder() { + assert!( + !Publisher::is_placeholder_proxy_secret("unit-test-proxy-secret"), + "should accept non-placeholder proxy_secret" + ); + } + #[test] fn test_settings_empty_toml() { let toml_str = ""; diff --git a/crates/common/src/settings_data.rs b/crates/common/src/settings_data.rs index 4f8e36a0..3eb799cb 100644 --- a/crates/common/src/settings_data.rs +++ b/crates/common/src/settings_data.rs @@ -3,7 +3,7 @@ use error_stack::{Report, ResultExt}; use validator::Validate; use crate::error::TrustedServerError; -use crate::settings::Settings; +use crate::settings::{Publisher, Settings, Synthetic}; pub use crate::auction_config_types::AuctionConfig; @@ -34,8 +34,17 @@ pub fn get_settings() -> Result> { message: "Failed to validate configuration".to_string(), })?; - if settings.synthetic.secret_key == "secret-key" { - return Err(Report::new(TrustedServerError::InsecureSecretKey)); + // Reject known placeholder values for secrets that feed into cryptographic operations. + if Synthetic::is_placeholder_secret_key(&settings.synthetic.secret_key) { + return Err(Report::new(TrustedServerError::InsecureDefault { + field: "synthetic.secret_key".to_string(), + })); + } + + if Publisher::is_placeholder_proxy_secret(&settings.publisher.proxy_secret) { + return Err(Report::new(TrustedServerError::InsecureDefault { + field: "publisher.proxy_secret".to_string(), + })); } if !settings.proxy.certificate_check { @@ -52,19 +61,15 @@ mod tests { use super::*; #[test] - fn test_get_settings() { - // Test that Settings::new() loads successfully - let settings = get_settings(); - assert!(settings.is_ok(), "Settings should load from embedded TOML"); - - let settings = settings.expect("should load settings from embedded TOML"); - // Verify basic structure is loaded - assert!(!settings.publisher.domain.is_empty()); - assert!(!settings.publisher.cookie_domain.is_empty()); - assert!(!settings.publisher.origin_url.is_empty()); - assert!(!settings.synthetic.counter_store.is_empty()); - assert!(!settings.synthetic.opid_store.is_empty()); - assert!(!settings.synthetic.secret_key.is_empty()); - assert!(!settings.synthetic.template.is_empty()); + fn rejects_default_placeholder_secrets() { + // The embedded trusted-server.toml ships with placeholder secrets. + // get_settings() must reject them so a deployment using defaults fails fast. + let err = get_settings() + .expect_err("should reject settings that contain placeholder secret values"); + let root = err.current_context(); + assert!( + matches!(root, TrustedServerError::InsecureDefault { .. }), + "should be InsecureDefault error, got: {root}" + ); } }