git.iko.soy/crush
on main help sign in
Enhance bash tool security and improve permission dialog UI
6419973Kujtim Hoxha
internal/llm/tools/bash.go+43−4
compare with main
unifiedsplit
@@ -38,8 +38,38 @@ var BannedCommands = []string{
38}
38}
39
39
40var SafeReadOnlyCommands = []string{
40var SafeReadOnlyCommands = []string{
41+ // Basic shell commands
41 "ls", "echo", "pwd", "date", "cal", "uptime", "whoami", "id", "groups", "env", "printenv", "set", "unset", "which", "type", "whereis",
42 "ls", "echo", "pwd", "date", "cal", "uptime", "whoami", "id", "groups", "env", "printenv", "set", "unset", "which", "type", "whereis",
42− "whatis", //...
43+ "whatis", "uname", "hostname", "df", "du", "free", "top", "ps", "kill", "killall", "nice", "nohup", "time", "timeout",
44+
45+ // Git read-only commands
46+ "git status", "git log", "git diff", "git show", "git branch", "git tag", "git remote", "git ls-files", "git ls-remote",
47+ "git rev-parse", "git config --get", "git config --list", "git describe", "git blame", "git grep", "git shortlog",
48+
49+ // Go commands
50+ "go version", "go list", "go env", "go doc", "go vet", "go fmt", "go mod", "go test", "go build", "go run", "go install", "go clean",
51+
52+ // Node.js commands
53+ "node", "npm", "npx", "yarn", "pnpm",
54+
55+ // Python commands
56+ "python", "python3", "pip", "pip3", "pytest", "pylint", "mypy", "black", "isort", "flake8", "ruff",
57+
58+ // Docker commands
59+ "docker ps", "docker images", "docker volume", "docker network", "docker info", "docker version",
60+ "docker-compose ps", "docker-compose config",
61+
62+ // Kubernetes commands
63+ "kubectl get", "kubectl describe", "kubectl logs", "kubectl version", "kubectl config",
64+
65+ // Rust commands
66+ "cargo", "rustc", "rustup",
67+
68+ // Java commands
69+ "java", "javac", "mvn", "gradle",
70+
71+ // Misc development tools
72+ "make", "cmake", "bazel", "terraform plan", "terraform validate", "ansible",
43}
73}
44
74
45func (b *bashTool) Info() ToolInfo {
75func (b *bashTool) Info() ToolInfo {
@@ -77,17 +107,26 @@ func (b *bashTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error)
77 return NewTextErrorResponse("missing command"), nil
107 return NewTextErrorResponse("missing command"), nil
78 }
108 }
79
109
110+ // Check for banned commands (first word only)
80 baseCmd := strings.Fields(params.Command)[0]
111 baseCmd := strings.Fields(params.Command)[0]
81 for _, banned := range BannedCommands {
112 for _, banned := range BannedCommands {
82 if strings.EqualFold(baseCmd, banned) {
113 if strings.EqualFold(baseCmd, banned) {
83 return NewTextErrorResponse(fmt.Sprintf("command '%s' is not allowed", baseCmd)), nil
114 return NewTextErrorResponse(fmt.Sprintf("command '%s' is not allowed", baseCmd)), nil
84 }
115 }
85 }
116 }
117+
118+ // Check for safe commands (can be multi-word)
86 isSafeReadOnly := false
119 isSafeReadOnly := false
120+ cmdLower := strings.ToLower(params.Command)
121+
87 for _, safe := range SafeReadOnlyCommands {
122 for _, safe := range SafeReadOnlyCommands {
88− if strings.EqualFold(baseCmd, safe) {
123+ // Check if command starts with the safe command pattern
124+ if strings.HasPrefix(cmdLower, strings.ToLower(safe)) {
125+ // Make sure it's either an exact match or followed by a space or flag
126+ if len(cmdLower) == len(safe) || cmdLower[len(safe)] == ' ' || cmdLower[len(safe)] == '-' {
89− isSafeReadOnly = true
127+ isSafeReadOnly = true
90− break
128+ break
129+ }
91 }
130 }
92 }
131 }
93 if !isSafeReadOnly {
132 if !isSafeReadOnly {
internal/llm/tools/bash_test.go+24−13
@@ -119,27 +119,38 @@ func TestBashTool_Run(t *testing.T) {
119 }
119 }
120 })
120 })
121
121
122− t.Run("handles safe read-only commands without permission check", func(t *testing.T) {
122+ t.Run("handles multi-word safe commands without permission check", func(t *testing.T) {
123 permission.Default = newMockPermissionService(false)
123 permission.Default = newMockPermissionService(false)
124
124
125 tool := NewBashTool()
125 tool := NewBashTool()
126
126
127− // Test with a safe read-only command
127+ // Test with multi-word safe commands
128− params := BashParams{
128+ multiWordCommands := []string{
129− Command: "echo 'test'",
129+ "git status",
130+ "git log -n 5",
131+ "docker ps",
132+ "go test ./...",
133+ "kubectl get pods",
130 }
134 }
131
135
132− paramsJSON, err := json.Marshal(params)
136+ for _, cmd := range multiWordCommands {
133− require.NoError(t, err)
137+ params := BashParams{
138+ Command: cmd,
139+ }
134
140
135− call := ToolCall{
141+ paramsJSON, err := json.Marshal(params)
136− Name: BashToolName,
142+ require.NoError(t, err)
137− Input: string(paramsJSON),
138− }
139
143
144+ call := ToolCall{
145+ Name: BashToolName,
146+ Input: string(paramsJSON),
147+ }
148+
140− response, err := tool.Run(context.Background(), call)
149+ response, err := tool.Run(context.Background(), call)
141− require.NoError(t, err)
150+ require.NoError(t, err)
142− assert.Equal(t, "test\n", response.Content)
151+ assert.NotContains(t, response.Content, "permission denied",
152+ "Command %s should be allowed without permission", cmd)
153+ }
143 })
154 })
144
155
145 t.Run("handles permission denied", func(t *testing.T) {
156 t.Run("handles permission denied", func(t *testing.T) {
internal/tui/components/dialog/permission.go+73−14
@@ -92,16 +92,7 @@ func formatDiff(diffText string) string {
92 }
92 }
93
93
94 // Join all formatted lines
94 // Join all formatted lines
95− content := strings.Join(formattedLines, "\n")
95+ return strings.Join(formattedLines, "\n")
96−
97− // Create a bordered box for the content
98− contentStyle := lipgloss.NewStyle().
99− MarginTop(1).
100− Padding(0, 1).
101− Border(lipgloss.RoundedBorder()).
102− BorderForeground(styles.Flamingo)
103−
104− return contentStyle.Render(content)
105}
96}
106
97
107func (p *permissionDialogCmp) Init() tea.Cmd {
98func (p *permissionDialogCmp) Init() tea.Cmd {
@@ -241,12 +232,46 @@ func (p *permissionDialogCmp) render() string {
241 headerParts = append(headerParts, keyStyle.Render("Update"))
232 headerParts = append(headerParts, keyStyle.Render("Update"))
242 // Recreate header content with the updated headerParts
233 // Recreate header content with the updated headerParts
243 headerContent = lipgloss.NewStyle().Padding(0, 1).Render(lipgloss.JoinVertical(lipgloss.Left, headerParts...))
234 headerContent = lipgloss.NewStyle().Padding(0, 1).Render(lipgloss.JoinVertical(lipgloss.Left, headerParts...))
235+
244− // Format the diff with colors instead of using markdown code block
236+ // Format the diff with colors
245 formattedDiff := formatDiff(pr.Diff)
237 formattedDiff := formatDiff(pr.Diff)
238+
239+ // Set up viewport for the diff content
240+ p.contentViewPort.Width = p.width - 2 - 2
241+
242+ // Calculate content height dynamically based on window size
243+ maxContentHeight := p.height - lipgloss.Height(headerContent) - lipgloss.Height(form) - 2 - 2 - 1
244+ p.contentViewPort.Height = maxContentHeight
245+ p.contentViewPort.SetContent(formattedDiff)
246+
247+ // Style the viewport
248+ var contentBorder lipgloss.Border
249+ var borderColor lipgloss.TerminalColor
250+
251+ if p.isViewportFocus {
252+ contentBorder = lipgloss.DoubleBorder()
253+ borderColor = styles.Blue
254+ } else {
255+ contentBorder = lipgloss.RoundedBorder()
256+ borderColor = styles.Flamingo
257+ }
258+
259+ contentStyle := lipgloss.NewStyle().
260+ MarginTop(1).
261+ Padding(0, 1).
262+ Border(contentBorder).
263+ BorderForeground(borderColor)
264+
265+ if p.isViewportFocus {
266+ contentStyle = contentStyle.BorderBackground(styles.Surface0)
267+ }
268+
269+ contentFinal := contentStyle.Render(p.contentViewPort.View())
270+
246 return lipgloss.JoinVertical(
271 return lipgloss.JoinVertical(
247 lipgloss.Top,
272 lipgloss.Top,
248 headerContent,
273 headerContent,
249− formattedDiff,
274+ contentFinal,
250 form,
275 form,
251 )
276 )
252
277
@@ -255,12 +280,46 @@ func (p *permissionDialogCmp) render() string {
255 headerParts = append(headerParts, keyStyle.Render("Content"))
280 headerParts = append(headerParts, keyStyle.Render("Content"))
256 // Recreate header content with the updated headerParts
281 // Recreate header content with the updated headerParts
257 headerContent = lipgloss.NewStyle().Padding(0, 1).Render(lipgloss.JoinVertical(lipgloss.Left, headerParts...))
282 headerContent = lipgloss.NewStyle().Padding(0, 1).Render(lipgloss.JoinVertical(lipgloss.Left, headerParts...))
283+
258− // Format the diff with colors instead of using markdown code block
284+ // Format the diff with colors
259 formattedDiff := formatDiff(pr.Content)
285 formattedDiff := formatDiff(pr.Content)
286+
287+ // Set up viewport for the content
288+ p.contentViewPort.Width = p.width - 2 - 2
289+
290+ // Calculate content height dynamically based on window size
291+ maxContentHeight := p.height - lipgloss.Height(headerContent) - lipgloss.Height(form) - 2 - 2 - 1
292+ p.contentViewPort.Height = maxContentHeight
293+ p.contentViewPort.SetContent(formattedDiff)
294+
295+ // Style the viewport
296+ var contentBorder lipgloss.Border
297+ var borderColor lipgloss.TerminalColor
298+
299+ if p.isViewportFocus {
300+ contentBorder = lipgloss.DoubleBorder()
301+ borderColor = styles.Blue
302+ } else {
303+ contentBorder = lipgloss.RoundedBorder()
304+ borderColor = styles.Flamingo
305+ }
306+
307+ contentStyle := lipgloss.NewStyle().
308+ MarginTop(1).
309+ Padding(0, 1).
310+ Border(contentBorder).
311+ BorderForeground(borderColor)
312+
313+ if p.isViewportFocus {
314+ contentStyle = contentStyle.BorderBackground(styles.Surface0)
315+ }
316+
317+ contentFinal := contentStyle.Render(p.contentViewPort.View())
318+
260 return lipgloss.JoinVertical(
319 return lipgloss.JoinVertical(
261 lipgloss.Top,
320 lipgloss.Top,
262 headerContent,
321 headerContent,
263− formattedDiff,
322+ contentFinal,
264 form,
323 form,
265 )
324 )
266
325
internal/tui/tui.go+9−6
@@ -123,8 +123,6 @@ func (a appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
123 a.status, _ = a.status.Update(msg)
123 a.status, _ = a.status.Update(msg)
124 case util.ErrorMsg:
124 case util.ErrorMsg:
125 a.status, _ = a.status.Update(msg)
125 a.status, _ = a.status.Update(msg)
126− case util.ClearStatusMsg:
127− a.status, _ = a.status.Update(msg)
128 case tea.KeyMsg:
126 case tea.KeyMsg:
129 if a.editorMode == vimtea.ModeNormal {
127 if a.editorMode == vimtea.ModeNormal {
130 switch {
128 switch {
@@ -163,16 +161,21 @@ func (a appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
163 }
161 }
164 }
162 }
165 }
163 }
164+
165+ var cmds []tea.Cmd
166+ s, cmd := a.status.Update(msg)
167+ a.status = s
168+ cmds = append(cmds, cmd)
166 if a.dialogVisible {
169 if a.dialogVisible {
167 d, cmd := a.dialog.Update(msg)
170 d, cmd := a.dialog.Update(msg)
168 a.dialog = d.(core.DialogCmp)
171 a.dialog = d.(core.DialogCmp)
172+ cmds = append(cmds, cmd)
169− return a, cmd
173+ return a, tea.Batch(cmds...)
170 }
174 }
171− s, _ := a.status.Update(msg)
172− a.status = s
173 p, cmd := a.pages[a.currentPage].Update(msg)
175 p, cmd := a.pages[a.currentPage].Update(msg)
174 a.pages[a.currentPage] = p
176 a.pages[a.currentPage] = p
177+ cmds = append(cmds, cmd)
175− return a, cmd
178+ return a, tea.Batch(cmds...)
176}
179}
177
180
178func (a *appModel) ToggleHelp() {
181func (a *appModel) ToggleHelp() {