Skip to content

Commit 272b7d0

Browse files
authored
Merge pull request #1785 from dgageot/always-ask-permission
permissions: add Ask list to force confirmation for tools
2 parents 57844d4 + 51b380e commit 272b7d0

File tree

9 files changed

+212
-64
lines changed

9 files changed

+212
-64
lines changed

cagent-schema.json

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,21 @@
636636
]
637637
]
638638
},
639+
"ask": {
640+
"type": "array",
641+
"description": "Tool patterns that always require user confirmation, even for tools that are normally auto-approved (e.g. read-only tools). Supports the same pattern syntax as allow: tool names with globs and argument matching (e.g., 'fetch' to always ask before fetching URLs).",
642+
"items": {
643+
"type": "string"
644+
},
645+
"examples": [
646+
[
647+
"fetch"
648+
],
649+
[
650+
"mcp:github:get_*"
651+
]
652+
]
653+
},
639654
"deny": {
640655
"type": "array",
641656
"description": "Tool patterns that are always rejected. Takes priority over allow patterns. Supports the same pattern syntax as allow: tool names with globs and argument matching (e.g., 'shell:cmd=rm -rf*' to block dangerous rm commands).",

pkg/app/app.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,9 +532,10 @@ func (a *App) PermissionsInfo() *runtime.PermissionsInfo {
532532
// Get session-level permissions
533533
var sessionPerms *runtime.PermissionsInfo
534534
if a.session != nil && a.session.Permissions != nil {
535-
if len(a.session.Permissions.Allow) > 0 || len(a.session.Permissions.Deny) > 0 {
535+
if len(a.session.Permissions.Allow) > 0 || len(a.session.Permissions.Ask) > 0 || len(a.session.Permissions.Deny) > 0 {
536536
sessionPerms = &runtime.PermissionsInfo{
537537
Allow: a.session.Permissions.Allow,
538+
Ask: a.session.Permissions.Ask,
538539
Deny: a.session.Permissions.Deny,
539540
}
540541
}
@@ -549,10 +550,12 @@ func (a *App) PermissionsInfo() *runtime.PermissionsInfo {
549550
result := &runtime.PermissionsInfo{}
550551
if sessionPerms != nil {
551552
result.Allow = append(result.Allow, sessionPerms.Allow...)
553+
result.Ask = append(result.Ask, sessionPerms.Ask...)
552554
result.Deny = append(result.Deny, sessionPerms.Deny...)
553555
}
554556
if teamPerms != nil {
555557
result.Allow = append(result.Allow, teamPerms.Allow...)
558+
result.Ask = append(result.Ask, teamPerms.Ask...)
556559
result.Deny = append(result.Deny, teamPerms.Deny...)
557560
}
558561

pkg/config/latest/types.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,14 +1069,18 @@ type RAGFusionConfig struct {
10691069
// PermissionsConfig represents tool permission configuration.
10701070
// Allow/Ask/Deny model. This controls tool call approval behavior:
10711071
// - Allow: Tools matching these patterns are auto-approved (like --yolo for specific tools)
1072-
// - Ask: Tools matching these patterns always require user approval (default behavior)
1072+
// - Ask: Tools matching these patterns always require user approval, even if the tool is read-only
10731073
// - Deny: Tools matching these patterns are always rejected, even with --yolo
10741074
//
10751075
// Patterns support glob-style matching (e.g., "shell", "read_*", "mcp:github:*")
1076-
// The evaluation order is: Deny (checked first), then Allow, then Ask (default)
1076+
// The evaluation order is: Deny (checked first), then Allow, then Ask (explicit), then default
1077+
// (read-only tools auto-approved, others ask)
10771078
type PermissionsConfig struct {
10781079
// Allow lists tool name patterns that are auto-approved without user confirmation
10791080
Allow []string `json:"allow,omitempty"`
1081+
// Ask lists tool name patterns that always require user confirmation,
1082+
// even for tools that are normally auto-approved (e.g. read-only tools)
1083+
Ask []string `json:"ask,omitempty"`
10801084
// Deny lists tool name patterns that are always rejected
10811085
Deny []string `json:"deny,omitempty"`
10821086
}

pkg/config/v4/types.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,14 +1056,18 @@ type RAGFusionConfig struct {
10561056
// PermissionsConfig represents tool permission configuration.
10571057
// Allow/Ask/Deny model. This controls tool call approval behavior:
10581058
// - Allow: Tools matching these patterns are auto-approved (like --yolo for specific tools)
1059-
// - Ask: Tools matching these patterns always require user approval (default behavior)
1059+
// - Ask: Tools matching these patterns always require user approval, even if the tool is read-only
10601060
// - Deny: Tools matching these patterns are always rejected, even with --yolo
10611061
//
10621062
// Patterns support glob-style matching (e.g., "shell", "read_*", "mcp:github:*")
1063-
// The evaluation order is: Deny (checked first), then Allow, then Ask (default)
1063+
// The evaluation order is: Deny (checked first), then Allow, then Ask (explicit), then default
1064+
// (read-only tools auto-approved, others ask)
10641065
type PermissionsConfig struct {
10651066
// Allow lists tool name patterns that are auto-approved without user confirmation
10661067
Allow []string `json:"allow,omitempty"`
1068+
// Ask lists tool name patterns that always require user confirmation,
1069+
// even for tools that are normally auto-approved (e.g. read-only tools)
1070+
Ask []string `json:"ask,omitempty"`
10671071
// Deny lists tool name patterns that are always rejected
10681072
Deny []string `json:"deny,omitempty"`
10691073
}

pkg/permissions/permissions.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ const (
2020
Allow
2121
// Deny means the tool is rejected and should not be executed
2222
Deny
23+
// ForceAsk means an explicit ask pattern matched; the tool must be
24+
// confirmed even if it would normally be auto-approved (e.g. read-only).
25+
ForceAsk
2326
)
2427

2528
// String returns a human-readable representation of the decision
@@ -31,6 +34,8 @@ func (d Decision) String() string {
3134
return "allow"
3235
case Deny:
3336
return "deny"
37+
case ForceAsk:
38+
return "force_ask"
3439
default:
3540
return "unknown"
3641
}
@@ -39,6 +44,7 @@ func (d Decision) String() string {
3944
// Checker evaluates tool permissions based on configured patterns
4045
type Checker struct {
4146
allowPatterns []string
47+
askPatterns []string
4248
denyPatterns []string
4349
}
4450

@@ -49,6 +55,7 @@ func NewChecker(cfg *latest.PermissionsConfig) *Checker {
4955
}
5056
return &Checker{
5157
allowPatterns: cfg.Allow,
58+
askPatterns: cfg.Ask,
5259
denyPatterns: cfg.Deny,
5360
}
5461
}
@@ -61,7 +68,7 @@ func (c *Checker) Check(toolName string) Decision {
6168
}
6269

6370
// CheckWithArgs evaluates the permission for a given tool name and its arguments.
64-
// Evaluation order: Deny (checked first), then Allow, then Ask (default)
71+
// Evaluation order: Deny (checked first), then Allow, then Ask (explicit), then Ask (default).
6572
//
6673
// The toolName can be a simple name like "shell" or a qualified name like
6774
// "mcp:github:create_issue".
@@ -71,6 +78,10 @@ func (c *Checker) Check(toolName string) Decision {
7178
// - Argument matching: "shell:cmd=ls*" matches shell tool with cmd argument starting with "ls"
7279
// - Multiple arguments: "shell:cmd=ls*:cwd=/home/*" matches both conditions
7380
// - Glob patterns in both tool names and argument values
81+
//
82+
// Returns ForceAsk when an explicit ask pattern matches. ForceAsk means the
83+
// tool must always be confirmed, even when it would normally be auto-approved
84+
// (e.g. read-only tools or --yolo mode).
7485
func (c *Checker) CheckWithArgs(toolName string, args map[string]any) Decision {
7586
// Deny patterns are checked first - they take priority
7687
for _, pattern := range c.denyPatterns {
@@ -86,20 +97,32 @@ func (c *Checker) CheckWithArgs(toolName string, args map[string]any) Decision {
8697
}
8798
}
8899

100+
// Explicit ask patterns override auto-approval (e.g. read-only hints)
101+
for _, pattern := range c.askPatterns {
102+
if matchToolPattern(pattern, toolName, args) {
103+
return ForceAsk
104+
}
105+
}
106+
89107
// Default is Ask
90108
return Ask
91109
}
92110

93111
// IsEmpty returns true if no permissions are configured
94112
func (c *Checker) IsEmpty() bool {
95-
return len(c.allowPatterns) == 0 && len(c.denyPatterns) == 0
113+
return len(c.allowPatterns) == 0 && len(c.askPatterns) == 0 && len(c.denyPatterns) == 0
96114
}
97115

98116
// AllowPatterns returns the list of allow patterns.
99117
func (c *Checker) AllowPatterns() []string {
100118
return c.allowPatterns
101119
}
102120

121+
// AskPatterns returns the list of ask patterns.
122+
func (c *Checker) AskPatterns() []string {
123+
return c.askPatterns
124+
}
125+
103126
// DenyPatterns returns the list of deny patterns.
104127
func (c *Checker) DenyPatterns() []string {
105128
return c.denyPatterns

pkg/permissions/permissions_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,15 @@ func TestNewChecker(t *testing.T) {
3535
require.NotNil(t, checker)
3636
assert.False(t, checker.IsEmpty())
3737
})
38+
39+
t.Run("with only ask patterns", func(t *testing.T) {
40+
t.Parallel()
41+
checker := NewChecker(&latest.PermissionsConfig{
42+
Ask: []string{"fetch"},
43+
})
44+
require.NotNil(t, checker)
45+
assert.False(t, checker.IsEmpty())
46+
})
3847
}
3948

4049
func TestChecker_Check(t *testing.T) {
@@ -43,6 +52,7 @@ func TestChecker_Check(t *testing.T) {
4352
tests := []struct {
4453
name string
4554
allow []string
55+
ask []string
4656
deny []string
4757
toolName string
4858
want Decision
@@ -138,13 +148,35 @@ func TestChecker_Check(t *testing.T) {
138148
toolName: "read_file",
139149
want: Allow,
140150
},
151+
// Ask patterns
152+
{
153+
name: "ask pattern returns ForceAsk",
154+
ask: []string{"fetch"},
155+
toolName: "fetch",
156+
want: ForceAsk,
157+
},
158+
{
159+
name: "deny takes priority over ask",
160+
ask: []string{"fetch"},
161+
deny: []string{"fetch"},
162+
toolName: "fetch",
163+
want: Deny,
164+
},
165+
{
166+
name: "allow takes priority over ask",
167+
allow: []string{"fetch"},
168+
ask: []string{"fetch"},
169+
toolName: "fetch",
170+
want: Allow,
171+
},
141172
}
142173

143174
for _, tt := range tests {
144175
t.Run(tt.name, func(t *testing.T) {
145176
t.Parallel()
146177
checker := NewChecker(&latest.PermissionsConfig{
147178
Allow: tt.allow,
179+
Ask: tt.ask,
148180
Deny: tt.deny,
149181
})
150182
got := checker.Check(tt.toolName)
@@ -383,6 +415,7 @@ func TestDecision_String(t *testing.T) {
383415
{Ask, "ask"},
384416
{Allow, "allow"},
385417
{Deny, "deny"},
418+
{ForceAsk, "force_ask"},
386419
{Decision(99), "unknown"},
387420
}
388421

@@ -394,6 +427,38 @@ func TestDecision_String(t *testing.T) {
394427
}
395428
}
396429

430+
func TestChecker_ForceAsk(t *testing.T) {
431+
t.Parallel()
432+
433+
tests := []struct {
434+
name string
435+
allow []string
436+
ask []string
437+
deny []string
438+
toolName string
439+
want Decision
440+
}{
441+
{name: "no patterns returns Ask", toolName: "fetch", want: Ask},
442+
{name: "ask pattern returns ForceAsk", ask: []string{"fetch"}, toolName: "fetch", want: ForceAsk},
443+
{name: "ask glob returns ForceAsk", ask: []string{"fetch*"}, toolName: "fetch_url", want: ForceAsk},
444+
{name: "ask pattern does not match other tool", ask: []string{"fetch"}, toolName: "shell", want: Ask},
445+
{name: "deny takes priority over ask", ask: []string{"fetch"}, deny: []string{"fetch"}, toolName: "fetch", want: Deny},
446+
{name: "allow takes priority over ask", ask: []string{"fetch"}, allow: []string{"fetch"}, toolName: "fetch", want: Allow},
447+
}
448+
449+
for _, tt := range tests {
450+
t.Run(tt.name, func(t *testing.T) {
451+
t.Parallel()
452+
checker := NewChecker(&latest.PermissionsConfig{
453+
Allow: tt.allow,
454+
Ask: tt.ask,
455+
Deny: tt.deny,
456+
})
457+
assert.Equal(t, tt.want, checker.Check(tt.toolName))
458+
})
459+
}
460+
}
461+
397462
func TestParsePattern(t *testing.T) {
398463
t.Parallel()
399464

0 commit comments

Comments
 (0)