fix(permissions): enforce manual approval for code mode (#10528)

crates/goose/src/agents/agent.rs

@@ -757,10 +757,6 @@ impl Agent {
 
         let goose_mode = *self.current_goose_mode.lock().await;
 
-        if goose_mode == GooseMode::SmartApprove {
-            self.tool_inspection_manager.apply_tool_annotations(&tools);
-        }
-
         let tool_call_cut_off = match Config::global().get_param::<usize>("GOOSE_TOOL_CALL_CUTOFF")
         {
             Ok(v) => v,

crates/goose/src/agents/platform_extensions/code_execution.rs

@@ -497,11 +497,11 @@ impl McpClientTrait for CodeExecutionClient {
                         schema::<ExecuteBashInput>(),
                     )
                     .annotate(ToolAnnotations::from_raw(
-                        Some("Get function details".to_string()),
-                        Some(true),
+                        Some("Execute Bash".to_string()),
                         Some(false),
                         Some(true),
                         Some(false),
+                        Some(true),
                     )),
                     McpTool::new(
                         "execute_typescript".to_string(),
@@ -874,6 +874,40 @@ mod tests {
         assert!(!moim.contains("ask_heimdall"));
     }
 
+    #[tokio::test]
+    async fn execute_bash_annotations_require_approval() {
+        let temp = tempfile::tempdir().unwrap();
+        let client = CodeExecutionClient::new(
+            PlatformExtensionContext {
+                extension_manager: None,
+                session_manager: Arc::new(crate::session::SessionManager::new(
+                    temp.path().join("sessions"),
+                )),
+                session: None,
+                use_login_shell_path: false,
+            },
+            ToolDisclosure::Filesystem,
+        )
+        .unwrap();
+
+        let tools = client
+            .list_tools("test", None, CancellationToken::new())
+            .await
+            .unwrap()
+            .tools;
+        let execute_bash = tools
+            .iter()
+            .find(|tool| tool.name == "execute_bash")
+            .unwrap();
+        let annotations = execute_bash.annotations.as_ref().unwrap();
+
+        assert_eq!(annotations.title.as_deref(), Some("Execute Bash"));
+        assert_eq!(annotations.read_only_hint, Some(false));
+        assert_eq!(annotations.destructive_hint, Some(true));
+        assert_eq!(annotations.idempotent_hint, Some(false));
+        assert_eq!(annotations.open_world_hint, Some(true));
+    }
+
     fn self_referential_any_schema() -> Value {
         json!({
             "$ref": "#/$defs/Any",

crates/goose/src/agents/reply_parts.rs

@@ -11,7 +11,7 @@ use tracing::debug;
 use super::super::agents::Agent;
 #[cfg(feature = "code-mode")]
 use crate::agents::platform_extensions::code_execution;
-use crate::config::Config;
+use crate::config::{Config, GooseMode};
 use crate::conversation::message::{Message, MessageContent, MessageUsage, ToolRequest};
 use crate::conversation::{fix_conversation, Conversation};
 #[cfg(test)]
@@ -251,6 +251,10 @@ impl Agent {
 
         let goose_mode = *self.current_goose_mode.lock().await;
 
+        if goose_mode == GooseMode::SmartApprove {
+            self.tool_inspection_manager.apply_tool_annotations(&tools);
+        }
+
         let prompt_manager = self.prompt_manager.lock().await;
         let mut system_prompt = prompt_manager
             .builder()
@@ -672,6 +676,7 @@ pub fn is_tool_visible_to_model(tool: &Tool) -> bool {
 mod tests {
     use super::*;
     use crate::agents::{AgentConfig, GoosePlatform};
+    use crate::config::permission::PermissionLevel;
     use crate::config::{GooseMode, PermissionManager};
     use crate::conversation::message::{Message, SystemNotificationType};
     use crate::providers::base::Provider;
@@ -679,7 +684,7 @@ mod tests {
     use async_trait::async_trait;
     use goose_providers::conversation::token_usage::{ProviderStats, ProviderUsage, Usage};
     use goose_providers::model::ModelConfig;
-    use rmcp::model::{AnnotateAble, RawTextContent, Role};
+    use rmcp::model::{AnnotateAble, RawTextContent, Role, ToolAnnotations};
     use rmcp::object;
     use std::sync::Mutex;
     use std::time::{Duration, Instant};
@@ -935,6 +940,69 @@ mod tests {
         Ok(())
     }
 
+    #[tokio::test]
+    async fn prepare_toolshim_tools_applies_writable_annotations() -> anyhow::Result<()> {
+        let data_dir = tempfile::tempdir()?;
+        let data_path = data_dir.path().to_path_buf();
+        let session_manager = Arc::new(SessionManager::new(data_path.clone()));
+        let permission_manager = Arc::new(PermissionManager::new(data_path));
+        permission_manager
+            .update_smart_approve_permission("frontend__write_tool", PermissionLevel::AlwaysAllow);
+        let agent = Agent::with_config(AgentConfig::new(
+            Arc::clone(&session_manager),
+            Arc::clone(&permission_manager),
+            None,
+            GooseMode::SmartApprove,
+            false,
+            GoosePlatform::GooseCli,
+        ));
+        let session = session_manager
+            .create_session(
+                std::env::current_dir()?,
+                "test-toolshim-annotations".to_string(),
+                SessionType::Hidden,
+                GooseMode::SmartApprove,
+            )
+            .await?;
+        let model_config = ModelConfig::new("test-model").with_toolshim(true);
+        agent
+            .update_provider(Arc::new(MockProvider), model_config, &session.id)
+            .await?;
+        agent
+            .add_extension(
+                crate::agents::extension::ExtensionConfig::Frontend {
+                    name: "frontend".to_string(),
+                    description: "desc".to_string(),
+                    tools: vec![Tool::new(
+                        "frontend__write_tool",
+                        "Write tool",
+                        object!({ "type": "object", "properties": { } }),
+                    )
+                    .annotate(ToolAnnotations::new().read_only(false))],
+                    instructions: None,
+                    bundled: None,
+                    available_tools: vec![],
+                },
+                &session.id,
+            )
+            .await?;
+
+        let (tools, toolshim_tools, _, _) = agent
+            .prepare_tools_and_prompt(&session.id, session.working_dir.as_path())
+            .await?;
+
+        assert!(tools.is_empty());
+        assert!(toolshim_tools
+            .iter()
+            .any(|tool| tool.name == "frontend__write_tool"));
+        assert_eq!(
+            permission_manager.get_smart_approve_permission("frontend__write_tool"),
+            Some(PermissionLevel::AskBefore)
+        );
+
+        Ok(())
+    }
+
     #[tokio::test]
     async fn test_stream_error_propagation() {
         use futures::StreamExt;

crates/goose/src/permission/permission_inspector.rs

@@ -169,8 +169,10 @@ impl ToolInspector for PermissionInspector {
                                     InspectionAction::RequireApproval(None)
                                 }
                             }
-                        // 2. Check if the tool is explicitly annotated as read-only
-                        } else if self.is_readonly_annotated_tool(tool_name) {
+                        // 2. Check for a read-only annotation in SmartApprove mode
+                        } else if goose_mode == GooseMode::SmartApprove
+                            && self.is_readonly_annotated_tool(tool_name)
+                        {
                             InspectionAction::Allow
                         // 3. Special case for extension management
                         } else if tool_name == MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE {
@@ -321,6 +323,7 @@ mod tests {
     #[test_case(GooseMode::SmartApprove, false, None, InspectionAction::RequireApproval(None); "smart_approve_unknown_defers")]
     #[test_case(GooseMode::Approve, false, None, InspectionAction::RequireApproval(None); "approve_requires_approval")]
     #[test_case(GooseMode::Approve, false, Some(PermissionLevel::AlwaysAllow), InspectionAction::RequireApproval(None); "approve_ignores_cache")]
+    #[test_case(GooseMode::Approve, true, None, InspectionAction::RequireApproval(None); "approve_ignores_annotation")]
     #[tokio::test]
     async fn test_inspect_action(
         mode: GooseMode,