From 290d971d5ec4ae8e858490ddbc74d0108b686df7 Mon Sep 17 00:00:00 2001
From: Alex Gorevski <agorevski@users.noreply.github.com>
Date: Tue, 17 Feb 2026 12:13:17 -0800
Subject: [PATCH] fix(security): reject shell-unsafe chars in screenshot
 filename (#625)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #601

The Linux screenshot path uses sh -c with single-quote interpolation.
A filename containing quote characters could break quoting and inject
shell tokens. Add a check that rejects filenames with any shell-breaking
characters (quotes, backticks, dollar signs, semicolons, pipes, etc.)
before passing to the shell command.

Severity: High — command injection in tool execution path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 src/tools/screenshot.rs | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

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();