diff --git a/src/tools/screenshot.rs b/src/tools/screenshot.rs index 7581bc1..e7911c3 100644 --- a/src/tools/screenshot.rs +++ b/src/tools/screenshot.rs @@ -68,6 +68,17 @@ impl ScreenshotTool { |n| n.to_string_lossy().to_string(), ); + // Reject filenames with shell-breaking characters to prevent injection in sh -c + const SHELL_UNSAFE: &[char] = + &['\'', '"', '`', '$', '\\', ';', '|', '&', '\n', '\0', '(', ')']; + if safe_name.contains(SHELL_UNSAFE) { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Filename contains characters unsafe for shell execution".into()), + }); + } + let output_path = self.security.workspace_dir.join(&safe_name); let output_str = output_path.to_string_lossy().to_string(); @@ -288,6 +299,20 @@ mod tests { assert!(!args.is_empty()); } + #[tokio::test] + async fn screenshot_rejects_shell_injection_filename() { + let tool = ScreenshotTool::new(test_security()); + let result = tool + .execute(json!({"filename": "test'injection.png"})) + .await + .unwrap(); + assert!(!result.success); + assert!(result + .error + .unwrap() + .contains("unsafe for shell execution")); + } + #[test] fn screenshot_command_contains_output_path() { let cmd = ScreenshotTool::screenshot_command("/tmp/my_screenshot.png").unwrap();