git.iko.soy/crush
on main help sign in
feat(permissions): require a permission prompt for chained commands
96728b1Kieran Klukas
internal/agent/tools/bash.go+7−5
compare with main
unifiedsplit
@@ -205,11 +205,13 @@ func NewBashTool(permissions permission.Service, workingDir string, attribution
205 isSafeReadOnly := false
205 isSafeReadOnly := false
206 cmdLower := strings.ToLower(params.Command)
206 cmdLower := strings.ToLower(params.Command)
207
207
208+ if !containsCommandChaining(params.Command) {
208− for _, safe := range safeCommands {
209+ for _, safe := range safeCommands {
209− if strings.HasPrefix(cmdLower, safe) {
210+ if strings.HasPrefix(cmdLower, safe) {
210− if len(cmdLower) == len(safe) || cmdLower[len(safe)] == ' ' || cmdLower[len(safe)] == '-' {
211+ if len(cmdLower) == len(safe) || cmdLower[len(safe)] == ' ' || cmdLower[len(safe)] == '-' {
211− isSafeReadOnly = true
212+ isSafeReadOnly = true
212− break
213+ break
214+ }
213 }
215 }
214 }
216 }
215 }
217 }
internal/agent/tools/bash_test.go+77
@@ -79,12 +79,89 @@ func TestBashTool_CustomAutoBackgroundThreshold(t *testing.T) {
79 require.NoError(t, bgManager.Kill(meta.ShellID))
79 require.NoError(t, bgManager.Kill(meta.ShellID))
80}
80}
81
81
82+type recordingPermissionService struct {
83+ *pubsub.Broker[permission.PermissionRequest]
84+ requestCount int
85+ allow bool
86+}
87+
88+func (m *recordingPermissionService) Request(ctx context.Context, req permission.CreatePermissionRequest) (bool, error) {
89+ m.requestCount++
90+ return m.allow, nil
91+}
92+
93+func (m *recordingPermissionService) Grant(req permission.PermissionRequest) {}
94+
95+func (m *recordingPermissionService) Deny(req permission.PermissionRequest) {}
96+
97+func (m *recordingPermissionService) GrantPersistent(req permission.PermissionRequest) {}
98+
99+func (m *recordingPermissionService) AutoApproveSession(sessionID string) {}
100+
101+func (m *recordingPermissionService) SetSkipRequests(skip bool) {}
102+
103+func (m *recordingPermissionService) SkipRequests() bool {
104+ return false
105+}
106+
107+func (m *recordingPermissionService) SubscribeNotifications(ctx context.Context) <-chan pubsub.Event[permission.PermissionNotification] {
108+ return make(<-chan pubsub.Event[permission.PermissionNotification])
109+}
110+
82func newBashToolForTest(workingDir string) fantasy.AgentTool {
111func newBashToolForTest(workingDir string) fantasy.AgentTool {
83 permissions := &mockBashPermissionService{Broker: pubsub.NewBroker[permission.PermissionRequest]()}
112 permissions := &mockBashPermissionService{Broker: pubsub.NewBroker[permission.PermissionRequest]()}
84 attribution := &config.Attribution{TrailerStyle: config.TrailerStyleNone}
113 attribution := &config.Attribution{TrailerStyle: config.TrailerStyleNone}
85 return NewBashTool(permissions, workingDir, attribution, "test-model")
114 return NewBashTool(permissions, workingDir, attribution, "test-model")
86}
115}
87
116
117+func newBashToolWithRecordingPerms(workingDir string, allow bool) (fantasy.AgentTool, *recordingPermissionService) {
118+ perms := &recordingPermissionService{
119+ Broker: pubsub.NewBroker[permission.PermissionRequest](),
120+ allow: allow,
121+ }
122+ attribution := &config.Attribution{TrailerStyle: config.TrailerStyleNone}
123+ return NewBashTool(perms, workingDir, attribution, "test-model"), perms
124+}
125+
126+func TestBashTool_ChainedCommandsRequirePermission(t *testing.T) {
127+ workingDir := t.TempDir()
128+ tool, perms := newBashToolWithRecordingPerms(workingDir, true)
129+ ctx := context.WithValue(context.Background(), SessionIDContextKey, "test-session")
130+
131+ // ls && echo should trigger permission check.
132+ resp := runBashTool(t, tool, ctx, BashParams{
133+ Description: "chained ls",
134+ Command: "ls && echo done",
135+ })
136+
137+ require.False(t, resp.IsError)
138+ require.Equal(t, 1, perms.requestCount, "chained command should trigger permission request")
139+
140+ // Plain ls should NOT trigger permission check.
141+ perms.requestCount = 0
142+ resp = runBashTool(t, tool, ctx, BashParams{
143+ Description: "plain ls",
144+ Command: "ls -la",
145+ })
146+
147+ require.False(t, resp.IsError)
148+ require.Equal(t, 0, perms.requestCount, "plain ls should not trigger permission request")
149+}
150+
151+func TestBashTool_ChainedCommandsDenied(t *testing.T) {
152+ workingDir := t.TempDir()
153+ tool, perms := newBashToolWithRecordingPerms(workingDir, false)
154+ ctx := context.WithValue(context.Background(), SessionIDContextKey, "test-session")
155+
156+ resp := runBashTool(t, tool, ctx, BashParams{
157+ Description: "chained ls denied",
158+ Command: "ls && rm -rf /",
159+ })
160+
161+ require.Equal(t, 1, perms.requestCount)
162+ require.Contains(t, resp.Content, "User denied permission")
163+}
164+
88func runBashTool(t *testing.T, tool fantasy.AgentTool, ctx context.Context, params BashParams) fantasy.ToolResponse {
165func runBashTool(t *testing.T, tool fantasy.AgentTool, ctx context.Context, params BashParams) fantasy.ToolResponse {
89 t.Helper()
166 t.Helper()
90
167
internal/agent/tools/safe.go+21−1
@@ -1,6 +1,10 @@
1package tools
1package tools
2
2
3+import (
3−import "runtime"
4+ "runtime"
5+ "slices"
6+ "strings"
7+)
4
8
5var safeCommands = []string{
9var safeCommands = []string{
6 // Bash builtins and core utils
10 // Bash builtins and core utils
@@ -54,6 +58,22 @@ var safeCommands = []string{
54 "git tag",
58 "git tag",
55}
59}
56
60
61+var chainingMetacharacters = []string{
62+ ";",
63+ "|",
64+ "&&",
65+ "$(",
66+ "`",
67+}
68+
69+// containsCommandChaining reports whether s contains shell metacharacters
70+// that enable command chaining or substitution.
71+func containsCommandChaining(s string) bool {
72+ return slices.ContainsFunc(chainingMetacharacters, func(c string) bool {
73+ return strings.Contains(s, c)
74+ })
75+}
76+
57func init() {
77func init() {
58 if runtime.GOOS == "windows" {
78 if runtime.GOOS == "windows" {
59 safeCommands = append(
79 safeCommands = append(
internal/agent/tools/safe_test.gonew file+47
@@ -0,0 +1,47 @@
1+package tools
2+
3+import (
4+ "testing"
5+
6+ "github.com/stretchr/testify/assert"
7+)
8+
9+func TestContainsCommandChaining(t *testing.T) {
10+ t.Parallel()
11+
12+ tests := []struct {
13+ name string
14+ input string
15+ expected bool
16+ }{
17+ {"plain ls", "ls -la", false},
18+ {"plain echo", "echo hello world", false},
19+ {"plain pwd", "pwd", false},
20+ {"plain git status", "git status", false},
21+ {"ls with redirect", "ls > /tmp/out", false},
22+ {"ls with pipe", "ls | grep foo", true},
23+ {"ls with double ampersand", "ls && echo done", true},
24+ {"ls with semicolon", "ls; echo done", true},
25+ {"ls with pipe pipe", "ls || echo fail", true},
26+ {"ls with backticks", "ls `echo foo`", true},
27+ {"ls with subshell", "ls $(echo foo)", true},
28+ {"ls with background ampersand", "ls & echo done", false},
29+ {"rm -rf with && ls (rm first)", "rm -rf / && ls", true},
30+ {"redirect with ampersand gt", "ls &> /dev/null", false},
31+ {"redirect with gt ampersand", "ls >& /dev/null", false},
32+ {"simple kill", "kill 1234", false},
33+ {"kill with pipe", "kill 1234 | echo foo", true},
34+ {"git log", "git log --oneline", false},
35+ {"git log with pipe", "git log | head", true},
36+ {"empty string", "", false},
37+ {"dollar sign in argument", "echo $HOME", false},
38+ }
39+
40+ for _, tt := range tests {
41+ t.Run(tt.name, func(t *testing.T) {
42+ t.Parallel()
43+ got := containsCommandChaining(tt.input)
44+ assert.Equal(t, tt.expected, got, "containsCommandChaining(%q)", tt.input)
45+ })
46+ }
47+}