git.iko.soy/crush
on main help sign in
chore: implement correct banned commands
28be21fKujtim Hoxha
internal/llm/tools/bash.go+230−6
compare with main
unifiedsplit
@@ -10,6 +10,7 @@ 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"
13)
14)
14
15
15type BashParams struct {
16type BashParams struct {
@@ -40,10 +41,235 @@ const (
40 BashNoOutput = "no output"
41 BashNoOutput = "no output"
41)
42)
42
43
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+
43var bannedCommands = []string{
269var bannedCommands = []string{
44 "alias", "curl", "curlie", "wget", "axel", "aria2c",
270 "alias", "curl", "curlie", "wget", "axel", "aria2c",
45 "nc", "telnet", "lynx", "w3m", "links", "httpie", "xh",
271 "nc", "telnet", "lynx", "w3m", "links", "httpie", "xh",
46− "http-prompt", "chrome", "firefox", "safari",
272+ "http-prompt", "chrome", "firefox", "safari", "sudo",
47}
273}
48
274
49// getSafeReadOnlyCommands returns platform-appropriate safe commands
275// getSafeReadOnlyCommands returns platform-appropriate safe commands
@@ -289,11 +515,9 @@ func (b *bashTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error)
289 return NewTextErrorResponse("missing command"), nil
515 return NewTextErrorResponse("missing command"), nil
290 }
516 }
291
517
292− baseCmd := strings.Fields(params.Command)[0]
518+
293− for _, banned := range bannedCommands {
519+ if err := validateCommand(params.Command); err != nil {
294− if strings.EqualFold(baseCmd, banned) {
295− return NewTextErrorResponse(fmt.Sprintf("command '%s' is not allowed", baseCmd)), nil
520+ return NewTextErrorResponse(err.Error()), nil
296− }
297 }
521 }
298
522
299 isSafeReadOnly := false
523 isSafeReadOnly := false
internal/llm/tools/bash_test.go+165
@@ -94,3 +94,168 @@ 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+}