Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion crates/common/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
11 changes: 6 additions & 5 deletions crates/common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}")]
Expand Down Expand Up @@ -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,
Expand Down
69 changes: 58 additions & 11 deletions crates/common/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 thinking — Placeholder matching is case-sensitive. Someone setting secret_key = "Secret-Key" bypasses the check while still using a predictable value. Consider case-insensitive matching:

pub fn is_placeholder_secret_key(secret_key: &str) -> bool {
    Self::SECRET_KEY_PLACEHOLDERS
        .iter()
        .any(|p| p.eq_ignore_ascii_case(secret_key))
}

}

Expand Down Expand Up @@ -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, "");

Expand All @@ -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 = "";
Expand Down
39 changes: 22 additions & 17 deletions crates/common/src/settings_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -34,8 +34,17 @@ pub fn get_settings() -> Result<Settings, Report<TrustedServerError>> {
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 {
Expand All @@ -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}"
);
}
}
Loading