From 9fed8453c7c135ae12d32558e1bff65a678f11b3 Mon Sep 17 00:00:00 2001
From: AI-Reviewer-QS <qs-ai-reviewer@quantstamp.com>
Date: Sun, 15 Feb 2026 05:54:13 +0800
Subject: [PATCH] fix: shell destructive-command check bypassed by
 Value::Object arguments (#72)

Co-authored-by: Yi LIU <yi@quantstamp.com>
Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
---
 src/agent/agent_loop.rs    | 16 +++++++++++----
 src/tools/builtin/shell.rs | 42 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/src/agent/agent_loop.rs b/src/agent/agent_loop.rs
index 02912a15..044981b8 100644
--- a/src/agent/agent_loop.rs
+++ b/src/agent/agent_loop.rs
@@ -1171,10 +1171,18 @@ impl Agent {
                                 && tc.name == "shell"
                                 && let Some(cmd) = tc
                                     .arguments
-                                    .as_str()
-                                    .and_then(|s| serde_json::from_str::<serde_json::Value>(s).ok())
-                                    .and_then(|v| {
-                                        v.get("command").and_then(|c| c.as_str().map(String::from))
+                                    .get("command")
+                                    .and_then(|c| c.as_str().map(String::from))
+                                    .or_else(|| {
+                                        tc.arguments
+                                            .as_str()
+                                            .and_then(|s| {
+                                                serde_json::from_str::<serde_json::Value>(s).ok()
+                                            })
+                                            .and_then(|v| {
+                                                v.get("command")
+                                                    .and_then(|c| c.as_str().map(String::from))
+                                            })
                                     })
                                 && crate::tools::builtin::shell::requires_explicit_approval(&cmd)
                             {
diff --git a/src/tools/builtin/shell.rs b/src/tools/builtin/shell.rs
index b8b948e4..56caba40 100644
--- a/src/tools/builtin/shell.rs
+++ b/src/tools/builtin/shell.rs
@@ -527,6 +527,48 @@ mod tests {
         ));
     }
 
+    /// Replicate the extraction logic from agent_loop.rs to prove it works
+    /// when `arguments` is a `serde_json::Value::Object` (the common case
+    /// that was previously broken because `Value::Object.as_str()` returns None).
+    #[test]
+    fn test_destructive_command_extraction_from_object_args() {
+        let arguments = serde_json::json!({"command": "rm -rf /tmp/stuff"});
+
+        let cmd = arguments
+            .get("command")
+            .and_then(|c| c.as_str().map(String::from))
+            .or_else(|| {
+                arguments
+                    .as_str()
+                    .and_then(|s| serde_json::from_str::<serde_json::Value>(s).ok())
+                    .and_then(|v| v.get("command").and_then(|c| c.as_str().map(String::from)))
+            });
+
+        assert_eq!(cmd.as_deref(), Some("rm -rf /tmp/stuff"));
+        assert!(requires_explicit_approval(cmd.as_deref().unwrap()));
+    }
+
+    /// Verify extraction still works when `arguments` is a JSON string
+    /// (rare, but possible if the LLM provider returns string-encoded JSON).
+    #[test]
+    fn test_destructive_command_extraction_from_string_args() {
+        let arguments =
+            serde_json::Value::String(r#"{"command": "git push --force origin main"}"#.to_string());
+
+        let cmd = arguments
+            .get("command")
+            .and_then(|c| c.as_str().map(String::from))
+            .or_else(|| {
+                arguments
+                    .as_str()
+                    .and_then(|s| serde_json::from_str::<serde_json::Value>(s).ok())
+                    .and_then(|v| v.get("command").and_then(|c| c.as_str().map(String::from)))
+            });
+
+        assert_eq!(cmd.as_deref(), Some("git push --force origin main"));
+        assert!(requires_explicit_approval(cmd.as_deref().unwrap()));
+    }
+
     #[test]
     fn test_sandbox_policy_builder() {
         let tool = ShellTool::new()