fix: root-level permission config not being respected#14070
Open
bfirsh wants to merge 1 commit into
Open
Conversation
In fromConfig, wildcard permission keys like "*" could end up after specific tool keys (like "bash") due to key reordering from mergeDeep during config merging. Since evaluate uses findLast, the catch-all "*" rule would win over the more specific tool rule. Fix by sorting wildcard permission keys before specific keys in fromConfig so specific tool rules always appear later and take precedence. Closes anomalyco#8832
6 tasks
|
Thanks a lot for the fix Ben, can't wait till it will land! |
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
Fixes #8832
Root-level
permissionconfig inopencode.jsonwas being ignored. The same config worked when placed underagent.build.permissionbut not at the root.Examples that didn't work
Setting permissions at the root level had no effect — bash commands would still be auto-allowed instead of prompting:
{ "permission": { "*": "ask", "bash": { "*": "ask", "ls *": "allow" } } }Similarly, this config intended to require approval for all bash except
ls, but all commands ran without asking:{ "permission": { "*": "allow", "bash": { "*": "ask", "git log *": "allow", "ls *": "allow" }, "edit": "ask" } }Moving the exact same config under
agent.build.permissionworked fine.Cause
When multiple config sources are merged via
mergeDeep(e.g. global config + project config), JavaScript object key ordering is not guaranteed. The catch-all"*": "ask"key could end up after specific tool keys like"bash"in the merged object.fromConfigiterates withObject.entries(), so the{ permission: "*", pattern: "*", action: "ask" }rule would appear after{ permission: "bash", pattern: "ls *", action: "allow" }in the ruleset. SinceevaluateusesfindLast, the wildcard*rule won over the specificbashrule.Fix
In
fromConfig, wildcard permission keys (like"*") are now always processed before specific tool keys. This ensures specific tool rules appear later in the ruleset and take precedence viafindLast, regardless of the key ordering in the config object.