git.iko.soy/crush
on main help sign in
Revert "chore: implement correct banned commands"
de3b815Kujtim Hoxha
internal/llm/tools/bash.go+6−230
compare with main
unifiedsplit
@@ -10,7 +10,6 @@ import (
10
10
11 "github.com/charmbracelet/crush/internal/permission"
11 "github.com/charmbracelet/crush/internal/permission"
12 "github.com/charmbracelet/crush/internal/shell"
12 "github.com/charmbracelet/crush/internal/shell"
13− "mvdan.cc/sh/v3/syntax"
14)
13)
15
14
16type BashParams struct {
15type BashParams struct {
@@ -41,235 +40,10 @@ const (
41 BashNoOutput = "no output"
40 BashNoOutput = "no output"
42)
41)
43
42
44−func containsBannedCommand(node syntax.Node) bool {
45− if node == nil {
46− return false
47− }
48−
49− switch n := node.(type) {
50− case *syntax.CallExpr:
51− if len(n.Args) > 0 {
52− cmdName := getWordValue(n.Args[0])
53− for _, banned := range bannedCommands {
54− if strings.EqualFold(cmdName, banned) {
55− return true
56− }
57− }
58− }
59− for _, arg := range n.Args {
60− if containsBannedCommand(arg) {
61− return true
62− }
63− }
64− case *syntax.Word:
65− if checkWordForBannedCommands(n) {
66− return true
67− }
68− for _, part := range n.Parts {
69− if containsBannedCommand(part) {
70− return true
71− }
72− }
73− case *syntax.CmdSubst:
74− for _, stmt := range n.Stmts {
75− if containsBannedCommand(stmt) {
76− return true
77− }
78− }
79− case *syntax.Subshell:
80− for _, stmt := range n.Stmts {
81− if containsBannedCommand(stmt) {
82− return true
83− }
84− }
85− case *syntax.Stmt:
86− if containsBannedCommand(n.Cmd) {
87− return true
88− }
89− for _, redir := range n.Redirs {
90− if containsBannedCommand(redir) {
91− return true
92− }
93− }
94− case *syntax.BinaryCmd:
95− return containsBannedCommand(n.X) || containsBannedCommand(n.Y)
96− case *syntax.Block:
97− for _, stmt := range n.Stmts {
98− if containsBannedCommand(stmt) {
99− return true
100− }
101− }
102− case *syntax.IfClause:
103− for _, stmt := range n.Cond {
104− if containsBannedCommand(stmt) {
105− return true
106− }
107− }
108− for _, stmt := range n.Then {
109− if containsBannedCommand(stmt) {
110− return true
111− }
112− }
113− if n.Else != nil && containsBannedCommand(n.Else) {
114− return true
115− }
116− case *syntax.WhileClause:
117− for _, stmt := range n.Cond {
118− if containsBannedCommand(stmt) {
119− return true
120− }
121− }
122− for _, stmt := range n.Do {
123− if containsBannedCommand(stmt) {
124− return true
125− }
126− }
127− case *syntax.ForClause:
128− for _, stmt := range n.Do {
129− if containsBannedCommand(stmt) {
130− return true
131− }
132− }
133− if containsBannedCommand(n.Loop) {
134− return true
135− }
136− case *syntax.CaseClause:
137− for _, item := range n.Items {
138− for _, stmt := range item.Stmts {
139− if containsBannedCommand(stmt) {
140− return true
141− }
142− }
143− }
144− case *syntax.FuncDecl:
145− return containsBannedCommand(n.Body)
146− case *syntax.ArithmExp:
147− return containsBannedCommand(n.X)
148− case *syntax.Redirect:
149− return containsBannedCommand(n.Word)
150− }
151− return false
152−}
153−
154−func checkWordForBannedCommands(word *syntax.Word) bool {
155− if word == nil {
156− return false
157− }
158−
159− for _, part := range word.Parts {
160− switch p := part.(type) {
161− case *syntax.SglQuoted:
162− if checkQuotedStringForBannedCommands(p.Value) {
163− return true
164− }
165− case *syntax.DblQuoted:
166− var content strings.Builder
167− for _, qpart := range p.Parts {
168− if lit, ok := qpart.(*syntax.Lit); ok {
169− content.WriteString(lit.Value)
170− }
171− }
172− if checkQuotedStringForBannedCommands(content.String()) {
173− return true
174− }
175− }
176− }
177− return false
178−}
179−
180−func checkQuotedStringForBannedCommands(content string) bool {
181− parser := syntax.NewParser()
182− file, err := parser.Parse(strings.NewReader(content), "")
183− if err != nil {
184− return false
185− }
186−
187− if len(file.Stmts) == 0 {
188− return false
189− }
190−
191− // Simple heuristic: if it looks like prose rather than commands, don't flag it
192− if len(file.Stmts) == 1 {
193− stmt := file.Stmts[0]
194− if callExpr, ok := stmt.Cmd.(*syntax.CallExpr); ok {
195− if len(callExpr.Args) > 2 {
196− allText := true
197− for i, arg := range callExpr.Args {
198− if i == 0 {
199− continue
200− }
201− argStr := getWordValue(arg)
202− if strings.HasPrefix(argStr, "-") {
203− allText = false
204− break
205− }
206− }
207− if allText {
208− return false
209− }
210− }
211− }
212− }
213−
214− for _, stmt := range file.Stmts {
215− if containsBannedCommand(stmt) {
216− return true
217− }
218− }
219− return false
220−}
221−
222−func getWordValue(word *syntax.Word) string {
223− if word == nil || len(word.Parts) == 0 {
224− return ""
225− }
226−
227− var result strings.Builder
228− for _, part := range word.Parts {
229− switch p := part.(type) {
230− case *syntax.Lit:
231− result.WriteString(p.Value)
232− case *syntax.SglQuoted:
233− result.WriteString(p.Value)
234− case *syntax.DblQuoted:
235− for _, qpart := range p.Parts {
236− if lit, ok := qpart.(*syntax.Lit); ok {
237− result.WriteString(lit.Value)
238− }
239− }
240− }
241− }
242− return result.String()
243−}
244−
245−func validateCommand(command string) error {
246− parser := syntax.NewParser()
247− file, err := parser.Parse(strings.NewReader(command), "")
248− if err != nil {
249− parts := strings.Fields(command)
250− if len(parts) > 0 {
251− baseCmd := parts[0]
252− for _, banned := range bannedCommands {
253− if strings.EqualFold(baseCmd, banned) {
254− return fmt.Errorf("command '%s' is not allowed", baseCmd)
255− }
256− }
257− }
258− return nil
259− }
260−
261− for _, stmt := range file.Stmts {
262− if containsBannedCommand(stmt) {
263− return fmt.Errorf("command contains banned operations")
264− }
265− }
266− return nil
267−}
268−
269var bannedCommands = []string{
43var bannedCommands = []string{
270 "alias", "curl", "curlie", "wget", "axel", "aria2c",
44 "alias", "curl", "curlie", "wget", "axel", "aria2c",
271 "nc", "telnet", "lynx", "w3m", "links", "httpie", "xh",
45 "nc", "telnet", "lynx", "w3m", "links", "httpie", "xh",
272− "http-prompt", "chrome", "firefox", "safari", "sudo",
46+ "http-prompt", "chrome", "firefox", "safari",
273}
47}
274
48
275// getSafeReadOnlyCommands returns platform-appropriate safe commands
49// getSafeReadOnlyCommands returns platform-appropriate safe commands
@@ -515,9 +289,11 @@ func (b *bashTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error)
515 return NewTextErrorResponse("missing command"), nil
289 return NewTextErrorResponse("missing command"), nil
516 }
290 }
517
291
518−
292+ baseCmd := strings.Fields(params.Command)[0]
519− if err := validateCommand(params.Command); err != nil {
293+ for _, banned := range bannedCommands {
294+ if strings.EqualFold(baseCmd, banned) {
520− return NewTextErrorResponse(err.Error()), nil
295+ return NewTextErrorResponse(fmt.Sprintf("command '%s' is not allowed", baseCmd)), nil
296+ }
521 }
297 }
522
298
523 isSafeReadOnly := false
299 isSafeReadOnly := false
internal/llm/tools/bash_test.go−165
@@ -94,168 +94,3 @@ func TestPlatformSpecificSafeCommands(t *testing.T) {
94 }
94 }
95 }
95 }
96}
96}
97−
98−func TestValidateCommand(t *testing.T) {
99− tests := []struct {
100− name string
101− command string
102− shouldError bool
103− }{
104− // Commands that should be blocked
105− {
106− name: "direct sudo",
107− command: "sudo ls",
108− shouldError: true,
109− },
110− {
111− name: "sudo in script",
112− command: "bash -c 'sudo ls'",
113− shouldError: true,
114− },
115− {
116− name: "sudo in command substitution",
117− command: "$(sudo whoami)",
118− shouldError: true,
119− },
120− {
121− name: "sudo in echo command substitution",
122− command: "echo $(sudo id)",
123− shouldError: true,
124− },
125− {
126− name: "sudo in command chain",
127− command: "ls && sudo rm file",
128− shouldError: true,
129− },
130− {
131− name: "sudo in if statement",
132− command: "if true; then sudo ls; fi",
133− shouldError: true,
134− },
135− {
136− name: "sudo in for loop",
137− command: "for i in 1; do sudo echo $i; done",
138− shouldError: true,
139− },
140− {
141− name: "direct curl",
142− command: "curl http://example.com",
143− shouldError: true,
144− },
145− {
146− name: "curl in script",
147− command: "bash -c 'curl malicious.com'",
148− shouldError: true,
149− },
150− {
151− name: "wget command",
152− command: "wget http://example.com",
153− shouldError: true,
154− },
155− {
156− name: "nc command",
157− command: "nc -l 8080",
158− shouldError: true,
159− },
160− // Commands that should be allowed
161− {
162− name: "simple ls",
163− command: "ls -la",
164− shouldError: false,
165− },
166− {
167− name: "echo command",
168− command: "echo hello",
169− shouldError: false,
170− },
171− {
172− name: "git status",
173− command: "git status",
174− shouldError: false,
175− },
176− {
177− name: "go build",
178− command: "go build",
179− shouldError: false,
180− },
181− {
182− name: "sudo as literal text",
183− command: "echo 'sudo is just text here'",
184− shouldError: false,
185− },
186− {
187− name: "complex allowed command",
188− command: "find . -name '*.go' | head -10",
189− shouldError: false,
190− },
191− {
192− name: "command with environment variables",
193− command: "FOO=bar go test",
194− shouldError: false,
195− },
196− }
197−
198− for _, tt := range tests {
199− t.Run(tt.name, func(t *testing.T) {
200− err := validateCommand(tt.command)
201− if tt.shouldError && err == nil {
202− t.Errorf("Expected error for command %q, but got none", tt.command)
203− }
204− if !tt.shouldError && err != nil {
205− t.Errorf("Expected no error for command %q, but got: %v", tt.command, err)
206− }
207− })
208− }
209−}
210−
211−func TestContainsBannedCommand(t *testing.T) {
212− // Test the helper functions directly with some edge cases
213− tests := []struct {
214− name string
215− command string
216− shouldError bool
217− }{
218− {
219− name: "nested command substitution",
220− command: "echo $(echo $(sudo id))",
221− shouldError: true,
222− },
223− {
224− name: "subshell with banned command",
225− command: "(sudo ls)",
226− shouldError: true,
227− },
228− {
229− name: "case statement with banned command",
230− command: "case $1 in start) sudo systemctl start service ;; esac",
231− shouldError: true,
232− },
233− {
234− name: "while loop with banned command",
235− command: "while true; do sudo echo test; done",
236− shouldError: true,
237− },
238− {
239− name: "function with banned command",
240− command: "function test() { sudo ls; }",
241− shouldError: true,
242− },
243− {
244− name: "complex valid command",
245− command: "if [ -f file ]; then echo exists; else echo missing; fi",
246− shouldError: false,
247− },
248− }
249−
250− for _, tt := range tests {
251− t.Run(tt.name, func(t *testing.T) {
252− err := validateCommand(tt.command)
253− if tt.shouldError && err == nil {
254− t.Errorf("Expected error for command %q, but got none", tt.command)
255− }
256− if !tt.shouldError && err != nil {
257− t.Errorf("Expected no error for command %q, but got: %v", tt.command, err)
258− }
259− })
260− }
261−}