Add tests for user-reported rm security bypass variants (#48647)

Richard Feldman created 2 months ago

Builds on top of #48620 to add explicit test coverage for the exact
bypass scenarios reported by users:

- `rm -rf /etc/../` — path traversal via single parent dir that
normalizes to `/`
- `rm -rf --no-preserve-root /` — long flag without `=value` that could
bypass the old regex
- `rm --no-preserve-root -rf /` — long flag positioned before short
flags
- `rm / -rf --no-preserve-root` — trailing long flag without `=value`
after the path operand
- `sudo rm -rf /`, `sudo rm -rf /*`, `sudo rm -rf --no-preserve-root /`
— sudo-prefixed variants

All of these cases are already correctly blocked by the hardened regex
patterns and path normalization logic added in #48620. These tests
confirm that the reported bypasses are addressed and guard against
regressions.

Release Notes:

- N/A

Change summary

crates/agent/src/tool_permissions.rs | 40 ++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)

Detailed changes

crates/agent/src/tool_permissions.rs

@@ -1356,6 +1356,10 @@ mod tests {
         // End-of-options marker
         t("rm -rf -- /").is_deny();
         t("rm -- /").is_deny();
+        // Prefixed with sudo or other commands
+        t("sudo rm -rf /").is_deny();
+        t("sudo rm -rf /*").is_deny();
+        t("sudo rm -rf --no-preserve-root /").is_deny();
     }
 
     #[test]
@@ -1540,9 +1544,20 @@ mod tests {
         t("rm --no-preserve-root=yes -rf .").is_deny();
         t("rm --no-preserve-root=yes -rf ..").is_deny();
         t("rm --no-preserve-root=yes -rf $HOME").is_deny();
+        // --flag (without =value) should also not bypass the rules
+        t("rm -rf --no-preserve-root /").is_deny();
+        t("rm --no-preserve-root -rf /").is_deny();
+        t("rm --no-preserve-root --recursive --force /").is_deny();
+        t("rm -rf --no-preserve-root ~").is_deny();
+        t("rm -rf --no-preserve-root .").is_deny();
+        t("rm -rf --no-preserve-root ..").is_deny();
+        t("rm -rf --no-preserve-root $HOME").is_deny();
         // Trailing --flag=value after path
         t("rm / --no-preserve-root=yes -rf").is_deny();
         t("rm ~ -rf --no-preserve-root=yes").is_deny();
+        // Trailing --flag (without =value) after path
+        t("rm / -rf --no-preserve-root").is_deny();
+        t("rm ~ -rf --no-preserve-root").is_deny();
         // Safe paths with --flag=value should NOT be blocked
         t("rm --no-preserve-root=yes -rf ./build")
             .mode(ToolPermissionMode::Allow)
@@ -1550,11 +1565,16 @@ mod tests {
         t("rm --interactive=never -rf /tmp/test")
             .mode(ToolPermissionMode::Allow)
             .is_allow();
+        // Safe paths with --flag (without =value) should NOT be blocked
+        t("rm --no-preserve-root -rf ./build")
+            .mode(ToolPermissionMode::Allow)
+            .is_allow();
     }
 
     #[test]
     fn hardcoded_blocks_rm_with_path_traversal() {
         // Traversal to root via ..
+        t("rm -rf /etc/../").is_deny();
         t("rm -rf /tmp/../../").is_deny();
         t("rm -rf /tmp/../..").is_deny();
         t("rm -rf /var/log/../../").is_deny();
@@ -1620,6 +1640,26 @@ mod tests {
         t("rm -rf /safe /var/log/../../").is_deny();
     }
 
+    #[test]
+    fn hardcoded_blocks_user_reported_bypass_variants() {
+        // User report: "rm -rf /etc/../" normalizes to "rm -rf /" via path traversal
+        t("rm -rf /etc/../").is_deny();
+        t("rm -rf /etc/..").is_deny();
+        // User report: --no-preserve-root (without =value) should not bypass
+        t("rm -rf --no-preserve-root /").is_deny();
+        t("rm --no-preserve-root -rf /").is_deny();
+        // User report: "rm -rf /*" should be caught (glob expands to all top-level entries)
+        t("rm -rf /*").is_deny();
+        // Chained with sudo
+        t("sudo rm -rf /").is_deny();
+        t("sudo rm -rf --no-preserve-root /").is_deny();
+        // Traversal cannot be bypassed even with global allow or allow patterns
+        t("rm -rf /etc/../").global(true).is_deny();
+        t("rm -rf /etc/../").allow(&[".*"]).is_deny();
+        t("rm -rf --no-preserve-root /").global(true).is_deny();
+        t("rm -rf --no-preserve-root /").allow(&[".*"]).is_deny();
+    }
+
     #[test]
     fn normalize_path_relative_no_change() {
         assert_eq!(normalize_path("foo/bar"), "foo/bar");