feat(permission): implement Phase 1 — complete permission system

Implement all 15 todo!() items across 4 files:

- denial_tracker: consecutive/total counters with 3/20 thresholds
- explainer: decision explanations + allow-rule suggestions for
  Bash, Edit, Read, MCP tool patterns
- shadowed_rules: detect unreachable rules via tool name coverage
  and glob/exact content subsumption
- path_validator: 6-gate validation pipeline (UTF-8 → shell expansion
  → UNC → NTFS bypass patterns → resolve/normalize → denied patterns
  → allowed dirs), symlink escape detection, tilde expansion,
  case-insensitive comparison on Windows/macOS

50 new tests added (260+ total in permission module).

Author: lingcoder

crates/core/src/permission/denial_tracker.rs

@@ -8,6 +8,16 @@
 
 use std::time::Instant;
 
+// ---------------------------------------------------------------------------
+// Constants
+// ---------------------------------------------------------------------------
+
+/// Maximum consecutive denials before triggering a fallback warning.
+const MAX_CONSECUTIVE_DENIALS: u32 = 3;
+
+/// Maximum total denials before triggering a fallback warning.
+const MAX_TOTAL_DENIALS: usize = 20;
+
 // ---------------------------------------------------------------------------
 // Types
 // ---------------------------------------------------------------------------
