Skip to content
This repository was archived by the owner on Sep 18, 2025. It is now read-only.

Commit 6419973

Browse files
kujtimiihoxhatermai
andcommitted
Enhance bash tool security and improve permission dialog UI
- Expand safe command list with common dev tools (git, go, node, python, etc.) - Improve multi-word command detection for better security checks - Add scrollable viewport to permission dialog for better diff viewing - Fix command batching in TUI update to properly handle multiple commands 🤖 Generated with termai Co-Authored-By: termai <noreply@termai.io>
1 parent 8f8b403 commit 6419973

File tree

4 files changed

+149
-37
lines changed

4 files changed

+149
-37
lines changed

internal/llm/tools/bash.go

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,38 @@ var BannedCommands = []string{
3838
}
3939

4040
var SafeReadOnlyCommands = []string{
41+
// Basic shell commands
4142
"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",
4373
}
4474

4575
func (b *bashTool) Info() ToolInfo {
@@ -77,17 +107,26 @@ func (b *bashTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error)
77107
return NewTextErrorResponse("missing command"), nil
78108
}
79109

110+
// Check for banned commands (first word only)
80111
baseCmd := strings.Fields(params.Command)[0]
81112
for _, banned := range BannedCommands {
82113
if strings.EqualFold(baseCmd, banned) {
83114
return NewTextErrorResponse(fmt.Sprintf("command '%s' is not allowed", baseCmd)), nil
84115
}
85116
}
117+
118+
// Check for safe commands (can be multi-word)
86119
isSafeReadOnly := false
120+
cmdLower := strings.ToLower(params.Command)
121+
87122
for _, safe := range SafeReadOnlyCommands {
88-
if strings.EqualFold(baseCmd, safe) {
89-
isSafeReadOnly = true
90-
break
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)] == '-' {
127+
isSafeReadOnly = true
128+
break
129+
}
91130
}
92131
}
93132
if !isSafeReadOnly {

internal/llm/tools/bash_test.go

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -119,27 +119,38 @@ func TestBashTool_Run(t *testing.T) {
119119
}
120120
})
121121

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) {
123123
permission.Default = newMockPermissionService(false)
124124

125125
tool := NewBashTool()
126126

127-
// Test with a safe read-only command
128-
params := BashParams{
129-
Command: "echo 'test'",
127+
// Test with multi-word safe commands
128+
multiWordCommands := []string{
129+
"git status",
130+
"git log -n 5",
131+
"docker ps",
132+
"go test ./...",
133+
"kubectl get pods",
130134
}
131135

132-
paramsJSON, err := json.Marshal(params)
133-
require.NoError(t, err)
136+
for _, cmd := range multiWordCommands {
137+
params := BashParams{
138+
Command: cmd,
139+
}
134140

135-
call := ToolCall{
136-
Name: BashToolName,
137-
Input: string(paramsJSON),
138-
}
141+
paramsJSON, err := json.Marshal(params)
142+
require.NoError(t, err)
139143

140-
response, err := tool.Run(context.Background(), call)
141-
require.NoError(t, err)
142-
assert.Equal(t, "test\n", response.Content)
144+
call := ToolCall{
145+
Name: BashToolName,
146+
Input: string(paramsJSON),
147+
}
148+
149+
response, err := tool.Run(context.Background(), call)
150+
require.NoError(t, err)
151+
assert.NotContains(t, response.Content, "permission denied",
152+
"Command %s should be allowed without permission", cmd)
153+
}
143154
})
144155

145156
t.Run("handles permission denied", func(t *testing.T) {

internal/tui/components/dialog/permission.go

Lines changed: 73 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,16 +92,7 @@ func formatDiff(diffText string) string {
9292
}
9393

9494
// Join all formatted lines
95-
content := 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)
95+
return strings.Join(formattedLines, "\n")
10596
}
10697

10798
func (p *permissionDialogCmp) Init() tea.Cmd {
@@ -241,12 +232,46 @@ func (p *permissionDialogCmp) render() string {
241232
headerParts = append(headerParts, keyStyle.Render("Update"))
242233
// Recreate header content with the updated headerParts
243234
headerContent = lipgloss.NewStyle().Padding(0, 1).Render(lipgloss.JoinVertical(lipgloss.Left, headerParts...))
244-
// Format the diff with colors instead of using markdown code block
235+
236+
// Format the diff with colors
245237
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+
246271
return lipgloss.JoinVertical(
247272
lipgloss.Top,
248273
headerContent,
249-
formattedDiff,
274+
contentFinal,
250275
form,
251276
)
252277

@@ -255,12 +280,46 @@ func (p *permissionDialogCmp) render() string {
255280
headerParts = append(headerParts, keyStyle.Render("Content"))
256281
// Recreate header content with the updated headerParts
257282
headerContent = lipgloss.NewStyle().Padding(0, 1).Render(lipgloss.JoinVertical(lipgloss.Left, headerParts...))
258-
// Format the diff with colors instead of using markdown code block
283+
284+
// Format the diff with colors
259285
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+
260319
return lipgloss.JoinVertical(
261320
lipgloss.Top,
262321
headerContent,
263-
formattedDiff,
322+
contentFinal,
264323
form,
265324
)
266325

internal/tui/tui.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,6 @@ func (a appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
123123
a.status, _ = a.status.Update(msg)
124124
case util.ErrorMsg:
125125
a.status, _ = a.status.Update(msg)
126-
case util.ClearStatusMsg:
127-
a.status, _ = a.status.Update(msg)
128126
case tea.KeyMsg:
129127
if a.editorMode == vimtea.ModeNormal {
130128
switch {
@@ -163,16 +161,21 @@ func (a appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
163161
}
164162
}
165163
}
164+
165+
var cmds []tea.Cmd
166+
s, cmd := a.status.Update(msg)
167+
a.status = s
168+
cmds = append(cmds, cmd)
166169
if a.dialogVisible {
167170
d, cmd := a.dialog.Update(msg)
168171
a.dialog = d.(core.DialogCmp)
169-
return a, cmd
172+
cmds = append(cmds, cmd)
173+
return a, tea.Batch(cmds...)
170174
}
171-
s, _ := a.status.Update(msg)
172-
a.status = s
173175
p, cmd := a.pages[a.currentPage].Update(msg)
174176
a.pages[a.currentPage] = p
175-
return a, cmd
177+
cmds = append(cmds, cmd)
178+
return a, tea.Batch(cmds...)
176179
}
177180

178181
func (a *appModel) ToggleHelp() {

0 commit comments

Comments
 (0)