diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 9ea05bb12..f809e300a 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -219,6 +219,10 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult return ErrorResult(guardError) } + if guardError := t.guardSymlinkOperands(command, cwd); guardError != "" { + return ErrorResult(guardError) + } + // Re-resolve symlinks immediately before execution to shrink the TOCTOU window // between validation and cmd.Dir assignment. if t.restrictToWorkspace && t.workingDir != "" && cwd != t.workingDir { @@ -427,6 +431,135 @@ func (t *ExecTool) guardCommand(command, cwd string) string { return "" } +// guardSymlinkOperands extracts path-like tokens from the command and +// checks whether any of them resolve (via symlinks) to a location +// outside the workspace. This closes the gap where guardCommand +// validates the textual path but the filesystem resolves a symlink +// to a different location at execution time. +func (t *ExecTool) guardSymlinkOperands(command, cwd string) string { + if !t.restrictToWorkspace || t.workingDir == "" { + return "" + } + + absWorkspace, err := filepath.Abs(t.workingDir) + if err != nil { + return "" + } + workspaceReal := absWorkspace + if resolved, err := filepath.EvalSymlinks(absWorkspace); err == nil { + workspaceReal = resolved + } + + tokens := extractPathTokens(command) + for _, token := range tokens { + // Absolute paths are already validated by guardCommand. + if filepath.IsAbs(token) { + continue + } + + candidate := filepath.Join(cwd, token) + + // Try resolving the full path first. If it exists, EvalSymlinks + // follows every symlink component in the path. + resolved, err := filepath.EvalSymlinks(candidate) + if err != nil { + // Path doesn't exist — try resolving the nearest existing + // ancestor to catch dangling paths under a symlinked dir. + if resolved, err = resolveExistingAncestor(candidate); err != nil { + continue + } + } + + if !isWithinWorkspace(resolved, workspaceReal) { + return "Command blocked by safety guard (symlink resolves outside workspace)" + } + } + + return "" +} + +// extractPathTokens splits a shell command into tokens and returns +// those that look like filesystem path operands (not flags, operators, +// environment assignments, or URLs). This is intentionally a +// best-effort heuristic — shell expansion and subshell constructs +// cannot be statically analyzed, but this catches the common case of +// `cat leak/secret.txt` where `leak` is a symlink. +func extractPathTokens(command string) []string { + raw := tokenizeShellCommand(command) + + var paths []string + for _, tok := range raw { + // Skip flags + if strings.HasPrefix(tok, "-") { + continue + } + // Skip environment variable assignments (VAR=value) + if strings.ContainsRune(tok, '=') { + continue + } + // Skip shell operators and keywords + if isShellOperator(tok) { + continue + } + // Skip URLs + if strings.Contains(tok, "://") { + continue + } + // Skip tokens that look like pure commands (no path separator + // and no dot-prefix). This avoids resolving "cat", "grep", etc. + if !strings.ContainsRune(tok, filepath.Separator) && + !strings.ContainsRune(tok, '/') && + !strings.HasPrefix(tok, ".") { + continue + } + paths = append(paths, tok) + } + return paths +} + +// tokenizeShellCommand performs basic shell-like tokenization, handling +// single and double quotes. It does not handle escape sequences, +// variable expansion, or subshells — those are out of scope for a +// best-effort safety guard. +func tokenizeShellCommand(cmd string) []string { + var tokens []string + var current strings.Builder + inSingle := false + inDouble := false + + for i := 0; i < len(cmd); i++ { + ch := cmd[i] + switch { + case ch == '\'' && !inDouble: + inSingle = !inSingle + case ch == '"' && !inSingle: + inDouble = !inDouble + case (ch == ' ' || ch == '\t') && !inSingle && !inDouble: + if current.Len() > 0 { + tokens = append(tokens, current.String()) + current.Reset() + } + default: + current.WriteByte(ch) + } + } + if current.Len() > 0 { + tokens = append(tokens, current.String()) + } + return tokens +} + +// isShellOperator returns true for common shell operators and control +// characters that should not be treated as path operands. +func isShellOperator(tok string) bool { + switch tok { + case "|", "||", "&&", ";", "&", ">", ">>", "<", "<<", "<<<", + "2>", "2>>", "&>", "2>&1", "1>&2": + return true + } + return false +} + func (t *ExecTool) SetTimeout(timeout time.Duration) { t.timeout = timeout } diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index c4553020f..d353ce6a8 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -301,6 +301,211 @@ func TestShellTool_WorkingDir_SymlinkEscape(t *testing.T) { } } +// TestShellTool_SymlinkOperandEscape verifies that symlinks in command operands +// that resolve outside the workspace are blocked. +func TestShellTool_SymlinkOperandEscape(t *testing.T) { + root := t.TempDir() + workspace := filepath.Join(root, "workspace") + outside := filepath.Join(root, "outside") + if err := os.MkdirAll(workspace, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + if err := os.MkdirAll(outside, 0o755); err != nil { + t.Fatalf("mkdir outside: %v", err) + } + os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("top secret"), 0o600) + + // Create a symlink inside workspace pointing to outside + link := filepath.Join(workspace, "leak") + if err := os.Symlink(outside, link); err != nil { + t.Skipf("symlinks not supported: %v", err) + } + + tool, err := NewExecTool(workspace, true) + if err != nil { + t.Fatalf("NewExecTool: %v", err) + } + + result := tool.Execute(context.Background(), map[string]any{ + "command": "cat leak/secret.txt", + }) + + if !result.IsError { + t.Fatalf("expected symlink operand escape to be blocked, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "symlink resolves outside workspace") { + t.Errorf("expected symlink message, got: %s", result.ForLLM) + } +} + +// TestShellTool_SymlinkOperandEscape_SymlinkedFile verifies blocking when a +// direct file symlink is used as an operand. +func TestShellTool_SymlinkOperandEscape_SymlinkedFile(t *testing.T) { + root := t.TempDir() + workspace := filepath.Join(root, "workspace") + outside := filepath.Join(root, "outside") + if err := os.MkdirAll(workspace, 0o755); err != nil { + t.Fatalf("mkdir workspace: %v", err) + } + if err := os.MkdirAll(outside, 0o755); err != nil { + t.Fatalf("mkdir outside: %v", err) + } + secretFile := filepath.Join(outside, "passwd") + os.WriteFile(secretFile, []byte("root:x:0:0"), 0o600) + + // Symlink a file (not directory) + link := filepath.Join(workspace, "./stolen") + if err := os.Symlink(secretFile, link); err != nil { + t.Skipf("symlinks not supported: %v", err) + } + + tool, err := NewExecTool(workspace, true) + if err != nil { + t.Fatalf("NewExecTool: %v", err) + } + + result := tool.Execute(context.Background(), map[string]any{ + "command": "cat ./stolen", + }) + + if !result.IsError { + t.Fatalf("expected symlinked file operand to be blocked, got: %s", result.ForLLM) + } +} + +// TestShellTool_SymlinkOperandEscape_NestedSymlinks verifies that chains of +// symlinks are fully resolved. +func TestShellTool_SymlinkOperandEscape_NestedSymlinks(t *testing.T) { + root := t.TempDir() + workspace := filepath.Join(root, "workspace") + intermediate := filepath.Join(root, "intermediate") + outside := filepath.Join(root, "outside") + for _, d := range []string{workspace, intermediate, outside} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + } + os.WriteFile(filepath.Join(outside, "data.txt"), []byte("sensitive"), 0o600) + + // intermediate/hop -> outside + if err := os.Symlink(outside, filepath.Join(intermediate, "hop")); err != nil { + t.Skipf("symlinks not supported: %v", err) + } + // workspace/step1 -> intermediate + if err := os.Symlink(intermediate, filepath.Join(workspace, "step1")); err != nil { + t.Skipf("symlinks not supported: %v", err) + } + + tool, err := NewExecTool(workspace, true) + if err != nil { + t.Fatalf("NewExecTool: %v", err) + } + + result := tool.Execute(context.Background(), map[string]any{ + "command": "cat step1/hop/data.txt", + }) + + if !result.IsError { + t.Fatalf("expected nested symlink escape to be blocked, got: %s", result.ForLLM) + } +} + +// TestShellTool_SymlinkOperand_SafeSymlink verifies that symlinks within +// the workspace are allowed. +func TestShellTool_SymlinkOperand_SafeSymlink(t *testing.T) { + workspace := t.TempDir() + subdir := filepath.Join(workspace, "data") + if err := os.MkdirAll(subdir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + os.WriteFile(filepath.Join(subdir, "file.txt"), []byte("safe content"), 0o644) + + // Symlink within workspace: workspace/link -> workspace/data + if err := os.Symlink(subdir, filepath.Join(workspace, "link")); err != nil { + t.Skipf("symlinks not supported: %v", err) + } + + tool, err := NewExecTool(workspace, true) + if err != nil { + t.Fatalf("NewExecTool: %v", err) + } + + result := tool.Execute(context.Background(), map[string]any{ + "command": "cat link/file.txt", + }) + + if result.IsError { + t.Fatalf("expected safe symlink within workspace to be allowed, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "safe content") { + t.Errorf("expected file content in output, got: %s", result.ForLLM) + } +} + +// TestShellTool_SymlinkOperand_QuotedPath verifies that quoted paths with +// symlinks are also caught. +func TestShellTool_SymlinkOperand_QuotedPath(t *testing.T) { + root := t.TempDir() + workspace := filepath.Join(root, "workspace") + outside := filepath.Join(root, "outside") + for _, d := range []string{workspace, outside} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + } + os.WriteFile(filepath.Join(outside, "data.txt"), []byte("secret"), 0o600) + + if err := os.Symlink(outside, filepath.Join(workspace, "escape")); err != nil { + t.Skipf("symlinks not supported: %v", err) + } + + tool, err := NewExecTool(workspace, true) + if err != nil { + t.Fatalf("NewExecTool: %v", err) + } + + // Test with double-quoted path + result := tool.Execute(context.Background(), map[string]any{ + "command": `cat "escape/data.txt"`, + }) + + if !result.IsError { + t.Fatalf("expected quoted symlink operand to be blocked, got: %s", result.ForLLM) + } +} + +// TestShellTool_SymlinkOperand_MultipleOperands verifies that when multiple +// operands are present, a symlink escape in any one blocks the command. +func TestShellTool_SymlinkOperand_MultipleOperands(t *testing.T) { + root := t.TempDir() + workspace := filepath.Join(root, "workspace") + outside := filepath.Join(root, "outside") + for _, d := range []string{workspace, outside} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + } + os.WriteFile(filepath.Join(workspace, "safe.txt"), []byte("ok"), 0o644) + os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("stolen"), 0o600) + + if err := os.Symlink(outside, filepath.Join(workspace, "leak")); err != nil { + t.Skipf("symlinks not supported: %v", err) + } + + tool, err := NewExecTool(workspace, true) + if err != nil { + t.Fatalf("NewExecTool: %v", err) + } + + result := tool.Execute(context.Background(), map[string]any{ + "command": "cat ./safe.txt leak/secret.txt", + }) + + if !result.IsError { + t.Fatalf("expected mixed operands with one escape to be blocked, got: %s", result.ForLLM) + } +} + // TestShellTool_RemoteChannelBlockedByDefault verifies exec is blocked for remote channels func TestShellTool_RemoteChannelBlockedByDefault(t *testing.T) { cfg := &config.Config{}