fix: fail closed when permissions settings file cannot be read or parsed (fixes #4282)

src/core/runner.rs

@@ -15,6 +15,7 @@ use crate::core::truncate::{CAP_LIST, CAP_WARNINGS};
 /// emitted so the caller tracks exactly that.
 pub fn emit_guarded(filtered: &str, hint: Option<&str>, raw: &str) -> String {
     let body = match hint {
+        Some(h) if filtered.is_empty() => h.to_string(),
         Some(h) => format!("{}\n{}", filtered, h),
         None => filtered.to_string(),
     };
@@ -1192,6 +1193,51 @@ mod forwarded_stderr_tests {
     }
 }
 
+#[cfg(test)]
+mod emit_guarded_tests {
+    use super::*;
+
+    #[test]
+    fn test_emit_guarded_empty_filtered_with_hint_no_leading_blank() {
+        // Use a long raw matching the repro (600 newlines) so never_worse prefers the hint
+        let long_raw = "x".repeat(600);
+        let result = emit_guarded(
+            "",
+            Some("[full output: ~/.cache/rtk/tee/1_uv.log]"),
+            &long_raw,
+        );
+        assert!(
+            !result.starts_with('\n'),
+            "should not start with blank line: {:?}",
+            result
+        );
+        assert_eq!(result, "[full output: ~/.cache/rtk/tee/1_uv.log]");
+    }
+
+    #[test]
+    fn test_emit_guarded_non_empty_filtered_with_hint_has_separator() {
+        // Use a long raw matching the repro so never_worse prefers the filtered+body
+        let long_raw = "x".repeat(600);
+        let result = emit_guarded("body content", Some("[hint]"), &long_raw);
+        assert!(
+            result.starts_with("body content"),
+            "should start with body: {:?}",
+            result
+        );
+        assert!(
+            result.contains("\n[hint]"),
+            "should have separator: {:?}",
+            result
+        );
+    }
+
+    #[test]
+    fn test_emit_guarded_no_hint_returns_filtered() {
+        let result = emit_guarded("body content", None, "raw raw raw");
+        assert_eq!(result, "body content");
+    }
+}
+
 #[cfg(test)]
 mod err_test_runner_tests {
     use super::*;

src/discover/mod.rs

@@ -301,7 +301,7 @@ pub fn run(
         registry::normalize_transparent_prefixes(&transparent_prefixes);
 
     // Loaded once up front (see `PermissionRules`), not once per command.
-    let (deny, ask, allow) = permissions::load_rules_for(permissions::Host::Claude);
+    let (deny, ask, allow, _config_error) = permissions::load_rules_for(permissions::Host::Claude);
     let rules = PermissionRules { deny, ask, allow };
 
     let cutoff = crate::core::utils::days_ago_cutoff(since_days);

src/hooks/decision.rs

@@ -79,7 +79,8 @@ pub(crate) fn decide_with_params(
     excluded: &[String],
     transparent_prefixes: &[String],
 ) -> HookDecision {
-    if verdict == PermissionVerdict::Deny {
+    // ConfigError means a settings file exists but couldn't be read/parsed — fail closed.
+    if verdict == PermissionVerdict::ConfigError || verdict == PermissionVerdict::Deny {
         return HookDecision::Deny;
     }
 
@@ -419,7 +420,8 @@ mod tests {
             check_command_for("git status", Host::Codex),
             PermissionVerdict::Default
         );
-        let (deny, ask, allow) = super::super::permissions::load_rules_for(Host::Codex);
+        let (deny, ask, allow, _config_error) =
+            super::super::permissions::load_rules_for(Host::Codex);
         assert!(deny.is_empty() && ask.is_empty() && allow.is_empty());
     }
 

src/hooks/permissions.rs

@@ -19,6 +19,8 @@ pub enum PermissionVerdict {
     Ask,
     /// No rule matched — default to ask (matches Claude Code's least-privilege default).
     Default,
+    /// A settings file exists but could not be read or parsed — fail closed.
+    ConfigError,
 }
 
 /// Check `cmd` against Claude Code's deny/ask/allow permission rules.
@@ -43,7 +45,10 @@ pub enum Host {
 }
 
 pub fn check_command_for(cmd: &str, host: Host) -> PermissionVerdict {
-    let (deny_rules, ask_rules, allow_rules) = load_rules_for(host);
+    let (deny_rules, ask_rules, allow_rules, config_error) = load_rules_for(host);
+    if config_error {
+        return PermissionVerdict::ConfigError;
+    }
     check_command_with_rules(cmd, &deny_rules, &ask_rules, &allow_rules)
 }
 
@@ -53,17 +58,29 @@ pub fn check_command_for(cmd: &str, host: Host) -> PermissionVerdict {
 /// can load once up front and reuse `check_command_with_rules` per command instead
 /// of going through `check_command_for` and re-reading every settings file from
 /// disk on every single call.
-pub(crate) fn load_rules_for(host: Host) -> (Vec<String>, Vec<String>, Vec<String>) {
+pub(crate) fn load_rules_for(host: Host) -> (Vec<String>, Vec<String>, Vec<String>, bool) {
     match host {
-        Host::Claude => load_permission_rules(),
-        Host::Cursor => load_cursor_rules(),
-        Host::Gemini => load_gemini_rules(),
-        Host::Droid => load_droid_rules(),
+        Host::Claude => {
+            let (deny, ask, allow, config_error) = load_permission_rules();
+            (deny, ask, allow, config_error)
+        }
+        Host::Cursor => {
+            let (deny, ask, allow) = load_cursor_rules();
+            (deny, ask, allow, false)
+        }
+        Host::Gemini => {
+            let (deny, ask, allow) = load_gemini_rules();
+            (deny, ask, allow, false)
+        }
+        Host::Droid => {
+            let (deny, ask, allow) = load_droid_rules();
+            (deny, ask, allow, false)
+        }
         // Hosts with no RTK-side rule source. Codex enforces its native
         // execution rules after updatedInput. Do not interpret these hosts'
         // rules as Claude Bash patterns or borrow another host's settings.
         // No RTK-side match means Default, not an explicit Allow.
-        Host::Codex | Host::Trae | Host::Vibe => (Vec::new(), Vec::new(), Vec::new()),
+        Host::Codex | Host::Trae | Host::Vibe => (Vec::new(), Vec::new(), Vec::new(), false),
     }
 }
 
@@ -146,22 +163,39 @@ pub(crate) fn check_command_with_rules(
 /// 3. `~/.claude/settings.json`
 /// 4. `~/.claude/settings.local.json`
 ///
-/// Missing files and malformed JSON are silently skipped.
-fn load_permission_rules() -> (Vec<String>, Vec<String>, Vec<String>) {
+/// If any settings file exists but cannot be read or parsed, returns config_error=true
+/// to trigger fail-closed behavior (ConfigError verdict).
+fn load_permission_rules() -> (Vec<String>, Vec<String>, Vec<String>, bool) {
     let mut deny_rules = Vec::new();
     let mut ask_rules = Vec::new();
     let mut allow_rules = Vec::new();
+    let mut config_error = false;
 
     for path in get_settings_paths() {
-        let Ok(content) = std::fs::read_to_string(&path) else {
+        if !path.exists() {
             continue;
+        }
+        let content = match std::fs::read_to_string(&path) {
+            Ok(c) => c,
+            Err(_) => {
+                eprintln!(
+                    "[rtk] error: failed to read permissions from {}",
+                    path.display()
+                );
+                config_error = true;
+                continue;
+            }
         };
-        let Ok(json) = crate::core::utils::from_json_str::<Value>(&content) else {
-            eprintln!(
-                "[rtk] warning: failed to parse permissions from {}",
-                path.display()
-            );
-            continue;
+        let json = match crate::core::utils::from_json_str::<Value>(&content) {
+            Ok(j) => j,
+            Err(_) => {
+                eprintln!(
+                    "[rtk] error: failed to parse permissions from {}",
+                    path.display()
+                );
+                config_error = true;
+                continue;
+            }
         };
         let Some(permissions) = json.get("permissions") else {
             continue;
@@ -172,7 +206,7 @@ fn load_permission_rules() -> (Vec<String>, Vec<String>, Vec<String>) {
         append_bash_rules(permissions.get("allow"), &mut allow_rules);
     }
 
-    (deny_rules, ask_rules, allow_rules)
+    (deny_rules, ask_rules, allow_rules, config_error)
 }
 
 /// Extract Bash-scoped patterns from a JSON array and append them to `target`.

src/hooks/rewrite_cmd.rs

@@ -211,10 +211,12 @@ mod tests {
         ///   Ask    → 3 (process::exit(3))
         ///   Default→ 3 (process::exit(3)) — grouped with Ask
         ///   Deny   → 2 (process::exit(2)) — handled before rewrite match
+        ///   ConfigError → 2 (fail closed, same as Deny)
         fn expected_exit_code(verdict: &PermissionVerdict) -> i32 {
             match verdict {
                 PermissionVerdict::Allow => 0,
                 PermissionVerdict::Deny => 2,
+                PermissionVerdict::ConfigError => 2,
                 PermissionVerdict::Ask => 3,
                 PermissionVerdict::Default => 3, // MUST be 3, not 0!
             }
@@ -256,6 +258,14 @@ mod tests {
             assert_eq!(expected_exit_code(&verdict), 2);
         }
 
+        #[test]
+        fn test_config_error_verdict_maps_to_deny_exit_code() {
+            // ConfigError (unreadable/unparseable settings file) maps to Deny (exit 2) — fail closed.
+            // This is tested indirectly by ensuring the enum variant exists and maps to 2.
+            // A direct test would require triggering a config error, which requires file I/O.
+            assert_eq!(expected_exit_code(&PermissionVerdict::ConfigError), 2);
+        }
+
         #[test]
         fn test_no_auto_allow_bypass_for_unrecognized_commands() {
             // SECURITY: A command with no permission rules and no matching allow rule