fix: check flags to ripgrep when deciding whether the invocation is "trusted" (#1644)

With this change, if any of `--pre`, `--hostname-bin`, `--search-zip`, or `-z` are used with a proposed invocation of `rg`, do not auto-approve.

codex-cli/src/approvals.ts

@@ -370,11 +370,26 @@ export function isSafeCommand(
         reason: "View file with line numbers",
         group: "Reading files",
       };
-    case "rg":
+    case "rg": {
+      // Certain ripgrep options execute external commands or invoke other
+      // processes, so we must reject them.
+      const isUnsafe = command.some(
+        (arg: string) =>
+          UNSAFE_OPTIONS_FOR_RIPGREP_WITHOUT_ARGS.has(arg) ||
+          [...UNSAFE_OPTIONS_FOR_RIPGREP_WITH_ARGS].some(
+            (opt) => arg === opt || arg.startsWith(`${opt}=`),
+          ),
+      );
+
+      if (isUnsafe) {
+        break;
+      }
+
       return {
         reason: "Ripgrep search",
         group: "Searching",
       };
+    }
     case "find": {
       // Certain options to `find` allow executing arbitrary processes, so we
       // cannot auto-approve them.
@@ -495,6 +510,22 @@ const UNSAFE_OPTIONS_FOR_FIND_COMMAND: ReadonlySet<string> = new Set([
   "-fprintf",
 ]);
 
+// Ripgrep options that are considered unsafe because they may execute
+// arbitrary commands or spawn auxiliary processes.
+const UNSAFE_OPTIONS_FOR_RIPGREP_WITH_ARGS: ReadonlySet<string> = new Set([
+  // Executes an arbitrary command for each matching file.
+  "--pre",
+  // Allows custom hostname command which could leak environment details.
+  "--hostname-bin",
+]);
+
+const UNSAFE_OPTIONS_FOR_RIPGREP_WITHOUT_ARGS: ReadonlySet<string> = new Set([
+  // Enables searching inside archives which triggers external decompression
+  // utilities – reject out of an abundance of caution.
+  "--search-zip",
+  "-z",
+]);
+
 // ---------------- Helper utilities for complex shell expressions -----------------
 
 // A conservative allow-list of bash operators that do not, on their own, cause

codex-cli/tests/approvals.test.ts

@@ -44,6 +44,14 @@ describe("canAutoApprove()", () => {
       group: "Navigating",
       runInSandbox: false,
     });
+
+    // Ripgrep safe invocation.
+    expect(check(["rg", "TODO"])).toEqual({
+      type: "auto-approve",
+      reason: "Ripgrep search",
+      group: "Searching",
+      runInSandbox: false,
+    });
   });
 
   test("simple safe commands within a `bash -lc` call", () => {
@@ -67,6 +75,24 @@ describe("canAutoApprove()", () => {
     });
   });
 
+  test("ripgrep unsafe flags", () => {
+    // Flags that do not take arguments
+    expect(check(["rg", "--search-zip", "TODO"])).toEqual({ type: "ask-user" });
+    expect(check(["rg", "-z", "TODO"])).toEqual({ type: "ask-user" });
+
+    // Flags that take arguments (provided separately)
+    expect(check(["rg", "--pre", "cat", "TODO"])).toEqual({ type: "ask-user" });
+    expect(check(["rg", "--hostname-bin", "hostname", "TODO"])).toEqual({
+      type: "ask-user",
+    });
+
+    // Flags that take arguments in = form
+    expect(check(["rg", "--pre=cat", "TODO"])).toEqual({ type: "ask-user" });
+    expect(check(["rg", "--hostname-bin=hostname", "TODO"])).toEqual({
+      type: "ask-user",
+    });
+  });
+
   test("bash -lc commands with unsafe redirects", () => {
     expect(check(["bash", "-lc", "echo hello > file.txt"])).toEqual({
       type: "ask-user",

codex-rs/core/src/is_safe_command.rs

@@ -23,9 +23,9 @@ fn is_safe_to_call_with_exec(command: &[String]) -> bool {
     let cmd0 = command.first().map(String::as_str);
 
     match cmd0 {
-        Some(
-            "cat" | "cd" | "echo" | "grep" | "head" | "ls" | "pwd" | "rg" | "tail" | "wc" | "which",
-        ) => true,
+        Some("cat" | "cd" | "echo" | "grep" | "head" | "ls" | "pwd" | "tail" | "wc" | "which") => {
+            true
+        }
 
         Some("find") => {
             // Certain options to `find` can delete files, write to files, or
@@ -46,6 +46,29 @@ fn is_safe_to_call_with_exec(command: &[String]) -> bool {
                 .any(|arg| UNSAFE_FIND_OPTIONS.contains(&arg.as_str()))
         }
 
+        // Ripgrep
+        Some("rg") => {
+            const UNSAFE_RIPGREP_OPTIONS_WITH_ARGS: &[&str] = &[
+                // Takes an arbitrary command that is executed for each match.
+                "--pre",
+                // Takes a command that can be used to obtain the local hostname.
+                "--hostname-bin",
+            ];
+            const UNSAFE_RIPGREP_OPTIONS_WITHOUT_ARGS: &[&str] = &[
+                // Calls out to other decompression tools, so do not auto-approve
+                // out of an abundance of caution.
+                "--search-zip",
+                "-z",
+            ];
+
+            !command.iter().any(|arg| {
+                UNSAFE_RIPGREP_OPTIONS_WITHOUT_ARGS.contains(&arg.as_str())
+                    || UNSAFE_RIPGREP_OPTIONS_WITH_ARGS
+                        .iter()
+                        .any(|&opt| arg == opt || arg.starts_with(&format!("{opt}=")))
+            })
+        }
+
         // Git
         Some("git") => matches!(
             command.get(1).map(String::as_str),
@@ -245,6 +268,40 @@ mod tests {
         }
     }
 
+    #[test]
+    fn ripgrep_rules() {
+        // Safe ripgrep invocations – none of the unsafe flags are present.
+        assert!(is_safe_to_call_with_exec(&vec_str(&[
+            "rg",
+            "Cargo.toml",
+            "-n"
+        ])));
+
+        // Unsafe flags that do not take an argument (present verbatim).
+        for args in [
+            vec_str(&["rg", "--search-zip", "files"]),
+            vec_str(&["rg", "-z", "files"]),
+        ] {
+            assert!(
+                !is_safe_to_call_with_exec(&args),
+                "expected {args:?} to be considered unsafe due to zip-search flag",
+            );
+        }
+
+        // Unsafe flags that expect a value, provided in both split and = forms.
+        for args in [
+            vec_str(&["rg", "--pre", "pwned", "files"]),
+            vec_str(&["rg", "--pre=pwned", "files"]),
+            vec_str(&["rg", "--hostname-bin", "pwned", "files"]),
+            vec_str(&["rg", "--hostname-bin=pwned", "files"]),
+        ] {
+            assert!(
+                !is_safe_to_call_with_exec(&args),
+                "expected {args:?} to be considered unsafe due to external-command flag",
+            );
+        }
+    }
+
     #[test]
     fn bash_lc_safe_examples() {
         assert!(is_known_safe_command(&vec_str(&["bash", "-lc", "ls"])));