@@ -53,10 +63,12 @@ impl DenialTracker {
 
     /// Record a new denial event.
     pub fn record_denial(&mut self, tool_name: &str, reason: &str) {
-        todo!(
-            "Record denial for tool '{tool_name}' with reason '{reason}', \
-             increment consecutive_count, push DenialRecord"
-        )
+        self.consecutive_count += 1;
+        self.denials.push(DenialRecord {
+            tool_name: tool_name.to_string(),
+            timestamp: Instant::now(),
+            reason: reason.to_string(),
+        });
     }
 
     /// Return the number of consecutive denials since the last allow/reset.
@@ -67,14 +79,14 @@ impl DenialTracker {
     /// Check whether the denial count has reached a threshold that warrants
     /// a warning to the user or agent.
     ///
-    /// Default threshold: 3 consecutive denials.
+    /// Returns `true` if consecutive denials >= 3 or total denials >= 20.
     pub fn should_warn(&self) -> bool {
-        todo!("Return true if consecutive_count >= warning threshold (3)")
+        self.consecutive_count >= MAX_CONSECUTIVE_DENIALS || self.denials.len() >= MAX_TOTAL_DENIALS
     }
 
     /// Reset the consecutive denial counter (called after a successful allow).
     pub fn reset(&mut self) {
-        todo!("Reset consecutive_count to 0")
+        self.consecutive_count = 0;
     }
 
     /// Return a slice of all recorded denial records.
@@ -104,9 +116,79 @@ mod tests {
         assert!(tracker.history().is_empty());
     }
 
-    // Additional tests to be added with implementation:
-    // - record_denial increments consecutive_count
-    // - should_warn returns false below threshold
-    // - should_warn returns true at/above threshold
-    // - reset clears consecutive_count but preserves history
+    #[test]
+    fn record_denial_increments_counters() {
+        let mut tracker = DenialTracker::new();
+        tracker.record_denial("Bash", "denied by policy");
+        assert_eq!(tracker.consecutive_denials(), 1);
+        assert_eq!(tracker.total_denials(), 1);
+        assert_eq!(tracker.history()[0].tool_name, "Bash");
+        assert_eq!(tracker.history()[0].reason, "denied by policy");
+    }
+
+    #[test]
+    fn should_warn_false_below_threshold() {
+        let mut tracker = DenialTracker::new();
+        tracker.record_denial("Bash", "denied");
+        tracker.record_denial("Bash", "denied");
+        assert!(!tracker.should_warn());
+    }
+
+    #[test]
+    fn should_warn_true_at_consecutive_threshold() {
+        let mut tracker = DenialTracker::new();
+        for _ in 0..3 {
+            tracker.record_denial("Bash", "denied");
+        }
+        assert!(tracker.should_warn());
+    }
+
+    #[test]
+    fn should_warn_true_at_total_threshold() {
+        let mut tracker = DenialTracker::new();
+        for i in 0..20 {
+            tracker.record_denial("Bash", "denied");
+            // Reset consecutive every 2 to avoid hitting consecutive threshold
+            if i % 2 == 1 {
+                tracker.reset();
+            }
+        }
+        assert!(tracker.should_warn());
+    }
+
+    #[test]
+    fn reset_clears_consecutive_but_preserves_history() {
+        let mut tracker = DenialTracker::new();
+        tracker.record_denial("Bash", "denied");
+        tracker.record_denial("Bash", "denied");
+        assert_eq!(tracker.consecutive_denials(), 2);
+        assert_eq!(tracker.total_denials(), 2);
+
+        tracker.reset();
+        assert_eq!(tracker.consecutive_denials(), 0);
+        assert_eq!(tracker.total_denials(), 2); // history preserved
+    }
+
+    #[test]
+    fn reset_after_warn_clears_warning() {
+        let mut tracker = DenialTracker::new();
+        for _ in 0..3 {
+            tracker.record_denial("Bash", "denied");
+        }
+        assert!(tracker.should_warn());
+
+        tracker.reset();
+        assert!(!tracker.should_warn());
+    }
+
+    #[test]
+    fn multiple_tools_tracked() {
+        let mut tracker = DenialTracker::new();
+        tracker.record_denial("Bash", "policy");
+        tracker.record_denial("Edit", "read-only mode");
+        assert_eq!(tracker.total_denials(), 2);
+        assert_eq!(tracker.consecutive_denials(), 2);
+        assert_eq!(tracker.history()[0].tool_name, "Bash");
+        assert_eq!(tracker.history()[1].tool_name, "Edit");
+    }
 }

crates/core/src/permission/explainer.rs

@@ -7,7 +7,7 @@
 //! debugging permission rules and for surfacing context in the TUI.
 
 use super::PermissionDecision;
-use super::rule_parser::PermissionRule;
+use super::rule_parser::{PermissionRule, matches_rule};
 
 // ---------------------------------------------------------------------------
 // Types
@@ -34,40 +34,120 @@ pub struct PermissionExplanation {
 /// Given a tool name, the final decision, and the full set of permission rules,
 /// produces a [`PermissionExplanation`] describing why the decision was reached
 /// and what rule (if any) matched.
-///
-/// # Arguments
-///
-/// * `tool_name` - Name of the tool that was checked.
-/// * `decision` - The final [`PermissionDecision`] that was reached.
-/// * `rules` - The list of [`PermissionRule`]s that were evaluated.
-///
-/// # Examples
-///
-/// ```ignore
-/// let explanation = explain_decision("Bash", &PermissionDecision::Allow, &rules);
-/// println!("{}", explanation.decision);
-/// // => "Allowed: tool 'Bash' matches whitelist rule 'Bash(command:git*)'"
-/// ```
 pub fn explain_decision(
     tool_name: &str,
     decision: &PermissionDecision,
     rules: &[PermissionRule],
 ) -> PermissionExplanation {
-    todo!(
-        "Explain why tool '{tool_name}' got decision {:?} given {} rules",
-        decision,
-        rules.len()
-    )
+    // Try to find the first matching rule for context
+    let empty_input = serde_json::json!({});
+    let matched = rules
+        .iter()
+        .find(|r| matches_rule(r, tool_name, &empty_input));
+
+    match decision {
+        PermissionDecision::Allow => {
+            if let Some(rule) = matched {
+                PermissionExplanation {
+                    decision: format!("Allowed: tool '{tool_name}' matches rule '{rule}'"),
+                    matched_rule: Some(rule.to_string()),
+                    suggestion: None,
+                }
+            } else {
+                PermissionExplanation {
+                    decision: format!(
+                        "Allowed: tool '{tool_name}' permitted by current permission mode"
+                    ),
+                    matched_rule: None,
+                    suggestion: None,
+                }
+            }
+        }
+        PermissionDecision::Deny(reason) => {
+            let suggestion = suggest_allow_rule(tool_name, &empty_input);
+            if let Some(rule) = matched {
+                PermissionExplanation {
+                    decision: format!(
+                        "Denied: tool '{tool_name}' blocked by rule '{rule}' — {reason}"
+                    ),
+                    matched_rule: Some(rule.to_string()),
+                    suggestion,
+                }
+            } else {
+                PermissionExplanation {
+                    decision: format!("Denied: tool '{tool_name}' — {reason}"),
+                    matched_rule: None,
+                    suggestion,
+                }
+            }
+        }
+        PermissionDecision::AskUser(prompt) => {
+            let suggestion = suggest_allow_rule(tool_name, &empty_input);
+            PermissionExplanation {
+                decision: format!("Requires confirmation: tool '{tool_name}' — {prompt}"),
+                matched_rule: matched.map(std::string::ToString::to_string),
+                suggestion,
+            }
+        }
+    }
 }
 
 /// Generate a suggestion for how to allow a denied tool invocation.
 ///
 /// Returns `None` if no useful suggestion can be generated.
 pub fn suggest_allow_rule(tool_name: &str, tool_input: &serde_json::Value) -> Option<String> {
-    todo!(
-        "Generate a suggestion for allowing tool '{tool_name}' with input {:?}",
-        tool_input
-    )
+    // For Bash tools, suggest a command-specific allow rule if possible
+    if tool_name == "Bash" || tool_name == "bash" {
+        if let Some(command) = tool_input.get("command").and_then(|v| v.as_str()) {
+            // Extract the base command (first word) for a prefix suggestion
+            let base_cmd = command.split_whitespace().next().unwrap_or(command);
+            return Some(format!(
+                "Add 'Bash(command:{base_cmd}*)' to `allowed_tools` to auto-approve {base_cmd} commands"
+            ));
+        }
+        return Some(
+            "Add 'Bash(*)' to `allowed_tools` to auto-approve all shell commands".to_string(),
+        );
+    }
+
+    // For Edit/Write tools, suggest a path-scoped rule if possible
+    if (tool_name == "Edit" || tool_name == "Write" || tool_name == "edit" || tool_name == "write")
+        && let Some(path) = tool_input.get("file_path").and_then(|v| v.as_str())
+        && let Some(parent) = std::path::Path::new(path).parent()
+    {
+        return Some(format!(
+            "Add '{tool_name}(file_path:{}/**)' to `allowed_tools` to auto-approve edits in that directory",
+            parent.display()
+        ));
+    }
+
+    // For Read tools, suggest a path-scoped rule if possible
+    if (tool_name == "Read" || tool_name == "read")
+        && let Some(path) = tool_input.get("file_path").and_then(|v| v.as_str())
+        && let Some(parent) = std::path::Path::new(path).parent()
+    {
+        return Some(format!(
+            "Add 'Read(file_path:{}/**)' to `allowed_tools` to auto-approve reads in that directory",
+            parent.display()
+        ));
+    }
+
+    // For MCP tools, suggest the server-level wildcard
+    if tool_name.starts_with("mcp__") {
+        // Extract the server name: mcp__<server>__<tool>
+        let parts: Vec<&str> = tool_name.splitn(3, "__").collect();
+        if parts.len() >= 2 {
+            let server = parts[1];
+            return Some(format!(
+                "Add 'mcp__{server}__*' to `allowed_tools` to auto-approve all tools from this MCP server"
+            ));
+        }
+    }
+
+    // Generic: suggest allowing the tool by name
+    Some(format!(
+        "Add '{tool_name}' to `allowed_tools` to auto-approve this tool"
+    ))
 }
 
 // ---------------------------------------------------------------------------
@@ -76,10 +156,94 @@ pub fn suggest_allow_rule(tool_name: &str, tool_input: &serde_json::Value) -> Op
 
 #[cfg(test)]
 mod tests {
-    // Tests will be added as the implementation progresses.
-    // Key test scenarios:
-    // - Explain an Allow decision with a matching whitelist rule
-    // - Explain a Deny decision with a matching denied rule
-    // - Explain an AskUser decision (no matching rule, default behavior)
-    // - Suggestion generation for common tool patterns
+    use super::*;
+    use crate::permission::rule_parser::parse_rule;
+
+    #[test]
+    fn explain_allow_with_matching_rule() {
+        // Use a tool-wide rule (no content constraint) so it matches with empty input
+        let rules = vec![parse_rule("Bash").unwrap()];
+        let explanation = explain_decision("Bash", &PermissionDecision::Allow, &rules);
+        assert!(explanation.decision.contains("Allowed"));
+        assert!(explanation.decision.contains("Bash"));
+        assert!(explanation.matched_rule.is_some());
+        assert!(explanation.suggestion.is_none());
+    }
+
+    #[test]
+    fn explain_allow_without_matching_rule() {
+        let rules = vec![parse_rule("Edit").unwrap()];
+        let explanation = explain_decision("Bash", &PermissionDecision::Allow, &rules);
+        assert!(explanation.decision.contains("Allowed"));
+        assert!(explanation.decision.contains("permission mode"));
+        assert!(explanation.matched_rule.is_none());
+    }
+
+    #[test]
+    fn explain_deny_with_reason() {
+        let rules = vec![parse_rule("Bash").unwrap()];
+        let explanation = explain_decision(
+            "Bash",
+            &PermissionDecision::Deny("tool is in denied list".to_string()),
+            &rules,
+        );
+        assert!(explanation.decision.contains("Denied"));
+        assert!(explanation.decision.contains("denied list"));
+        assert!(explanation.matched_rule.is_some());
+        assert!(explanation.suggestion.is_some());
+    }
+
+    #[test]
+    fn explain_ask_user() {
+        let rules = vec![];
+        let explanation = explain_decision(
+            "Bash",
+            &PermissionDecision::AskUser("confirm execution".to_string()),
+            &rules,
+        );
+        assert!(explanation.decision.contains("Requires confirmation"));
+        assert!(explanation.suggestion.is_some());
+    }
+
+    #[test]
+    fn suggest_bash_command_rule() {
+        let input = serde_json::json!({"command": "git status"});
+        let suggestion = suggest_allow_rule("Bash", &input);
+        assert!(suggestion.is_some());
+        let s = suggestion.unwrap();
+        assert!(s.contains("git"));
+        assert!(s.contains("allowed_tools"));
+    }
+
+    #[test]
+    fn suggest_bash_generic() {
+        let input = serde_json::json!({});
+        let suggestion = suggest_allow_rule("Bash", &input);
+        assert!(suggestion.is_some());
+        assert!(suggestion.unwrap().contains("Bash(*)"));
+    }
+
+    #[test]
+    fn suggest_mcp_server_rule() {
+        let input = serde_json::json!({});
+        let suggestion = suggest_allow_rule("mcp__playwright__click", &input);
+        assert!(suggestion.is_some());
+        assert!(suggestion.unwrap().contains("mcp__playwright__*"));
+    }
+
+    #[test]
+    fn suggest_generic_tool_rule() {
+        let input = serde_json::json!({});
+        let suggestion = suggest_allow_rule("CustomTool", &input);
+        assert!(suggestion.is_some());
+        assert!(suggestion.unwrap().contains("CustomTool"));
+    }
+
+    #[test]
+    fn explain_allow_with_wildcard_rule() {
+        let rules = vec![parse_rule("*").unwrap()];
+        let explanation = explain_decision("AnyTool", &PermissionDecision::Allow, &rules);
+        assert!(explanation.decision.contains("Allowed"));
+        assert_eq!(explanation.matched_rule, Some("*".to_string()));
+    }
 }