From 87dcd7a7a059df42e5564f0bbdbeb086f005363e Mon Sep 17 00:00:00 2001
From: fettpl <38704082+fettpl@users.noreply.github.com>
Date: Tue, 17 Feb 2026 13:51:08 +0100
Subject: [PATCH] fix(security): expand git argument sanitization (#523)

* fix(security): expand git argument sanitization

Expand sanitize_git_args() blocklist to also reject --pager=, --editor=,
-c (config injection), --no-verify, and > in arguments. Apply validation
to git_add() paths and git_diff() files argument (previously only called
from git_checkout()). The -c check uses exact match to avoid
false-positives on --cached.

Closes #516

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* style: apply rustfmt to providers/mod.rs

Fix pre-existing formatting issue from main.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/tools/git_operations.rs | 63 +++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/src/tools/git_operations.rs b/src/tools/git_operations.rs
index 9fcb453..8635216 100644
--- a/src/tools/git_operations.rs
+++ b/src/tools/git_operations.rs
@@ -28,13 +28,22 @@ impl GitOperationsTool {
             if arg_lower.starts_with("--exec=")
                 || arg_lower.starts_with("--upload-pack=")
                 || arg_lower.starts_with("--receive-pack=")
+                || arg_lower.starts_with("--pager=")
+                || arg_lower.starts_with("--editor=")
+                || arg_lower == "--no-verify"
                 || arg_lower.contains("$(")
                 || arg_lower.contains('`')
                 || arg.contains('|')
                 || arg.contains(';')
+                || arg.contains('>')
             {
                 anyhow::bail!("Blocked potentially dangerous git argument: {arg}");
             }
+            // Block `-c` config injection (exact match or `-c=...` prefix).
+            // This must not false-positive on `--cached` or `-cached`.
+            if arg_lower == "-c" || arg_lower.starts_with("-c=") {
+                anyhow::bail!("Blocked potentially dangerous git argument: {arg}");
+            }
             result.push(arg.to_string());
         }
         Ok(result)
@@ -129,6 +138,9 @@ impl GitOperationsTool {
             .and_then(|v| v.as_bool())
             .unwrap_or(false);
 
+        // Validate files argument against injection patterns
+        self.sanitize_git_args(files)?;
+
         let mut git_args = vec!["diff", "--unified=3"];
         if cached {
             git_args.push("--cached");
@@ -314,6 +326,9 @@ impl GitOperationsTool {
             .and_then(|v| v.as_str())
             .ok_or_else(|| anyhow::anyhow!("Missing 'paths' parameter"))?;
 
+        // Validate paths against injection patterns
+        self.sanitize_git_args(paths)?;
+
         let output = self.run_git_command(&["add", "--", paths]).await;
 
         match output {
@@ -574,6 +589,52 @@ mod tests {
         assert!(tool.sanitize_git_args("arg; rm file").is_err());
     }
 
+    #[test]
+    fn sanitize_git_blocks_pager_editor_injection() {
+        let tmp = TempDir::new().unwrap();
+        let tool = test_tool(tmp.path());
+
+        assert!(tool.sanitize_git_args("--pager=less").is_err());
+        assert!(tool.sanitize_git_args("--editor=vim").is_err());
+    }
+
+    #[test]
+    fn sanitize_git_blocks_config_injection() {
+        let tmp = TempDir::new().unwrap();
+        let tool = test_tool(tmp.path());
+
+        // Exact `-c` flag (config injection)
+        assert!(tool.sanitize_git_args("-c core.sshCommand=evil").is_err());
+        assert!(tool.sanitize_git_args("-c=core.pager=less").is_err());
+    }
+
+    #[test]
+    fn sanitize_git_blocks_no_verify() {
+        let tmp = TempDir::new().unwrap();
+        let tool = test_tool(tmp.path());
+
+        assert!(tool.sanitize_git_args("--no-verify").is_err());
+    }
+
+    #[test]
+    fn sanitize_git_blocks_redirect_in_args() {
+        let tmp = TempDir::new().unwrap();
+        let tool = test_tool(tmp.path());
+
+        assert!(tool.sanitize_git_args("file.txt > /tmp/out").is_err());
+    }
+
+    #[test]
+    fn sanitize_git_cached_not_blocked() {
+        let tmp = TempDir::new().unwrap();
+        let tool = test_tool(tmp.path());
+
+        // --cached must NOT be blocked by the `-c` check
+        assert!(tool.sanitize_git_args("--cached").is_ok());
+        // Other safe flags starting with -c prefix
+        assert!(tool.sanitize_git_args("-cached").is_ok());
+    }
+
     #[test]
     fn sanitize_git_allows_safe() {
         let tmp = TempDir::new().unwrap();
@@ -583,6 +644,8 @@ mod tests {
         assert!(tool.sanitize_git_args("main").is_ok());
         assert!(tool.sanitize_git_args("feature/test-branch").is_ok());
         assert!(tool.sanitize_git_args("--cached").is_ok());
+        assert!(tool.sanitize_git_args("src/main.rs").is_ok());
+        assert!(tool.sanitize_git_args(".").is_ok());
     }
 
     #[test]