Allow always_allow for nushell, elvish, and rc (#48395)

Richard Feldman created 2 months ago

`brush-parser` handles `;` (sequential execution) and `|` (piping) which
all these shells use, so we can safely parse their commands for
`always_allow` pattern matching.

(No release notes because granular tool permissions haven't been
released yet.)

Release Notes:

- N/A

Change summary

crates/agent/src/tool_permissions.rs | 13 +++++++++----
crates/util/src/shell.rs             | 21 ++++++++++++++++-----
2 files changed, 25 insertions(+), 9 deletions(-)

Detailed changes

crates/agent/src/tool_permissions.rs

@@ -982,8 +982,8 @@ mod tests {
     }
 
     #[test]
-    fn nushell_denies_when_always_allow_configured() {
-        t("ls").allow(&["^ls"]).shell(ShellKind::Nushell).is_deny();
+    fn nushell_allows_with_allow_pattern() {
+        t("ls").allow(&["^ls"]).shell(ShellKind::Nushell).is_allow();
     }
 
     #[test]
@@ -1012,8 +1012,13 @@ mod tests {
     }
 
     #[test]
-    fn elvish_denies_when_always_allow_configured() {
-        t("ls").allow(&["^ls"]).shell(ShellKind::Elvish).is_deny();
+    fn elvish_allows_with_allow_pattern() {
+        t("ls").allow(&["^ls"]).shell(ShellKind::Elvish).is_allow();
+    }
+
+    #[test]
+    fn rc_allows_with_allow_pattern() {
+        t("ls").allow(&["^ls"]).shell(ShellKind::Rc).is_allow();
     }
 
     #[test]

crates/util/src/shell.rs

@@ -256,16 +256,24 @@ impl ShellKind {
         Self::new(&get_system_shell(), cfg!(windows))
     }
 
-    /// Returns whether this shell uses POSIX-like command chaining syntax (`&&`, `||`, `;`, `|`).
+    /// Returns whether this shell's command chaining syntax can be parsed by brush-parser.
     ///
     /// This is used to determine if we can safely parse shell commands to extract sub-commands
     /// for security purposes (e.g., preventing shell injection in "always allow" patterns).
     ///
-    /// **Compatible shells:** Posix (sh, bash, dash, zsh), Fish 3.0+, PowerShell 7+/Pwsh,
-    /// Cmd, Xonsh, Csh, Tcsh
+    /// The brush-parser handles `;` (sequential execution) and `|` (piping), which are
+    /// supported by all common shells. It also handles `&&` and `||` for conditional
+    /// execution, `$()` and backticks for command substitution, and process substitution.
     ///
-    /// **Incompatible shells:** Nushell (uses `and`/`or` keywords), Elvish (uses `and`/`or`
-    /// keywords), Rc (Plan 9 shell - no `&&`/`||` operators)
+    /// # Shell Notes
+    ///
+    /// - **Nushell**: Uses `;` for sequential execution. The `and`/`or` keywords are boolean
+    ///   operators on values (e.g., `$true and $false`), not command chaining operators.
+    /// - **Elvish**: Uses `;` to separate pipelines, which brush-parser handles. Elvish does
+    ///   not have `&&` or `||` operators. Its `and`/`or` are special commands that operate
+    ///   on values, not command chaining (e.g., `and $true $false`).
+    /// - **Rc (Plan 9)**: Uses `;` for sequential execution and `|` for piping. Does not
+    ///   have `&&`/`||` operators for conditional chaining.
     pub fn supports_posix_chaining(&self) -> bool {
         matches!(
             self,
@@ -277,6 +285,9 @@ impl ShellKind {
                 | ShellKind::Xonsh
                 | ShellKind::Csh
                 | ShellKind::Tcsh
+                | ShellKind::Nushell
+                | ShellKind::Elvish
+                | ShellKind::Rc
         )
     }