Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
5 changes: 0 additions & 5 deletions crates/trusted-server-core/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ 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");

Expand Down
7 changes: 0 additions & 7 deletions crates/trusted-server-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,6 @@ pub enum TrustedServerError {
#[display("GDPR consent error: {message}")]
GdprConsent { message: String },

/// 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}")]
InvalidUtf8 { message: String },
Expand Down Expand Up @@ -99,7 +93,6 @@ impl IntoHttpResponse for TrustedServerError {
Self::Configuration { .. } | Self::Settings { .. } => StatusCode::INTERNAL_SERVER_ERROR,
Self::Gam { .. } => StatusCode::BAD_GATEWAY,
Self::GdprConsent { .. } => StatusCode::BAD_REQUEST,
Self::InsecureDefault { .. } => StatusCode::INTERNAL_SERVER_ERROR,
Self::InvalidHeaderValue { .. } => StatusCode::BAD_REQUEST,
Self::InvalidUtf8 { .. } => StatusCode::BAD_REQUEST,
Self::KvStore { .. } => StatusCode::SERVICE_UNAVAILABLE,
Expand Down
112 changes: 0 additions & 112 deletions crates/trusted-server-core/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,6 @@ pub struct Publisher {
}

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
/// (case-insensitive).
#[must_use]
pub fn is_placeholder_proxy_secret(proxy_secret: &str) -> bool {
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`.
///
/// # Examples
Expand Down Expand Up @@ -203,25 +191,8 @@ pub struct Synthetic {
}

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
/// (case-insensitive).
#[must_use]
pub fn is_placeholder_secret_key(secret_key: &str) -> bool {
Self::SECRET_KEY_PLACEHOLDERS
.iter()
.any(|p| p.eq_ignore_ascii_case(secret_key))
}

/// 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.
Expand Down Expand Up @@ -418,33 +389,6 @@ 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<TrustedServerError>> {
let mut insecure_fields: Vec<&str> = Vec::new();

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.expose()) {
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
Expand Down Expand Up @@ -744,62 +688,6 @@ 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_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!(
!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_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!(
!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
121 changes: 15 additions & 106 deletions crates/trusted-server-core/src/settings_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ pub fn get_settings() -> Result<Settings, Report<TrustedServerError>> {
message: "Failed to validate configuration".to_string(),
})?;

// Reject known placeholder values for secrets that feed into cryptographic operations.
settings.reject_placeholder_secrets()?;

if !settings.proxy.certificate_check {
log::warn!(
"INSECURE: proxy.certificate_check is disabled — TLS certificates will NOT be verified"
Expand All @@ -48,110 +45,22 @@ pub fn get_settings() -> Result<Settings, Report<TrustedServerError>> {

#[cfg(test)]
mod tests {
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.
///
/// # 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 {
let original = crate_test_settings_str();
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, after_secret_key,
"should have replaced proxy_secret value"
);
result
}

#[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}"
);
}
use super::*;

#[test]
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 { 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");
}

/// 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}"
);
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.expose().is_empty());
assert!(!settings.synthetic.template.is_empty());
}
}
12 changes: 0 additions & 12 deletions docs/guide/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ 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
Expand Down Expand Up @@ -360,7 +359,6 @@ fastly kv-store create --name=opid_store
**Security**:

- Must be non-empty
- Cannot be a known placeholder: `"secret-key"`, `"secret_key"`, or `"trusted-server"` (case-insensitive)
- Rotate periodically for security
- Store securely (environment variable recommended)

Expand All @@ -374,7 +372,6 @@ openssl rand -hex 32
**Validation**: Application startup fails if:

- Empty string
- Exactly `"secret-key"`, `"secret_key"`, or `"trusted-server"` (known placeholders, case-insensitive)

#### `template`

Expand Down Expand Up @@ -923,12 +920,10 @@ 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` ≠ known placeholders (`"secret-key"`, `"secret_key"`, `"trusted-server"` — case-insensitive)
- `template` non-empty

**Handler Validation**:
Expand Down Expand Up @@ -1038,13 +1033,6 @@ trusted-server.dev.toml # Development overrides
- Verify all required fields present
- Check environment variable format

**"Configuration field '...' is set to a known placeholder value"**:

- `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 a secure random value (see generation commands above)

**"Invalid regex"**:

- Handler `path` must be valid regex
Expand Down
Loading