From f785eea02e71b68940e21db630167079da69a155 Mon Sep 17 00:00:00 2001 From: Subash Date: Mon, 16 Mar 2026 08:59:20 +0530 Subject: [PATCH] fix: prevent symlink operand escape in restricted exec The exec tool's restrictToWorkspace guard validates command text for path traversal (../) and absolute paths, but does not resolve symlinks in relative path operands. A symlink inside the workspace pointing outside allows reading or executing arbitrary files: os.Symlink("/etc", "workspace/leak") exec("cat leak/passwd") // passes guard, shell follows symlink Add guardSymlinkOperands() which extracts path-like tokens from the command, resolves them via filepath.EvalSymlinks, and blocks if any resolve outside the workspace boundary. Uses the same resolveExistingAncestor() fallback as the filesystem tool to handle dangling paths under symlinked directories. The tokenizer handles quoted paths and filters out flags, env assignments, URLs, and shell operators to minimize false positives. Fixes #1526 --- pkg/tools/shell.go | 133 ++++++++++++++++++++++++++ pkg/tools/shell_test.go | 205 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 338 insertions(+) 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{}