Fail fast on invalid config regexes and enabled config

.claude/agents/pr-creator.md

@@ -49,6 +49,7 @@ Using the `.github/pull_request_template.md` structure, draft:
 - **Changes table**: list each file modified and what changed.
 - **Closes**: `Closes #<issue-number>` to auto-close the linked issue.
 - **Test plan**: check off which verification steps were run.
+- **Hardening note**: when config-derived regex or pattern compilation is touched, state how invalid enabled config fails startup and which regression tests cover that path.
 - **Checklist**: verify each item applies.
 
 ### 5. Create the PR
@@ -172,6 +173,7 @@ Do **not** use labels as a substitute for types.
 - Use sentence case for the title.
 - Use imperative mood (e.g., "Add caching to proxy" not "Added caching").
 - The summary should focus on _why_, not just _what_.
+- Do not describe config-derived regex/pattern compilation as safe unless invalid enabled config is handled without `panic!`, `unwrap()`, or `expect()`.
 - Always base PRs against `main` unless told otherwise.
 - Always assign the PR to the current user (`--assignee @me`).
 - Never force-push or rebase without explicit user approval.

.claude/agents/pr-reviewer.md

@@ -78,6 +78,8 @@ For each changed file, evaluate:
 - `expect("should ...")` instead of `unwrap()` in production code
 - `error-stack` (`Report<E>`) with `derive_more::Display` for errors (not thiserror/anyhow)
 - `log` macros (not `println!`)
+- Config-derived regex/pattern compilation must not use panic-prone `expect()`/`unwrap()`; invalid enabled config should surface as startup/config errors
+- Invalid enabled integrations/providers must not be silently logged-and-disabled during startup or registration
 - `vi.hoisted()` for mock definitions in JS tests
 - Integration IDs match JS directory names
 - Colocated tests with `#[cfg(test)]`
@@ -105,6 +107,7 @@ For each changed file, evaluate:
 
 - Are new code paths tested?
 - Are edge cases covered (empty input, max values, error paths)?
+- If config-derived regex/pattern compilation changed: are invalid enabled-config startup failures and explicit `enabled = false` bypass cases both covered?
 - Rust tests: `cargo test --workspace`
 - JS tests: `npx vitest run` in `crates/js/lib/`
 

Cargo.toml

@@ -74,8 +74,8 @@ jose-jwk = "0.1.2"
 log = "0.4.29"
 log-fastly = "0.11.12"
 lol_html = "2.7.2"
-once_cell = "1.21"
 matchit = "0.9"
+once_cell = "1"
 pin-project-lite = "0.2"
 rand = "0.8"
 regex = "1.12.3"

crates/trusted-server-adapter-fastly/src/main.rs

