[webdevsamran]

Summary

• Treat ErrorKind::NotADirectory identical to NotFound when reading Claude Code settings files so non-directory parents (e.g. .claude as a regular file) are skipped cleanly like a missing file instead of triggering unreadable-file warnings and the catch-all Ask.
• Strip leading UTF-8 BOM (\u{feff}) before checking content.trim().is_empty() so BOM-only settings files are correctly treated as empty and skipped rather than failing JSON parsing.
• Both skips happen before scan.every_file_read = false, ensuring a non-directory parent or BOM-only settings file does not force SettingsScan / claude_hook_deny_rules() to Unattested.
• Closes Permission loader: a settings file that cannot exist, or holds only a BOM, fails closed #4651.

Test plan

• Added unit tests in src/hooks/permissions.rs:

  • est_bom_only_settings_file_adds_no_rules: verifies that BOM-only and BOM-plus-whitespace settings files add no rules.
    

  • est_not_a_directory_settings_file_adds_no_rules: verifies that a settings path under a regular file parent adds no rules without error.
    

• Verified cargo fmt --check cleanly.

Important: All PRs must target the develop branch.

---

AI Disclosure: Implementation assisted by agentic workflow; logic, bounds, and regression tests thoroughly inspected.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[rtk-wshm-sync-bot]

wshm · Automated triage by AI

📊 Automated PR Analysis

🐛 Type	bug-fix
🟢 Risk	low

Summary

Fixes the Claude Code settings permission loader to treat ErrorKind::NotADirectory the same as NotFound (so a non-directory parent like a .claude file fails closed instead of warning/catch-all Ask), and strips a leading UTF-8 BOM before checking if settings content is empty so BOM-only files are skipped rather than failing JSON parsing. Adds two unit tests covering both cases.

Review Checklist

• Tests present
• Breaking change
• Docs updated

Linked issues: #4651

---

Analyzed automatically by wshm · This is an automated analysis, not a human review.

[houtinghang]

I tested this locally. The fix does what #4651 asks, and the two new tests fail without it and pass with it. The one thing in the way is the conflict with develop.

Conflict with develop. #4127 (62cf6ba7, merged 10-23) moved this loop into load_permission_rules_tracking() and added a three-line comment above the NotFound arm. That is the only conflicting hunk: keeping the comment and taking your matches! arm resolves it, and the rest of the patch applies unchanged. This is the resolution I tested:

            Ok(content) => content,
            // A file that is not there denies nothing. One that is there and
            // could not be read — permissions, or bytes that are not UTF-8 —
            // may hold anything.
            Err(e)
                if matches!(
                    e.kind(),
                    std::io::ErrorKind::NotFound | std::io::ErrorKind::NotADirectory
                ) =>
            {
                continue;
            }

After the rebase the change also reaches claude_hook_deny_rules(), the gate for the opt-in fish wrap from #4127. Both new skips happen before scan.every_file_read = false, so a .claude regular file or a BOM-only settings file no longer forces that gate to Unattested. That matches the SettingsScan doc ("Every settings file that exists was opened, decoded and parsed"), but since the issue is labelled area:security it may be worth a line in the description. (This part is from reading the code; I did not exercise the wrap path.)

What I ran (WSL2 Ubuntu 26.04 x86_64, rustc 1.99.0, debug builds of the PR's merge base 9a501c8d, the PR head 1c85e65c, develop 32b2ec43, and develop + this PR with the conflict resolved as above):

rtk rewrite "git status" exit codes, with an isolated HOME and CLAUDE_CONFIG_DIR, and allow: ["Bash(git status)"] in the other settings file:

case	base	PR	develop	develop + PR
project .claude is a regular file	3	0	3	0
settings.local.json = EF BB BF	3	0	3	0
settings.local.json = EF BB BF + " \r\n"	3	0	3	0
CLAUDE_CONFIG_DIR points at a regular file	3	0	3	0

The .claude file, BOM-only and CLAUDE_CONFIG_DIR cases under RTK_REWRITE_HOST=openclaw, with git log (no rule matches), also go from 3 to 0. In all of these the Not a directory (os error 20) / failed to parse warnings are gone after the fix.

Controls, the same on all four builds: invalid JSON → 3, BOM + invalid JSON → 3, BOM + deny: ["Bash(git status)"] → 2, settings.local.json that is a directory → 3, non-UTF-8 bytes → 3, a plain allow file → 0. The controls still print their warnings.

• cargo test --lib hooks::permissions: on the base and on develop with only the two new tests added, 147 passed and the 2 new tests failed; with the fix, 149/149 on both.
• develop + PR: cargo test --no-fail-fast 4915 passed, 0 failed (develop with only the two tests added: 4913 passed, the 2 new tests failed). cargo fmt --all --check and cargo clippy --all-targets are clean on both the PR head and the rebased commit.
• Not tested: Windows and macOS. Per the issue, Windows already reports NotFound for the regular-file layout, so test_not_a_directory_settings_file_adds_no_rules passes there with or without the fix.

Two small things, neither blocking:

• The first summary bullet says non-directory parents now "fail closed". After this change they are skipped like a missing file; the catch-all Ask is what #4287 calls failing closed. Rewording it would keep the description from reading as the reverse of the change.
• #4680 (for #4652) reworks read_json() for the Cursor and Droid loaders with the same NotFound-only arm and the same trim() check before the BOM is stripped. Whichever of the two lands second may want to carry both checks there too.

AI disclosure: I used Claude Code to build the binaries, run the tests above and draft this comment, and I checked the results and the wording before posting.

[webdevsamran]

Hi @houtinghang, thank you so much for the thorough review and for running the extensive local test matrices across Ubuntu, WSL2, and RTK rewrite modes!

Addressed in commit 455ea7f:

1. Rebased onto develop: resolved the conflict in load_permission_rules_tracking() from feat(hooks): wrap unambiguously-fish scripts as rtk run --shell fish #4127, preserving the explanatory comment above the matches! arm as suggested.
2. PR Description Polish: reworded the summary to clarify that non-directory parents are skipped cleanly like missing files rather than triggering unreadable-file warnings / catch-all Ask, and noted the SettingsScan / claude_hook_deny_rules() interaction.

Verified with cargo fmt --all --check cleanly.