From 3bdabdc7ec720c9819d78000b96f88d5490f094d Mon Sep 17 00:00:00 2001 From: Chummy Date: Mon, 16 Feb 2026 14:57:58 +0800 Subject: [PATCH] fix(security): enforce action guards in file_write and scheduler (#269) --- src/cron/scheduler.rs | 49 ++++++++++++++++++++++++ src/tools/file_write.rs | 83 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+) diff --git a/src/cron/scheduler.rs b/src/cron/scheduler.rs index 0453999..bab1965 100644 --- a/src/cron/scheduler.rs +++ b/src/cron/scheduler.rs @@ -138,6 +138,20 @@ async fn run_job_command( security: &SecurityPolicy, job: &CronJob, ) -> (bool, String) { + if !security.can_act() { + return ( + false, + "blocked by security policy: autonomy is read-only".to_string(), + ); + } + + if security.is_rate_limited() { + return ( + false, + "blocked by security policy: rate limit exceeded".to_string(), + ); + } + if !security.is_command_allowed(&job.command) { return ( false, @@ -155,6 +169,13 @@ async fn run_job_command( ); } + if !security.record_action() { + return ( + false, + "blocked by security policy: action budget exhausted".to_string(), + ); + } + let output = Command::new("sh") .arg("-lc") .arg(&job.command) @@ -261,6 +282,34 @@ mod tests { assert!(output.contains("/etc/passwd")); } + #[tokio::test] + async fn run_job_command_blocks_readonly_mode() { + let tmp = TempDir::new().unwrap(); + let mut config = test_config(&tmp); + config.autonomy.level = crate::security::AutonomyLevel::ReadOnly; + let job = test_job("echo should-not-run"); + let security = SecurityPolicy::from_config(&config.autonomy, &config.workspace_dir); + + let (success, output) = run_job_command(&config, &security, &job).await; + assert!(!success); + assert!(output.contains("blocked by security policy")); + assert!(output.contains("read-only")); + } + + #[tokio::test] + async fn run_job_command_blocks_rate_limited() { + let tmp = TempDir::new().unwrap(); + let mut config = test_config(&tmp); + config.autonomy.max_actions_per_hour = 0; + let job = test_job("echo should-not-run"); + let security = SecurityPolicy::from_config(&config.autonomy, &config.workspace_dir); + + let (success, output) = run_job_command(&config, &security, &job).await; + assert!(!success); + assert!(output.contains("blocked by security policy")); + assert!(output.contains("rate limit exceeded")); + } + #[tokio::test] async fn execute_job_with_retry_recovers_after_first_failure() { let tmp = TempDir::new().unwrap(); diff --git a/src/tools/file_write.rs b/src/tools/file_write.rs index 7b0079d..620487f 100644 --- a/src/tools/file_write.rs +++ b/src/tools/file_write.rs @@ -53,6 +53,22 @@ impl Tool for FileWriteTool { .and_then(|v| v.as_str()) .ok_or_else(|| anyhow::anyhow!("Missing 'content' parameter"))?; + if !self.security.can_act() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Action blocked: autonomy is read-only".into()), + }); + } + + if self.security.is_rate_limited() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded: too many actions in the last hour".into()), + }); + } + // Security check: validate path is within workspace if !self.security.is_path_allowed(path) { return Ok(ToolResult { @@ -122,6 +138,14 @@ impl Tool for FileWriteTool { } } + if !self.security.record_action() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded: action budget exhausted".into()), + }); + } + match tokio::fs::write(&resolved_target, content).await { Ok(()) => Ok(ToolResult { success: true, @@ -150,6 +174,19 @@ mod tests { }) } + fn test_security_with( + workspace: std::path::PathBuf, + autonomy: AutonomyLevel, + max_actions_per_hour: u32, + ) -> Arc { + Arc::new(SecurityPolicy { + autonomy, + workspace_dir: workspace, + max_actions_per_hour, + ..SecurityPolicy::default() + }) + } + #[test] fn file_write_name() { let tool = FileWriteTool::new(test_security(std::env::temp_dir())); @@ -324,4 +361,50 @@ mod tests { let _ = tokio::fs::remove_dir_all(&root).await; } + + #[tokio::test] + async fn file_write_blocks_readonly_mode() { + let dir = std::env::temp_dir().join("zeroclaw_test_file_write_readonly"); + let _ = tokio::fs::remove_dir_all(&dir).await; + tokio::fs::create_dir_all(&dir).await.unwrap(); + + let tool = FileWriteTool::new(test_security_with(dir.clone(), AutonomyLevel::ReadOnly, 20)); + let result = tool + .execute(json!({"path": "out.txt", "content": "should-block"})) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_deref().unwrap_or("").contains("read-only")); + assert!(!dir.join("out.txt").exists()); + + let _ = tokio::fs::remove_dir_all(&dir).await; + } + + #[tokio::test] + async fn file_write_blocks_when_rate_limited() { + let dir = std::env::temp_dir().join("zeroclaw_test_file_write_rate_limited"); + let _ = tokio::fs::remove_dir_all(&dir).await; + tokio::fs::create_dir_all(&dir).await.unwrap(); + + let tool = FileWriteTool::new(test_security_with( + dir.clone(), + AutonomyLevel::Supervised, + 0, + )); + let result = tool + .execute(json!({"path": "out.txt", "content": "should-block"})) + .await + .unwrap(); + + assert!(!result.success); + assert!(result + .error + .as_deref() + .unwrap_or("") + .contains("Rate limit exceeded")); + assert!(!dir.join("out.txt").exists()); + + let _ = tokio::fs::remove_dir_all(&dir).await; + } }