From 575afae83ef0c0adf5468c65a56b67868f8a2187 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 9 Mar 2026 20:51:55 -0500 Subject: [PATCH 1/5] Fix weak and inconsistent secret default validation Reject all known placeholder values for synthetic.secret_key and publisher.proxy_secret at runtime startup so deployments using default secrets fail fast instead of running with predictable cryptographic keys. --- crates/common/build.rs | 5 +++ crates/common/src/error.rs | 11 ++--- crates/common/src/settings.rs | 69 ++++++++++++++++++++++-------- crates/common/src/settings_data.rs | 37 ++++++++-------- 4 files changed, 82 insertions(+), 40 deletions(-) diff --git a/crates/common/build.rs b/crates/common/build.rs index cce3beab..3dddd230 100644 --- a/crates/common/build.rs +++ b/crates/common/build.rs @@ -29,6 +29,11 @@ fn main() { // Merge base TOML with environment variable overrides and write output. // Panics if admin endpoints are not covered by a handler. + // 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..5ed5cafe 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 536cfcdc..f93defa7 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) } } @@ -659,15 +669,38 @@ 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, ""); + 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}'" + ); + } + } - let settings = Settings::from_toml(&toml_str); + #[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!( - settings.is_err(), - "Should fail when required fields are missing" + !Publisher::is_placeholder_proxy_secret("unit-test-proxy-secret"), + "should accept non-placeholder proxy_secret" ); } diff --git a/crates/common/src/settings_data.rs b/crates/common/src/settings_data.rs index 4f8e36a0..49e85e63 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,13 @@ 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 result = get_settings(); + assert!( + result.is_err(), + "should reject settings that contain placeholder secret values" + ); } } From ccf22b0c5d6ce1574b37d96e56721557e0b32d76 Mon Sep 17 00:00:00 2001 From: Christian Date: Tue, 10 Mar 2026 11:00:00 -0500 Subject: [PATCH 2/5] Address PR review: restore missing test, assert specific error variant, fix em dash --- crates/common/src/error.rs | 2 +- crates/common/src/settings.rs | 14 ++++++++++++++ crates/common/src/settings_data.rs | 8 +++++--- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/common/src/error.rs b/crates/common/src/error.rs index 5ed5cafe..0e4a7456 100644 --- a/crates/common/src/error.rs +++ b/crates/common/src/error.rs @@ -35,7 +35,7 @@ pub enum TrustedServerError { /// 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" + "Configuration field '{field}' is set to a known placeholder value - this is insecure" )] InsecureDefault { field: String }, diff --git a/crates/common/src/settings.rs b/crates/common/src/settings.rs index f93defa7..311c3dee 100644 --- a/crates/common/src/settings.rs +++ b/crates/common/src/settings.rs @@ -668,6 +668,20 @@ 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, ""); + + let settings = Settings::from_toml(&toml_str); + assert!( + settings.is_err(), + "Should fail when required fields are missing" + ); + } + #[test] fn is_placeholder_secret_key_rejects_all_known_placeholders() { for placeholder in Synthetic::SECRET_KEY_PLACEHOLDERS { diff --git a/crates/common/src/settings_data.rs b/crates/common/src/settings_data.rs index 49e85e63..3eb799cb 100644 --- a/crates/common/src/settings_data.rs +++ b/crates/common/src/settings_data.rs @@ -64,10 +64,12 @@ mod tests { 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 result = get_settings(); + let err = get_settings() + .expect_err("should reject settings that contain placeholder secret values"); + let root = err.current_context(); assert!( - result.is_err(), - "should reject settings that contain placeholder secret values" + matches!(root, TrustedServerError::InsecureDefault { .. }), + "should be InsecureDefault error, got: {root}" ); } } From fec8ce96d46d2d211d2682d490a99092045cc87a Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 11 Mar 2026 14:20:10 -0500 Subject: [PATCH 3/5] Address PR review: add proxy_secret validation, centralize placeholder checks - Add #[validate(length(min = 1))] to proxy_secret to prevent empty strings from producing a deterministic crypto key - Centralize placeholder validation into Settings::reject_placeholder_secrets() which collects all violations into a single error - Rewrite tests to use explicit TOML input instead of build-time embedded config, making them deterministic across CI environments - Remove spurious #[allow(dead_code)] from public items that are used --- crates/common/src/settings.rs | 32 +++++++++-- crates/common/src/settings_data.rs | 92 +++++++++++++++++++++++------- 2 files changed, 100 insertions(+), 24 deletions(-) diff --git a/crates/common/src/settings.rs b/crates/common/src/settings.rs index 311c3dee..bb15ae33 100644 --- a/crates/common/src/settings.rs +++ b/crates/common/src/settings.rs @@ -23,16 +23,15 @@ 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, } 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) @@ -199,11 +198,9 @@ pub struct Synthetic { impl Synthetic { /// 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) @@ -391,6 +388,33 @@ impl Settings { Ok(settings) } + /// Checks all secret fields for known placeholder values and returns an + /// error listing every offending field. This centralises the placeholder + /// policy so callers don't need to know which fields are secrets. + /// + /// # Errors + /// + /// Returns [`TrustedServerError::InsecureDefault`] when one or more secret + /// fields still contain a placeholder value. + 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) { + insecure_fields.push("synthetic.secret_key"); + } + if Publisher::is_placeholder_proxy_secret(&self.publisher.proxy_secret) { + insecure_fields.push("publisher.proxy_secret"); + } + + if insecure_fields.is_empty() { + Ok(()) + } else { + Err(Report::new(TrustedServerError::InsecureDefault { + field: insecure_fields.join(", "), + })) + } + } + #[must_use] pub fn handler_for_path(&self, path: &str) -> Option<&Handler> { self.handlers diff --git a/crates/common/src/settings_data.rs b/crates/common/src/settings_data.rs index 3eb799cb..b21d9efe 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::{Publisher, Settings, Synthetic}; +use crate::settings::Settings; pub use crate::auction_config_types::AuctionConfig; @@ -35,17 +35,7 @@ pub fn get_settings() -> Result> { })?; // 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(), - })); - } + settings.reject_placeholder_secrets()?; if !settings.proxy.certificate_check { log::warn!( @@ -58,18 +48,80 @@ pub fn get_settings() -> Result> { #[cfg(test)] mod tests { - use super::*; + use crate::error::TrustedServerError; + use crate::settings::Settings; + use crate::test_support::tests::crate_test_settings_str; + + /// Builds a TOML string with the given secret values swapped in. + fn toml_with_secrets(secret_key: &str, proxy_secret: &str) -> String { + crate_test_settings_str() + .replace( + r#"secret_key = "test-secret-key""#, + &format!(r#"secret_key = "{secret_key}""#), + ) + .replace( + r#"proxy_secret = "unit-test-proxy-secret""#, + &format!(r#"proxy_secret = "{proxy_secret}""#), + ) + } + + #[test] + fn rejects_placeholder_secret_key() { + let toml = toml_with_secrets("secret-key", "real-proxy-secret"); + let settings = Settings::from_toml(&toml).expect("should parse TOML"); + let err = settings + .reject_placeholder_secrets() + .expect_err("should reject placeholder secret_key"); + let root = err.current_context(); + assert!( + matches!(root, TrustedServerError::InsecureDefault { field } if field.contains("synthetic.secret_key")), + "error should mention synthetic.secret_key, got: {root}" + ); + } #[test] - 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"); + fn rejects_placeholder_proxy_secret() { + let toml = toml_with_secrets("real-secret-key", "change-me-proxy-secret"); + let settings = Settings::from_toml(&toml).expect("should parse TOML"); + let err = settings + .reject_placeholder_secrets() + .expect_err("should reject placeholder proxy_secret"); let root = err.current_context(); assert!( - matches!(root, TrustedServerError::InsecureDefault { .. }), - "should be InsecureDefault error, got: {root}" + matches!(root, TrustedServerError::InsecureDefault { field } if field.contains("publisher.proxy_secret")), + "error should mention publisher.proxy_secret, got: {root}" ); } + + #[test] + fn rejects_both_placeholders_in_single_error() { + let toml = toml_with_secrets("secret_key", "change-me-proxy-secret"); + let settings = Settings::from_toml(&toml).expect("should parse TOML"); + let err = settings + .reject_placeholder_secrets() + .expect_err("should reject both placeholder secrets"); + let root = err.current_context(); + match root { + TrustedServerError::InsecureDefault { field } => { + assert!( + field.contains("synthetic.secret_key"), + "error should mention synthetic.secret_key, got: {field}" + ); + assert!( + field.contains("publisher.proxy_secret"), + "error should mention publisher.proxy_secret, got: {field}" + ); + } + other => panic!("expected InsecureDefault, got: {other}"), + } + } + + #[test] + fn accepts_non_placeholder_secrets() { + let toml = toml_with_secrets("production-secret-key", "production-proxy-secret"); + let settings = Settings::from_toml(&toml).expect("should parse TOML"); + settings + .reject_placeholder_secrets() + .expect("non-placeholder secrets should pass validation"); + } } From ea9722542f0c1f6fc4596749855afcdf96cd5d38 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 16 Mar 2026 13:58:24 -0500 Subject: [PATCH 4/5] Address review feedback: case-insensitive placeholders, test robustness, smoke test --- crates/common/src/settings.rs | 16 ++++++++++----- crates/common/src/settings_data.rs | 31 ++++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/crates/common/src/settings.rs b/crates/common/src/settings.rs index bb15ae33..b4793e9b 100644 --- a/crates/common/src/settings.rs +++ b/crates/common/src/settings.rs @@ -31,10 +31,13 @@ impl Publisher { /// Known placeholder values that must not be used in production. pub const PROXY_SECRET_PLACEHOLDERS: &[&str] = &["change-me-proxy-secret"]; - /// Returns `true` if `proxy_secret` matches a known placeholder value. + /// Returns `true` if `proxy_secret` matches a known placeholder value + /// (case-insensitive). #[must_use] pub fn is_placeholder_proxy_secret(proxy_secret: &str) -> bool { - Self::PROXY_SECRET_PLACEHOLDERS.contains(&proxy_secret) + Self::PROXY_SECRET_PLACEHOLDERS + .iter() + .any(|p| p.eq_ignore_ascii_case(proxy_secret)) } /// Extracts the host (including port if present) from the `origin_url`. @@ -200,10 +203,13 @@ impl Synthetic { /// Known placeholder values that must not be used in production. pub const SECRET_KEY_PLACEHOLDERS: &[&str] = &["secret-key", "secret_key", "trusted-server"]; - /// Returns `true` if `secret_key` matches a known placeholder value. + /// Returns `true` if `secret_key` matches a known placeholder value + /// (case-insensitive). #[must_use] pub fn is_placeholder_secret_key(secret_key: &str) -> bool { - Self::SECRET_KEY_PLACEHOLDERS.contains(&secret_key) + Self::SECRET_KEY_PLACEHOLDERS + .iter() + .any(|p| p.eq_ignore_ascii_case(secret_key)) } } @@ -389,7 +395,7 @@ impl Settings { } /// Checks all secret fields for known placeholder values and returns an - /// error listing every offending field. This centralises the placeholder + /// error listing every offending field. This centralises the placeholder /// policy so callers don't need to know which fields are secrets. /// /// # Errors diff --git a/crates/common/src/settings_data.rs b/crates/common/src/settings_data.rs index b21d9efe..52767132 100644 --- a/crates/common/src/settings_data.rs +++ b/crates/common/src/settings_data.rs @@ -53,8 +53,14 @@ mod tests { use crate::test_support::tests::crate_test_settings_str; /// Builds a TOML string with the given secret values swapped in. + /// + /// # Panics + /// + /// Panics if the replacement patterns no longer match the test TOML, + /// which would cause the substitution to silently no-op. fn toml_with_secrets(secret_key: &str, proxy_secret: &str) -> String { - crate_test_settings_str() + let original = crate_test_settings_str(); + let result = original .replace( r#"secret_key = "test-secret-key""#, &format!(r#"secret_key = "{secret_key}""#), @@ -62,7 +68,12 @@ mod tests { .replace( r#"proxy_secret = "unit-test-proxy-secret""#, &format!(r#"proxy_secret = "{proxy_secret}""#), - ) + ); + assert_ne!( + result, original, + "should have replaced at least one secret value" + ); + result } #[test] @@ -124,4 +135,20 @@ mod tests { .reject_placeholder_secrets() .expect("non-placeholder secrets should pass validation"); } + + /// Smoke-test the full `get_settings()` pipeline (embedded bytes → UTF-8 → + /// parse → validate → placeholder check). The build-time TOML ships with + /// placeholder secrets, so the expected outcome is an [`InsecureDefault`] + /// error — but reaching that error proves every earlier stage succeeded. + #[test] + fn get_settings_rejects_embedded_placeholder_secrets() { + let err = super::get_settings().expect_err("should reject embedded placeholder secrets"); + assert!( + matches!( + err.current_context(), + TrustedServerError::InsecureDefault { .. } + ), + "should fail with InsecureDefault, got: {err}" + ); + } } From 85304bd9cbf9a3026e61fd3b06152d8218ec285c Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 16 Mar 2026 14:09:06 -0500 Subject: [PATCH 5/5] Improve test assertions, add case-insensitive tests, update docs for placeholder validation --- crates/common/src/settings.rs | 20 ++++++++++++++++++++ crates/common/src/settings_data.rs | 25 ++++++++++++++----------- docs/guide/configuration.md | 15 +++++++++------ 3 files changed, 43 insertions(+), 17 deletions(-) diff --git a/crates/common/src/settings.rs b/crates/common/src/settings.rs index b4793e9b..989d5ca0 100644 --- a/crates/common/src/settings.rs +++ b/crates/common/src/settings.rs @@ -722,6 +722,18 @@ mod tests { } } + #[test] + fn is_placeholder_secret_key_is_case_insensitive() { + assert!( + Synthetic::is_placeholder_secret_key("SECRET-KEY"), + "should detect case-insensitive placeholder secret_key" + ); + assert!( + Synthetic::is_placeholder_secret_key("Trusted-Server"), + "should detect mixed-case placeholder secret_key" + ); + } + #[test] fn is_placeholder_secret_key_accepts_non_placeholder() { assert!( @@ -740,6 +752,14 @@ mod tests { } } + #[test] + fn is_placeholder_proxy_secret_is_case_insensitive() { + assert!( + Publisher::is_placeholder_proxy_secret("CHANGE-ME-PROXY-SECRET"), + "should detect case-insensitive placeholder proxy_secret" + ); + } + #[test] fn is_placeholder_proxy_secret_accepts_non_placeholder() { assert!( diff --git a/crates/common/src/settings_data.rs b/crates/common/src/settings_data.rs index 52767132..d764f0bd 100644 --- a/crates/common/src/settings_data.rs +++ b/crates/common/src/settings_data.rs @@ -60,18 +60,21 @@ mod tests { /// which would cause the substitution to silently no-op. fn toml_with_secrets(secret_key: &str, proxy_secret: &str) -> String { let original = crate_test_settings_str(); - let result = original - .replace( - r#"secret_key = "test-secret-key""#, - &format!(r#"secret_key = "{secret_key}""#), - ) - .replace( - r#"proxy_secret = "unit-test-proxy-secret""#, - &format!(r#"proxy_secret = "{proxy_secret}""#), - ); + let after_secret_key = original.replace( + r#"secret_key = "test-secret-key""#, + &format!(r#"secret_key = "{secret_key}""#), + ); + assert_ne!( + after_secret_key, original, + "should have replaced secret_key value" + ); + let result = after_secret_key.replace( + r#"proxy_secret = "unit-test-proxy-secret""#, + &format!(r#"proxy_secret = "{proxy_secret}""#), + ); assert_ne!( - result, original, - "should have replaced at least one secret value" + result, after_secret_key, + "should have replaced proxy_secret value" ); result } diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 7fa3cfdf..cc83660b 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -244,6 +244,7 @@ TRUSTED_SERVER__PUBLISHER__PROXY_SECRET=your-secret-here **Security**: - Keep confidential and secure +- Cannot be the placeholder `"change-me-proxy-secret"` (case-insensitive) — startup will fail - Rotate periodically (90 days recommended) - Use cryptographically random values (32+ bytes) - Never commit to version control @@ -359,7 +360,7 @@ fastly kv-store create --name=opid_store **Security**: - Must be non-empty -- Cannot be `"secret_key"` or `"secret-key"` (reserved/invalid) +- Cannot be a known placeholder: `"secret-key"`, `"secret_key"`, or `"trusted-server"` (case-insensitive) - Rotate periodically for security - Store securely (environment variable recommended) @@ -373,7 +374,7 @@ openssl rand -hex 32 **Validation**: Application startup fails if: - Empty string -- Exactly `"secret_key"` or `"secret-key"` (default placeholders) +- Exactly `"secret-key"`, `"secret_key"`, or `"trusted-server"` (known placeholders, case-insensitive) #### `template` @@ -922,11 +923,12 @@ Configuration is validated at startup: - All fields non-empty - `origin_url` is valid URL +- `proxy_secret` ≠ known placeholder (`"change-me-proxy-secret"` — case-insensitive) **Synthetic Validation**: - `secret_key` ≥ 1 character -- `secret_key` ≠ `"secret-key"` +- `secret_key` ≠ known placeholders (`"secret-key"`, `"secret_key"`, `"trusted-server"` — case-insensitive) - `template` non-empty **Handler Validation**: @@ -1036,11 +1038,12 @@ trusted-server.dev.toml # Development overrides - Verify all required fields present - Check environment variable format -**"Secret key is not valid"**: +**"Configuration field '...' is set to a known placeholder value"**: -- Cannot use `"secret-key"` (placeholder) +- `synthetic.secret_key` cannot be `"secret-key"`, `"secret_key"`, or `"trusted-server"` (case-insensitive) +- `publisher.proxy_secret` cannot be `"change-me-proxy-secret"` (case-insensitive) - Must be non-empty -- Change to secure random value +- Change to a secure random value (see generation commands above) **"Invalid regex"**: