Skip to content

Commit 2e706ad

Browse files
Remove runtime secret placeholder validation that breaks deployments
The runtime placeholder checks introduced in #467 reject known placeholder values for synthetic.secret_key and publisher.proxy_secret at startup. The expanded placeholder list and case-insensitive matching now catches deployments that were previously working. Remove the runtime validation, the InsecureDefault error variant, and all associated tests and docs while keeping the existing non-empty validation for secret_key.
1 parent 5817deb commit 2e706ad

5 files changed

Lines changed: 15 additions & 242 deletions

File tree

crates/trusted-server-core/build.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,6 @@ fn main() {
3838

3939
// Merge base TOML with environment variable overrides and write output.
4040
// Panics if admin endpoints are not covered by a handler.
41-
// Note: placeholder secret rejection is intentionally NOT done here.
42-
// The base trusted-server.toml ships with placeholder secrets that
43-
// production deployments override via TRUSTED_SERVER__* env vars at
44-
// build time. Runtime startup (get_settings) rejects any remaining
45-
// placeholders so a misconfigured deployment fails fast.
4641
let settings = settings::Settings::from_toml_and_env(&toml_content)
4742
.expect("Failed to parse settings at build time");
4843

crates/trusted-server-core/src/error.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,6 @@ pub enum TrustedServerError {
3333
#[display("GDPR consent error: {message}")]
3434
GdprConsent { message: String },
3535

36-
/// A configuration secret is still set to a known placeholder value.
37-
#[display(
38-
"Configuration field '{field}' is set to a known placeholder value - this is insecure"
39-
)]
40-
InsecureDefault { field: String },
41-
4236
/// Invalid UTF-8 data encountered.
4337
#[display("Invalid UTF-8 data: {message}")]
4438
InvalidUtf8 { message: String },
@@ -99,7 +93,6 @@ impl IntoHttpResponse for TrustedServerError {
9993
Self::Configuration { .. } | Self::Settings { .. } => StatusCode::INTERNAL_SERVER_ERROR,
10094
Self::Gam { .. } => StatusCode::BAD_GATEWAY,
10195
Self::GdprConsent { .. } => StatusCode::BAD_REQUEST,
102-
Self::InsecureDefault { .. } => StatusCode::INTERNAL_SERVER_ERROR,
10396
Self::InvalidHeaderValue { .. } => StatusCode::BAD_REQUEST,
10497
Self::InvalidUtf8 { .. } => StatusCode::BAD_REQUEST,
10598
Self::KvStore { .. } => StatusCode::SERVICE_UNAVAILABLE,

crates/trusted-server-core/src/settings.rs

Lines changed: 0 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,6 @@ pub struct Publisher {
3030
}
3131

3232
impl Publisher {
33-
/// Known placeholder values that must not be used in production.
34-
pub const PROXY_SECRET_PLACEHOLDERS: &[&str] = &["change-me-proxy-secret"];
35-
36-
/// Returns `true` if `proxy_secret` matches a known placeholder value
37-
/// (case-insensitive).
38-
#[must_use]
39-
pub fn is_placeholder_proxy_secret(proxy_secret: &str) -> bool {
40-
Self::PROXY_SECRET_PLACEHOLDERS
41-
.iter()
42-
.any(|p| p.eq_ignore_ascii_case(proxy_secret))
43-
}
44-
4533
/// Extracts the host (including port if present) from the `origin_url`.
4634
///
4735
/// # Examples
@@ -203,25 +191,8 @@ pub struct Synthetic {
203191
}
204192

205193
impl Synthetic {
206-
/// Known placeholder values that must not be used in production.
207-
pub const SECRET_KEY_PLACEHOLDERS: &[&str] = &["secret-key", "secret_key", "trusted-server"];
208-
209-
/// Returns `true` if `secret_key` matches a known placeholder value
210-
/// (case-insensitive).
211-
#[must_use]
212-
pub fn is_placeholder_secret_key(secret_key: &str) -> bool {
213-
Self::SECRET_KEY_PLACEHOLDERS
214-
.iter()
215-
.any(|p| p.eq_ignore_ascii_case(secret_key))
216-
}
217-
218194
/// Validates that the secret key is not empty.
219195
///
220-
/// Placeholder detection is intentionally **not** performed here because
221-
/// this validator runs at build time (via `from_toml_and_env`) when the
222-
/// config legitimately contains placeholder values. Placeholder rejection
223-
/// happens at runtime via [`Settings::reject_placeholder_secrets`].
224-
///
225196
/// # Errors
226197
///
227198
/// Returns a validation error if the secret key is empty.
@@ -418,33 +389,6 @@ impl Settings {
418389
Ok(settings)
419390
}
420391

421-
/// Checks all secret fields for known placeholder values and returns an
422-
/// error listing every offending field. This centralises the placeholder
423-
/// policy so callers don't need to know which fields are secrets.
424-
///
425-
/// # Errors
426-
///
427-
/// Returns [`TrustedServerError::InsecureDefault`] when one or more secret
428-
/// fields still contain a placeholder value.
429-
pub fn reject_placeholder_secrets(&self) -> Result<(), Report<TrustedServerError>> {
430-
let mut insecure_fields: Vec<&str> = Vec::new();
431-
432-
if Synthetic::is_placeholder_secret_key(self.synthetic.secret_key.expose()) {
433-
insecure_fields.push("synthetic.secret_key");
434-
}
435-
if Publisher::is_placeholder_proxy_secret(self.publisher.proxy_secret.expose()) {
436-
insecure_fields.push("publisher.proxy_secret");
437-
}
438-
439-
if insecure_fields.is_empty() {
440-
Ok(())
441-
} else {
442-
Err(Report::new(TrustedServerError::InsecureDefault {
443-
field: insecure_fields.join(", "),
444-
}))
445-
}
446-
}
447-
448392
#[must_use]
449393
pub fn handler_for_path(&self, path: &str) -> Option<&Handler> {
450394
self.handlers
@@ -744,62 +688,6 @@ mod tests {
744688
);
745689
}
746690

747-
#[test]
748-
fn is_placeholder_secret_key_rejects_all_known_placeholders() {
749-
for placeholder in Synthetic::SECRET_KEY_PLACEHOLDERS {
750-
assert!(
751-
Synthetic::is_placeholder_secret_key(placeholder),
752-
"should detect placeholder secret_key '{placeholder}'"
753-
);
754-
}
755-
}
756-
757-
#[test]
758-
fn is_placeholder_secret_key_is_case_insensitive() {
759-
assert!(
760-
Synthetic::is_placeholder_secret_key("SECRET-KEY"),
761-
"should detect case-insensitive placeholder secret_key"
762-
);
763-
assert!(
764-
Synthetic::is_placeholder_secret_key("Trusted-Server"),
765-
"should detect mixed-case placeholder secret_key"
766-
);
767-
}
768-
769-
#[test]
770-
fn is_placeholder_secret_key_accepts_non_placeholder() {
771-
assert!(
772-
!Synthetic::is_placeholder_secret_key("test-secret-key"),
773-
"should accept non-placeholder secret_key"
774-
);
775-
}
776-
777-
#[test]
778-
fn is_placeholder_proxy_secret_rejects_all_known_placeholders() {
779-
for placeholder in Publisher::PROXY_SECRET_PLACEHOLDERS {
780-
assert!(
781-
Publisher::is_placeholder_proxy_secret(placeholder),
782-
"should detect placeholder proxy_secret '{placeholder}'"
783-
);
784-
}
785-
}
786-
787-
#[test]
788-
fn is_placeholder_proxy_secret_is_case_insensitive() {
789-
assert!(
790-
Publisher::is_placeholder_proxy_secret("CHANGE-ME-PROXY-SECRET"),
791-
"should detect case-insensitive placeholder proxy_secret"
792-
);
793-
}
794-
795-
#[test]
796-
fn is_placeholder_proxy_secret_accepts_non_placeholder() {
797-
assert!(
798-
!Publisher::is_placeholder_proxy_secret("unit-test-proxy-secret"),
799-
"should accept non-placeholder proxy_secret"
800-
);
801-
}
802-
803691
#[test]
804692
fn test_settings_empty_toml() {
805693
let toml_str = "";

crates/trusted-server-core/src/settings_data.rs

Lines changed: 15 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@ pub fn get_settings() -> Result<Settings, Report<TrustedServerError>> {
3434
message: "Failed to validate configuration".to_string(),
3535
})?;
3636

37-
// Reject known placeholder values for secrets that feed into cryptographic operations.
38-
settings.reject_placeholder_secrets()?;
39-
4037
if !settings.proxy.certificate_check {
4138
log::warn!(
4239
"INSECURE: proxy.certificate_check is disabled — TLS certificates will NOT be verified"
@@ -48,110 +45,22 @@ pub fn get_settings() -> Result<Settings, Report<TrustedServerError>> {
4845

4946
#[cfg(test)]
5047
mod tests {
51-
use crate::error::TrustedServerError;
52-
use crate::settings::Settings;
53-
use crate::test_support::tests::crate_test_settings_str;
54-
55-
/// Builds a TOML string with the given secret values swapped in.
56-
///
57-
/// # Panics
58-
///
59-
/// Panics if the replacement patterns no longer match the test TOML,
60-
/// which would cause the substitution to silently no-op.
61-
fn toml_with_secrets(secret_key: &str, proxy_secret: &str) -> String {
62-
let original = crate_test_settings_str();
63-
let after_secret_key = original.replace(
64-
r#"secret_key = "test-secret-key""#,
65-
&format!(r#"secret_key = "{secret_key}""#),
66-
);
67-
assert_ne!(
68-
after_secret_key, original,
69-
"should have replaced secret_key value"
70-
);
71-
let result = after_secret_key.replace(
72-
r#"proxy_secret = "unit-test-proxy-secret""#,
73-
&format!(r#"proxy_secret = "{proxy_secret}""#),
74-
);
75-
assert_ne!(
76-
result, after_secret_key,
77-
"should have replaced proxy_secret value"
78-
);
79-
result
80-
}
81-
82-
#[test]
83-
fn rejects_placeholder_secret_key() {
84-
let toml = toml_with_secrets("secret-key", "real-proxy-secret");
85-
let settings = Settings::from_toml(&toml).expect("should parse TOML");
86-
let err = settings
87-
.reject_placeholder_secrets()
88-
.expect_err("should reject placeholder secret_key");
89-
let root = err.current_context();
90-
assert!(
91-
matches!(root, TrustedServerError::InsecureDefault { field } if field.contains("synthetic.secret_key")),
92-
"error should mention synthetic.secret_key, got: {root}"
93-
);
94-
}
48+
use super::*;
9549

9650
#[test]
97-
fn rejects_placeholder_proxy_secret() {
98-
let toml = toml_with_secrets("real-secret-key", "change-me-proxy-secret");
99-
let settings = Settings::from_toml(&toml).expect("should parse TOML");
100-
let err = settings
101-
.reject_placeholder_secrets()
102-
.expect_err("should reject placeholder proxy_secret");
103-
let root = err.current_context();
104-
assert!(
105-
matches!(root, TrustedServerError::InsecureDefault { field } if field.contains("publisher.proxy_secret")),
106-
"error should mention publisher.proxy_secret, got: {root}"
107-
);
108-
}
109-
110-
#[test]
111-
fn rejects_both_placeholders_in_single_error() {
112-
let toml = toml_with_secrets("secret_key", "change-me-proxy-secret");
113-
let settings = Settings::from_toml(&toml).expect("should parse TOML");
114-
let err = settings
115-
.reject_placeholder_secrets()
116-
.expect_err("should reject both placeholder secrets");
117-
let root = err.current_context();
118-
match root {
119-
TrustedServerError::InsecureDefault { field } => {
120-
assert!(
121-
field.contains("synthetic.secret_key"),
122-
"error should mention synthetic.secret_key, got: {field}"
123-
);
124-
assert!(
125-
field.contains("publisher.proxy_secret"),
126-
"error should mention publisher.proxy_secret, got: {field}"
127-
);
128-
}
129-
other => panic!("expected InsecureDefault, got: {other}"),
130-
}
131-
}
132-
133-
#[test]
134-
fn accepts_non_placeholder_secrets() {
135-
let toml = toml_with_secrets("production-secret-key", "production-proxy-secret");
136-
let settings = Settings::from_toml(&toml).expect("should parse TOML");
137-
settings
138-
.reject_placeholder_secrets()
139-
.expect("non-placeholder secrets should pass validation");
140-
}
141-
142-
/// Smoke-test the full `get_settings()` pipeline (embedded bytes → UTF-8 →
143-
/// parse → validate → placeholder check). The build-time TOML ships with
144-
/// placeholder secrets, so the expected outcome is an [`InsecureDefault`]
145-
/// error — but reaching that error proves every earlier stage succeeded.
146-
#[test]
147-
fn get_settings_rejects_embedded_placeholder_secrets() {
148-
let err = super::get_settings().expect_err("should reject embedded placeholder secrets");
149-
assert!(
150-
matches!(
151-
err.current_context(),
152-
TrustedServerError::InsecureDefault { .. }
153-
),
154-
"should fail with InsecureDefault, got: {err}"
155-
);
51+
fn test_get_settings() {
52+
// Test that Settings::new() loads successfully
53+
let settings = get_settings();
54+
assert!(settings.is_ok(), "Settings should load from embedded TOML");
55+
56+
let settings = settings.expect("should load settings from embedded TOML");
57+
// Verify basic structure is loaded
58+
assert!(!settings.publisher.domain.is_empty());
59+
assert!(!settings.publisher.cookie_domain.is_empty());
60+
assert!(!settings.publisher.origin_url.is_empty());
61+
assert!(!settings.synthetic.counter_store.is_empty());
62+
assert!(!settings.synthetic.opid_store.is_empty());
63+
assert!(!settings.synthetic.secret_key.expose().is_empty());
64+
assert!(!settings.synthetic.template.is_empty());
15665
}
15766
}

docs/guide/configuration.md

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,6 @@ TRUSTED_SERVER__PUBLISHER__PROXY_SECRET=your-secret-here
244244
**Security**:
245245

246246
- Keep confidential and secure
247-
- Cannot be the placeholder `"change-me-proxy-secret"` (case-insensitive) — startup will fail
248247
- Rotate periodically (90 days recommended)
249248
- Use cryptographically random values (32+ bytes)
250249
- Never commit to version control
@@ -360,7 +359,6 @@ fastly kv-store create --name=opid_store
360359
**Security**:
361360

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

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

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

379376
#### `template`
380377

@@ -923,12 +920,10 @@ Configuration is validated at startup:
923920

924921
- All fields non-empty
925922
- `origin_url` is valid URL
926-
- `proxy_secret` ≠ known placeholder (`"change-me-proxy-secret"` — case-insensitive)
927923

928924
**Synthetic Validation**:
929925

930926
- `secret_key` ≥ 1 character
931-
- `secret_key` ≠ known placeholders (`"secret-key"`, `"secret_key"`, `"trusted-server"` — case-insensitive)
932927
- `template` non-empty
933928

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

1041-
**"Configuration field '...' is set to a known placeholder value"**:
1042-
1043-
- `synthetic.secret_key` cannot be `"secret-key"`, `"secret_key"`, or `"trusted-server"` (case-insensitive)
1044-
- `publisher.proxy_secret` cannot be `"change-me-proxy-secret"` (case-insensitive)
1045-
- Must be non-empty
1046-
- Change to a secure random value (see generation commands above)
1047-
10481036
**"Invalid regex"**:
10491037

10501038
- Handler `path` must be valid regex

0 commit comments

Comments
 (0)