Fix unsafe auto-approval of git branch deletion

Diagnosis: Although the bug report specifically mentioned worktrees, this problm isn't worktree-specific. `git branch -d/-D/--delete` isn’t treated as dangerous, and git branch is currently treated as “safe,” so it won’t prompt under on-request. In a worktree, that’s extra risky because branch deletion writes to the common gitdir outside the workdir.

* Classify git branch -d/-D/--delete as dangerous to force approval under on-request.
* Restrict git branch “safe” classification to read-only forms (e.g., --show-current, --list).
* Add focused safety tests for branch deletion and read-only branch queries.

This addresses #10160

codex-rs/core/src/command_safety/is_dangerous_command.rs

@@ -32,7 +32,11 @@ fn is_dangerous_to_call_with_exec(command: &[String]) -> bool {
 
     match cmd0 {
         Some(cmd) if cmd.ends_with("git") || cmd.ends_with("/git") => {
-            matches!(command.get(1).map(String::as_str), Some("reset" | "rm"))
+            match command.get(1).map(String::as_str) {
+                Some("reset" | "rm") => true,
+                Some("branch") => git_branch_is_delete(command),
+                _ => false,
+            }
         }
 
         Some("rm") => matches!(command.get(1).map(String::as_str), Some("-f" | "-rf")),
@@ -45,9 +49,19 @@ fn is_dangerous_to_call_with_exec(command: &[String]) -> bool {
     }
 }
 
+fn git_branch_is_delete(command: &[String]) -> bool {
+    command.iter().skip(2).any(|arg| {
+        matches!(arg.as_str(), "-d" | "-D" | "--delete")
+            || arg.starts_with("--delete=")
+            || (arg.starts_with("-d") && arg != "-d")
+            || (arg.starts_with("-D") && arg != "-D")
+    })
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
+    use pretty_assertions::assert_eq;
 
     fn vec_str(items: &[&str]) -> Vec<String> {
         items.iter().map(std::string::ToString::to_string).collect()
@@ -55,64 +69,89 @@ mod tests {
 
     #[test]
     fn git_reset_is_dangerous() {
-        assert!(command_might_be_dangerous(&vec_str(&["git", "reset"])));
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["git", "reset"])),
+            true
+        );
     }
 
     #[test]
     fn bash_git_reset_is_dangerous() {
-        assert!(command_might_be_dangerous(&vec_str(&[
-            "bash",
-            "-lc",
-            "git reset --hard"
-        ])));
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["bash", "-lc", "git reset --hard"])),
+            true
+        );
     }
 
     #[test]
     fn zsh_git_reset_is_dangerous() {
-        assert!(command_might_be_dangerous(&vec_str(&[
-            "zsh",
-            "-lc",
-            "git reset --hard"
-        ])));
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["zsh", "-lc", "git reset --hard"])),
+            true
+        );
     }
 
     #[test]
     fn git_status_is_not_dangerous() {
-        assert!(!command_might_be_dangerous(&vec_str(&["git", "status"])));
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["git", "status"])),
+            false
+        );
     }
 
     #[test]
     fn bash_git_status_is_not_dangerous() {
-        assert!(!command_might_be_dangerous(&vec_str(&[
-            "bash",
-            "-lc",
-            "git status"
-        ])));
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["bash", "-lc", "git status"])),
+            false
+        );
     }
 
     #[test]
     fn sudo_git_reset_is_dangerous() {
-        assert!(command_might_be_dangerous(&vec_str(&[
-            "sudo", "git", "reset", "--hard"
-        ])));
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["sudo", "git", "reset", "--hard"])),
+            true
+        );
     }
 
     #[test]
     fn usr_bin_git_is_dangerous() {
-        assert!(command_might_be_dangerous(&vec_str(&[
-            "/usr/bin/git",
-            "reset",
-            "--hard"
-        ])));
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["/usr/bin/git", "reset", "--hard"])),
+            true
+        );
+    }
+
+    #[test]
+    fn git_branch_delete_is_dangerous() {
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["git", "branch", "-d", "feature"])),
+            true
+        );
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["git", "branch", "-D", "feature"])),
+            true
+        );
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["bash", "-lc", "git branch --delete feature"])),
+            true
+        );
     }
 
     #[test]
     fn rm_rf_is_dangerous() {
-        assert!(command_might_be_dangerous(&vec_str(&["rm", "-rf", "/"])));
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["rm", "-rf", "/"])),
+            true
+        );
     }
 
     #[test]
     fn rm_f_is_dangerous() {
-        assert!(command_might_be_dangerous(&vec_str(&["rm", "-f", "/"])));
+        assert_eq!(
+            command_might_be_dangerous(&vec_str(&["rm", "-f", "/"])),
+            true
+        );
     }
 }

