diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 5f4103dcf..9816dfb30 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -95,12 +95,12 @@ var ( regexp.MustCompile(`\bssh\b.*@`), regexp.MustCompile(`\beval\b`), regexp.MustCompile(`\bsource\s+.*\.sh\b`), - regexp.MustCompile(`\bfind\s+/\b`), // find / - traverse entire filesystem - regexp.MustCompile(`\bls\s+/\b`), // ls / - list root directory + regexp.MustCompile(`\bfind\s+/(\s|[;&|><\n]|$)`), // find / - traverse entire filesystem + regexp.MustCompile(`\bls\s+/(\s|[;&|><\n]|$)`), // ls / - list root directory } // absolutePathPattern matches absolute file paths in commands (Unix and Windows). - absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/(?:[^\s\"']*)?`) + absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']*`) // safePaths are kernel pseudo-devices that are always safe to reference in // commands, regardless of workspace restriction. They contain no user data @@ -113,7 +113,7 @@ var ( "/dev/stdin": true, "/dev/stdout": true, "/dev/stderr": true, - "/": true, // root is a path boundary, not a regular file + "/": true, // root is a path boundary, not a regular file } ) diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index fedc99c45..6a19a6e25 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -1621,12 +1621,38 @@ func TestShellTool_FindRootBlocked(t *testing.T) { t.Fatalf("unable to configure exec tool: %s", err) } + // Commands that attempt to traverse or list the root filesystem. + // Each must be blocked by either a deny-pattern match or the + // workspace-boundary check. blocked := []string{ + // deny pattern: find / "find / -name 'private*' -type f 2>/dev/null", - "find /etc -name 'passwd'", "find / -type f -name '*.key'", + "find / -maxdepth 1", + // deny pattern: ls / "ls /", + "ls / -la", + // workspace check: find / + "find /etc -name 'passwd'", + "find /var/log -name '*.log'", + // workspace check: ls / "ls /etc", + "ls /root", + // shell metacharacter bypass attempts + "find /;", + "find /& ", + "find /| cat", + "find /&& echo done", + "find /|| echo fail", + "ls /;", + "ls /& ", + "ls /| wc -l", + "ls /&& echo done", + "ls /|| echo fail", + // uppercase / mixed case (matching is case-insensitive) + "FIND /", + "Ls /", + "Find / -name passwd", } for _, cmd := range blocked { @@ -1650,10 +1676,19 @@ func TestShellTool_FindInWorkspaceAllowed(t *testing.T) { t.Fatalf("unable to configure exec tool: %s", err) } + // Legitimate workspace-relative commands that must not be blocked. allowed := []string{ "find . -name '*.go'", "find -name '*.txt'", + "find workspace -type d", "echo hello", + "ls -la", + // these match the find/ls prefix but not the root-path pattern + "findfile", + "lst", + // find/ls used on single-component relative paths (no slash) + "find sub -name '*.go'", + "ls sub", } for _, cmd := range allowed { @@ -1666,3 +1701,54 @@ func TestShellTool_FindInWorkspaceAllowed(t *testing.T) { } } } + +func TestShellTool_FindRootBlocked_DenyDisabled(t *testing.T) { + // When deny patterns are disabled, find / and ls / should still be + // caught by the workspace-boundary check (since / is outside the + // workspace). + tmpDir := t.TempDir() + cfg := &config.Config{} + cfg.Tools.Exec.EnableDenyPatterns = false + + tool, err := NewExecToolWithConfig(tmpDir, true, cfg) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + // These are blocked by workspace restriction even without deny patterns. + blocked := []string{ + "find /etc -name passwd", + "ls /etc", + "find /;", + "ls /;", + } + + for _, cmd := range blocked { + result := tool.Execute(context.Background(), map[string]any{ + "action": "run", + "command": cmd, + }) + if !result.IsError { + t.Errorf("expected command to be blocked (workspace check): %s", cmd) + } + } + + // find / and ls / with bare root *can* execute when deny patterns + // are disabled because / is in safePaths. + bareRoot := []string{ + "find / -maxdepth 1", + "ls /", + } + for _, cmd := range bareRoot { + result := tool.Execute(context.Background(), map[string]any{ + "action": "run", + "command": cmd, + }) + if result.IsError && strings.Contains(result.ForLLM, "blocked") { + t.Errorf( + "bare root command should not be workspace-blocked (safePaths): %s\n error: %s", + cmd, result.ForLLM, + ) + } + } +}