fix(security): enforce action guards in file_write and scheduler (#269)
This commit is contained in:
parent
60f3282ad4
commit
3bdabdc7ec
2 changed files with 132 additions and 0 deletions
|
|
@ -138,6 +138,20 @@ async fn run_job_command(
|
||||||
security: &SecurityPolicy,
|
security: &SecurityPolicy,
|
||||||
job: &CronJob,
|
job: &CronJob,
|
||||||
) -> (bool, String) {
|
) -> (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) {
|
if !security.is_command_allowed(&job.command) {
|
||||||
return (
|
return (
|
||||||
false,
|
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")
|
let output = Command::new("sh")
|
||||||
.arg("-lc")
|
.arg("-lc")
|
||||||
.arg(&job.command)
|
.arg(&job.command)
|
||||||
|
|
@ -261,6 +282,34 @@ mod tests {
|
||||||
assert!(output.contains("/etc/passwd"));
|
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]
|
#[tokio::test]
|
||||||
async fn execute_job_with_retry_recovers_after_first_failure() {
|
async fn execute_job_with_retry_recovers_after_first_failure() {
|
||||||
let tmp = TempDir::new().unwrap();
|
let tmp = TempDir::new().unwrap();
|
||||||
|
|
|
||||||
|
|
@ -53,6 +53,22 @@ impl Tool for FileWriteTool {
|
||||||
.and_then(|v| v.as_str())
|
.and_then(|v| v.as_str())
|
||||||
.ok_or_else(|| anyhow::anyhow!("Missing 'content' parameter"))?;
|
.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
|
// Security check: validate path is within workspace
|
||||||
if !self.security.is_path_allowed(path) {
|
if !self.security.is_path_allowed(path) {
|
||||||
return Ok(ToolResult {
|
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 {
|
match tokio::fs::write(&resolved_target, content).await {
|
||||||
Ok(()) => Ok(ToolResult {
|
Ok(()) => Ok(ToolResult {
|
||||||
success: true,
|
success: true,
|
||||||
|
|
@ -150,6 +174,19 @@ mod tests {
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn test_security_with(
|
||||||
|
workspace: std::path::PathBuf,
|
||||||
|
autonomy: AutonomyLevel,
|
||||||
|
max_actions_per_hour: u32,
|
||||||
|
) -> Arc<SecurityPolicy> {
|
||||||
|
Arc::new(SecurityPolicy {
|
||||||
|
autonomy,
|
||||||
|
workspace_dir: workspace,
|
||||||
|
max_actions_per_hour,
|
||||||
|
..SecurityPolicy::default()
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn file_write_name() {
|
fn file_write_name() {
|
||||||
let tool = FileWriteTool::new(test_security(std::env::temp_dir()));
|
let tool = FileWriteTool::new(test_security(std::env::temp_dir()));
|
||||||
|
|
@ -324,4 +361,50 @@ mod tests {
|
||||||
|
|
||||||
let _ = tokio::fs::remove_dir_all(&root).await;
|
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;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue