fix(config): fail startup on malformed numeric env overrides (#277)

nodedb/src/config/server/env/cluster.rs

@@ -118,6 +118,7 @@ mod tests {
 
     #[test]
     fn env_cluster_overrides() {
+        let _env_guard = super::super::test_support::env_lock().lock().unwrap();
         // Always start clean.
         unsafe {
             std::env::remove_var("NODEDB_NODE_ID");
@@ -131,7 +132,7 @@ mod tests {
             cluster: Some(make_cluster(1)),
             ..Default::default()
         };
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         assert_eq!(
             cfg.cluster.as_ref().unwrap().node_id,
             42,
@@ -143,7 +144,7 @@ mod tests {
 
         unsafe { std::env::set_var("NODEDB_NODE_ID", "99") };
         let mut cfg = ServerConfig::default();
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         assert!(
             cfg.cluster.is_none(),
             "NODEDB_NODE_ID with no [cluster] section must not create cluster"
@@ -157,7 +158,7 @@ mod tests {
             cluster: Some(make_cluster(7)),
             ..Default::default()
         };
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         assert_eq!(
             cfg.cluster.as_ref().unwrap().node_id,
             7,
@@ -172,7 +173,7 @@ mod tests {
             cluster: Some(make_cluster(1)),
             ..Default::default()
         };
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         let seeds = &cfg.cluster.as_ref().unwrap().seed_nodes;
         assert_eq!(seeds.len(), 2, "two seed addresses should be applied");
         assert_eq!(seeds[0].to_string(), "10.0.0.1:9400");
@@ -190,7 +191,7 @@ mod tests {
             }),
             ..Default::default()
         };
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         let seeds = &cfg.cluster.as_ref().unwrap().seed_nodes;
         assert_eq!(
             seeds.len(),

nodedb/src/config/server/env/dispatch.rs

@@ -55,13 +55,14 @@ use crate::config::server::ServerConfig;
 /// handled separately by `crate::config::auth::AuthConfig::resolve_superuser_password()`
 /// (called from `main.rs`) so that the value is never passed through logging
 /// code paths or stored in `ServerConfig` where it could appear in debug output.
-pub fn apply_env_overrides(config: &mut ServerConfig) {
+pub fn apply_env_overrides(config: &mut ServerConfig) -> crate::Result<()> {
     apply_host_and_ports(config);
     apply_cluster_overrides(config);
-    apply_numeric_settings(config);
+    apply_numeric_settings(config)?;
     apply_tls_overrides(config);
     apply_wal_tuning(config);
     apply_checkpoint_tuning(config);
     apply_timeseries_overrides(config);
     super::super::observability::apply_observability_env(&mut config.observability);
+    Ok(())
 }

nodedb/src/config/server/env/host_ports.rs

@@ -91,9 +91,10 @@ mod tests {
 
     #[test]
     fn env_data_dir_override() {
+        let _env_guard = super::super::test_support::env_lock().lock().unwrap();
         unsafe { std::env::set_var("NODEDB_DATA_DIR", "/tmp/test-nodedb") };
         let mut cfg = ServerConfig::default();
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         assert_eq!(
             cfg.server.data_dir,
             std::path::PathBuf::from("/tmp/test-nodedb")
@@ -105,17 +106,18 @@ mod tests {
     /// env-var races (env vars are process-global, Rust tests run in parallel).
     #[test]
     fn env_memory_limit_overrides() {
+        let _env_guard = super::super::test_support::env_lock().lock().unwrap();
         // ── Valid value → overrides memory_limit ──
         unsafe { std::env::set_var("NODEDB_MEMORY_LIMIT", "2GiB") };
         let mut cfg = ServerConfig::default();
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         assert_eq!(cfg.server.memory_limit, 2 * 1024 * 1024 * 1024);
 
         // ── Malformed value → memory_limit unchanged ──
         unsafe { std::env::set_var("NODEDB_MEMORY_LIMIT", "notanumber") };
         let mut cfg = ServerConfig::default();
         let before = cfg.server.memory_limit;
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         assert_eq!(
             cfg.server.memory_limit, before,
             "malformed value must not change config"
@@ -128,17 +130,18 @@ mod tests {
     /// env-var races (env vars are process-global, Rust tests run in parallel).
     #[test]
     fn env_sync_port_overrides() {
+        let _env_guard = super::super::test_support::env_lock().lock().unwrap();
         // ── Valid value → overrides ports.sync ──
         unsafe { std::env::set_var("NODEDB_PORT_SYNC", "19090") };
         let mut cfg = ServerConfig::default();
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         assert_eq!(cfg.server.ports.sync, 19090);
 
         // ── Malformed value → ports.sync unchanged ──
         unsafe { std::env::set_var("NODEDB_PORT_SYNC", "notaport") };
         let mut cfg = ServerConfig::default();
         let before = cfg.server.ports.sync;
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         assert_eq!(
             cfg.server.ports.sync, before,
             "malformed value must not change config"

nodedb/src/config/server/env/mod.rs

@@ -18,3 +18,16 @@ mod wal;
 pub use cluster::parse_seed_nodes;
 pub use dispatch::apply_env_overrides;
 pub use memory_size::parse_memory_size;
+
+/// Serialize tests that read/write process env vars. Parallel cargo test
+/// threads otherwise race on `std::env` (e.g. a strict-numeric test setting
+/// NODEDB_DATA_PLANE_CORES while a host_ports test reads it).
+#[cfg(test)]
+pub(crate) mod test_support {
+    use std::sync::{Mutex, OnceLock};
+
+    pub(crate) fn env_lock() -> &'static Mutex<()> {
+        static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
+        LOCK.get_or_init(|| Mutex::new(()))
+    }
+}

nodedb/src/config/server/env/numeric.rs

@@ -5,47 +5,49 @@
 
 use crate::config::server::{LogFormat, ServerConfig};
 
-pub(super) fn apply_numeric_settings(config: &mut ServerConfig) {
-    if let Ok(val) = std::env::var("NODEDB_DATA_PLANE_CORES") {
-        match val.trim().parse::<usize>() {
-            Ok(cores) => {
-                tracing::info!(
-                    env_var = "NODEDB_DATA_PLANE_CORES",
-                    value = cores,
-                    "environment variable override applied"
-                );
-                config.server.data_plane_cores = cores;
-            }
-            Err(_) => {
-                tracing::warn!(
-                    env_var = "NODEDB_DATA_PLANE_CORES",
-                    value = %val,
-                    "ignoring malformed environment variable (expected usize), using config value"
-                );
-            }
+/// Parse a positive-or-nonzero usize env override, failing boot on malformed
+/// input instead of silently keeping the compiled default (issue #277).
+fn apply_positive_usize(var: &str, target: &mut usize, allow_zero: bool) -> crate::Result<()> {
+    if let Ok(val) = std::env::var(var) {
+        let parsed = val
+            .trim()
+            .parse::<usize>()
+            .map_err(|_| crate::Error::Config {
+                detail: format!("invalid value '{val}' for {var}: expected positive integer"),
+            })?;
+        if !allow_zero && parsed == 0 {
+            return Err(crate::Error::Config {
+                detail: format!("{var} must be greater than zero"),
+            });
         }
+        tracing::info!(
+            env_var = var,
+            value = parsed,
+            "environment variable override applied"
+        );
+        *target = parsed;
     }
+    Ok(())
+}
 
-    if let Ok(val) = std::env::var("NODEDB_MAX_CONNECTIONS") {
-        match val.trim().parse::<usize>() {
-            Ok(n) => {
-                tracing::info!(
-                    env_var = "NODEDB_MAX_CONNECTIONS",
-                    value = n,
-                    "environment variable override applied"
-                );
-                config.server.max_connections = n;
-            }
-            Err(_) => {
-                tracing::warn!(
-                    env_var = "NODEDB_MAX_CONNECTIONS",
-                    value = %val,
-                    "ignoring malformed environment variable (expected usize), using config value"
-                );
-            }
-        }
-    }
+pub(super) fn apply_numeric_settings(config: &mut ServerConfig) -> crate::Result<()> {
+    apply_positive_usize(
+        "NODEDB_DATA_PLANE_CORES",
+        &mut config.server.data_plane_cores,
+        false,
+    )?;
+    apply_positive_usize(
+        "NODEDB_MAX_CONNECTIONS",
+        &mut config.server.max_connections,
+        false,
+    )?;
+    apply_log_format_override(config);
+    Ok(())
+}
 
+/// NODEDB_LOG_FORMAT is a string-valued override. Malformed values stay a
+/// warning (non-numeric path, out of C3 scope).
+fn apply_log_format_override(config: &mut ServerConfig) {
     if let Ok(val) = std::env::var("NODEDB_LOG_FORMAT") {
         let normalised = val.trim().to_lowercase();
         match normalised.as_str() {
@@ -75,3 +77,44 @@ pub(super) fn apply_numeric_settings(config: &mut ServerConfig) {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn c3_malformed_env_errors_instead_of_silent_default() {
+        let _env_guard = super::super::test_support::env_lock().lock().unwrap();
+        unsafe { std::env::set_var("NODEDB_DATA_PLANE_CORES", "abc") };
+        let mut cfg = ServerConfig::default();
+        let err = apply_numeric_settings(&mut cfg).unwrap_err();
+        assert!(
+            err.to_string().contains("NODEDB_DATA_PLANE_CORES"),
+            "C3: error must name the env var, got: {err}"
+        );
+        unsafe { std::env::remove_var("NODEDB_DATA_PLANE_CORES") };
+    }
+
+    #[test]
+    fn c3_valid_env_still_overrides() {
+        let _env_guard = super::super::test_support::env_lock().lock().unwrap();
+        unsafe { std::env::set_var("NODEDB_DATA_PLANE_CORES", "8") };
+        let mut cfg = ServerConfig::default();
+        apply_numeric_settings(&mut cfg).unwrap();
+        assert_eq!(cfg.server.data_plane_cores, 8);
+        unsafe { std::env::remove_var("NODEDB_DATA_PLANE_CORES") };
+    }
+
+    #[test]
+    fn c3_zero_value_rejected() {
+        let _env_guard = super::super::test_support::env_lock().lock().unwrap();
+        unsafe { std::env::set_var("NODEDB_DATA_PLANE_CORES", "0") };
+        let mut cfg = ServerConfig::default();
+        let err = apply_numeric_settings(&mut cfg).unwrap_err();
+        assert!(
+            err.to_string().contains("greater than zero"),
+            "C3: zero cores must be rejected, got: {err}"
+        );
+        unsafe { std::env::remove_var("NODEDB_DATA_PLANE_CORES") };
+    }
+}

nodedb/src/config/server/env/wal.rs

@@ -50,18 +50,19 @@ mod tests {
     /// it off — an absent or malformed env var must never be read as one.
     #[test]
     fn env_wal_direct_io_override() {
+        let _env_guard = super::super::test_support::env_lock().lock().unwrap();
         let mut cfg = ServerConfig::default();
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         assert!(cfg.tuning.wal.direct_io, "default must be direct I/O");
 
         unsafe { std::env::set_var("NODEDB_WAL_DIRECT_IO", "false") };
         let mut cfg = ServerConfig::default();
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         assert!(!cfg.tuning.wal.direct_io);
 
         unsafe { std::env::set_var("NODEDB_WAL_DIRECT_IO", "nonsense") };
         let mut cfg = ServerConfig::default();
-        apply_env_overrides(&mut cfg);
+        apply_env_overrides(&mut cfg).unwrap();
         assert!(
             cfg.tuning.wal.direct_io,
             "a malformed value must not silently disable direct I/O"

nodedb/src/main.rs

@@ -96,7 +96,7 @@ async fn server_main() -> anyhow::Result<()> {
     // correct in case NODEDB_DATA_DIR / NODEDB_MEMORY_LIMIT also affect it.
     // The overrides are re-applied silently here; the real log messages
     // will be emitted by the second call after the subscriber is registered.
-    apply_env_overrides(&mut config);
+    apply_env_overrides(&mut config)?;
 
     // Own the black-box recorder before the subscriber is built: the panic hook
     // it installs chains in front of the one above, and the reports directory
@@ -126,7 +126,7 @@ async fn server_main() -> anyhow::Result<()> {
 
     // Re-apply env overrides after tracing initializes so that
     // info!/warn! messages are actually emitted for operators.
-    apply_env_overrides(&mut config);
+    apply_env_overrides(&mut config)?;
 
     let cluster_mode_str = startup_log::log_boot_banner(&config_path, &config);