@@ -49,7 +49,13 @@ fn main(req: Request) -> Result<Response, Error> {
     log::debug!("Settings {settings:?}");
 
     // Build the auction orchestrator once at startup
-    let orchestrator = build_orchestrator(&settings);
+    let orchestrator = match build_orchestrator(&settings) {
+        Ok(orchestrator) => orchestrator,
+        Err(e) => {
+            log::error!("Failed to build auction orchestrator: {:?}", e);
+            return Ok(to_error_response(&e));
+        }
+    };
 
     let integration_registry = match IntegrationRegistry::new(&settings) {
         Ok(r) => r,
@@ -81,7 +87,13 @@ async fn route_request(
     // Extract geo info before auth check or routing consumes the request
     let geo_info = GeoInfo::from_request(&req);
 
-    if let Some(mut response) = enforce_basic_auth(settings, &req) {
+    // `get_settings()` should already have rejected invalid handler regexes.
+    // Keep this fallback so manually-constructed or otherwise unprepared
+    // settings still become an error response instead of panicking.
+    if let Some(mut response) = enforce_basic_auth(settings, &req).unwrap_or_else(|e| {
+        log::error!("Failed to evaluate basic auth: {:?}", e);
+        Some(to_error_response(&e))
+    }) {
         finalize_response(settings, geo_info.as_ref(), &mut response);
         return Ok(response);
     }

crates/trusted-server-core/src/auction/mod.rs

@@ -7,6 +7,9 @@
 //! Note: Individual auction providers are located in the `integrations` module
 //! (e.g., `crate::integrations::aps`, `crate::integrations::prebid`).
 
+use error_stack::Report;
+
+use crate::error::TrustedServerError;
 use crate::settings::Settings;
 use std::sync::Arc;
 
@@ -27,7 +30,8 @@ pub use types::{
 };
 
 /// Type alias for provider builder functions.
-type ProviderBuilder = fn(&Settings) -> Vec<Arc<dyn AuctionProvider>>;
+type ProviderBuilder =
+    fn(&Settings) -> Result<Vec<Arc<dyn AuctionProvider>>, Report<TrustedServerError>>;
 
 /// Returns the list of all available provider builder functions.
 ///
@@ -49,15 +53,20 @@ fn provider_builders() -> &'static [ProviderBuilder] {
 ///
 /// # Arguments
 /// * `settings` - Application settings used to configure the orchestrator and providers
-#[must_use]
-pub fn build_orchestrator(settings: &Settings) -> AuctionOrchestrator {
+///
+/// # Errors
+///
+/// Returns an error when an enabled auction provider has invalid configuration.
+pub fn build_orchestrator(
+    settings: &Settings,
+) -> Result<AuctionOrchestrator, Report<TrustedServerError>> {
     log::info!("Building auction orchestrator");
 
     let mut orchestrator = AuctionOrchestrator::new(settings.auction.clone());
 
     // Auto-discover and register all auction providers from settings
     for builder in provider_builders() {
-        for provider in builder(settings) {
+        for provider in builder(settings)? {
             orchestrator.register_provider(provider);
         }
     }
@@ -67,5 +76,5 @@ pub fn build_orchestrator(settings: &Settings) -> AuctionOrchestrator {
         orchestrator.provider_count()
     );
 
-    orchestrator
+    Ok(orchestrator)
 }

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

@@ -1,35 +1,36 @@
 use base64::{engine::general_purpose::STANDARD, Engine as _};
+use error_stack::Report;
 use fastly::http::{header, StatusCode};
 use fastly::{Request, Response};
 use sha2::{Digest as _, Sha256};
 use subtle::ConstantTimeEq as _;
 
+use crate::error::TrustedServerError;
 use crate::settings::Settings;
 
 const BASIC_AUTH_REALM: &str = r#"Basic realm="Trusted Server""#;
 
-/// Enforces Basic-auth for incoming requests.
+/// Enforce HTTP basic auth for the matched handler, if any.
 ///
-/// Authentication is required when a configured handler's `path` regex matches
-/// the request path. Paths not covered by any handler pass through without
-/// authentication.
+/// Returns `Ok(None)` when the request does not target a protected handler or
+/// when the supplied credentials are valid. Returns `Ok(Some(Response))` with
+/// the auth challenge when credentials are missing or invalid.
 ///
-/// Admin endpoints are protected by requiring a handler at build time — see
-/// [`Settings::from_toml_and_env`].
+/// # Errors
 ///
-/// # Returns
-///
-/// * `Some(Response)` — a `401 Unauthorized` response that should be sent back
-///   to the client (credentials missing or incorrect).
-/// * `None` — the request is allowed to proceed.
-pub fn enforce_basic_auth(settings: &Settings, req: &Request) -> Option<Response> {
-    let path = req.get_path();
-
-    let handler = settings.handler_for_path(path)?;
+/// Returns an error when handler configuration is invalid, such as an
+/// un-compilable path regex.
+pub fn enforce_basic_auth(
+    settings: &Settings,
+    req: &Request,
+) -> Result<Option<Response>, Report<TrustedServerError>> {
+    let Some(handler) = settings.handler_for_path(req.get_path())? else {
+        return Ok(None);
+    };
 
     let (username, password) = match extract_credentials(req) {
         Some(credentials) => credentials,
-        None => return Some(unauthorized_response()),
+        None => return Ok(Some(unauthorized_response())),
     };
 
     // Hash before comparing to normalise lengths — `ct_eq` on raw byte slices
@@ -45,9 +46,9 @@ pub fn enforce_basic_auth(settings: &Settings, req: &Request) -> Option<Response
         .ct_eq(&Sha256::digest(password.as_bytes()));
 
     if bool::from(username_match & password_match) {
-        None
+        Ok(None)
     } else {
-        Some(unauthorized_response())
+        Ok(Some(unauthorized_response()))
     }
 }
 
@@ -90,22 +91,26 @@ mod tests {
     use base64::engine::general_purpose::STANDARD;
     use fastly::http::{header, Method};
 
-    use crate::test_support::tests::create_test_settings;
+    use crate::test_support::tests::{crate_test_settings_str, create_test_settings};
 
     #[test]
     fn no_challenge_for_non_protected_path() {
         let settings = create_test_settings();
         let req = Request::new(Method::GET, "https://example.com/open");
 
-        assert!(enforce_basic_auth(&settings, &req).is_none());
+        assert!(enforce_basic_auth(&settings, &req)
+            .expect("should evaluate auth")
+            .is_none());
     }
 
     #[test]
     fn challenge_when_missing_credentials() {
         let settings = create_test_settings();
         let req = Request::new(Method::GET, "https://example.com/secure");
 
-        let response = enforce_basic_auth(&settings, &req).expect("should challenge");
+        let response = enforce_basic_auth(&settings, &req)
+            .expect("should evaluate auth")
+            .expect("should challenge");
         assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED);
         let realm = response
             .get_header(header::WWW_AUTHENTICATE)
@@ -120,7 +125,9 @@ mod tests {
         let token = STANDARD.encode("user:pass");
         req.set_header(header::AUTHORIZATION, format!("Basic {token}"));
 
-        assert!(enforce_basic_auth(&settings, &req).is_none());
+        assert!(enforce_basic_auth(&settings, &req)
+            .expect("should evaluate auth")
+            .is_none());
     }
 
     #[test]
@@ -131,7 +138,8 @@ mod tests {
         req.set_header(header::AUTHORIZATION, format!("Basic {token}"));
 
         let response = enforce_basic_auth(&settings, &req)
-            .expect("should challenge when both username and password are wrong");
+            .expect("should evaluate auth")
+            .expect("should challenge");
         assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED);
     }
 
@@ -141,10 +149,23 @@ mod tests {
         let mut req = Request::new(Method::GET, "https://example.com/secure");
         req.set_header(header::AUTHORIZATION, "Bearer token");
 
-        let response = enforce_basic_auth(&settings, &req).expect("should challenge");
+        let response = enforce_basic_auth(&settings, &req)
+            .expect("should evaluate auth")
+            .expect("should challenge");
         assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED);
     }
 
+    #[test]
+    fn returns_error_for_invalid_handler_regex_without_panicking() {
+        let config = crate_test_settings_str().replace(r#"path = "^/secure""#, r#"path = "(""#);
+        let err = Settings::from_toml(&config).expect_err("should reject invalid handler regex");
+        assert!(
+            err.to_string()
+                .contains("Handler path regex `(` failed to compile"),
+            "should describe the invalid handler regex"
+        );
+    }
+
     #[test]
     fn allow_admin_path_with_valid_credentials() {
         let settings = create_test_settings();
@@ -153,7 +174,9 @@ mod tests {
         req.set_header(header::AUTHORIZATION, format!("Basic {token}"));
 
         assert!(
-            enforce_basic_auth(&settings, &req).is_none(),
+            enforce_basic_auth(&settings, &req)
+                .expect("should evaluate auth")
+                .is_none(),
             "should allow admin path with correct credentials"
         );
     }
@@ -166,6 +189,7 @@ mod tests {
         req.set_header(header::AUTHORIZATION, format!("Basic {token}"));
 
         let response = enforce_basic_auth(&settings, &req)
+            .expect("should evaluate auth")
             .expect("should challenge admin path with wrong credentials");
         assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED);
     }
@@ -176,6 +200,7 @@ mod tests {
         let req = Request::new(Method::POST, "https://example.com/admin/keys/rotate");
 
         let response = enforce_basic_auth(&settings, &req)
+            .expect("should evaluate auth")
             .expect("should challenge admin path with missing credentials");
         assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED);
     }
@@ -188,6 +213,7 @@ mod tests {
         req.set_header(header::AUTHORIZATION, format!("Basic {token}"));
 
         let response = enforce_basic_auth(&settings, &req)
+            .expect("should evaluate auth")
             .expect("should challenge when only username is wrong");
         assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED);
     }
@@ -200,6 +226,7 @@ mod tests {
         req.set_header(header::AUTHORIZATION, format!("Basic {token}"));
 
         let response = enforce_basic_auth(&settings, &req)
+            .expect("should evaluate auth")
             .expect("should challenge when only password is wrong");
         assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED);
     }

crates/trusted-server-core/src/integrations/adserver_mock.rs

@@ -35,6 +35,7 @@ pub struct AdServerMockConfig {
     pub enabled: bool,
 
     /// Mediation endpoint URL
+    #[validate(url)]
     pub endpoint: String,
 
     /// Timeout in milliseconds
@@ -406,8 +407,14 @@ impl AuctionProvider for AdServerMockProvider {
 // ============================================================================
 
 /// Auto-register ad server mock provider based on settings configuration.
-#[must_use]
-pub fn register_providers(settings: &Settings) -> Vec<Arc<dyn AuctionProvider>> {
+///
+/// # Errors
+///
+/// Returns an error when the ad server mock provider is enabled with invalid
+/// configuration.
+pub fn register_providers(
+    settings: &Settings,
+) -> Result<Vec<Arc<dyn AuctionProvider>>, Report<TrustedServerError>> {
     let mut providers: Vec<Arc<dyn AuctionProvider>> = Vec::new();
 
     match settings.integration_config::<AdServerMockConfig>("adserver_mock") {
@@ -422,11 +429,11 @@ pub fn register_providers(settings: &Settings) -> Vec<Arc<dyn AuctionProvider>>
             log::debug!("AdServer Mock config found but is disabled");
         }
         Err(e) => {
-            log::error!("Failed to load AdServer Mock config: {:?}", e);
+            return Err(e);
         }
     }
 
-    providers
+    Ok(providers)
 }
 
 // ============================================================================

crates/trusted-server-core/src/integrations/aps.rs

@@ -197,6 +197,7 @@ pub struct ApsConfig {
 
     /// APS API endpoint
     #[serde(default = "default_endpoint")]
+    #[validate(url)]
     pub endpoint: String,
 
     /// Timeout in milliseconds
@@ -597,8 +598,14 @@ use std::sync::Arc;
 /// Auto-register APS provider based on settings configuration.
 ///
 /// Returns the APS provider if enabled in settings.
-#[must_use]
-pub fn register_providers(settings: &Settings) -> Vec<Arc<dyn AuctionProvider>> {
+///
+/// # Errors
+///
+/// Returns an error when the APS provider is enabled with invalid
+/// configuration.
+pub fn register_providers(
+    settings: &Settings,
+) -> Result<Vec<Arc<dyn AuctionProvider>>, Report<TrustedServerError>> {
     let mut providers: Vec<Arc<dyn AuctionProvider>> = Vec::new();
 
     // Check for real APS provider configuration
@@ -615,11 +622,11 @@ pub fn register_providers(settings: &Settings) -> Vec<Arc<dyn AuctionProvider>>
             log::debug!("APS integration config found but is disabled");
         }
         Err(e) => {
-            log::error!("Failed to load APS configuration: {:?}", e);
+            return Err(e);
         }
     }
 
-    providers
+    Ok(providers)
 }
 
 // ============================================================================