From ae484149ae6581f03097cc8931ba21205fdce680 Mon Sep 17 00:00:00 2001 From: islobodan Date: Mon, 27 Apr 2026 13:44:24 +0000 Subject: [PATCH 1/2] fix: block find / from bypassing workspace sandbox (fixes #2688) --- pkg/tools/shell.go | 5 +++- pkg/tools/shell_test.go | 53 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index a570ac9ec..5f4103dcf 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -95,10 +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 } // 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 @@ -111,6 +113,7 @@ var ( "/dev/stdin": true, "/dev/stdout": true, "/dev/stderr": true, + "/": 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 a8de2f4c9..fedc99c45 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -1613,3 +1613,56 @@ func TestEncodeKeyTokenWithPtyKeyMode(t *testing.T) { }) } } + +func TestShellTool_FindRootBlocked(t *testing.T) { + tmpDir := t.TempDir() + tool, err := NewExecTool(tmpDir, true) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + blocked := []string{ + "find / -name 'private*' -type f 2>/dev/null", + "find /etc -name 'passwd'", + "find / -type f -name '*.key'", + "ls /", + "ls /etc", + } + + 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: %s", cmd) + } + if !strings.Contains(result.ForLLM, "blocked") { + t.Errorf("expected 'blocked' message for: %s\ngot: %s", cmd, result.ForLLM) + } + } +} + +func TestShellTool_FindInWorkspaceAllowed(t *testing.T) { + tmpDir := t.TempDir() + tool, err := NewExecTool(tmpDir, true) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + allowed := []string{ + "find . -name '*.go'", + "find -name '*.txt'", + "echo hello", + } + + for _, cmd := range allowed { + result := tool.Execute(context.Background(), map[string]any{ + "action": "run", + "command": cmd, + }) + if result.IsError && strings.Contains(result.ForLLM, "blocked") { + t.Errorf("command should not be blocked: %s\n error: %s", cmd, result.ForLLM) + } + } +} From b9ba5266bdfb7e15464fe26a7eae5fd4cec2b328 Mon Sep 17 00:00:00 2001 From: islobodan Date: Mon, 27 Apr 2026 14:51:34 +0000 Subject: [PATCH 2/2] fix: sandbox deny patterns - use (\s|[;&|><\n]|$) instead of \b for find / and ls / --- pkg/tools/shell.go | 8 ++-- pkg/tools/shell_test.go | 88 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 5 deletions(-) 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, + ) + } + } +}