Expand hardcoded agent terminal security rules (#48399)

Richard Feldman created 2 months ago

Expands the hardcoded security rules that block dangerous `rm` commands
in the agent terminal tool.

### New blocked patterns

- `rm -rf $HOME` / `rm -rf $HOME/` / `rm -rf ${HOME}` / `rm -rf
${HOME}/`
- `rm -rf .` / `rm -rf ./`
- `rm -rf ..` / `rm -rf ../`
- `rm -rf ~/` (previously only `rm -rf ~` was blocked)

### Flag handling improvements

- Simplified the flag character class from `[rRfF]` to `[rf]` since the
regex is already compiled with case-insensitive mode — less confusing,
same behavior.
- Added tests verifying that reversed flags (`-fr`), uppercase (`RM
-RF`), split flags (`-r -f`), and chained commands all get caught.

### Safe commands still allowed

Paths like `rm -rf ./build`, `rm -rf ~/Documents`, `rm -rf
$HOME/Documents`, `rm -rf ../some_dir`, and `rm -rf .hidden_dir` are
**not** blocked.

Release Notes:

- Auto-block a wider range of agent terminal commands, e.g. `rm -rf
$HOME` in addition to `rm -rf ~`

Change summary

crates/agent/src/tool_permissions.rs | 71 +++++++++++++++++++++++++++--
crates/settings_content/src/agent.rs |  5 +
2 files changed, 69 insertions(+), 7 deletions(-)

Detailed changes

crates/agent/src/tool_permissions.rs

@@ -17,12 +17,22 @@ pub struct HardcodedSecurityRules {
 
 pub static HARDCODED_SECURITY_RULES: LazyLock<HardcodedSecurityRules> = LazyLock::new(|| {
     HardcodedSecurityRules {
+        // Case-insensitive; `(-[rf]+\s+)*` handles `-rf`, `-fr`, `-RF`, `-r -f`, etc.
         terminal_deny: vec![
             // Recursive deletion of root - "rm -rf /" or "rm -rf / "
-            CompiledRegex::new(r"rm\s+(-[rRfF]+\s+)*/\s*$", false)
+            CompiledRegex::new(r"rm\s+(-[rf]+\s+)*/\s*$", false)
                 .expect("hardcoded regex should compile"),
-            // Recursive deletion of home - "rm -rf ~" (but not ~/subdir)
-            CompiledRegex::new(r"rm\s+(-[rRfF]+\s+)*~\s*$", false)
+            // Recursive deletion of home - "rm -rf ~" or "rm -rf ~/" (but not ~/subdir)
+            CompiledRegex::new(r"rm\s+(-[rf]+\s+)*~/?\s*$", false)
+                .expect("hardcoded regex should compile"),
+            // Recursive deletion of home via $HOME - "rm -rf $HOME" or "rm -rf ${HOME}"
+            CompiledRegex::new(r"rm\s+(-[rf]+\s+)*(\$HOME|\$\{HOME\})/?\s*$", false)
+                .expect("hardcoded regex should compile"),
+            // Recursive deletion of current directory - "rm -rf ." or "rm -rf ./"
+            CompiledRegex::new(r"rm\s+(-[rf]+\s+)*\./?\s*$", false)
+                .expect("hardcoded regex should compile"),
+            // Recursive deletion of parent directory - "rm -rf .." or "rm -rf ../"
+            CompiledRegex::new(r"rm\s+(-[rf]+\s+)*\.\./?\s*$", false)
                 .expect("hardcoded regex should compile"),
         ],
     }
@@ -1065,14 +1075,44 @@ mod tests {
 
     #[test]
     fn hardcoded_blocks_rm_rf_root() {
-        // rm -rf / should be blocked by hardcoded rules
         t("rm -rf /").is_deny();
+        t("rm -fr /").is_deny();
+        t("rm -RF /").is_deny();
+        t("rm -FR /").is_deny();
+        t("rm -r -f /").is_deny();
+        t("rm -f -r /").is_deny();
+        t("RM -RF /").is_deny();
     }
 
     #[test]
     fn hardcoded_blocks_rm_rf_home() {
-        // rm -rf ~ should be blocked by hardcoded rules
         t("rm -rf ~").is_deny();
+        t("rm -fr ~").is_deny();
+        t("rm -rf ~/").is_deny();
+        t("rm -rf $HOME").is_deny();
+        t("rm -fr $HOME").is_deny();
+        t("rm -rf $HOME/").is_deny();
+        t("rm -rf ${HOME}").is_deny();
+        t("rm -rf ${HOME}/").is_deny();
+        t("rm -RF $HOME").is_deny();
+        t("rm -FR ${HOME}/").is_deny();
+        t("rm -R -F ${HOME}/").is_deny();
+        t("RM -RF ~").is_deny();
+    }
+
+    #[test]
+    fn hardcoded_blocks_rm_rf_dot() {
+        t("rm -rf .").is_deny();
+        t("rm -fr .").is_deny();
+        t("rm -rf ./").is_deny();
+        t("rm -rf ..").is_deny();
+        t("rm -fr ..").is_deny();
+        t("rm -rf ../").is_deny();
+        t("rm -RF .").is_deny();
+        t("rm -FR ../").is_deny();
+        t("rm -R -F ../").is_deny();
+        t("RM -RF .").is_deny();
+        t("RM -RF ..").is_deny();
     }
 
     #[test]
@@ -1080,12 +1120,18 @@ mod tests {
         // Even with always_allow_tool_actions=true, hardcoded rules block
         t("rm -rf /").global(true).is_deny();
         t("rm -rf ~").global(true).is_deny();
+        t("rm -rf $HOME").global(true).is_deny();
+        t("rm -rf .").global(true).is_deny();
+        t("rm -rf ..").global(true).is_deny();
     }
 
     #[test]
     fn hardcoded_cannot_be_bypassed_by_allow_pattern() {
         // Even with an allow pattern that matches, hardcoded rules block
         t("rm -rf /").allow(&[".*"]).is_deny();
+        t("rm -rf $HOME").allow(&[".*"]).is_deny();
+        t("rm -rf .").allow(&[".*"]).is_deny();
+        t("rm -rf ..").allow(&[".*"]).is_deny();
     }
 
     #[test]
@@ -1097,6 +1143,18 @@ mod tests {
         t("rm -rf /tmp/test")
             .mode(ToolPermissionMode::Allow)
             .is_allow();
+        t("rm -rf ~/Documents")
+            .mode(ToolPermissionMode::Allow)
+            .is_allow();
+        t("rm -rf $HOME/Documents")
+            .mode(ToolPermissionMode::Allow)
+            .is_allow();
+        t("rm -rf ../some_dir")
+            .mode(ToolPermissionMode::Allow)
+            .is_allow();
+        t("rm -rf .hidden_dir")
+            .mode(ToolPermissionMode::Allow)
+            .is_allow();
     }
 
     #[test]
@@ -1105,5 +1163,8 @@ mod tests {
         t("ls && rm -rf /").is_deny();
         t("echo hello; rm -rf ~").is_deny();
         t("cargo build && rm -rf /").global(true).is_deny();
+        t("echo hello; rm -rf $HOME").is_deny();
+        t("echo hello; rm -rf .").is_deny();
+        t("echo hello; rm -rf ..").is_deny();
     }
 }

crates/settings_content/src/agent.rs

@@ -71,8 +71,9 @@ pub struct AgentSettingsContent {
     /// that you allow it, always choose to allow it.
     ///
     /// **Security note**: Even with this enabled, Zed's built-in security rules
-    /// still block some tool actions, such as the terminal tool running `rm -rf /` or `rm -rf ~`,
-    /// to prevent certain classes of failures from happening.
+    /// still block some tool actions, such as the terminal tool running `rm -rf /`, `rm -rf ~`,
+    /// `rm -rf $HOME`, `rm -rf .`, or `rm -rf ..`, to prevent certain classes of failures
+    /// from happening.
     ///
     /// This setting has no effect on external agents that support permission modes, such as Claude Code.
     ///