From 0847e97b792da5419654f6ca8814db13e4a4f549 Mon Sep 17 00:00:00 2001 From: Alix-007 Date: Wed, 18 Mar 2026 01:39:37 +0800 Subject: [PATCH] fix(channels): allow low-risk shell in non-interactive mode (#3771) Co-authored-by: Alix-007 <267018309+Alix-007@users.noreply.github.com> --- src/agent/loop_.rs | 62 +++++++++++++++++++++++++++++++++++++++++++++ src/approval/mod.rs | 15 +++++++++++ 2 files changed, 77 insertions(+) diff --git a/src/agent/loop_.rs b/src/agent/loop_.rs index 3b0b86f82..412d2f1fd 100644 --- a/src/agent/loop_.rs +++ b/src/agent/loop_.rs @@ -4731,6 +4731,68 @@ mod tests { assert!(tool_results.content.contains("Skipped duplicate tool call")); } + #[tokio::test] + async fn run_tool_call_loop_allows_low_risk_shell_in_non_interactive_mode() { + let provider = ScriptedProvider::from_text_responses(vec![ + r#" +{"name":"shell","arguments":{"command":"echo hello"}} +"#, + "done", + ]); + + let tmp = TempDir::new().expect("temp dir"); + let security = Arc::new(crate::security::SecurityPolicy { + autonomy: crate::security::AutonomyLevel::Supervised, + workspace_dir: tmp.path().to_path_buf(), + ..crate::security::SecurityPolicy::default() + }); + let runtime: Arc = + Arc::new(crate::runtime::NativeRuntime::new()); + let tools_registry: Vec> = vec![Box::new( + crate::tools::shell::ShellTool::new(security, runtime), + )]; + + let mut history = vec![ + ChatMessage::system("test-system"), + ChatMessage::user("run shell"), + ]; + let observer = NoopObserver; + let approval_mgr = + ApprovalManager::for_non_interactive(&crate::config::AutonomyConfig::default()); + + let result = run_tool_call_loop( + &provider, + &mut history, + &tools_registry, + &observer, + "mock-provider", + "mock-model", + 0.0, + true, + Some(&approval_mgr), + "telegram", + &crate::config::MultimodalConfig::default(), + 4, + None, + None, + None, + &[], + &[], + None, + ) + .await + .expect("non-interactive shell should succeed for low-risk command"); + + assert_eq!(result, "done"); + + let tool_results = history + .iter() + .find(|msg| msg.role == "user" && msg.content.starts_with("[Tool results]")) + .expect("tool results message should be present"); + assert!(tool_results.content.contains("hello")); + assert!(!tool_results.content.contains("Denied by user.")); + } + #[tokio::test] async fn run_tool_call_loop_dedup_exempt_allows_repeated_calls() { let provider = ScriptedProvider::from_text_responses(vec![ diff --git a/src/approval/mod.rs b/src/approval/mod.rs index a3060488c..3b2b9a4f6 100644 --- a/src/approval/mod.rs +++ b/src/approval/mod.rs @@ -126,6 +126,15 @@ impl ApprovalManager { return true; } + // Channel-driven shell execution is still guarded by the shell tool's + // own command allowlist and risk policy. Skipping the outer approval + // gate here lets low-risk allowlisted commands (e.g. `ls`) work in + // non-interactive channels without silently allowing medium/high-risk + // commands. + if self.non_interactive && tool_name == "shell" { + return false; + } + // auto_approve skips the prompt. if self.auto_approve.contains(tool_name) { return false; @@ -456,6 +465,12 @@ mod tests { assert!(!mgr.needs_approval("memory_recall")); } + #[test] + fn non_interactive_shell_skips_outer_approval_by_default() { + let mgr = ApprovalManager::for_non_interactive(&AutonomyConfig::default()); + assert!(!mgr.needs_approval("shell")); + } + #[test] fn non_interactive_always_ask_tools_need_approval() { let mgr = ApprovalManager::for_non_interactive(&supervised_config());