fix(hook): close branch-guard bypasses for bare filenames, new dirs, missing path fields

src/hook_redirect.rs

@@ -149,18 +149,39 @@ pub fn redirect_decision(tool_name: &str, tool_input: Option<&Value>) -> Option<
         // need it. When we do check, resolve the target file's repo, not
         // host cwd.
         let (current, default) = if MUTATING_TOOLS.contains(&tool_name.as_str()) {
-            let target_repo = tool_input.as_ref().and_then(|ti| {
+            let target_path = tool_input.as_ref().and_then(|ti| {
                 ti.get("file_path")
                     .or_else(|| ti.get("path"))
                     .and_then(Value::as_str)
                     .map(Path::new)
-                    .and_then(|p| p.parent())
-                    .and_then(flare_git_core::branch::repo_toplevel)
             });
-            match target_repo {
-                Some(repo) => (current_branch(Some(&repo)), default_branch(Some(&repo))),
-                // Target path not in any git repo → no branch guard.
-                None => (None, None),
+            // Walk up from the target to the first ancestor that actually
+            // exists on disk before asking git for its toplevel -- a bare
+            // filename's parent is "" (no such dir) and a new file's parent
+            // may not exist yet, either of which would otherwise make the
+            // git subprocess fail and silently skip the guard.
+            let target_repo = target_path.and_then(|p| {
+                p.ancestors().skip(1).find_map(|ancestor| {
+                    let check = if ancestor == Path::new("") {
+                        Path::new(".")
+                    } else {
+                        ancestor
+                    };
+                    check
+                        .exists()
+                        .then(|| flare_git_core::branch::repo_toplevel(check))
+                        .flatten()
+                })
+            });
+            match (target_path, target_repo) {
+                // Path was extracted but isn't in any git repo -- no guard.
+                (Some(_), None) => (None, None),
+                // Path couldn't be extracted (tool has no file_path/path,
+                // e.g. MultiEdit) -- fall back to cwd; repo found -- use it.
+                (_, repo) => (
+                    current_branch(repo.as_deref()),
+                    default_branch(repo.as_deref()),
+                ),
             }
         } else {
             (None, None)
@@ -320,4 +341,108 @@ mod tests {
             "a worker slower than the timeout must fail open to None"
         );
     }
+
+    // Guards `std::env::set_current_dir` below -- these are the only tests
+    // in this module that touch the real process cwd, so serialize just
+    // them rather than the whole (parallel) test binary.
+    static CWD_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
+
+    /// `git init` a temp repo with one commit on `branch`, past the point
+    /// where `rev-parse` calls succeed -- enough to exercise `redirect_decision`'s
+    /// real git subprocess path (`test_support` in flare-git-core is
+    /// `pub(crate)`, so this binary crate can't reuse it).
+    fn init_temp_repo(branch: &str) -> tempfile::TempDir {
+        let dir = tempfile::tempdir().unwrap();
+        let run = |args: &[&str]| {
+            let out = std::process::Command::new("git")
+                .args(args)
+                .current_dir(dir.path())
+                .output()
+                .unwrap();
+            assert!(
+                out.status.success(),
+                "git {args:?} failed: {}",
+                String::from_utf8_lossy(&out.stderr)
+            );
+        };
+        run(&["init", "-q", "-b", branch]);
+        run(&["config", "user.email", "test@example.com"]);
+        run(&["config", "user.name", "test"]);
+        std::fs::write(dir.path().join("seed.txt"), "seed").unwrap();
+        run(&["add", "seed.txt"]);
+        run(&["commit", "-q", "-m", "seed"]);
+        dir
+    }
+
+    /// Runs `body` with cwd set to `dir`, restoring the original cwd
+    /// afterward even if `body` panics.
+    fn with_cwd<T>(dir: &Path, body: impl FnOnce() -> T) -> T {
+        let _guard = CWD_LOCK.lock().unwrap_or_else(|e| e.into_inner());
+        let original = std::env::current_dir().unwrap();
+        std::env::set_current_dir(dir).unwrap();
+        let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(body));
+        std::env::set_current_dir(original).unwrap();
+        result.unwrap()
+    }
+
+    #[test]
+    fn redirect_decision_guards_bare_filename_via_cwd_fallback() {
+        // Regression for the CodeRabbit-flagged bypass on PR #283: a bare
+        // filename's `.parent()` is `""`, which used to be handed straight
+        // to `repo_toplevel` (ENOENT -> None -> guard silently skipped).
+        let repo = init_temp_repo("master");
+        with_cwd(repo.path(), || {
+            let decision = redirect_decision("Write", Some(&json!({"file_path": "file.txt"})));
+            assert!(
+                decision.is_some(),
+                "bare filename on the default branch must still be guarded"
+            );
+        });
+    }
+
+    #[test]
+    fn redirect_decision_guards_new_nested_path_via_ancestor_walk() {
+        // Second bypass: a new file under a directory that doesn't exist
+        // yet used to fail the git subprocess the same way.
+        let repo = init_temp_repo("master");
+        with_cwd(repo.path(), || {
+            let decision = redirect_decision(
+                "Write",
+                Some(&json!({"file_path": "new_dir/does_not_exist_yet.txt"})),
+            );
+            assert!(
+                decision.is_some(),
+                "a new file under a not-yet-created directory must still be guarded"
+            );
+        });
+    }
+
+    #[test]
+    fn redirect_decision_guards_missing_path_field_via_cwd_fallback() {
+        // Third bypass: MultiEdit-shaped input with no top-level file_path
+        // used to make target_repo resolution bail out to `(None, None)`
+        // unconditionally instead of falling back to cwd.
+        let repo = init_temp_repo("master");
+        with_cwd(repo.path(), || {
+            let decision = redirect_decision("MultiEdit", Some(&json!({"edits": []})));
+            assert!(
+                decision.is_some(),
+                "a MUTATING_TOOLS call with no file_path/path must still fall back to cwd"
+            );
+        });
+    }
+
+    #[test]
+    fn redirect_decision_still_skips_guard_outside_any_repo() {
+        // Not a regression case, but pins down the intended non-bypass
+        // behavior: a target genuinely outside any git repo must still
+        // pass through unguarded, ancestor walk or not.
+        let dir = tempfile::tempdir().unwrap();
+        let target = dir.path().join("file.txt");
+        let decision = redirect_decision(
+            "Write",
+            Some(&json!({"file_path": target.to_str().unwrap()})),
+        );
+        assert!(decision.is_none());
+    }
 }