Harden git safe command classification

codex-rs/shell-command/src/command_safety/is_safe_command.rs

@@ -187,7 +187,12 @@ pub(crate) fn is_safe_git_command(command: &[String]) -> bool {
     let subcommand_args = &command[subcommand_idx + 1..];
 
     match subcommand {
-        "status" | "log" | "diff" | "show" => git_subcommand_args_are_read_only(subcommand_args),
+        "status" | "log" | "diff" | "show" => {
+            // These subcommands can consult repository-controlled configuration
+            // for external helpers, so argv-only classification is not enough to
+            // auto-approve them.
+            false
+        }
         "branch" => {
             git_subcommand_args_are_read_only(subcommand_args)
                 && git_branch_is_read_only(subcommand_args)
@@ -345,7 +350,6 @@ mod tests {
     #[test]
     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",
@@ -376,6 +380,25 @@ mod tests {
         }
     }
 
+    #[test]
+    fn git_config_sensitive_subcommands_require_approval() {
+        for args in [
+            vec_str(&["git", "status"]),
+            vec_str(&["git", "log", "-1"]),
+            vec_str(&["git", "diff"]),
+            vec_str(&["git", "show", "HEAD"]),
+            vec_str(&["bash", "-lc", "git status"]),
+            vec_str(&["bash", "-lc", "git log -1"]),
+            vec_str(&["bash", "-lc", "git diff"]),
+            vec_str(&["bash", "-lc", "git show HEAD"]),
+        ] {
+            assert!(
+                !is_known_safe_command(&args),
+                "expected {args:?} to require approval because git may invoke repository-configured helpers",
+            );
+        }
+    }
+
     #[test]
     fn git_branch_mutating_flags_are_not_safe() {
         assert!(!is_known_safe_command(&vec_str(&[
@@ -461,13 +484,15 @@ mod tests {
     }
 
     #[test]
-    fn git_subcommand_patch_flags_remain_safe() {
-        assert!(is_known_safe_command(&vec_str(&["git", "log", "-p", "-1"])));
-        assert!(is_known_safe_command(&vec_str(&["git", "diff", "-p"])));
-        assert!(is_known_safe_command(&vec_str(&[
+    fn git_patch_display_subcommands_require_approval() {
+        assert!(!is_known_safe_command(&vec_str(&[
+            "git", "log", "-p", "-1"
+        ])));
+        assert!(!is_known_safe_command(&vec_str(&["git", "diff", "-p"])));
+        assert!(!is_known_safe_command(&vec_str(&[
             "git", "show", "-p", "HEAD",
         ])));
-        assert!(is_known_safe_command(&vec_str(&[
+        assert!(!is_known_safe_command(&vec_str(&[
             "bash",
             "-lc",
             "git log -p -1",
@@ -651,11 +676,6 @@ mod tests {
     fn bash_lc_safe_examples() {
         assert!(is_known_safe_command(&vec_str(&["bash", "-lc", "ls"])));
         assert!(is_known_safe_command(&vec_str(&["bash", "-lc", "ls -1"])));
-        assert!(is_known_safe_command(&vec_str(&[
-            "bash",
-            "-lc",
-            "git status"
-        ])));
         assert!(is_known_safe_command(&vec_str(&[
             "bash",
             "-lc",

codex-rs/shell-command/src/command_safety/windows_safe_commands.rs

@@ -242,13 +242,6 @@ mod tests {
             "Get-ChildItem -Path .",
         ])));
 
-        assert!(is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
-            "-NoProfile",
-            "-Command",
-            "git status",
-        ])));
-
         assert!(is_safe_command_windows(&vec_str(&[
             "powershell.exe",
             "Get-Content",
@@ -290,7 +283,7 @@ mod tests {
     }
 
     #[test]
-    fn allows_read_only_pipelines_and_git_usage() {
+    fn allows_read_only_pipelines_and_git_branch_usage() {
         let Some(pwsh) = try_find_pwsh_executable_blocking() else {
             return;
         };
@@ -316,7 +309,7 @@ mod tests {
         assert!(is_safe_command_windows(&[
             pwsh.clone(),
             "-Command".to_string(),
-            "git show HEAD:foo.rs".to_string()
+            "git branch --show-current".to_string()
         ]));
 
         assert!(is_safe_command_windows(&[
@@ -332,6 +325,34 @@ mod tests {
         ]));
     }
 
+    #[test]
+    fn rejects_git_config_sensitive_subcommands() {
+        let results: Vec<(&str, bool)> = ["git status", "git log -1", "git diff", "git show HEAD"]
+            .into_iter()
+            .map(|script| {
+                (
+                    script,
+                    is_safe_command_windows(&[
+                        "powershell.exe".to_string(),
+                        "-NoProfile".to_string(),
+                        "-Command".to_string(),
+                        script.to_string(),
+                    ]),
+                )
+            })
+            .collect();
+
+        assert_eq!(
+            vec![
+                ("git status", false),
+                ("git log -1", false),
+                ("git diff", false),
+                ("git show HEAD", false),
+            ],
+            results
+        );
+    }
+
     #[test]
     fn rejects_git_global_override_options() {
         let Some(pwsh) = try_find_pwsh_executable_blocking() else {