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
This commit is contained in:
parent
9ab1450ab5
commit
f785eea02e
2 changed files with 338 additions and 0 deletions
|
|
@ -219,6 +219,10 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
|
||||||
return ErrorResult(guardError)
|
return ErrorResult(guardError)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if guardError := t.guardSymlinkOperands(command, cwd); guardError != "" {
|
||||||
|
return ErrorResult(guardError)
|
||||||
|
}
|
||||||
|
|
||||||
// Re-resolve symlinks immediately before execution to shrink the TOCTOU window
|
// Re-resolve symlinks immediately before execution to shrink the TOCTOU window
|
||||||
// between validation and cmd.Dir assignment.
|
// between validation and cmd.Dir assignment.
|
||||||
if t.restrictToWorkspace && t.workingDir != "" && cwd != t.workingDir {
|
if t.restrictToWorkspace && t.workingDir != "" && cwd != t.workingDir {
|
||||||
|
|
@ -427,6 +431,135 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
return ""
|
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) {
|
func (t *ExecTool) SetTimeout(timeout time.Duration) {
|
||||||
t.timeout = timeout
|
t.timeout = timeout
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -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
|
// TestShellTool_RemoteChannelBlockedByDefault verifies exec is blocked for remote channels
|
||||||
func TestShellTool_RemoteChannelBlockedByDefault(t *testing.T) {
|
func TestShellTool_RemoteChannelBlockedByDefault(t *testing.T) {
|
||||||
cfg := &config.Config{}
|
cfg := &config.Config{}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue