From 1f9247092c39b391b3201a666df444ba949ce447 Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sat, 14 Feb 2026 23:51:57 +0100 Subject: [PATCH 01/21] fix: replace UUID v4 key generation with direct CSPRNG Use ChaCha20Poly1305::generate_key(&mut OsRng) to generate encryption keys directly from the OS CSPRNG, providing full 256-bit entropy without the fixed version/variant bits that UUID v4 introduces (6 fixed bits per 128-bit UUID = only 244 effective bits from two UUIDs). Closes #54 Co-Authored-By: Claude Opus 4.6 --- src/security/secrets.rs | 44 +++++++++++++++++++++++++++++++---------- src/tools/browser.rs | 1 + 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index bafad38..cd93ede 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -217,16 +217,12 @@ fn xor_cipher(data: &[u8], key: &[u8]) -> Vec { .collect() } -/// Generate a random key using system entropy (UUID v4 + process ID + timestamp). +/// Generate a random 256-bit key using the OS CSPRNG. +/// +/// Uses `OsRng` (via `getrandom`) directly, providing full 256-bit entropy +/// without the fixed version/variant bits that UUID v4 introduces. fn generate_random_key() -> Vec { - // Use two UUIDs (32 random bytes) as our key material - let u1 = uuid::Uuid::new_v4(); - let u2 = uuid::Uuid::new_v4(); - let mut key = Vec::with_capacity(KEY_LEN); - key.extend_from_slice(u1.as_bytes()); - key.extend_from_slice(u2.as_bytes()); - key.truncate(KEY_LEN); - key + ChaCha20Poly1305::generate_key(&mut OsRng).to_vec() } /// Hex-encode bytes to a lowercase hex string. @@ -241,7 +237,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. fn hex_decode(hex: &str) -> Result> { - if hex.len() % 2 != 0 { + if !hex.len().is_multiple_of(2) { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) @@ -751,6 +747,34 @@ mod tests { assert_ne!(k1, k2, "Two random keys should differ"); } + #[test] + fn generate_random_key_has_no_uuid_fixed_bits() { + // UUID v4 has fixed bits at positions 6 (version = 0b0100xxxx) and + // 8 (variant = 0b10xxxxxx). A direct CSPRNG key should not consistently + // have these patterns across multiple samples. + let mut version_match = 0; + let mut variant_match = 0; + let samples = 100; + for _ in 0..samples { + let key = generate_random_key(); + // In UUID v4, byte 6 always has top nibble = 0x4 + if key[6] & 0xf0 == 0x40 { + version_match += 1; + } + // In UUID v4, byte 8 always has top 2 bits = 0b10 + if key[8] & 0xc0 == 0x80 { + variant_match += 1; + } + } + // With true randomness, each pattern should appear ~1/16 and ~1/4 of + // the time. UUID would hit 100/100 on both. Allow generous margin. + assert!( + version_match < 30, + "byte[6] matched UUID v4 version nibble {version_match}/100 times — \ + likely still using UUID-based key generation" + ); + } + #[cfg(unix)] #[test] fn key_file_has_restricted_permissions() { diff --git a/src/tools/browser.rs b/src/tools/browser.rs index 5ee9505..25be13c 100644 --- a/src/tools/browser.rs +++ b/src/tools/browser.rs @@ -366,6 +366,7 @@ impl BrowserTool { } #[async_trait] +#[allow(clippy::too_many_lines)] impl Tool for BrowserTool { fn name(&self) -> &str { "browser" From 2942e5607d1270c0b862d32e8b10a4e1784e1a5e Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sat, 14 Feb 2026 23:59:36 +0100 Subject: [PATCH 02/21] fix: log warning when Windows key file permissions fail to set Replace silently discarded icacls result with proper error handling that logs a tracing::warn! on failure. Previously, if icacls failed (binary not found, permission denied), the key file would remain world-readable on Windows with no indication of the problem. Closes #56 Co-Authored-By: Claude Opus 4.6 --- src/security/secrets.rs | 18 +++++++++++++++--- src/tools/browser.rs | 1 + 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index bafad38..c845bd9 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -191,14 +191,26 @@ impl SecretStore { #[cfg(windows)] { // On Windows, use icacls to restrict permissions to current user only - let _ = std::process::Command::new("icacls") + match std::process::Command::new("icacls") .arg(&self.key_path) .args(["/inheritance:r", "/grant:r"]) .arg(format!( "{}:F", std::env::var("USERNAME").unwrap_or_default() )) - .output(); + .output() + { + Ok(o) if !o.status.success() => { + tracing::warn!( + "Failed to set key file permissions via icacls (exit code {:?})", + o.status.code() + ); + } + Err(e) => { + tracing::warn!("Could not set key file permissions: {e}"); + } + _ => {} + } } Ok(key) @@ -241,7 +253,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. fn hex_decode(hex: &str) -> Result> { - if hex.len() % 2 != 0 { + if !hex.len().is_multiple_of(2) { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) diff --git a/src/tools/browser.rs b/src/tools/browser.rs index 5ee9505..25be13c 100644 --- a/src/tools/browser.rs +++ b/src/tools/browser.rs @@ -366,6 +366,7 @@ impl BrowserTool { } #[async_trait] +#[allow(clippy::too_many_lines)] impl Tool for BrowserTool { fn name(&self) -> &str { "browser" From 6776373e8ef8db23eae68f779b6b6bac4b1888bf Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 00:01:23 +0100 Subject: [PATCH 03/21] fix: constant_time_eq no longer leaks secret length via early return Remove the early return on length mismatch that leaked length information via timing. Now iterates over max(a.len(), b.len()), padding the shorter input with zeros, and checks both byte-level differences and length equality at the end. Closes #57 Co-Authored-By: Claude Opus 4.6 --- src/security/pairing.rs | 26 +++++++++++++++++++------- src/security/secrets.rs | 2 +- src/tools/browser.rs | 1 + 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/security/pairing.rs b/src/security/pairing.rs index 5f55603..f9a9a05 100644 --- a/src/security/pairing.rs +++ b/src/security/pairing.rs @@ -174,15 +174,27 @@ fn generate_token() -> String { format!("zc_{}", uuid::Uuid::new_v4().as_simple()) } -/// Constant-time string comparison to prevent timing attacks on pairing code. +/// Constant-time string comparison to prevent timing attacks. +/// +/// Does not short-circuit on length mismatch — always iterates over the +/// longer input to avoid leaking length information via timing. pub fn constant_time_eq(a: &str, b: &str) -> bool { - if a.len() != b.len() { - return false; + let a = a.as_bytes(); + let b = b.as_bytes(); + + // Track length mismatch as a usize (non-zero = different lengths) + let len_diff = a.len() ^ b.len(); + + // XOR each byte, padding the shorter input with zeros. + // Iterates over max(a.len(), b.len()) to avoid timing differences. + let max_len = a.len().max(b.len()); + let mut byte_diff = 0u8; + for i in 0..max_len { + let x = if i < a.len() { a[i] } else { 0 }; + let y = if i < b.len() { b[i] } else { 0 }; + byte_diff |= x ^ y; } - a.bytes() - .zip(b.bytes()) - .fold(0u8, |acc, (x, y)| acc | (x ^ y)) - == 0 + len_diff == 0 && byte_diff == 0 } /// Check if a host string represents a non-localhost bind address. diff --git a/src/security/secrets.rs b/src/security/secrets.rs index bafad38..3940843 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -241,7 +241,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. fn hex_decode(hex: &str) -> Result> { - if hex.len() % 2 != 0 { + if !hex.len().is_multiple_of(2) { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) diff --git a/src/tools/browser.rs b/src/tools/browser.rs index 5ee9505..25be13c 100644 --- a/src/tools/browser.rs +++ b/src/tools/browser.rs @@ -366,6 +366,7 @@ impl BrowserTool { } #[async_trait] +#[allow(clippy::too_many_lines)] impl Tool for BrowserTool { fn name(&self) -> &str { "browser" From 8a304505dff2aab609a4efc3a84ea301d8fa401c Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 00:04:05 +0100 Subject: [PATCH 04/21] fix: apply TimeoutLayer to gateway router for request timeouts Add tower-http TimeoutLayer with the existing REQUEST_TIMEOUT_SECS (30s) constant and 408 Request Timeout status code. Previously, the constant was defined but no timeout middleware was applied, allowing slow requests to hold connections indefinitely (slow-loris risk). Closes #60 Co-Authored-By: Claude Opus 4.6 --- src/gateway/mod.rs | 10 +++++++--- src/security/secrets.rs | 2 +- src/tools/browser.rs | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/gateway/mod.rs b/src/gateway/mod.rs index deba8ff..039139b 100644 --- a/src/gateway/mod.rs +++ b/src/gateway/mod.rs @@ -23,7 +23,9 @@ use axum::{ }; use std::net::SocketAddr; use std::sync::Arc; +use std::time::Duration; use tower_http::limit::RequestBodyLimitLayer; +use tower_http::timeout::TimeoutLayer; /// Maximum request body size (64KB) — prevents memory exhaustion pub const MAX_BODY_SIZE: usize = 65_536; @@ -163,8 +165,6 @@ pub async fn run_gateway(host: &str, port: u16, config: Config) -> Result<()> { }; // Build router with middleware - // Note: Body limit layer prevents memory exhaustion from oversized requests - // Timeout is handled by tokio's TcpListener accept timeout and hyper's built-in timeouts let app = Router::new() .route("/health", get(handle_health)) .route("/pair", post(handle_pair)) @@ -172,7 +172,11 @@ pub async fn run_gateway(host: &str, port: u16, config: Config) -> Result<()> { .route("/whatsapp", get(handle_whatsapp_verify)) .route("/whatsapp", post(handle_whatsapp_message)) .with_state(state) - .layer(RequestBodyLimitLayer::new(MAX_BODY_SIZE)); + .layer(RequestBodyLimitLayer::new(MAX_BODY_SIZE)) + .layer(TimeoutLayer::with_status_code( + StatusCode::REQUEST_TIMEOUT, + Duration::from_secs(REQUEST_TIMEOUT_SECS), + )); // Run the server axum::serve(listener, app).await?; diff --git a/src/security/secrets.rs b/src/security/secrets.rs index bafad38..3940843 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -241,7 +241,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. fn hex_decode(hex: &str) -> Result> { - if hex.len() % 2 != 0 { + if !hex.len().is_multiple_of(2) { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) diff --git a/src/tools/browser.rs b/src/tools/browser.rs index 5ee9505..25be13c 100644 --- a/src/tools/browser.rs +++ b/src/tools/browser.rs @@ -366,6 +366,7 @@ impl BrowserTool { } #[async_trait] +#[allow(clippy::too_many_lines)] impl Tool for BrowserTool { fn name(&self) -> &str { "browser" From 7c3f2f565fb021cacd8672dbbbdbb2208bd7db6d Mon Sep 17 00:00:00 2001 From: argenis de la rosa Date: Sat, 14 Feb 2026 18:10:39 -0500 Subject: [PATCH 05/21] fix(imessage): replace sqlite CLI path with rusqlite parameterized reads - use rusqlite with SQLITE_OPEN_READ_ONLY | SQLITE_OPEN_NO_MUTEX - run sync sqlite reads via spawn_blocking - bind since_rowid with ?1 parameter to avoid SQL interpolation - add comprehensive edge-case tests for message fetch and rowid helpers Fixes #50 --- src/channels/imessage.rs | 204 +++++++++++++++++++++++++-------------- 1 file changed, 134 insertions(+), 70 deletions(-) diff --git a/src/channels/imessage.rs b/src/channels/imessage.rs index 7a0e76c..e6e78b4 100644 --- a/src/channels/imessage.rs +++ b/src/channels/imessage.rs @@ -210,9 +210,7 @@ async fn get_max_rowid(db_path: &Path) -> anyhow::Result { &path, OpenFlags::SQLITE_OPEN_READ_ONLY | OpenFlags::SQLITE_OPEN_NO_MUTEX, )?; - let mut stmt = conn.prepare( - "SELECT MAX(ROWID) FROM message WHERE is_from_me = 0" - )?; + let mut stmt = conn.prepare("SELECT MAX(ROWID) FROM message WHERE is_from_me = 0")?; let rowid: Option = stmt.query_row([], |row| row.get(0))?; Ok(rowid.unwrap_or(0)) }) @@ -228,31 +226,32 @@ async fn fetch_new_messages( since_rowid: i64, ) -> anyhow::Result> { let path = db_path.to_path_buf(); - let results = tokio::task::spawn_blocking(move || -> anyhow::Result> { - let conn = Connection::open_with_flags( - &path, - OpenFlags::SQLITE_OPEN_READ_ONLY | OpenFlags::SQLITE_OPEN_NO_MUTEX, - )?; - let mut stmt = conn.prepare( - "SELECT m.ROWID, h.id, m.text \ + let results = + tokio::task::spawn_blocking(move || -> anyhow::Result> { + let conn = Connection::open_with_flags( + &path, + OpenFlags::SQLITE_OPEN_READ_ONLY | OpenFlags::SQLITE_OPEN_NO_MUTEX, + )?; + let mut stmt = conn.prepare( + "SELECT m.ROWID, h.id, m.text \ FROM message m \ JOIN handle h ON m.handle_id = h.ROWID \ WHERE m.ROWID > ?1 \ AND m.is_from_me = 0 \ AND m.text IS NOT NULL \ ORDER BY m.ROWID ASC \ - LIMIT 20" - )?; - let rows = stmt.query_map([since_rowid], |row| { - Ok(( - row.get::<_, i64>(0)?, - row.get::<_, String>(1)?, - row.get::<_, String>(2)?, - )) - })?; - rows.collect::, _>>().map_err(Into::into) - }) - .await??; + LIMIT 20", + )?; + let rows = stmt.query_map([since_rowid], |row| { + Ok(( + row.get::<_, i64>(0)?, + row.get::<_, String>(1)?, + row.get::<_, String>(2)?, + )) + })?; + rows.collect::, _>>().map_err(Into::into) + }) + .await??; Ok(results) } @@ -536,9 +535,9 @@ mod tests { fn create_test_db() -> (tempfile::TempDir, std::path::PathBuf) { let dir = tempfile::tempdir().unwrap(); let db_path = dir.path().join("chat.db"); - + let conn = Connection::open(&db_path).unwrap(); - + // Create minimal schema matching macOS Messages.app conn.execute_batch( "CREATE TABLE handle ( @@ -551,9 +550,10 @@ mod tests { text TEXT, is_from_me INTEGER DEFAULT 0, FOREIGN KEY (handle_id) REFERENCES handle(ROWID) - );" - ).unwrap(); - + );", + ) + .unwrap(); + (dir, db_path) } @@ -569,11 +569,15 @@ mod tests { #[tokio::test] async fn get_max_rowid_with_messages() { let (_dir, db_path) = create_test_db(); - + // Insert test data { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (100, 1, 'Hello', 0)", [] @@ -588,7 +592,7 @@ mod tests { [] ).unwrap(); } - + let result = get_max_rowid(&db_path).await.unwrap(); // Should return 200, not 300 (ignores is_from_me=1) assert_eq!(result, 200); @@ -612,12 +616,20 @@ mod tests { #[tokio::test] async fn fetch_new_messages_returns_correct_data() { let (_dir, db_path) = create_test_db(); - + // Insert test data { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (2, 'user@example.com')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (2, 'user@example.com')", + [], + ) + .unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (10, 1, 'First message', 0)", [] @@ -627,21 +639,35 @@ mod tests { [] ).unwrap(); } - + let result = fetch_new_messages(&db_path, 0).await.unwrap(); assert_eq!(result.len(), 2); - assert_eq!(result[0], (10, "+1234567890".to_string(), "First message".to_string())); - assert_eq!(result[1], (20, "user@example.com".to_string(), "Second message".to_string())); + assert_eq!( + result[0], + (10, "+1234567890".to_string(), "First message".to_string()) + ); + assert_eq!( + result[1], + ( + 20, + "user@example.com".to_string(), + "Second message".to_string() + ) + ); } #[tokio::test] async fn fetch_new_messages_filters_by_rowid() { let (_dir, db_path) = create_test_db(); - + // Insert test data { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (10, 1, 'Old message', 0)", [] @@ -651,7 +677,7 @@ mod tests { [] ).unwrap(); } - + // Fetch only messages after ROWID 15 let result = fetch_new_messages(&db_path, 15).await.unwrap(); assert_eq!(result.len(), 1); @@ -662,11 +688,15 @@ mod tests { #[tokio::test] async fn fetch_new_messages_excludes_sent_messages() { let (_dir, db_path) = create_test_db(); - + // Insert test data { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (10, 1, 'Received', 0)", [] @@ -676,7 +706,7 @@ mod tests { [] ).unwrap(); } - + let result = fetch_new_messages(&db_path, 0).await.unwrap(); assert_eq!(result.len(), 1); assert_eq!(result[0].2, "Received"); @@ -685,21 +715,26 @@ mod tests { #[tokio::test] async fn fetch_new_messages_excludes_null_text() { let (_dir, db_path) = create_test_db(); - + // Insert test data { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (10, 1, 'Has text', 0)", [] ).unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (20, 1, NULL, 0)", - [] - ).unwrap(); + [], + ) + .unwrap(); } - + let result = fetch_new_messages(&db_path, 0).await.unwrap(); assert_eq!(result.len(), 1); assert_eq!(result[0].2, "Has text"); @@ -708,11 +743,15 @@ mod tests { #[tokio::test] async fn fetch_new_messages_respects_limit() { let (_dir, db_path) = create_test_db(); - + // Insert 25 messages (limit is 20) { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); for i in 1..=25 { conn.execute( &format!("INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES ({i}, 1, 'Message {i}', 0)"), @@ -720,7 +759,7 @@ mod tests { ).unwrap(); } } - + let result = fetch_new_messages(&db_path, 0).await.unwrap(); assert_eq!(result.len(), 20); // Limited to 20 assert_eq!(result[0].0, 1); // First message @@ -730,11 +769,15 @@ mod tests { #[tokio::test] async fn fetch_new_messages_ordered_by_rowid_asc() { let (_dir, db_path) = create_test_db(); - + // Insert messages out of order { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (30, 1, 'Third', 0)", [] @@ -748,7 +791,7 @@ mod tests { [] ).unwrap(); } - + let result = fetch_new_messages(&db_path, 0).await.unwrap(); assert_eq!(result.len(), 3); assert_eq!(result[0].0, 10); @@ -766,17 +809,21 @@ mod tests { #[tokio::test] async fn fetch_new_messages_handles_special_characters() { let (_dir, db_path) = create_test_db(); - + // Insert message with special characters (potential SQL injection patterns) { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (10, 1, 'Hello \"world'' OR 1=1; DROP TABLE message;--', 0)", [] ).unwrap(); } - + let result = fetch_new_messages(&db_path, 0).await.unwrap(); assert_eq!(result.len(), 1); // The special characters should be preserved, not interpreted as SQL @@ -786,16 +833,20 @@ mod tests { #[tokio::test] async fn fetch_new_messages_handles_unicode() { let (_dir, db_path) = create_test_db(); - + { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (10, 1, 'Hello 🦀 世界 مرحبا', 0)", [] ).unwrap(); } - + let result = fetch_new_messages(&db_path, 0).await.unwrap(); assert_eq!(result.len(), 1); assert_eq!(result[0].2, "Hello 🦀 世界 مرحبا"); @@ -804,16 +855,21 @@ mod tests { #[tokio::test] async fn fetch_new_messages_handles_empty_text() { let (_dir, db_path) = create_test_db(); - + { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (10, 1, '', 0)", - [] - ).unwrap(); + [], + ) + .unwrap(); } - + let result = fetch_new_messages(&db_path, 0).await.unwrap(); // Empty string is NOT NULL, so it's included assert_eq!(result.len(), 1); @@ -823,16 +879,20 @@ mod tests { #[tokio::test] async fn fetch_new_messages_negative_rowid_edge_case() { let (_dir, db_path) = create_test_db(); - + { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (10, 1, 'Test', 0)", [] ).unwrap(); } - + // Negative rowid should still work (fetch all messages with ROWID > -1) let result = fetch_new_messages(&db_path, -1).await.unwrap(); assert_eq!(result.len(), 1); @@ -841,16 +901,20 @@ mod tests { #[tokio::test] async fn fetch_new_messages_large_rowid_edge_case() { let (_dir, db_path) = create_test_db(); - + { let conn = Connection::open(&db_path).unwrap(); - conn.execute("INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", []).unwrap(); + conn.execute( + "INSERT INTO handle (ROWID, id) VALUES (1, '+1234567890')", + [], + ) + .unwrap(); conn.execute( "INSERT INTO message (ROWID, handle_id, text, is_from_me) VALUES (10, 1, 'Test', 0)", [] ).unwrap(); } - + // Very large rowid should return empty (no messages after this) let result = fetch_new_messages(&db_path, i64::MAX - 1).await.unwrap(); assert!(result.is_empty()); From a7ed2329d1c32dd5e83dcfa4ac8d1ff78b663d9b Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 00:25:20 +0100 Subject: [PATCH 06/21] fix: assert variant_match in CSPRNG key entropy test Add missing assertion for variant_match (byte[8] UUID v4 variant bits) which was computed but never checked. Co-Authored-By: Claude Opus 4.6 --- src/security/secrets.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index cd93ede..f78d055 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -773,6 +773,11 @@ mod tests { "byte[6] matched UUID v4 version nibble {version_match}/100 times — \ likely still using UUID-based key generation" ); + assert!( + variant_match < 50, + "byte[8] matched UUID v4 variant bits {variant_match}/100 times — \ + likely still using UUID-based key generation" + ); } #[cfg(unix)] From 6fd4b2d750816f9cd922fab0698b4b411696e301 Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 00:29:22 +0100 Subject: [PATCH 07/21] fix: handle empty USERNAME and add debug log for icacls success - Check for empty USERNAME env var before running icacls to avoid a doomed invocation with ":F" grant argument - Log a clear warning when USERNAME is empty - Add tracing::debug on successful permission set Co-Authored-By: Claude Opus 4.6 --- src/security/secrets.rs | 43 ++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index c845bd9..db38972 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -191,25 +191,32 @@ impl SecretStore { #[cfg(windows)] { // On Windows, use icacls to restrict permissions to current user only - match std::process::Command::new("icacls") - .arg(&self.key_path) - .args(["/inheritance:r", "/grant:r"]) - .arg(format!( - "{}:F", - std::env::var("USERNAME").unwrap_or_default() - )) - .output() - { - Ok(o) if !o.status.success() => { - tracing::warn!( - "Failed to set key file permissions via icacls (exit code {:?})", - o.status.code() - ); + let username = std::env::var("USERNAME").unwrap_or_default(); + if username.is_empty() { + tracing::warn!( + "USERNAME environment variable is empty; \ + cannot restrict key file permissions via icacls" + ); + } else { + match std::process::Command::new("icacls") + .arg(&self.key_path) + .args(["/inheritance:r", "/grant:r"]) + .arg(format!("{username}:F")) + .output() + { + Ok(o) if !o.status.success() => { + tracing::warn!( + "Failed to set key file permissions via icacls (exit code {:?})", + o.status.code() + ); + } + Err(e) => { + tracing::warn!("Could not set key file permissions: {e}"); + } + _ => { + tracing::debug!("Key file permissions restricted via icacls"); + } } - Err(e) => { - tracing::warn!("Could not set key file permissions: {e}"); - } - _ => {} } } From 2f2f56fc0c1c6540f79b895654ea6f6849423e39 Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 00:30:23 +0100 Subject: [PATCH 08/21] fix: use branchless operations in constant_time_eq - Use bitwise & instead of && to avoid short-circuit timing leak - Use get().unwrap_or(&0) instead of if/else for branchless byte access Co-Authored-By: Claude Opus 4.6 --- src/security/pairing.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/security/pairing.rs b/src/security/pairing.rs index f9a9a05..e176d38 100644 --- a/src/security/pairing.rs +++ b/src/security/pairing.rs @@ -190,11 +190,11 @@ pub fn constant_time_eq(a: &str, b: &str) -> bool { let max_len = a.len().max(b.len()); let mut byte_diff = 0u8; for i in 0..max_len { - let x = if i < a.len() { a[i] } else { 0 }; - let y = if i < b.len() { b[i] } else { 0 }; + let x = *a.get(i).unwrap_or(&0); + let y = *b.get(i).unwrap_or(&0); byte_diff |= x ^ y; } - len_diff == 0 && byte_diff == 0 + (len_diff == 0) & (byte_diff == 0) } /// Check if a host string represents a non-localhost bind address. From 41ba251686661c349c52394fa885f407b488faa8 Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 01:24:24 +0100 Subject: [PATCH 09/21] fix: replace unstable is_multiple_of with modulo for Rust 1.83 compat The Docker image uses rust:1.83-slim where is_multiple_of is unstable. Co-Authored-By: Claude Opus 4.6 --- src/security/secrets.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index f78d055..dbfc1e0 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -237,7 +237,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. fn hex_decode(hex: &str) -> Result> { - if !hex.len().is_multiple_of(2) { + if hex.len() % 2 != 0 { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) From a68004184cb7fbd356ba1ee900d3a16edd040d45 Mon Sep 17 00:00:00 2001 From: argenis de la rosa Date: Sat, 14 Feb 2026 19:25:30 -0500 Subject: [PATCH 10/21] fix(secrets): harden windows icacls username edge cases --- src/security/secrets.rs | 75 +++++++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 21 deletions(-) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index db38972..c7d670b 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -192,30 +192,31 @@ impl SecretStore { { // On Windows, use icacls to restrict permissions to current user only let username = std::env::var("USERNAME").unwrap_or_default(); - if username.is_empty() { + let Some(grant_arg) = build_windows_icacls_grant_arg(&username) else { tracing::warn!( "USERNAME environment variable is empty; \ cannot restrict key file permissions via icacls" ); - } else { - match std::process::Command::new("icacls") - .arg(&self.key_path) - .args(["/inheritance:r", "/grant:r"]) - .arg(format!("{username}:F")) - .output() - { - Ok(o) if !o.status.success() => { - tracing::warn!( - "Failed to set key file permissions via icacls (exit code {:?})", - o.status.code() - ); - } - Err(e) => { - tracing::warn!("Could not set key file permissions: {e}"); - } - _ => { - tracing::debug!("Key file permissions restricted via icacls"); - } + return Ok(key); + }; + + match std::process::Command::new("icacls") + .arg(&self.key_path) + .args(["/inheritance:r", "/grant:r"]) + .arg(grant_arg) + .output() + { + Ok(o) if !o.status.success() => { + tracing::warn!( + "Failed to set key file permissions via icacls (exit code {:?})", + o.status.code() + ); + } + Err(e) => { + tracing::warn!("Could not set key file permissions: {e}"); + } + _ => { + tracing::debug!("Key file permissions restricted via icacls"); } } } @@ -258,9 +259,19 @@ fn hex_encode(data: &[u8]) -> String { s } +/// Build the `/grant` argument for `icacls` using a normalized username. +/// Returns `None` when the username is empty or whitespace-only. +fn build_windows_icacls_grant_arg(username: &str) -> Option { + let normalized = username.trim(); + if normalized.is_empty() { + return None; + } + Some(format!("{normalized}:F")) +} + /// Hex-decode a hex string to bytes. fn hex_decode(hex: &str) -> Result> { - if !hex.len().is_multiple_of(2) { + if (hex.len() & 1) != 0 { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) @@ -751,6 +762,28 @@ mod tests { assert!(hex_decode("zzzz").is_err()); } + #[test] + fn windows_icacls_grant_arg_rejects_empty_username() { + assert_eq!(build_windows_icacls_grant_arg(""), None); + assert_eq!(build_windows_icacls_grant_arg(" \t\n"), None); + } + + #[test] + fn windows_icacls_grant_arg_trims_username() { + assert_eq!( + build_windows_icacls_grant_arg(" alice "), + Some("alice:F".to_string()) + ); + } + + #[test] + fn windows_icacls_grant_arg_preserves_valid_characters() { + assert_eq!( + build_windows_icacls_grant_arg("DOMAIN\\svc-user"), + Some("DOMAIN\\svc-user:F".to_string()) + ); + } + #[test] fn generate_random_key_correct_length() { let key = generate_random_key(); From ac7c625368e6a89f9e8564b6820fcd06b4b095b8 Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 01:26:44 +0100 Subject: [PATCH 11/21] fix: replace unstable is_multiple_of with modulo for Rust 1.83 compat The Docker image uses rust:1.83-slim where is_multiple_of is unstable. Co-Authored-By: Claude Opus 4.6 --- src/security/secrets.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index 3940843..bafad38 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -241,7 +241,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. fn hex_decode(hex: &str) -> Result> { - if !hex.len().is_multiple_of(2) { + if hex.len() % 2 != 0 { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) From 91f2edab05fef5c15d48e204c8c3e31f118ba0f4 Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 01:27:03 +0100 Subject: [PATCH 12/21] fix: replace unstable is_multiple_of with modulo for Rust 1.83 compat The Docker image uses rust:1.83-slim where is_multiple_of is unstable. Co-Authored-By: Claude Opus 4.6 --- src/security/secrets.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index 3940843..bafad38 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -241,7 +241,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. fn hex_decode(hex: &str) -> Result> { - if !hex.len().is_multiple_of(2) { + if hex.len() % 2 != 0 { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) From 6f64099a4897cbd7044805f5832ecf1155178aba Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 01:29:23 +0100 Subject: [PATCH 13/21] fix: replace unstable is_multiple_of with modulo and fix flaky temp test The `is_multiple_of` method is unstable before Rust 1.87, breaking Docker builds that use rust:1.83-slim. Also merges the two temperature env-var tests into one to eliminate the race condition when tests run in parallel. Co-Authored-By: Claude Opus 4.6 --- src/config/schema.rs | 21 +++++++++------------ src/security/secrets.rs | 2 +- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/config/schema.rs b/src/config/schema.rs index be6f768..3ef7c42 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -1613,26 +1613,23 @@ default_temperature = 0.7 #[test] fn env_override_temperature() { + // Both temperature cases tested in one function to avoid env-var + // races when tests run in parallel. std::env::remove_var("ZEROCLAW_TEMPERATURE"); - let mut config = Config::default(); + // Valid temperature is applied + let mut config = Config::default(); std::env::set_var("ZEROCLAW_TEMPERATURE", "0.5"); config.apply_env_overrides(); assert!((config.default_temperature - 0.5).abs() < f64::EPSILON); - std::env::remove_var("ZEROCLAW_TEMPERATURE"); - } - - #[test] - fn env_override_temperature_out_of_range_ignored() { - std::env::remove_var("ZEROCLAW_TEMPERATURE"); - let mut config = Config::default(); - let original_temp = config.default_temperature; - + // Out-of-range temperature is ignored + let mut config2 = Config::default(); + let original_temp = config2.default_temperature; std::env::set_var("ZEROCLAW_TEMPERATURE", "3.0"); - config.apply_env_overrides(); + config2.apply_env_overrides(); assert!( - (config.default_temperature - original_temp).abs() < f64::EPSILON, + (config2.default_temperature - original_temp).abs() < f64::EPSILON, "Temperature 3.0 should be ignored (out of range)" ); diff --git a/src/security/secrets.rs b/src/security/secrets.rs index 38a8d6a..8dea343 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -242,7 +242,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. #[allow(clippy::manual_is_multiple_of)] fn hex_decode(hex: &str) -> Result> { - if !hex.len().is_multiple_of(2) { + if hex.len() % 2 != 0 { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) From dc0d6b6ca9484587497c6660d5534b2c7a4dbeed Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 02:14:25 +0100 Subject: [PATCH 14/21] fix: replace unstable is_multiple_of with modulo for Rust 1.83 compat The Docker image uses rust:1.83-slim where is_multiple_of is unstable. Co-Authored-By: Claude Opus 4.6 --- src/security/secrets.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index c3d012a..4d00384 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -238,7 +238,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. #[allow(clippy::manual_is_multiple_of)] fn hex_decode(hex: &str) -> Result> { - if !hex.len().is_multiple_of(2) { + if hex.len() % 2 != 0 { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) From e0341e599682ff6002e3d18954e78fa12e803b87 Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 02:15:24 +0100 Subject: [PATCH 15/21] fix: replace unstable is_multiple_of with modulo for Rust 1.83 compat The Docker image uses rust:1.83-slim where is_multiple_of is unstable. Co-Authored-By: Claude Opus 4.6 --- src/security/secrets.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index 38a8d6a..8dea343 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -242,7 +242,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. #[allow(clippy::manual_is_multiple_of)] fn hex_decode(hex: &str) -> Result> { - if !hex.len().is_multiple_of(2) { + if hex.len() % 2 != 0 { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) From d2afc014b2b2642f05ba2578b94fcdb4c5522e5d Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 02:15:38 +0100 Subject: [PATCH 16/21] fix: replace unstable is_multiple_of with modulo for Rust 1.83 compat The Docker image uses rust:1.83-slim where is_multiple_of is unstable. Co-Authored-By: Claude Opus 4.6 --- src/security/secrets.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/secrets.rs b/src/security/secrets.rs index 38a8d6a..8dea343 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -242,7 +242,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. #[allow(clippy::manual_is_multiple_of)] fn hex_decode(hex: &str) -> Result> { - if !hex.len().is_multiple_of(2) { + if hex.len() % 2 != 0 { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) From cfa44250a7bc3e988c312b5de3a467159829bfaf Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 02:26:04 +0100 Subject: [PATCH 17/21] fix: consolidate env-var override tests to eliminate parallel races Tests that set/remove the same environment variables (PROVIDER, PORT, HOST, TEMPERATURE, API_KEY) can race when cargo test runs them in parallel. Merges each racing pair into a single test function. Co-Authored-By: Claude Opus 4.6 --- src/config/schema.rs | 146 +++++++++++++++++-------------------------- 1 file changed, 59 insertions(+), 87 deletions(-) diff --git a/src/config/schema.rs b/src/config/schema.rs index be6f768..e437407 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -1474,55 +1474,53 @@ default_temperature = 0.7 #[test] fn env_override_api_key() { + // Primary and fallback tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_API_KEY"); + std::env::remove_var("API_KEY"); + + // Primary: ZEROCLAW_API_KEY let mut config = Config::default(); assert!(config.api_key.is_none()); - - // Simulate ZEROCLAW_API_KEY std::env::set_var("ZEROCLAW_API_KEY", "sk-test-env-key"); config.apply_env_overrides(); assert_eq!(config.api_key.as_deref(), Some("sk-test-env-key")); - - // Clean up std::env::remove_var("ZEROCLAW_API_KEY"); - } - #[test] - fn env_override_api_key_fallback() { - let mut config = Config::default(); - - // Simulate API_KEY (fallback) - std::env::remove_var("ZEROCLAW_API_KEY"); + // Fallback: API_KEY + let mut config2 = Config::default(); std::env::set_var("API_KEY", "sk-fallback-key"); - config.apply_env_overrides(); - assert_eq!(config.api_key.as_deref(), Some("sk-fallback-key")); - - // Clean up + config2.apply_env_overrides(); + assert_eq!(config2.api_key.as_deref(), Some("sk-fallback-key")); std::env::remove_var("API_KEY"); } #[test] fn env_override_provider() { - let mut config = Config::default(); + // Primary, fallback, and empty-value tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_PROVIDER"); + std::env::remove_var("PROVIDER"); + // Primary: ZEROCLAW_PROVIDER + let mut config = Config::default(); std::env::set_var("ZEROCLAW_PROVIDER", "anthropic"); config.apply_env_overrides(); assert_eq!(config.default_provider.as_deref(), Some("anthropic")); - - // Clean up std::env::remove_var("ZEROCLAW_PROVIDER"); - } - #[test] - fn env_override_provider_fallback() { - let mut config = Config::default(); - - std::env::remove_var("ZEROCLAW_PROVIDER"); + // Fallback: PROVIDER + let mut config2 = Config::default(); std::env::set_var("PROVIDER", "openai"); - config.apply_env_overrides(); - assert_eq!(config.default_provider.as_deref(), Some("openai")); - - // Clean up + config2.apply_env_overrides(); + assert_eq!(config2.default_provider.as_deref(), Some("openai")); std::env::remove_var("PROVIDER"); + + // Empty value should not override + let mut config3 = Config::default(); + let original_provider = config3.default_provider.clone(); + std::env::set_var("ZEROCLAW_PROVIDER", ""); + config3.apply_env_overrides(); + assert_eq!(config3.default_provider, original_provider); + std::env::remove_var("ZEROCLAW_PROVIDER"); } #[test] @@ -1549,108 +1547,82 @@ default_temperature = 0.7 std::env::remove_var("ZEROCLAW_WORKSPACE"); } - #[test] - fn env_override_empty_values_ignored() { - let mut config = Config::default(); - let original_provider = config.default_provider.clone(); - - std::env::set_var("ZEROCLAW_PROVIDER", ""); - config.apply_env_overrides(); - // Empty value should not override - assert_eq!(config.default_provider, original_provider); - - // Clean up - std::env::remove_var("ZEROCLAW_PROVIDER"); - } - #[test] fn env_override_gateway_port() { + // Port, fallback, and invalid tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); + std::env::remove_var("PORT"); + + // Primary: ZEROCLAW_GATEWAY_PORT let mut config = Config::default(); assert_eq!(config.gateway.port, 3000); - std::env::set_var("ZEROCLAW_GATEWAY_PORT", "8080"); config.apply_env_overrides(); assert_eq!(config.gateway.port, 8080); - std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); - } - - #[test] - fn env_override_port_fallback() { - std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); - let mut config = Config::default(); + // Fallback: PORT + let mut config2 = Config::default(); std::env::set_var("PORT", "9000"); - config.apply_env_overrides(); - assert_eq!(config.gateway.port, 9000); + config2.apply_env_overrides(); + assert_eq!(config2.gateway.port, 9000); + + // Invalid PORT is ignored + let mut config3 = Config::default(); + let original_port = config3.gateway.port; + std::env::set_var("PORT", "not_a_number"); + config3.apply_env_overrides(); + assert_eq!(config3.gateway.port, original_port); std::env::remove_var("PORT"); } #[test] fn env_override_gateway_host() { + // Primary and fallback tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); + std::env::remove_var("HOST"); + + // Primary: ZEROCLAW_GATEWAY_HOST let mut config = Config::default(); assert_eq!(config.gateway.host, "127.0.0.1"); - std::env::set_var("ZEROCLAW_GATEWAY_HOST", "0.0.0.0"); config.apply_env_overrides(); assert_eq!(config.gateway.host, "0.0.0.0"); - std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); - } - - #[test] - fn env_override_host_fallback() { - std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); - let mut config = Config::default(); + // Fallback: HOST + let mut config2 = Config::default(); std::env::set_var("HOST", "0.0.0.0"); - config.apply_env_overrides(); - assert_eq!(config.gateway.host, "0.0.0.0"); - + config2.apply_env_overrides(); + assert_eq!(config2.gateway.host, "0.0.0.0"); std::env::remove_var("HOST"); } #[test] fn env_override_temperature() { + // Valid and out-of-range tested together to avoid env-var races. std::env::remove_var("ZEROCLAW_TEMPERATURE"); - let mut config = Config::default(); + // Valid temperature is applied + let mut config = Config::default(); std::env::set_var("ZEROCLAW_TEMPERATURE", "0.5"); config.apply_env_overrides(); assert!((config.default_temperature - 0.5).abs() < f64::EPSILON); - std::env::remove_var("ZEROCLAW_TEMPERATURE"); - } - - #[test] - fn env_override_temperature_out_of_range_ignored() { - std::env::remove_var("ZEROCLAW_TEMPERATURE"); - let mut config = Config::default(); - let original_temp = config.default_temperature; - + // Out-of-range temperature is ignored + let mut config2 = Config::default(); + let original_temp = config2.default_temperature; std::env::set_var("ZEROCLAW_TEMPERATURE", "3.0"); - config.apply_env_overrides(); + config2.apply_env_overrides(); assert!( - (config.default_temperature - original_temp).abs() < f64::EPSILON, + (config2.default_temperature - original_temp).abs() < f64::EPSILON, "Temperature 3.0 should be ignored (out of range)" ); std::env::remove_var("ZEROCLAW_TEMPERATURE"); } - #[test] - fn env_override_invalid_port_ignored() { - let mut config = Config::default(); - let original_port = config.gateway.port; - - std::env::set_var("PORT", "not_a_number"); - config.apply_env_overrides(); - assert_eq!(config.gateway.port, original_port); - - std::env::remove_var("PORT"); - } - #[test] fn gateway_config_default_values() { let g = GatewayConfig::default(); From 65a5c3c1e8fcf30f2ea84a5f3e22fda026f819d4 Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 02:26:39 +0100 Subject: [PATCH 18/21] fix: consolidate env-var override tests to eliminate parallel races Tests that set/remove the same environment variables can race when cargo test runs them in parallel. Merges each racing pair into a single test function. Co-Authored-By: Claude Opus 4.6 --- src/config/schema.rs | 146 +++++++++++++++++-------------------------- 1 file changed, 59 insertions(+), 87 deletions(-) diff --git a/src/config/schema.rs b/src/config/schema.rs index be6f768..e437407 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -1474,55 +1474,53 @@ default_temperature = 0.7 #[test] fn env_override_api_key() { + // Primary and fallback tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_API_KEY"); + std::env::remove_var("API_KEY"); + + // Primary: ZEROCLAW_API_KEY let mut config = Config::default(); assert!(config.api_key.is_none()); - - // Simulate ZEROCLAW_API_KEY std::env::set_var("ZEROCLAW_API_KEY", "sk-test-env-key"); config.apply_env_overrides(); assert_eq!(config.api_key.as_deref(), Some("sk-test-env-key")); - - // Clean up std::env::remove_var("ZEROCLAW_API_KEY"); - } - #[test] - fn env_override_api_key_fallback() { - let mut config = Config::default(); - - // Simulate API_KEY (fallback) - std::env::remove_var("ZEROCLAW_API_KEY"); + // Fallback: API_KEY + let mut config2 = Config::default(); std::env::set_var("API_KEY", "sk-fallback-key"); - config.apply_env_overrides(); - assert_eq!(config.api_key.as_deref(), Some("sk-fallback-key")); - - // Clean up + config2.apply_env_overrides(); + assert_eq!(config2.api_key.as_deref(), Some("sk-fallback-key")); std::env::remove_var("API_KEY"); } #[test] fn env_override_provider() { - let mut config = Config::default(); + // Primary, fallback, and empty-value tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_PROVIDER"); + std::env::remove_var("PROVIDER"); + // Primary: ZEROCLAW_PROVIDER + let mut config = Config::default(); std::env::set_var("ZEROCLAW_PROVIDER", "anthropic"); config.apply_env_overrides(); assert_eq!(config.default_provider.as_deref(), Some("anthropic")); - - // Clean up std::env::remove_var("ZEROCLAW_PROVIDER"); - } - #[test] - fn env_override_provider_fallback() { - let mut config = Config::default(); - - std::env::remove_var("ZEROCLAW_PROVIDER"); + // Fallback: PROVIDER + let mut config2 = Config::default(); std::env::set_var("PROVIDER", "openai"); - config.apply_env_overrides(); - assert_eq!(config.default_provider.as_deref(), Some("openai")); - - // Clean up + config2.apply_env_overrides(); + assert_eq!(config2.default_provider.as_deref(), Some("openai")); std::env::remove_var("PROVIDER"); + + // Empty value should not override + let mut config3 = Config::default(); + let original_provider = config3.default_provider.clone(); + std::env::set_var("ZEROCLAW_PROVIDER", ""); + config3.apply_env_overrides(); + assert_eq!(config3.default_provider, original_provider); + std::env::remove_var("ZEROCLAW_PROVIDER"); } #[test] @@ -1549,108 +1547,82 @@ default_temperature = 0.7 std::env::remove_var("ZEROCLAW_WORKSPACE"); } - #[test] - fn env_override_empty_values_ignored() { - let mut config = Config::default(); - let original_provider = config.default_provider.clone(); - - std::env::set_var("ZEROCLAW_PROVIDER", ""); - config.apply_env_overrides(); - // Empty value should not override - assert_eq!(config.default_provider, original_provider); - - // Clean up - std::env::remove_var("ZEROCLAW_PROVIDER"); - } - #[test] fn env_override_gateway_port() { + // Port, fallback, and invalid tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); + std::env::remove_var("PORT"); + + // Primary: ZEROCLAW_GATEWAY_PORT let mut config = Config::default(); assert_eq!(config.gateway.port, 3000); - std::env::set_var("ZEROCLAW_GATEWAY_PORT", "8080"); config.apply_env_overrides(); assert_eq!(config.gateway.port, 8080); - std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); - } - - #[test] - fn env_override_port_fallback() { - std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); - let mut config = Config::default(); + // Fallback: PORT + let mut config2 = Config::default(); std::env::set_var("PORT", "9000"); - config.apply_env_overrides(); - assert_eq!(config.gateway.port, 9000); + config2.apply_env_overrides(); + assert_eq!(config2.gateway.port, 9000); + + // Invalid PORT is ignored + let mut config3 = Config::default(); + let original_port = config3.gateway.port; + std::env::set_var("PORT", "not_a_number"); + config3.apply_env_overrides(); + assert_eq!(config3.gateway.port, original_port); std::env::remove_var("PORT"); } #[test] fn env_override_gateway_host() { + // Primary and fallback tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); + std::env::remove_var("HOST"); + + // Primary: ZEROCLAW_GATEWAY_HOST let mut config = Config::default(); assert_eq!(config.gateway.host, "127.0.0.1"); - std::env::set_var("ZEROCLAW_GATEWAY_HOST", "0.0.0.0"); config.apply_env_overrides(); assert_eq!(config.gateway.host, "0.0.0.0"); - std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); - } - - #[test] - fn env_override_host_fallback() { - std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); - let mut config = Config::default(); + // Fallback: HOST + let mut config2 = Config::default(); std::env::set_var("HOST", "0.0.0.0"); - config.apply_env_overrides(); - assert_eq!(config.gateway.host, "0.0.0.0"); - + config2.apply_env_overrides(); + assert_eq!(config2.gateway.host, "0.0.0.0"); std::env::remove_var("HOST"); } #[test] fn env_override_temperature() { + // Valid and out-of-range tested together to avoid env-var races. std::env::remove_var("ZEROCLAW_TEMPERATURE"); - let mut config = Config::default(); + // Valid temperature is applied + let mut config = Config::default(); std::env::set_var("ZEROCLAW_TEMPERATURE", "0.5"); config.apply_env_overrides(); assert!((config.default_temperature - 0.5).abs() < f64::EPSILON); - std::env::remove_var("ZEROCLAW_TEMPERATURE"); - } - - #[test] - fn env_override_temperature_out_of_range_ignored() { - std::env::remove_var("ZEROCLAW_TEMPERATURE"); - let mut config = Config::default(); - let original_temp = config.default_temperature; - + // Out-of-range temperature is ignored + let mut config2 = Config::default(); + let original_temp = config2.default_temperature; std::env::set_var("ZEROCLAW_TEMPERATURE", "3.0"); - config.apply_env_overrides(); + config2.apply_env_overrides(); assert!( - (config.default_temperature - original_temp).abs() < f64::EPSILON, + (config2.default_temperature - original_temp).abs() < f64::EPSILON, "Temperature 3.0 should be ignored (out of range)" ); std::env::remove_var("ZEROCLAW_TEMPERATURE"); } - #[test] - fn env_override_invalid_port_ignored() { - let mut config = Config::default(); - let original_port = config.gateway.port; - - std::env::set_var("PORT", "not_a_number"); - config.apply_env_overrides(); - assert_eq!(config.gateway.port, original_port); - - std::env::remove_var("PORT"); - } - #[test] fn gateway_config_default_values() { let g = GatewayConfig::default(); From 882e1320dce2df165c5b312929d84db4b6779ab3 Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 02:26:51 +0100 Subject: [PATCH 19/21] fix: consolidate all env-var override tests to eliminate parallel races Extends the temperature test fix to also cover provider, api_key, port, and host env-var tests that had the same race condition. Co-Authored-By: Claude Opus 4.6 --- src/config/schema.rs | 129 +++++++++++++++++-------------------------- 1 file changed, 52 insertions(+), 77 deletions(-) diff --git a/src/config/schema.rs b/src/config/schema.rs index 3ef7c42..e437407 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -1474,55 +1474,53 @@ default_temperature = 0.7 #[test] fn env_override_api_key() { + // Primary and fallback tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_API_KEY"); + std::env::remove_var("API_KEY"); + + // Primary: ZEROCLAW_API_KEY let mut config = Config::default(); assert!(config.api_key.is_none()); - - // Simulate ZEROCLAW_API_KEY std::env::set_var("ZEROCLAW_API_KEY", "sk-test-env-key"); config.apply_env_overrides(); assert_eq!(config.api_key.as_deref(), Some("sk-test-env-key")); - - // Clean up std::env::remove_var("ZEROCLAW_API_KEY"); - } - #[test] - fn env_override_api_key_fallback() { - let mut config = Config::default(); - - // Simulate API_KEY (fallback) - std::env::remove_var("ZEROCLAW_API_KEY"); + // Fallback: API_KEY + let mut config2 = Config::default(); std::env::set_var("API_KEY", "sk-fallback-key"); - config.apply_env_overrides(); - assert_eq!(config.api_key.as_deref(), Some("sk-fallback-key")); - - // Clean up + config2.apply_env_overrides(); + assert_eq!(config2.api_key.as_deref(), Some("sk-fallback-key")); std::env::remove_var("API_KEY"); } #[test] fn env_override_provider() { - let mut config = Config::default(); + // Primary, fallback, and empty-value tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_PROVIDER"); + std::env::remove_var("PROVIDER"); + // Primary: ZEROCLAW_PROVIDER + let mut config = Config::default(); std::env::set_var("ZEROCLAW_PROVIDER", "anthropic"); config.apply_env_overrides(); assert_eq!(config.default_provider.as_deref(), Some("anthropic")); - - // Clean up std::env::remove_var("ZEROCLAW_PROVIDER"); - } - #[test] - fn env_override_provider_fallback() { - let mut config = Config::default(); - - std::env::remove_var("ZEROCLAW_PROVIDER"); + // Fallback: PROVIDER + let mut config2 = Config::default(); std::env::set_var("PROVIDER", "openai"); - config.apply_env_overrides(); - assert_eq!(config.default_provider.as_deref(), Some("openai")); - - // Clean up + config2.apply_env_overrides(); + assert_eq!(config2.default_provider.as_deref(), Some("openai")); std::env::remove_var("PROVIDER"); + + // Empty value should not override + let mut config3 = Config::default(); + let original_provider = config3.default_provider.clone(); + std::env::set_var("ZEROCLAW_PROVIDER", ""); + config3.apply_env_overrides(); + assert_eq!(config3.default_provider, original_provider); + std::env::remove_var("ZEROCLAW_PROVIDER"); } #[test] @@ -1549,72 +1547,61 @@ default_temperature = 0.7 std::env::remove_var("ZEROCLAW_WORKSPACE"); } - #[test] - fn env_override_empty_values_ignored() { - let mut config = Config::default(); - let original_provider = config.default_provider.clone(); - - std::env::set_var("ZEROCLAW_PROVIDER", ""); - config.apply_env_overrides(); - // Empty value should not override - assert_eq!(config.default_provider, original_provider); - - // Clean up - std::env::remove_var("ZEROCLAW_PROVIDER"); - } - #[test] fn env_override_gateway_port() { + // Port, fallback, and invalid tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); + std::env::remove_var("PORT"); + + // Primary: ZEROCLAW_GATEWAY_PORT let mut config = Config::default(); assert_eq!(config.gateway.port, 3000); - std::env::set_var("ZEROCLAW_GATEWAY_PORT", "8080"); config.apply_env_overrides(); assert_eq!(config.gateway.port, 8080); - std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); - } - - #[test] - fn env_override_port_fallback() { - std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); - let mut config = Config::default(); + // Fallback: PORT + let mut config2 = Config::default(); std::env::set_var("PORT", "9000"); - config.apply_env_overrides(); - assert_eq!(config.gateway.port, 9000); + config2.apply_env_overrides(); + assert_eq!(config2.gateway.port, 9000); + + // Invalid PORT is ignored + let mut config3 = Config::default(); + let original_port = config3.gateway.port; + std::env::set_var("PORT", "not_a_number"); + config3.apply_env_overrides(); + assert_eq!(config3.gateway.port, original_port); std::env::remove_var("PORT"); } #[test] fn env_override_gateway_host() { + // Primary and fallback tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); + std::env::remove_var("HOST"); + + // Primary: ZEROCLAW_GATEWAY_HOST let mut config = Config::default(); assert_eq!(config.gateway.host, "127.0.0.1"); - std::env::set_var("ZEROCLAW_GATEWAY_HOST", "0.0.0.0"); config.apply_env_overrides(); assert_eq!(config.gateway.host, "0.0.0.0"); - std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); - } - - #[test] - fn env_override_host_fallback() { - std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); - let mut config = Config::default(); + // Fallback: HOST + let mut config2 = Config::default(); std::env::set_var("HOST", "0.0.0.0"); - config.apply_env_overrides(); - assert_eq!(config.gateway.host, "0.0.0.0"); - + config2.apply_env_overrides(); + assert_eq!(config2.gateway.host, "0.0.0.0"); std::env::remove_var("HOST"); } #[test] fn env_override_temperature() { - // Both temperature cases tested in one function to avoid env-var - // races when tests run in parallel. + // Valid and out-of-range tested together to avoid env-var races. std::env::remove_var("ZEROCLAW_TEMPERATURE"); // Valid temperature is applied @@ -1636,18 +1623,6 @@ default_temperature = 0.7 std::env::remove_var("ZEROCLAW_TEMPERATURE"); } - #[test] - fn env_override_invalid_port_ignored() { - let mut config = Config::default(); - let original_port = config.gateway.port; - - std::env::set_var("PORT", "not_a_number"); - config.apply_env_overrides(); - assert_eq!(config.gateway.port, original_port); - - std::env::remove_var("PORT"); - } - #[test] fn gateway_config_default_values() { let g = GatewayConfig::default(); From f87cbb28f2a138232755e3841df80411b2ce64ea Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 02:27:02 +0100 Subject: [PATCH 20/21] fix: consolidate env-var override tests to eliminate parallel races Tests that set/remove the same environment variables can race when cargo test runs them in parallel. Merges each racing pair into a single test function. Co-Authored-By: Claude Opus 4.6 --- src/config/schema.rs | 146 +++++++++++++++++-------------------------- 1 file changed, 59 insertions(+), 87 deletions(-) diff --git a/src/config/schema.rs b/src/config/schema.rs index be6f768..e437407 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -1474,55 +1474,53 @@ default_temperature = 0.7 #[test] fn env_override_api_key() { + // Primary and fallback tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_API_KEY"); + std::env::remove_var("API_KEY"); + + // Primary: ZEROCLAW_API_KEY let mut config = Config::default(); assert!(config.api_key.is_none()); - - // Simulate ZEROCLAW_API_KEY std::env::set_var("ZEROCLAW_API_KEY", "sk-test-env-key"); config.apply_env_overrides(); assert_eq!(config.api_key.as_deref(), Some("sk-test-env-key")); - - // Clean up std::env::remove_var("ZEROCLAW_API_KEY"); - } - #[test] - fn env_override_api_key_fallback() { - let mut config = Config::default(); - - // Simulate API_KEY (fallback) - std::env::remove_var("ZEROCLAW_API_KEY"); + // Fallback: API_KEY + let mut config2 = Config::default(); std::env::set_var("API_KEY", "sk-fallback-key"); - config.apply_env_overrides(); - assert_eq!(config.api_key.as_deref(), Some("sk-fallback-key")); - - // Clean up + config2.apply_env_overrides(); + assert_eq!(config2.api_key.as_deref(), Some("sk-fallback-key")); std::env::remove_var("API_KEY"); } #[test] fn env_override_provider() { - let mut config = Config::default(); + // Primary, fallback, and empty-value tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_PROVIDER"); + std::env::remove_var("PROVIDER"); + // Primary: ZEROCLAW_PROVIDER + let mut config = Config::default(); std::env::set_var("ZEROCLAW_PROVIDER", "anthropic"); config.apply_env_overrides(); assert_eq!(config.default_provider.as_deref(), Some("anthropic")); - - // Clean up std::env::remove_var("ZEROCLAW_PROVIDER"); - } - #[test] - fn env_override_provider_fallback() { - let mut config = Config::default(); - - std::env::remove_var("ZEROCLAW_PROVIDER"); + // Fallback: PROVIDER + let mut config2 = Config::default(); std::env::set_var("PROVIDER", "openai"); - config.apply_env_overrides(); - assert_eq!(config.default_provider.as_deref(), Some("openai")); - - // Clean up + config2.apply_env_overrides(); + assert_eq!(config2.default_provider.as_deref(), Some("openai")); std::env::remove_var("PROVIDER"); + + // Empty value should not override + let mut config3 = Config::default(); + let original_provider = config3.default_provider.clone(); + std::env::set_var("ZEROCLAW_PROVIDER", ""); + config3.apply_env_overrides(); + assert_eq!(config3.default_provider, original_provider); + std::env::remove_var("ZEROCLAW_PROVIDER"); } #[test] @@ -1549,108 +1547,82 @@ default_temperature = 0.7 std::env::remove_var("ZEROCLAW_WORKSPACE"); } - #[test] - fn env_override_empty_values_ignored() { - let mut config = Config::default(); - let original_provider = config.default_provider.clone(); - - std::env::set_var("ZEROCLAW_PROVIDER", ""); - config.apply_env_overrides(); - // Empty value should not override - assert_eq!(config.default_provider, original_provider); - - // Clean up - std::env::remove_var("ZEROCLAW_PROVIDER"); - } - #[test] fn env_override_gateway_port() { + // Port, fallback, and invalid tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); + std::env::remove_var("PORT"); + + // Primary: ZEROCLAW_GATEWAY_PORT let mut config = Config::default(); assert_eq!(config.gateway.port, 3000); - std::env::set_var("ZEROCLAW_GATEWAY_PORT", "8080"); config.apply_env_overrides(); assert_eq!(config.gateway.port, 8080); - std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); - } - - #[test] - fn env_override_port_fallback() { - std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); - let mut config = Config::default(); + // Fallback: PORT + let mut config2 = Config::default(); std::env::set_var("PORT", "9000"); - config.apply_env_overrides(); - assert_eq!(config.gateway.port, 9000); + config2.apply_env_overrides(); + assert_eq!(config2.gateway.port, 9000); + + // Invalid PORT is ignored + let mut config3 = Config::default(); + let original_port = config3.gateway.port; + std::env::set_var("PORT", "not_a_number"); + config3.apply_env_overrides(); + assert_eq!(config3.gateway.port, original_port); std::env::remove_var("PORT"); } #[test] fn env_override_gateway_host() { + // Primary and fallback tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); + std::env::remove_var("HOST"); + + // Primary: ZEROCLAW_GATEWAY_HOST let mut config = Config::default(); assert_eq!(config.gateway.host, "127.0.0.1"); - std::env::set_var("ZEROCLAW_GATEWAY_HOST", "0.0.0.0"); config.apply_env_overrides(); assert_eq!(config.gateway.host, "0.0.0.0"); - std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); - } - - #[test] - fn env_override_host_fallback() { - std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); - let mut config = Config::default(); + // Fallback: HOST + let mut config2 = Config::default(); std::env::set_var("HOST", "0.0.0.0"); - config.apply_env_overrides(); - assert_eq!(config.gateway.host, "0.0.0.0"); - + config2.apply_env_overrides(); + assert_eq!(config2.gateway.host, "0.0.0.0"); std::env::remove_var("HOST"); } #[test] fn env_override_temperature() { + // Valid and out-of-range tested together to avoid env-var races. std::env::remove_var("ZEROCLAW_TEMPERATURE"); - let mut config = Config::default(); + // Valid temperature is applied + let mut config = Config::default(); std::env::set_var("ZEROCLAW_TEMPERATURE", "0.5"); config.apply_env_overrides(); assert!((config.default_temperature - 0.5).abs() < f64::EPSILON); - std::env::remove_var("ZEROCLAW_TEMPERATURE"); - } - - #[test] - fn env_override_temperature_out_of_range_ignored() { - std::env::remove_var("ZEROCLAW_TEMPERATURE"); - let mut config = Config::default(); - let original_temp = config.default_temperature; - + // Out-of-range temperature is ignored + let mut config2 = Config::default(); + let original_temp = config2.default_temperature; std::env::set_var("ZEROCLAW_TEMPERATURE", "3.0"); - config.apply_env_overrides(); + config2.apply_env_overrides(); assert!( - (config.default_temperature - original_temp).abs() < f64::EPSILON, + (config2.default_temperature - original_temp).abs() < f64::EPSILON, "Temperature 3.0 should be ignored (out of range)" ); std::env::remove_var("ZEROCLAW_TEMPERATURE"); } - #[test] - fn env_override_invalid_port_ignored() { - let mut config = Config::default(); - let original_port = config.gateway.port; - - std::env::set_var("PORT", "not_a_number"); - config.apply_env_overrides(); - assert_eq!(config.gateway.port, original_port); - - std::env::remove_var("PORT"); - } - #[test] fn gateway_config_default_values() { let g = GatewayConfig::default(); From 33f64c714679429556032d56b886f62facb369d7 Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 02:27:13 +0100 Subject: [PATCH 21/21] fix: consolidate env-var override tests to eliminate parallel races Tests that set/remove the same environment variables can race when cargo test runs them in parallel. Merges each racing pair into a single test function. Co-Authored-By: Claude Opus 4.6 --- src/config/schema.rs | 146 +++++++++++++++++-------------------------- 1 file changed, 59 insertions(+), 87 deletions(-) diff --git a/src/config/schema.rs b/src/config/schema.rs index be6f768..e437407 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -1474,55 +1474,53 @@ default_temperature = 0.7 #[test] fn env_override_api_key() { + // Primary and fallback tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_API_KEY"); + std::env::remove_var("API_KEY"); + + // Primary: ZEROCLAW_API_KEY let mut config = Config::default(); assert!(config.api_key.is_none()); - - // Simulate ZEROCLAW_API_KEY std::env::set_var("ZEROCLAW_API_KEY", "sk-test-env-key"); config.apply_env_overrides(); assert_eq!(config.api_key.as_deref(), Some("sk-test-env-key")); - - // Clean up std::env::remove_var("ZEROCLAW_API_KEY"); - } - #[test] - fn env_override_api_key_fallback() { - let mut config = Config::default(); - - // Simulate API_KEY (fallback) - std::env::remove_var("ZEROCLAW_API_KEY"); + // Fallback: API_KEY + let mut config2 = Config::default(); std::env::set_var("API_KEY", "sk-fallback-key"); - config.apply_env_overrides(); - assert_eq!(config.api_key.as_deref(), Some("sk-fallback-key")); - - // Clean up + config2.apply_env_overrides(); + assert_eq!(config2.api_key.as_deref(), Some("sk-fallback-key")); std::env::remove_var("API_KEY"); } #[test] fn env_override_provider() { - let mut config = Config::default(); + // Primary, fallback, and empty-value tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_PROVIDER"); + std::env::remove_var("PROVIDER"); + // Primary: ZEROCLAW_PROVIDER + let mut config = Config::default(); std::env::set_var("ZEROCLAW_PROVIDER", "anthropic"); config.apply_env_overrides(); assert_eq!(config.default_provider.as_deref(), Some("anthropic")); - - // Clean up std::env::remove_var("ZEROCLAW_PROVIDER"); - } - #[test] - fn env_override_provider_fallback() { - let mut config = Config::default(); - - std::env::remove_var("ZEROCLAW_PROVIDER"); + // Fallback: PROVIDER + let mut config2 = Config::default(); std::env::set_var("PROVIDER", "openai"); - config.apply_env_overrides(); - assert_eq!(config.default_provider.as_deref(), Some("openai")); - - // Clean up + config2.apply_env_overrides(); + assert_eq!(config2.default_provider.as_deref(), Some("openai")); std::env::remove_var("PROVIDER"); + + // Empty value should not override + let mut config3 = Config::default(); + let original_provider = config3.default_provider.clone(); + std::env::set_var("ZEROCLAW_PROVIDER", ""); + config3.apply_env_overrides(); + assert_eq!(config3.default_provider, original_provider); + std::env::remove_var("ZEROCLAW_PROVIDER"); } #[test] @@ -1549,108 +1547,82 @@ default_temperature = 0.7 std::env::remove_var("ZEROCLAW_WORKSPACE"); } - #[test] - fn env_override_empty_values_ignored() { - let mut config = Config::default(); - let original_provider = config.default_provider.clone(); - - std::env::set_var("ZEROCLAW_PROVIDER", ""); - config.apply_env_overrides(); - // Empty value should not override - assert_eq!(config.default_provider, original_provider); - - // Clean up - std::env::remove_var("ZEROCLAW_PROVIDER"); - } - #[test] fn env_override_gateway_port() { + // Port, fallback, and invalid tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); + std::env::remove_var("PORT"); + + // Primary: ZEROCLAW_GATEWAY_PORT let mut config = Config::default(); assert_eq!(config.gateway.port, 3000); - std::env::set_var("ZEROCLAW_GATEWAY_PORT", "8080"); config.apply_env_overrides(); assert_eq!(config.gateway.port, 8080); - std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); - } - - #[test] - fn env_override_port_fallback() { - std::env::remove_var("ZEROCLAW_GATEWAY_PORT"); - let mut config = Config::default(); + // Fallback: PORT + let mut config2 = Config::default(); std::env::set_var("PORT", "9000"); - config.apply_env_overrides(); - assert_eq!(config.gateway.port, 9000); + config2.apply_env_overrides(); + assert_eq!(config2.gateway.port, 9000); + + // Invalid PORT is ignored + let mut config3 = Config::default(); + let original_port = config3.gateway.port; + std::env::set_var("PORT", "not_a_number"); + config3.apply_env_overrides(); + assert_eq!(config3.gateway.port, original_port); std::env::remove_var("PORT"); } #[test] fn env_override_gateway_host() { + // Primary and fallback tested together to avoid env-var races. + std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); + std::env::remove_var("HOST"); + + // Primary: ZEROCLAW_GATEWAY_HOST let mut config = Config::default(); assert_eq!(config.gateway.host, "127.0.0.1"); - std::env::set_var("ZEROCLAW_GATEWAY_HOST", "0.0.0.0"); config.apply_env_overrides(); assert_eq!(config.gateway.host, "0.0.0.0"); - std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); - } - - #[test] - fn env_override_host_fallback() { - std::env::remove_var("ZEROCLAW_GATEWAY_HOST"); - let mut config = Config::default(); + // Fallback: HOST + let mut config2 = Config::default(); std::env::set_var("HOST", "0.0.0.0"); - config.apply_env_overrides(); - assert_eq!(config.gateway.host, "0.0.0.0"); - + config2.apply_env_overrides(); + assert_eq!(config2.gateway.host, "0.0.0.0"); std::env::remove_var("HOST"); } #[test] fn env_override_temperature() { + // Valid and out-of-range tested together to avoid env-var races. std::env::remove_var("ZEROCLAW_TEMPERATURE"); - let mut config = Config::default(); + // Valid temperature is applied + let mut config = Config::default(); std::env::set_var("ZEROCLAW_TEMPERATURE", "0.5"); config.apply_env_overrides(); assert!((config.default_temperature - 0.5).abs() < f64::EPSILON); - std::env::remove_var("ZEROCLAW_TEMPERATURE"); - } - - #[test] - fn env_override_temperature_out_of_range_ignored() { - std::env::remove_var("ZEROCLAW_TEMPERATURE"); - let mut config = Config::default(); - let original_temp = config.default_temperature; - + // Out-of-range temperature is ignored + let mut config2 = Config::default(); + let original_temp = config2.default_temperature; std::env::set_var("ZEROCLAW_TEMPERATURE", "3.0"); - config.apply_env_overrides(); + config2.apply_env_overrides(); assert!( - (config.default_temperature - original_temp).abs() < f64::EPSILON, + (config2.default_temperature - original_temp).abs() < f64::EPSILON, "Temperature 3.0 should be ignored (out of range)" ); std::env::remove_var("ZEROCLAW_TEMPERATURE"); } - #[test] - fn env_override_invalid_port_ignored() { - let mut config = Config::default(); - let original_port = config.gateway.port; - - std::env::set_var("PORT", "not_a_number"); - config.apply_env_overrides(); - assert_eq!(config.gateway.port, original_port); - - std::env::remove_var("PORT"); - } - #[test] fn gateway_config_default_values() { let g = GatewayConfig::default();