Cherry-pick removing default-allow tool call patterns (#47392)

Richard Feldman created 2 months ago

Nothing should be auto-allowed by default. Users can add their own
always_allow patterns if they want to skip confirmation for specific
commands.

Keeps default always_deny and always_confirm patterns for dangerous
commands like rm -rf, sudo, etc.

Release Notes:

- N/A

Change summary

assets/settings/default.json                | 22 ----------
crates/agent_settings/src/agent_settings.rs | 47 +---------------------
2 files changed, 3 insertions(+), 66 deletions(-)

Detailed changes

assets/settings/default.json

@@ -997,22 +997,6 @@
             // Privileged commands
             { "pattern": "sudo\\s" },
           ],
-          "always_allow": [
-            // Build and test commands
-            { "pattern": "^cargo\\s+(build|test|check|clippy|run)" },
-            { "pattern": "^npm\\s+(test|run|install)" },
-            { "pattern": "^pnpm\\s+(test|run|install)" },
-            { "pattern": "^yarn\\s+(test|run|install)" },
-            // Safe read-only commands
-            { "pattern": "^ls(\\s|$)" },
-            { "pattern": "^cat\\s" },
-            { "pattern": "^head\\s" },
-            { "pattern": "^tail\\s" },
-            { "pattern": "^grep\\s" },
-            { "pattern": "^find\\s" },
-            // Safe git commands
-            { "pattern": "^git\\s+(status|log|diff|branch|show)" },
-          ],
         },
         "edit_file": {
           "default_mode": "confirm",
@@ -1048,12 +1032,6 @@
         },
         "fetch": {
           "default_mode": "confirm",
-          "always_allow": [
-            // Common documentation sites
-            { "pattern": "^https://(docs\\.|api\\.)?github\\.com" },
-            { "pattern": "^https://docs\\.rs" },
-            { "pattern": "^https://crates\\.io" },
-          ],
         },
       },
     },

crates/agent_settings/src/agent_settings.rs

@@ -600,10 +600,6 @@ mod tests {
             !terminal.always_confirm.is_empty(),
             "terminal should have confirm rules"
         );
-        assert!(
-            !terminal.always_allow.is_empty(),
-            "terminal should have allow rules"
-        );
 
         let edit_file = permissions
             .tools
@@ -623,13 +619,10 @@ mod tests {
             "delete_path should have deny rules"
         );
 
-        let fetch = permissions
-            .tools
-            .get("fetch")
-            .expect("fetch tool should be configured");
+        // fetch tool should be configured (with default_mode: confirm)
         assert!(
-            !fetch.always_allow.is_empty(),
-            "fetch should have allow rules"
+            permissions.tools.contains_key("fetch"),
+            "fetch tool should be configured"
         );
     }
 
@@ -665,40 +658,6 @@ mod tests {
         }
     }
 
-    #[test]
-    fn test_default_allow_rules_match_safe_commands() {
-        let default_json = include_str!("../../../assets/settings/default.json");
-        let value: serde_json::Value = serde_json_lenient::from_str(default_json).unwrap();
-        let tool_permissions = value["agent"]["tool_permissions"].clone();
-        let content: ToolPermissionsContent = serde_json::from_value(tool_permissions).unwrap();
-        let permissions = compile_tool_permissions(Some(content));
-
-        let terminal = permissions.tools.get("terminal").unwrap();
-
-        let safe_commands = [
-            "cargo build",
-            "cargo test",
-            "cargo check",
-            "npm test",
-            "pnpm install",
-            "yarn run build",
-            "ls",
-            "ls -la",
-            "cat file.txt",
-            "git status",
-            "git log",
-            "git diff",
-        ];
-
-        for cmd in &safe_commands {
-            assert!(
-                terminal.always_allow.iter().any(|r| r.is_match(cmd)),
-                "Command '{}' should be allowed by allow rules",
-                cmd
-            );
-        }
-    }
-
     #[test]
     fn test_deny_takes_precedence_over_allow_and_confirm() {
         let json = json!({