Repository navigation
fix(permissions): fail closed when a settings file cannot be read or parsed - #4287
Open
mrhard9090 wants to merge 1 commit into
Open
mrhard9090 wants to merge 1 commit into
mrhard9090 wants to merge 1 commit into
Conversation
…parsed A Claude Code settings file that exists but cannot be read or parsed adds a catch-all Ask rule and a warning instead of being skipped. Refs rtk-ai#4282.
📊 Automated PR Analysis
SummaryFixes rtk's permission rule loader so that a settings.json file which exists but fails to read or parse no longer counts as having no rules. Instead, it now prints a warning and injects a catch-all Ask rule, ensuring commands fail closed (deny/ask) rather than falling through to Default. The loader body was also extracted into a testable load_permission_rules_from(paths) function with accompanying unit tests. Review Checklist
Linked issues: #4282 Analyzed automatically by wshm · This is an automated analysis, not a human review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refs #4282
load_permission_rulesnow prints a warning for both cases (the read failure was silent before) and adds a catch-allAskrule, so every command gets at leastAsk, the verdict nothing relaxes. Deny rules from the other settings files still apply first.load_permission_rules_from(paths)so it can be tested with real files.What changes in practice:
AskandDefaultboth exit 3 fromrtk rewrite, but a delegate that owns approval (RTK_REWRITE_HOST=openclaw, #3909) relaxesDefaultto an allowed rewrite. With a brokensettings.json, a command that file denies was rewritten and allowed there (exit 0); now it exits 3.Not in this PR: the hooks still call
rtk rewrite ... 2>/dev/null, so the warning is not shown in that path, and the Cursor / Gemini / Droid loaders (read_json) are unchanged.Test plan
test_valid_settings_file_rules_apply,test_unparseable_settings_file_fails_closed_to_ask,test_unreadable_settings_file_fails_closed_to_ask,test_unreadable_settings_file_keeps_other_files_deny_rules,test_missing_and_empty_settings_files_add_no_rules.rtk rewrite "git push origin main"on Linux (WSL Ubuntu 24.04),CLAUDE_CONFIG_DIRwith{"permissions":{"deny":["Bash(git push *)"]}}(unreadable = a directory in place ofsettings.json, since the shell runs as root andchmod 000does not block it):developdevelop, openclawcargo test --bin rtkon Linux: 3787 passed, 8 ignored.cargo fmt --all -- --checkandcargo clippy --all-targets(clean) on Windows (MSVC).Written with help from Claude Opus 5.5.