git.iko.soy/crush
on main help sign in
Merge pull request #138 from charmbracelet/proper-banned-commands

chore: implement correct banned commands

commitfae5ac8d8be4f34e3f084ab0a3664ee5e41b22cc authorKujtim Hoxha <kujtimii.h@gmail.com> dateMon 14 Jul 2025, 13:23 (1 year ago) parent95f5e750a52748
3 files changed+315−17
compare with main
unifiedsplit
internal/llm/tools/bash.go+117−10 internal/shell/command_block_test.go+123 internal/shell/shell.go+75−7
internal/llm/tools/bash.go+117−10
@@ -4,6 +4,7 @@ import (
44 "context"
55 "encoding/json"
66 "fmt"
7+ "log/slog"
78 "runtime"
89 "strings"
910 "time"
@@ -41,9 +42,74 @@ const (
4142)
4243
4344var bannedCommands = []string{
44− "alias", "curl", "curlie", "wget", "axel", "aria2c",
45− "nc", "telnet", "lynx", "w3m", "links", "httpie", "xh",
46− "http-prompt", "chrome", "firefox", "safari",
45+ // Network/Download tools
46+ "alias",
47+ "aria2c",
48+ "axel",
49+ "chrome",
50+ "curl",
51+ "curlie",
52+ "firefox",
53+ "http-prompt",
54+ "httpie",
55+ "links",
56+ "lynx",
57+ "nc",
58+ "safari",
59+ "telnet",
60+ "w3m",
61+ "wget",
62+ "xh",
63+
64+ // System administration
65+ "doas",
66+ "su",
67+ "sudo",
68+
69+ // Package managers
70+ "apk",
71+ "apt",
72+ "apt-cache",
73+ "apt-get",
74+ "dnf",
75+ "dpkg",
76+ "emerge",
77+ "home-manager",
78+ "makepkg",
79+ "opkg",
80+ "pacman",
81+ "paru",
82+ "pkg",
83+ "pkg_add",
84+ "pkg_delete",
85+ "portage",
86+ "rpm",
87+ "yay",
88+ "yum",
89+ "zypper",
90+
91+ // System modification
92+ "at",
93+ "batch",
94+ "chkconfig",
95+ "crontab",
96+ "fdisk",
97+ "mkfs",
98+ "mount",
99+ "parted",
100+ "service",
101+ "systemctl",
102+ "umount",
103+
104+ // Network configuration
105+ "firewall-cmd",
106+ "ifconfig",
107+ "ip",
108+ "iptables",
109+ "netstat",
110+ "pfctl",
111+ "route",
112+ "ufw",
47113}
48114
49115// getSafeReadOnlyCommands returns platform-appropriate safe commands
@@ -244,7 +310,42 @@ Important:
244310- Never update git config`, bannedCommandsStr, MaxOutputLength)
245311}
246312
313+func blockFuncs() []shell.BlockFunc {
314+ return []shell.BlockFunc{
315+ shell.CommandsBlocker(bannedCommands),
316+ shell.ArgumentsBlocker([][]string{
317+ // System package managers
318+ {"apk", "add"},
319+ {"apt", "install"},
320+ {"apt-get", "install"},
321+ {"dnf", "install"},
322+ {"emerge"},
323+ {"pacman", "-S"},
324+ {"pkg", "install"},
325+ {"yum", "install"},
326+ {"zypper", "install"},
327+
328+ // Language-specific package managers
329+ {"brew", "install"},
330+ {"cargo", "install"},
331+ {"gem", "install"},
332+ {"go", "install"},
333+ {"npm", "install", "-g"},
334+ {"npm", "install", "--global"},
335+ {"pip", "install", "--user"},
336+ {"pip3", "install", "--user"},
337+ {"pnpm", "add", "-g"},
338+ {"pnpm", "add", "--global"},
339+ {"yarn", "global", "add"},
340+ }),
341+ }
342+}
343+
247344func NewBashTool(permission permission.Service, workingDir string) BaseTool {
345+ // Set up command blocking on the persistent shell
346+ persistentShell := shell.GetPersistentShell(workingDir)
347+ persistentShell.SetBlockFuncs(blockFuncs())
348+
248349 return &bashTool{
249350 permissions: permission,
250351 workingDir: workingDir,
@@ -289,13 +390,6 @@ func (b *bashTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error)
289390 return NewTextErrorResponse("missing command"), nil
290391 }
291392
292− baseCmd := strings.Fields(params.Command)[0]
293− for _, banned := range bannedCommands {
294− if strings.EqualFold(baseCmd, banned) {
295− return NewTextErrorResponse(fmt.Sprintf("command '%s' is not allowed", baseCmd)), nil
296− }
297− }
298−
299393 isSafeReadOnly := false
300394 cmdLower := strings.ToLower(params.Command)
301395
@@ -349,7 +443,20 @@ func (b *bashTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error)
349443 stdout = truncateOutput(stdout)
350444 stderr = truncateOutput(stderr)
351445
446+ slog.Info("Bash command executed",
447+ "command", params.Command,
448+ "stdout", stdout,
449+ "stderr", stderr,
450+ "exit_code", exitCode,
451+ "interrupted", interrupted,
452+ "err", err,
453+ )
454+
352455 errorMessage := stderr
456+ if errorMessage == "" && err != nil {
457+ errorMessage = err.Error()
458+ }
459+
353460 if interrupted {
354461 if errorMessage != "" {
355462 errorMessage += "\n"
internal/shell/command_block_test.gonew file+123
@@ -0,0 +1,123 @@
1+package shell
2+
3+import (
4+ "context"
5+ "os"
6+ "strings"
7+ "testing"
8+)
9+
10+func TestCommandBlocking(t *testing.T) {
11+ tests := []struct {
12+ name string
13+ blockFuncs []BlockFunc
14+ command string
15+ shouldBlock bool
16+ }{
17+ {
18+ name: "block simple command",
19+ blockFuncs: []BlockFunc{
20+ func(args []string) bool {
21+ return len(args) > 0 && args[0] == "curl"
22+ },
23+ },
24+ command: "curl https://example.com",
25+ shouldBlock: true,
26+ },
27+ {
28+ name: "allow non-blocked command",
29+ blockFuncs: []BlockFunc{
30+ func(args []string) bool {
31+ return len(args) > 0 && args[0] == "curl"
32+ },
33+ },
34+ command: "echo hello",
35+ shouldBlock: false,
36+ },
37+ {
38+ name: "block subcommand",
39+ blockFuncs: []BlockFunc{
40+ func(args []string) bool {
41+ return len(args) >= 2 && args[0] == "brew" && args[1] == "install"
42+ },
43+ },
44+ command: "brew install wget",
45+ shouldBlock: true,
46+ },
47+ {
48+ name: "allow different subcommand",
49+ blockFuncs: []BlockFunc{
50+ func(args []string) bool {
51+ return len(args) >= 2 && args[0] == "brew" && args[1] == "install"
52+ },
53+ },
54+ command: "brew list",
55+ shouldBlock: false,
56+ },
57+ {
58+ name: "block npm global install with -g",
59+ blockFuncs: []BlockFunc{
60+ ArgumentsBlocker([][]string{
61+ {"npm", "install", "-g"},
62+ {"npm", "install", "--global"},
63+ }),
64+ },
65+ command: "npm install -g typescript",
66+ shouldBlock: true,
67+ },
68+ {
69+ name: "block npm global install with --global",
70+ blockFuncs: []BlockFunc{
71+ ArgumentsBlocker([][]string{
72+ {"npm", "install", "-g"},
73+ {"npm", "install", "--global"},
74+ }),
75+ },
76+ command: "npm install --global typescript",
77+ shouldBlock: true,
78+ },
79+ {
80+ name: "allow npm local install",
81+ blockFuncs: []BlockFunc{
82+ ArgumentsBlocker([][]string{
83+ {"npm", "install", "-g"},
84+ {"npm", "install", "--global"},
85+ }),
86+ },
87+ command: "npm install typescript",
88+ shouldBlock: false,
89+ },
90+ }
91+
92+ for _, tt := range tests {
93+ t.Run(tt.name, func(t *testing.T) {
94+ // Create a temporary directory for each test
95+ tmpDir, err := os.MkdirTemp("", "shell-test-*")
96+ if err != nil {
97+ t.Fatalf("Failed to create temp dir: %v", err)
98+ }
99+ defer os.RemoveAll(tmpDir)
100+
101+ shell := NewShell(&Options{
102+ WorkingDir: tmpDir,
103+ BlockFuncs: tt.blockFuncs,
104+ })
105+
106+ _, _, err = shell.Exec(context.Background(), tt.command)
107+
108+ if tt.shouldBlock {
109+ if err == nil {
110+ t.Errorf("Expected command to be blocked, but it was allowed")
111+ } else if !strings.Contains(err.Error(), "not allowed for security reasons") {
112+ t.Errorf("Expected security error, got: %v", err)
113+ }
114+ } else {
115+ // For non-blocked commands, we might get other errors (like command not found)
116+ // but we shouldn't get the security error
117+ if err != nil && strings.Contains(err.Error(), "not allowed for security reasons") {
118+ t.Errorf("Command was unexpectedly blocked: %v", err)
119+ }
120+ }
121+ })
122+ }
123+}
internal/shell/shell.go+75−7
@@ -44,12 +44,16 @@ type noopLogger struct{}
4444
4545func (noopLogger) InfoPersist(msg string, keysAndValues ...interface{}) {}
4646
47+// BlockFunc is a function that determines if a command should be blocked
48+type BlockFunc func(args []string) bool
49+
4750// Shell provides cross-platform shell execution with optional state persistence
4851type Shell struct {
49− env []string
50− cwd string
51− mu sync.Mutex
52− logger Logger
52+ env []string
53+ cwd string
54+ mu sync.Mutex
55+ logger Logger
56+ blockFuncs []BlockFunc
5357}
5458
5559// Options for creating a new shell
@@ -57,6 +61,7 @@ type Options struct {
5761 WorkingDir string
5862 Env []string
5963 Logger Logger
64+ BlockFuncs []BlockFunc
6065}
6166
6267// NewShell creates a new shell instance with the given options
@@ -81,9 +86,10 @@ func NewShell(opts *Options) *Shell {
8186 }
8287
8388 return &Shell{
84− cwd: cwd,
85− env: env,
86− logger: logger,
89+ cwd: cwd,
90+ env: env,
91+ logger: logger,
92+ blockFuncs: opts.BlockFuncs,
8793 }
8894}
8995
@@ -152,6 +158,13 @@ func (s *Shell) SetEnv(key, value string) {
152158 s.env = append(s.env, keyPrefix+value)
153159}
154160
161+// SetBlockFuncs sets the command block functions for the shell
162+func (s *Shell) SetBlockFuncs(blockFuncs []BlockFunc) {
163+ s.mu.Lock()
164+ defer s.mu.Unlock()
165+ s.blockFuncs = blockFuncs
166+}
167+
155168// Windows-specific commands that should use native shell
156169var windowsNativeCommands = map[string]bool{
157170 "dir": true,
@@ -203,6 +216,60 @@ func (s *Shell) determineShellType(command string) ShellType {
203216 return ShellTypePOSIX
204217}
205218
219+// CommandsBlocker creates a BlockFunc that blocks exact command matches
220+func CommandsBlocker(bannedCommands []string) BlockFunc {
221+ bannedSet := make(map[string]bool)
222+ for _, cmd := range bannedCommands {
223+ bannedSet[cmd] = true
224+ }
225+
226+ return func(args []string) bool {
227+ if len(args) == 0 {
228+ return false
229+ }
230+ return bannedSet[args[0]]
231+ }
232+}
233+
234+// ArgumentsBlocker creates a BlockFunc that blocks specific subcommands
235+func ArgumentsBlocker(blockedSubCommands [][]string) BlockFunc {
236+ return func(args []string) bool {
237+ for _, blocked := range blockedSubCommands {
238+ if len(args) >= len(blocked) {
239+ match := true
240+ for i, part := range blocked {
241+ if args[i] != part {
242+ match = false
243+ break
244+ }
245+ }
246+ if match {
247+ return true
248+ }
249+ }
250+ }
251+ return false
252+ }
253+}
254+
255+func (s *Shell) blockHandler() func(next interp.ExecHandlerFunc) interp.ExecHandlerFunc {
256+ return func(next interp.ExecHandlerFunc) interp.ExecHandlerFunc {
257+ return func(ctx context.Context, args []string) error {
258+ if len(args) == 0 {
259+ return next(ctx, args)
260+ }
261+
262+ for _, blockFunc := range s.blockFuncs {
263+ if blockFunc(args) {
264+ return fmt.Errorf("command is not allowed for security reasons: %s", strings.Join(args, " "))
265+ }
266+ }
267+
268+ return next(ctx, args)
269+ }
270+ }
271+}
272+
206273// execWindows executes commands using native Windows shells (cmd.exe or PowerShell)
207274func (s *Shell) execWindows(ctx context.Context, command string, shell string) (string, string, error) {
208275 var cmd *exec.Cmd
@@ -291,6 +358,7 @@ func (s *Shell) execPOSIX(ctx context.Context, command string) (string, string,
291358 interp.Interactive(false),
292359 interp.Env(expand.ListEnviron(s.env...)),
293360 interp.Dir(s.cwd),
361+ interp.ExecHandlers(s.blockHandler()),
294362 )
295363 if err != nil {
296364 return "", "", fmt.Errorf("could not run command: %w", err)