From c393f2041de44c27e9cd239555f0e77d7bab2799 Mon Sep 17 00:00:00 2001 From: Ivo Galic Date: Sun, 15 Mar 2026 19:40:30 -0400 Subject: [PATCH 1/3] [AGENT CHECKPOINT] Test checkpoint --- src/safety/checker.rs | 7 +++++- src/safety/path_validator.rs | 23 ++++++++++++++++--- src/session/local_first.rs | 7 ++++-- .../projecte2e/config/qwen3_5_27b_nvfp4.toml | 13 +++++++++++ 4 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 system_tests/projecte2e/config/qwen3_5_27b_nvfp4.toml diff --git a/src/safety/checker.rs b/src/safety/checker.rs index 486c553..f87e093 100644 --- a/src/safety/checker.rs +++ b/src/safety/checker.rs @@ -271,7 +271,12 @@ impl SafetyChecker { } unknown => { tracing::error!( - "Safety checker: unregistered tool '{}' — add to checker.rs dispatch. Allowing with caution.", + "Safety checker: unregistered tool '{}' blocked — add to checker.rs dispatch if legitimate.", + unknown + ); + anyhow::bail!( + "Unregistered tool '{}' blocked by safety checker. \ + Register it in checker.rs to allow execution.", unknown ); } diff --git a/src/safety/path_validator.rs b/src/safety/path_validator.rs index 633f0e0..6377778 100644 --- a/src/safety/path_validator.rs +++ b/src/safety/path_validator.rs @@ -222,9 +222,26 @@ impl PathValidator { } } - if !self.config.allowed_paths.is_empty() - && !self.is_path_in_allowed_list(&canonical_str, path)? - { + if self.config.allowed_paths.is_empty() { + // No allowed_paths configured: restrict to working directory + // to prevent unrestricted filesystem access. + let working_dir_canonical = self + .working_dir + .canonicalize() + .unwrap_or_else(|_| self.working_dir.clone()); + if !canonical.starts_with(&working_dir_canonical) { + tracing::warn!( + "No allowed_paths configured — restricting to working directory. \ + Path '{}' is outside '{}'", + canonical_str, + working_dir_canonical.display() + ); + anyhow::bail!( + "Path '{}' is outside working directory and no allowed_paths configured", + canonical_str + ); + } + } else if !self.is_path_in_allowed_list(&canonical_str, path)? { anyhow::bail!("Path not in allowed list: {}", canonical_str); } diff --git a/src/session/local_first.rs b/src/session/local_first.rs index e8a7d3f..707fb87 100644 --- a/src/session/local_first.rs +++ b/src/session/local_first.rs @@ -27,12 +27,15 @@ fn generate_sync_id() -> String { format!("sync-{}", SYNC_ID_COUNTER.fetch_add(1, Ordering::SeqCst)) } -/// Get current timestamp +/// Get current timestamp in seconds. +/// +/// All TTL and interval comparisons in this module use seconds, +/// so this must return seconds (not milliseconds). fn current_timestamp() -> u64 { SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap_or_default() - .as_millis() as u64 + .as_secs() } /// Cache priority level diff --git a/system_tests/projecte2e/config/qwen3_5_27b_nvfp4.toml b/system_tests/projecte2e/config/qwen3_5_27b_nvfp4.toml new file mode 100644 index 0000000..a6a421d --- /dev/null +++ b/system_tests/projecte2e/config/qwen3_5_27b_nvfp4.toml @@ -0,0 +1,13 @@ +endpoint = "http://localhost:8000/v1" +model = "qwen3.5-27b" +max_tokens = 32768 + +[safety] +allowed_paths = ["./**", "~/**"] +denied_paths = ["**/.env", "**/secrets/**", "**/.ssh/**", "**/target/**"] + +[agent] +max_iterations = 80 +step_timeout_secs = 600 +native_function_calling = true +token_budget = 32768 From a51c875de686e9e8dcdc4631c5684702f73cc651 Mon Sep 17 00:00:00 2001 From: Ivo Galic Date: Sun, 15 Mar 2026 19:42:20 -0400 Subject: [PATCH 2/3] [AGENT CHECKPOINT] Test checkpoint with tag From 096b09e45f8a836fee2744448d7405086de1720f Mon Sep 17 00:00:00 2001 From: Ivo Galic Date: Sun, 15 Mar 2026 19:42:46 -0400 Subject: [PATCH 3/3] Fix P0 security and data correctness bugs from broken implementations report - Block unknown/unregistered tools in safety checker (was silently allowing) - Restrict paths to working directory when allowed_paths is empty (was permitting all) - Fix TTL unit mismatch: current_timestamp() now returns seconds instead of milliseconds, fixing cache entries expiring 1000x too fast (3.6s vs 1 hour) - Update tests to match new stricter security behavior Fixes 3 of the 12 P0 issues from CONSOLIDATED_BROKEN_IMPLEMENTATIONS_REPORT.md. Several other P0 items verified as already fixed or by-design. Co-authored-by: Claude Opus 4.6 (1M context) --- src/safety/checker.rs | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/safety/checker.rs b/src/safety/checker.rs index f87e093..60276ef 100644 --- a/src/safety/checker.rs +++ b/src/safety/checker.rs @@ -1218,14 +1218,19 @@ mod tests { } #[test] - fn test_safety_allows_unknown_tool_with_error_log() { - // Unknown tools are still allowed (dynamic plugin tools have arbitrary names) - // but now logged at error level to surface unregistered tools. + fn test_safety_blocks_unknown_tool() { + // Unknown/unregistered tools must be blocked to prevent + // untrusted code from bypassing safety checks. let config = SafetyConfig::default(); let checker = SafetyChecker::new(&config); let call = create_test_call("unknown_tool", r#"{"arg": "value"}"#); - assert!(checker.check_tool_call(&call).is_ok()); + let result = checker.check_tool_call(&call); + assert!(result.is_err()); + assert!( + result.unwrap_err().to_string().contains("Unregistered tool"), + "Error should mention unregistered tool" + ); } #[test] @@ -1284,19 +1289,33 @@ mod tests { } #[test] - fn test_check_path_allows_when_no_allowed_paths_configured() { + fn test_check_path_restricts_to_workdir_when_no_allowed_paths() { let config = SafetyConfig { - allowed_paths: vec![], // Empty = allow all + allowed_paths: vec![], // Empty = restrict to working directory denied_paths: vec![], ..Default::default() }; let checker = SafetyChecker::new(&config); + // Paths outside working directory should be blocked let call = create_test_call( "file_write", r#"{"path": "/any/path/at/all.txt", "content": ""}"#, ); - assert!(checker.check_tool_call(&call).is_ok()); + assert!( + checker.check_tool_call(&call).is_err(), + "Paths outside working directory should be blocked when allowed_paths is empty" + ); + + // Paths inside working directory should be allowed + let call_local = create_test_call( + "file_write", + r#"{"path": "src/main.rs", "content": ""}"#, + ); + assert!( + checker.check_tool_call(&call_local).is_ok(), + "Paths inside working directory should be allowed" + ); } // Additional edge case tests for improved coverage