Add global-level permissions from user config#2230
Conversation
|
/review |
pkg/runtime/runtime.go
Outdated
| // WithGlobalPermissions sets user-level permission patterns loaded from the | ||
| // user config (~/.config/cagent/config.yaml). These are evaluated after session | ||
| // and team permissions in the approval cascade, acting as user-wide defaults. | ||
| func WithGlobalPermissions(checker *permissions.Checker) Opt { |
There was a problem hiding this comment.
I'm not sure the runtime needs to know that global permissions exist, the runtime should just get a permissions checker that is already merged
There was a problem hiding this comment.
Agreed — reworked. Global patterns are now merged into the team's checker in run.go before the runtime is created (permissions.Merge() + team.SetPermissions()). The runtime has no concept of "global" and receives a single pre-merged checker.
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
This PR correctly implements global-level permissions from user config. The code is well-structured with proper nil checks and comprehensive test coverage.
Key strengths:
- Proper permission cascade: Session → Team → Global → ReadOnlyHint → Ask
- Safe nil handling throughout (checks before creating options, checks before accessing fields)
- Comprehensive test coverage (7 new tests covering deny, allow, cascade priority, YOLO override, and merging)
- Clear documentation in comments
Verified:
- No nil pointer dereferences (all accesses properly guarded)
- Permission cascade logic is correct and matches documentation
- Global permissions are properly integrated into the runtime via options pattern
- Tests validate all priority scenarios
The implementation follows Go best practices and the project's existing patterns.
733673b to
f7d632b
Compare
Allow users to define permission patterns in ~/.config/cagent/config.yaml that apply across all sessions and agents as user-wide defaults. Global patterns are merged into the team's permission checker before the runtime is created, so the runtime receives a single pre-merged set.
f7d632b to
1fe3f42
Compare
|
/review |
Assessment: 🟢 APPROVENo bugs found in the changed code. The implementation correctly:
The permission merge logic is sound and follows Go best practices. |
Summary
Closes #52
Adds a
permissionsfield to the user config (~/.config/cagent/config.yaml) so users can define permission patterns that apply across all sessions and agents as user-wide defaults.Configuration
Same pattern syntax as agent-level permissions — globs, argument matching (
shell:cmd=git*), and MCP-qualified names (mcp:server:tool).How it works
Global patterns are merged into the team's checker before the runtime is created. The runtime has no concept of "global" — it sees a single pre-merged checker.
Before
After
* Ask = no matching pattern, fall through to next checkerThe runtime evaluation is identical — the only difference is the checker now contains patterns from both sources.