codex-rs/core/src/command_safety/is_safe_command.rs

@@ -131,10 +131,11 @@ fn is_safe_to_call_with_exec(command: &[String]) -> bool {
         }
 
         // Git
-        Some("git") => matches!(
-            command.get(1).map(String::as_str),
-            Some("branch" | "status" | "log" | "diff" | "show")
-        ),
+        Some("git") => match command.get(1).map(String::as_str) {
+            Some("status" | "log" | "diff" | "show") => true,
+            Some("branch") => git_branch_is_read_only(command),
+            _ => false,
+        },
 
         // Rust
         Some("cargo") if command.get(1).map(String::as_str) == Some("check") => true,
@@ -155,6 +156,34 @@ fn is_safe_to_call_with_exec(command: &[String]) -> bool {
     }
 }
 
+// Treat `git branch` as safe only when the arguments clearly indicate
+// a read-only query, not a branch mutation (create/rename/delete).
+fn git_branch_is_read_only(command: &[String]) -> bool {
+    if command.len() <= 2 {
+        // `git branch` with no additional args lists branches.
+        return true;
+    }
+
+    let mut saw_read_only_flag = false;
+    for arg in command.iter().skip(2).map(String::as_str) {
+        match arg {
+            "--list" | "-l" | "--show-current" | "-a" | "--all" | "-r" | "--remotes" | "-v"
+            | "-vv" | "--verbose" => {
+                saw_read_only_flag = true;
+            }
+            _ if arg.starts_with("--format=") => {
+                saw_read_only_flag = true;
+            }
+            _ => {
+                // Any other flag or positional argument may create, rename, or delete branches.
+                return false;
+            }
+        }
+    }
+
+    saw_read_only_flag
+}
+
 // (bash parsing helpers implemented in crate::bash)
 
 /* ----------------------------------------------------------
@@ -207,6 +236,12 @@ mod tests {
     fn known_safe_examples() {
         assert!(is_safe_to_call_with_exec(&vec_str(&["ls"])));
         assert!(is_safe_to_call_with_exec(&vec_str(&["git", "status"])));
+        assert!(is_safe_to_call_with_exec(&vec_str(&["git", "branch"])));
+        assert!(is_safe_to_call_with_exec(&vec_str(&[
+            "git",
+            "branch",
+            "--show-current"
+        ])));
         assert!(is_safe_to_call_with_exec(&vec_str(&["base64"])));
         assert!(is_safe_to_call_with_exec(&vec_str(&[
             "sed", "-n", "1,5p", "file.txt"
@@ -231,6 +266,18 @@ mod tests {
         }
     }
 
+    #[test]
+    fn git_branch_mutating_flags_are_not_safe() {
+        assert!(!is_known_safe_command(&vec_str(&[
+            "git", "branch", "-d", "feature"
+        ])));
+        assert!(!is_known_safe_command(&vec_str(&[
+            "git",
+            "branch",
+            "new-branch"
+        ])));
+    }
+
     #[test]
     fn zsh_lc_safe_command_sequence() {
         assert!(is_known_safe_command(&vec_str(&["zsh", "-lc", "ls"])));