From 60f3282ad439e9ef5d33c154fd6005904db22449 Mon Sep 17 00:00:00 2001 From: Chummy Date: Mon, 16 Feb 2026 14:57:56 +0800 Subject: [PATCH] fix(security): enforce action budget checks in file_read (#270) --- src/tools/file_read.rs | 80 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 3 deletions(-) diff --git a/src/tools/file_read.rs b/src/tools/file_read.rs index 264dcc4..eee80d2 100644 --- a/src/tools/file_read.rs +++ b/src/tools/file_read.rs @@ -4,6 +4,8 @@ use async_trait::async_trait; use serde_json::json; use std::sync::Arc; +const MAX_FILE_SIZE_BYTES: u64 = 10 * 1024 * 1024; + /// Read file contents with path sandboxing pub struct FileReadTool { security: Arc, @@ -44,6 +46,14 @@ impl Tool for FileReadTool { .and_then(|v| v.as_str()) .ok_or_else(|| anyhow::anyhow!("Missing 'path' parameter"))?; + 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 { @@ -79,15 +89,14 @@ impl Tool for FileReadTool { } // Check file size AFTER canonicalization to prevent TOCTOU symlink bypass - const MAX_FILE_SIZE: u64 = 10 * 1024 * 1024; match tokio::fs::metadata(&resolved_path).await { Ok(meta) => { - if meta.len() > MAX_FILE_SIZE { + if meta.len() > MAX_FILE_SIZE_BYTES { return Ok(ToolResult { success: false, output: String::new(), error: Some(format!( - "File too large: {} bytes (limit: {MAX_FILE_SIZE} bytes)", + "File too large: {} bytes (limit: {MAX_FILE_SIZE_BYTES} bytes)", meta.len() )), }); @@ -102,6 +111,14 @@ impl Tool for FileReadTool { } } + 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::read_to_string(&resolved_path).await { Ok(contents) => Ok(ToolResult { success: true, @@ -130,6 +147,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_read_name() { let tool = FileReadTool::new(test_security(std::env::temp_dir())); @@ -204,6 +234,50 @@ mod tests { assert!(result.error.as_ref().unwrap().contains("not allowed")); } + #[tokio::test] + async fn file_read_blocks_when_rate_limited() { + let dir = std::env::temp_dir().join("zeroclaw_test_file_read_rate_limited"); + let _ = tokio::fs::remove_dir_all(&dir).await; + tokio::fs::create_dir_all(&dir).await.unwrap(); + tokio::fs::write(dir.join("test.txt"), "hello world") + .await + .unwrap(); + + let tool = FileReadTool::new(test_security_with( + dir.clone(), + AutonomyLevel::Supervised, + 0, + )); + let result = tool.execute(json!({"path": "test.txt"})).await.unwrap(); + + assert!(!result.success); + assert!(result + .error + .as_deref() + .unwrap_or("") + .contains("Rate limit exceeded")); + + let _ = tokio::fs::remove_dir_all(&dir).await; + } + + #[tokio::test] + async fn file_read_allows_readonly_mode() { + let dir = std::env::temp_dir().join("zeroclaw_test_file_read_readonly"); + let _ = tokio::fs::remove_dir_all(&dir).await; + tokio::fs::create_dir_all(&dir).await.unwrap(); + tokio::fs::write(dir.join("test.txt"), "readonly ok") + .await + .unwrap(); + + let tool = FileReadTool::new(test_security_with(dir.clone(), AutonomyLevel::ReadOnly, 20)); + let result = tool.execute(json!({"path": "test.txt"})).await.unwrap(); + + assert!(result.success); + assert_eq!(result.output, "readonly ok"); + + let _ = tokio::fs::remove_dir_all(&dir).await; + } + #[tokio::test] async fn file_read_missing_path_param() { let tool = FileReadTool::new(test_security(std::env::temp_dir()));