diff --git a/crates/agent/src/tool_permissions.rs b/crates/agent/src/tool_permissions.rs index 2437b64df803d307946efc098031fb8e486f31df..07342ad07037dd318fb53acc989dae7e9ad96e80 100644 --- a/crates/agent/src/tool_permissions.rs +++ b/crates/agent/src/tool_permissions.rs @@ -3,6 +3,7 @@ use crate::shell_parser::extract_commands; use crate::tools::TerminalTool; use agent_settings::{AgentSettings, CompiledRegex, ToolPermissions, ToolRules}; use settings::ToolPermissionMode; +use std::path::{Component, Path}; use std::sync::LazyLock; use util::shell::ShellKind; @@ -16,24 +17,44 @@ pub struct HardcodedSecurityRules { } pub static HARDCODED_SECURITY_RULES: LazyLock = LazyLock::new(|| { + // Flag group matches any short flags (-rf, -rfv, -v, etc.) or long flags (--recursive, --force, etc.) + // This ensures extra flags like -rfv, -v -rf, --recursive --force don't bypass the rules. + const FLAGS: &str = r"(--[a-zA-Z0-9][-a-zA-Z0-9_]*(=[^\s]*)?\s+|-[a-zA-Z]+\s+)*"; + // Trailing flags that may appear after the path operand (GNU rm accepts flags after operands) + const TRAILING_FLAGS: &str = r"(\s+--[a-zA-Z0-9][-a-zA-Z0-9_]*(=[^\s]*)?|\s+-[a-zA-Z]+)*\s*"; + 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+(-[rf]+\s+)*/\s*$", false) - .expect("hardcoded regex should compile"), - // 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"), + // Recursive deletion of root - "rm -rf /", "rm -rfv /", "rm -rf /*", "rm / -rf" + CompiledRegex::new( + &format!(r"\brm\s+{FLAGS}(--\s+)?/\*?{TRAILING_FLAGS}$"), + false, + ) + .expect("hardcoded regex should compile"), + // Recursive deletion of home - "rm -rf ~" or "rm -rf ~/" or "rm -rf ~/*" or "rm ~ -rf" (but not ~/subdir) + CompiledRegex::new( + &format!(r"\brm\s+{FLAGS}(--\s+)?~/?\*?{TRAILING_FLAGS}$"), + false, + ) + .expect("hardcoded regex should compile"), + // Recursive deletion of home via $HOME - "rm -rf $HOME" or "rm -rf ${HOME}" or "rm $HOME -rf" or with /* + CompiledRegex::new( + &format!(r"\brm\s+{FLAGS}(--\s+)?(\$HOME|\$\{{HOME\}})/?(\*)?{TRAILING_FLAGS}$"), + false, + ) + .expect("hardcoded regex should compile"), + // Recursive deletion of current directory - "rm -rf ." or "rm -rf ./" or "rm -rf ./*" or "rm . -rf" + CompiledRegex::new( + &format!(r"\brm\s+{FLAGS}(--\s+)?\./?\*?{TRAILING_FLAGS}$"), + false, + ) + .expect("hardcoded regex should compile"), + // Recursive deletion of parent directory - "rm -rf .." or "rm -rf ../" or "rm -rf ../*" or "rm .. -rf" + CompiledRegex::new( + &format!(r"\brm\s+{FLAGS}(--\s+)?\.\./?\*?{TRAILING_FLAGS}$"), + false, + ) + .expect("hardcoded regex should compile"), ], } }); @@ -53,25 +74,21 @@ fn check_hardcoded_security_rules( let rules = &*HARDCODED_SECURITY_RULES; let terminal_patterns = &rules.terminal_deny; - // First: check the original input as-is - for pattern in terminal_patterns { - if pattern.is_match(input) { - return Some(ToolPermissionDecision::Deny( - HARDCODED_SECURITY_DENIAL_MESSAGE.into(), - )); - } + // First: check the original input as-is (and its path-normalized form) + if matches_hardcoded_patterns(input, terminal_patterns) { + return Some(ToolPermissionDecision::Deny( + HARDCODED_SECURITY_DENIAL_MESSAGE.into(), + )); } // Second: parse and check individual sub-commands (for chained commands) if shell_kind.supports_posix_chaining() { if let Some(commands) = extract_commands(input) { for command in &commands { - for pattern in terminal_patterns { - if pattern.is_match(command) { - return Some(ToolPermissionDecision::Deny( - HARDCODED_SECURITY_DENIAL_MESSAGE.into(), - )); - } + if matches_hardcoded_patterns(command, terminal_patterns) { + return Some(ToolPermissionDecision::Deny( + HARDCODED_SECURITY_DENIAL_MESSAGE.into(), + )); } } } @@ -80,6 +97,105 @@ fn check_hardcoded_security_rules( None } +/// Checks a single command against hardcoded patterns, both as-is and with +/// path arguments normalized (to catch traversal bypasses like `rm -rf /tmp/../../` +/// and multi-path bypasses like `rm -rf /tmp /`). +fn matches_hardcoded_patterns(command: &str, patterns: &[CompiledRegex]) -> bool { + for pattern in patterns { + if pattern.is_match(command) { + return true; + } + } + + for expanded in expand_rm_to_single_path_commands(command) { + for pattern in patterns { + if pattern.is_match(&expanded) { + return true; + } + } + } + + false +} + +/// For rm commands, expands multi-path arguments into individual single-path +/// commands with normalized paths. This catches both traversal bypasses like +/// `rm -rf /tmp/../../` and multi-path bypasses like `rm -rf /tmp /`. +fn expand_rm_to_single_path_commands(command: &str) -> Vec { + let trimmed = command.trim(); + + let first_token = trimmed.split_whitespace().next(); + if !first_token.is_some_and(|t| t.eq_ignore_ascii_case("rm")) { + return vec![]; + } + + let parts: Vec<&str> = trimmed.split_whitespace().collect(); + let mut flags = Vec::new(); + let mut paths = Vec::new(); + let mut past_double_dash = false; + + for part in parts.iter().skip(1) { + if !past_double_dash && *part == "--" { + past_double_dash = true; + flags.push(*part); + continue; + } + if !past_double_dash && part.starts_with('-') { + flags.push(*part); + } else { + paths.push(*part); + } + } + + let flags_str = if flags.is_empty() { + String::new() + } else { + format!("{} ", flags.join(" ")) + }; + + let mut results = Vec::new(); + for path in &paths { + if path.starts_with('$') { + let home_prefix = if path.starts_with("${HOME}") { + Some("${HOME}") + } else if path.starts_with("$HOME") { + Some("$HOME") + } else { + None + }; + + if let Some(prefix) = home_prefix { + let suffix = &path[prefix.len()..]; + if suffix.is_empty() { + results.push(format!("rm {flags_str}{path}")); + } else if suffix.starts_with('/') { + let normalized_suffix = normalize_path(suffix); + let reconstructed = if normalized_suffix == "/" { + prefix.to_string() + } else { + format!("{prefix}{normalized_suffix}") + }; + results.push(format!("rm {flags_str}{reconstructed}")); + } else { + results.push(format!("rm {flags_str}{path}")); + } + } else { + results.push(format!("rm {flags_str}{path}")); + } + continue; + } + + let mut normalized = normalize_path(path); + if normalized.is_empty() && !Path::new(path).has_root() { + normalized = ".".to_string(); + } + + results.push(format!("rm {flags_str}{normalized}")); + } + + results +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum ToolPermissionDecision { Allow, @@ -92,28 +208,27 @@ impl ToolPermissionDecision { /// /// # Precedence Order (highest to lowest) /// - /// 1. **`always_allow_tool_actions`** - When enabled, allows all tool actions without - /// prompting. This global setting bypasses all other checks including deny patterns. - /// Use with caution as it disables all security rules. - /// 2. **`always_deny`** - If any deny pattern matches, the tool call is blocked immediately. + /// 1. **Hardcoded security rules** - Critical safety checks (e.g., blocking `rm -rf /`) + /// that cannot be bypassed by any user settings, including `always_allow_tool_actions`. + /// 2. **`always_allow_tool_actions`** - When enabled, allows all tool actions without + /// prompting. This global setting bypasses user-configured deny/confirm/allow patterns, + /// but does **not** bypass hardcoded security rules. + /// 3. **`always_deny`** - If any deny pattern matches, the tool call is blocked immediately. /// This takes precedence over `always_confirm` and `always_allow` patterns. - /// 3. **`always_confirm`** - If any confirm pattern matches (and no deny matched), + /// 4. **`always_confirm`** - If any confirm pattern matches (and no deny matched), /// the user is prompted for confirmation. - /// 4. **`always_allow`** - If any allow pattern matches (and no deny/confirm matched), + /// 5. **`always_allow`** - If any allow pattern matches (and no deny/confirm matched), /// the tool call proceeds without prompting. - /// 5. **`default_mode`** - If no patterns match, falls back to the tool's default mode. + /// 6. **`default_mode`** - If no patterns match, falls back to the tool's default mode. /// /// # Shell Compatibility (Terminal Tool Only) /// /// For the terminal tool, commands are parsed to extract sub-commands for security. - /// This parsing only works for shells with POSIX-like `&&` / `||` / `;` / `|` syntax: - /// - /// **Compatible shells:** Posix (sh, bash, dash, zsh), Fish 3.0+, PowerShell 7+/Pwsh, - /// Cmd, Xonsh, Csh, Tcsh - /// - /// **Incompatible shells:** Nushell, Elvish, Rc (Plan 9) - /// - /// For incompatible shells, `always_allow` patterns are disabled for safety. + /// All currently supported `ShellKind` variants are treated as compatible because + /// brush-parser can handle their command chaining syntax. If a new `ShellKind` + /// variant is added that brush-parser cannot safely parse, it should be excluded + /// from `ShellKind::supports_posix_chaining()`, which will cause `always_allow` + /// patterns to be disabled for that shell. /// /// # Pattern Matching Tips /// @@ -303,15 +418,119 @@ pub fn decide_permission_from_settings( ) } +/// Normalizes a path by collapsing `.` and `..` segments without touching the filesystem. +fn normalize_path(raw: &str) -> String { + let is_absolute = Path::new(raw).has_root(); + let mut components: Vec<&str> = Vec::new(); + for component in Path::new(raw).components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + if components.last() == Some(&"..") { + components.push(".."); + } else if !components.is_empty() { + components.pop(); + } else if !is_absolute { + components.push(".."); + } + } + Component::Normal(segment) => { + if let Some(s) = segment.to_str() { + components.push(s); + } + } + Component::RootDir | Component::Prefix(_) => {} + } + } + let joined = components.join("/"); + if is_absolute { + format!("/{joined}") + } else { + joined + } +} + +/// Decides permission by checking both the raw input path and a simplified/canonicalized +/// version. Returns the most restrictive decision (Deny > Confirm > Allow). +pub fn decide_permission_for_path( + tool_name: &str, + raw_path: &str, + settings: &AgentSettings, +) -> ToolPermissionDecision { + let raw_decision = decide_permission_from_settings(tool_name, raw_path, settings); + + let simplified = normalize_path(raw_path); + if simplified == raw_path { + return raw_decision; + } + + let simplified_decision = decide_permission_from_settings(tool_name, &simplified, settings); + + most_restrictive(raw_decision, simplified_decision) +} + +fn most_restrictive( + a: ToolPermissionDecision, + b: ToolPermissionDecision, +) -> ToolPermissionDecision { + match (&a, &b) { + (ToolPermissionDecision::Deny(_), _) => a, + (_, ToolPermissionDecision::Deny(_)) => b, + (ToolPermissionDecision::Confirm, _) | (_, ToolPermissionDecision::Confirm) => { + ToolPermissionDecision::Confirm + } + _ => a, + } +} + #[cfg(test)] mod tests { use super::*; use crate::AgentTool; use crate::pattern_extraction::extract_terminal_pattern; use crate::tools::{EditFileTool, TerminalTool}; - use agent_settings::{CompiledRegex, InvalidRegexPattern, ToolRules}; + use agent_settings::{AgentProfileId, CompiledRegex, InvalidRegexPattern, ToolRules}; + use gpui::px; + use settings::{DefaultAgentView, DockPosition, DockSide, NotifyWhenAgentWaiting}; use std::sync::Arc; + fn test_agent_settings( + tool_permissions: ToolPermissions, + always_allow_tool_actions: bool, + ) -> AgentSettings { + AgentSettings { + enabled: true, + button: true, + dock: DockPosition::Right, + agents_panel_dock: DockSide::Left, + default_width: px(300.), + default_height: px(600.), + default_model: None, + inline_assistant_model: None, + inline_assistant_use_streaming_tools: false, + commit_message_model: None, + thread_summary_model: None, + inline_alternatives: vec![], + favorite_models: vec![], + default_profile: AgentProfileId::default(), + default_view: DefaultAgentView::Thread, + profiles: Default::default(), + always_allow_tool_actions, + notify_when_agent_waiting: NotifyWhenAgentWaiting::default(), + play_sound_when_agent_done: false, + single_file_review: false, + model_parameters: vec![], + enable_feedback: false, + expand_edit_card: true, + expand_terminal_card: true, + cancel_generation_on_terminal_stop: true, + use_modifier_to_send: true, + message_editor_min_lines: 1, + tool_permissions, + show_turn_stats: false, + } + } + fn pattern(command: &str) -> &'static str { Box::leak( extract_terminal_pattern(command) @@ -1125,6 +1344,18 @@ mod tests { t("rm -r -f /").is_deny(); t("rm -f -r /").is_deny(); t("RM -RF /").is_deny(); + // Long flags + t("rm --recursive --force /").is_deny(); + t("rm --force --recursive /").is_deny(); + // Extra short flags + t("rm -rfv /").is_deny(); + t("rm -v -rf /").is_deny(); + // Glob wildcards + t("rm -rf /*").is_deny(); + t("rm -rf /* ").is_deny(); + // End-of-options marker + t("rm -rf -- /").is_deny(); + t("rm -- /").is_deny(); } #[test] @@ -1141,6 +1372,40 @@ mod tests { t("rm -FR ${HOME}/").is_deny(); t("rm -R -F ${HOME}/").is_deny(); t("RM -RF ~").is_deny(); + // Long flags + t("rm --recursive --force ~").is_deny(); + t("rm --recursive --force ~/").is_deny(); + t("rm --recursive --force $HOME").is_deny(); + t("rm --force --recursive ${HOME}/").is_deny(); + // Extra short flags + t("rm -rfv ~").is_deny(); + t("rm -v -rf ~/").is_deny(); + // Glob wildcards + t("rm -rf ~/*").is_deny(); + t("rm -rf $HOME/*").is_deny(); + t("rm -rf ${HOME}/*").is_deny(); + // End-of-options marker + t("rm -rf -- ~").is_deny(); + t("rm -rf -- ~/").is_deny(); + t("rm -rf -- $HOME").is_deny(); + } + + #[test] + fn hardcoded_blocks_rm_rf_home_with_traversal() { + // Path traversal after $HOME / ${HOME} should still be blocked + t("rm -rf $HOME/./").is_deny(); + t("rm -rf $HOME/foo/..").is_deny(); + t("rm -rf ${HOME}/.").is_deny(); + t("rm -rf ${HOME}/./").is_deny(); + t("rm -rf $HOME/a/b/../..").is_deny(); + t("rm -rf ${HOME}/foo/bar/../..").is_deny(); + // Subdirectories should NOT be blocked + t("rm -rf $HOME/subdir") + .mode(ToolPermissionMode::Allow) + .is_allow(); + t("rm -rf ${HOME}/Documents") + .mode(ToolPermissionMode::Allow) + .is_allow(); } #[test] @@ -1156,6 +1421,18 @@ mod tests { t("rm -R -F ../").is_deny(); t("RM -RF .").is_deny(); t("RM -RF ..").is_deny(); + // Long flags + t("rm --recursive --force .").is_deny(); + t("rm --force --recursive ../").is_deny(); + // Extra short flags + t("rm -rfv .").is_deny(); + t("rm -v -rf ../").is_deny(); + // Glob wildcards + t("rm -rf ./*").is_deny(); + t("rm -rf ../*").is_deny(); + // End-of-options marker + t("rm -rf -- .").is_deny(); + t("rm -rf -- ../").is_deny(); } #[test] @@ -1210,4 +1487,320 @@ mod tests { t("echo hello; rm -rf .").is_deny(); t("echo hello; rm -rf ..").is_deny(); } + + #[test] + fn hardcoded_blocks_rm_with_trailing_flags() { + // GNU rm accepts flags after operands by default + t("rm / -rf").is_deny(); + t("rm / -fr").is_deny(); + t("rm / -RF").is_deny(); + t("rm / -r -f").is_deny(); + t("rm / --recursive --force").is_deny(); + t("rm / -rfv").is_deny(); + t("rm /* -rf").is_deny(); + // Mixed: some flags before path, some after + t("rm -r / -f").is_deny(); + t("rm -f / -r").is_deny(); + // Home + t("rm ~ -rf").is_deny(); + t("rm ~/ -rf").is_deny(); + t("rm ~ -r -f").is_deny(); + t("rm $HOME -rf").is_deny(); + t("rm ${HOME} -rf").is_deny(); + // Dot / dotdot + t("rm . -rf").is_deny(); + t("rm ./ -rf").is_deny(); + t("rm . -r -f").is_deny(); + t("rm .. -rf").is_deny(); + t("rm ../ -rf").is_deny(); + t("rm .. -r -f").is_deny(); + // Trailing flags in chained commands + t("ls && rm / -rf").is_deny(); + t("echo hello; rm ~ -rf").is_deny(); + // Safe paths with trailing flags should NOT be blocked + t("rm ./build -rf") + .mode(ToolPermissionMode::Allow) + .is_allow(); + t("rm /tmp/test -rf") + .mode(ToolPermissionMode::Allow) + .is_allow(); + t("rm ~/Documents -rf") + .mode(ToolPermissionMode::Allow) + .is_allow(); + } + + #[test] + fn hardcoded_blocks_rm_with_flag_equals_value() { + // --flag=value syntax should not bypass the rules + t("rm --no-preserve-root=yes -rf /").is_deny(); + t("rm --no-preserve-root=yes --recursive --force /").is_deny(); + t("rm -rf --no-preserve-root=yes /").is_deny(); + t("rm --interactive=never -rf /").is_deny(); + 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 ..").is_deny(); + t("rm --no-preserve-root=yes -rf $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(); + // Safe paths with --flag=value should NOT be blocked + t("rm --no-preserve-root=yes -rf ./build") + .mode(ToolPermissionMode::Allow) + .is_allow(); + t("rm --interactive=never -rf /tmp/test") + .mode(ToolPermissionMode::Allow) + .is_allow(); + } + + #[test] + fn hardcoded_blocks_rm_with_path_traversal() { + // Traversal to root via .. + t("rm -rf /tmp/../../").is_deny(); + t("rm -rf /tmp/../..").is_deny(); + t("rm -rf /var/log/../../").is_deny(); + // Root via /./ + t("rm -rf /./").is_deny(); + t("rm -rf /.").is_deny(); + // Double slash (equivalent to /) + t("rm -rf //").is_deny(); + // Home traversal via ~/./ + t("rm -rf ~/./").is_deny(); + t("rm -rf ~/.").is_deny(); + // Dot traversal via indirect paths + t("rm -rf ./foo/..").is_deny(); + t("rm -rf ../foo/..").is_deny(); + // Traversal in chained commands + t("ls && rm -rf /tmp/../../").is_deny(); + t("echo hello; rm -rf /./").is_deny(); + // Traversal cannot be bypassed by global or allow patterns + t("rm -rf /tmp/../../").global(true).is_deny(); + t("rm -rf /./").allow(&[".*"]).is_deny(); + // Safe paths with traversal should still be allowed + t("rm -rf /tmp/../tmp/foo") + .mode(ToolPermissionMode::Allow) + .is_allow(); + t("rm -rf ~/Documents/./subdir") + .mode(ToolPermissionMode::Allow) + .is_allow(); + } + + #[test] + fn hardcoded_blocks_rm_multi_path_with_dangerous_last() { + t("rm -rf /tmp /").is_deny(); + t("rm -rf /tmp/foo /").is_deny(); + t("rm -rf /var/log ~").is_deny(); + t("rm -rf /safe $HOME").is_deny(); + } + + #[test] + fn hardcoded_blocks_rm_multi_path_with_dangerous_first() { + t("rm -rf / /tmp").is_deny(); + t("rm -rf ~ /var/log").is_deny(); + t("rm -rf . /tmp/foo").is_deny(); + t("rm -rf .. /safe").is_deny(); + } + + #[test] + fn hardcoded_allows_rm_multi_path_all_safe() { + t("rm -rf /tmp /home/user") + .mode(ToolPermissionMode::Allow) + .is_allow(); + t("rm -rf ./build ./dist") + .mode(ToolPermissionMode::Allow) + .is_allow(); + t("rm -rf /var/log/app /tmp/cache") + .mode(ToolPermissionMode::Allow) + .is_allow(); + } + + #[test] + fn hardcoded_blocks_rm_multi_path_with_traversal() { + t("rm -rf /safe /tmp/../../").is_deny(); + t("rm -rf /tmp/../../ /safe").is_deny(); + t("rm -rf /safe /var/log/../../").is_deny(); + } + + #[test] + fn normalize_path_relative_no_change() { + assert_eq!(normalize_path("foo/bar"), "foo/bar"); + } + + #[test] + fn normalize_path_relative_with_curdir() { + assert_eq!(normalize_path("foo/./bar"), "foo/bar"); + } + + #[test] + fn normalize_path_relative_with_parent() { + assert_eq!(normalize_path("foo/bar/../baz"), "foo/baz"); + } + + #[test] + fn normalize_path_absolute_preserved() { + assert_eq!(normalize_path("/etc/passwd"), "/etc/passwd"); + } + + #[test] + fn normalize_path_absolute_with_traversal() { + assert_eq!(normalize_path("/tmp/../etc/passwd"), "/etc/passwd"); + } + + #[test] + fn normalize_path_root() { + assert_eq!(normalize_path("/"), "/"); + } + + #[test] + fn normalize_path_parent_beyond_root_clamped() { + assert_eq!(normalize_path("/../../../etc/passwd"), "/etc/passwd"); + } + + #[test] + fn normalize_path_curdir_only() { + assert_eq!(normalize_path("."), ""); + } + + #[test] + fn normalize_path_empty() { + assert_eq!(normalize_path(""), ""); + } + + #[test] + fn normalize_path_relative_traversal_above_start() { + assert_eq!(normalize_path("../../../etc/passwd"), "../../../etc/passwd"); + } + + #[test] + fn normalize_path_relative_traversal_with_curdir() { + assert_eq!(normalize_path("../../."), "../.."); + } + + #[test] + fn normalize_path_relative_partial_traversal_above_start() { + assert_eq!(normalize_path("foo/../../bar"), "../bar"); + } + + #[test] + fn most_restrictive_deny_vs_allow() { + assert!(matches!( + most_restrictive( + ToolPermissionDecision::Deny("x".into()), + ToolPermissionDecision::Allow + ), + ToolPermissionDecision::Deny(_) + )); + } + + #[test] + fn most_restrictive_allow_vs_deny() { + assert!(matches!( + most_restrictive( + ToolPermissionDecision::Allow, + ToolPermissionDecision::Deny("x".into()) + ), + ToolPermissionDecision::Deny(_) + )); + } + + #[test] + fn most_restrictive_deny_vs_confirm() { + assert!(matches!( + most_restrictive( + ToolPermissionDecision::Deny("x".into()), + ToolPermissionDecision::Confirm + ), + ToolPermissionDecision::Deny(_) + )); + } + + #[test] + fn most_restrictive_confirm_vs_deny() { + assert!(matches!( + most_restrictive( + ToolPermissionDecision::Confirm, + ToolPermissionDecision::Deny("x".into()) + ), + ToolPermissionDecision::Deny(_) + )); + } + + #[test] + fn most_restrictive_deny_vs_deny() { + assert!(matches!( + most_restrictive( + ToolPermissionDecision::Deny("a".into()), + ToolPermissionDecision::Deny("b".into()) + ), + ToolPermissionDecision::Deny(_) + )); + } + + #[test] + fn most_restrictive_confirm_vs_allow() { + assert_eq!( + most_restrictive( + ToolPermissionDecision::Confirm, + ToolPermissionDecision::Allow + ), + ToolPermissionDecision::Confirm + ); + } + + #[test] + fn most_restrictive_allow_vs_confirm() { + assert_eq!( + most_restrictive( + ToolPermissionDecision::Allow, + ToolPermissionDecision::Confirm + ), + ToolPermissionDecision::Confirm + ); + } + + #[test] + fn most_restrictive_allow_vs_allow() { + assert_eq!( + most_restrictive(ToolPermissionDecision::Allow, ToolPermissionDecision::Allow), + ToolPermissionDecision::Allow + ); + } + + #[test] + fn decide_permission_for_path_no_dots_early_return() { + // When the path has no `.` or `..`, normalize_path returns the same string, + // so decide_permission_for_path returns the raw decision directly. + let settings = test_agent_settings( + ToolPermissions { + tools: Default::default(), + }, + false, + ); + let decision = decide_permission_for_path(EditFileTool::NAME, "src/main.rs", &settings); + assert_eq!(decision, ToolPermissionDecision::Confirm); + } + + #[test] + fn decide_permission_for_path_traversal_triggers_deny() { + let deny_regex = CompiledRegex::new("/etc/passwd", false).unwrap(); + let mut tools = collections::HashMap::default(); + tools.insert( + Arc::from(EditFileTool::NAME), + ToolRules { + default_mode: ToolPermissionMode::Allow, + always_allow: vec![], + always_deny: vec![deny_regex], + always_confirm: vec![], + invalid_patterns: vec![], + }, + ); + let settings = test_agent_settings(ToolPermissions { tools }, false); + + let decision = + decide_permission_for_path(EditFileTool::NAME, "/tmp/../etc/passwd", &settings); + assert!( + matches!(decision, ToolPermissionDecision::Deny(_)), + "expected Deny for traversal to /etc/passwd, got {:?}", + decision + ); + } }