fix(security): block shell escape bypasses, symlink TOCTOU, and working_dir escape

- Add 5 regex patterns to block ANSI-C/locale quoting, hex/octal escapes, and escaped metacharacters that bypassed shell denylist in restricted mode - Add safeReadFile/safeWriteFile/safeOpenFile wrappers that re-verify symlink targets right before I/O to close TOCTOU race window - Validate working_dir parameter stays within workspace when restricted - Document all three protections in README security section Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-16 23:03:41 +03:00 · 2026-02-16 23:03:41 +03:00 · ac77d77b01
commit ac77d77b01
parent 9d467b9434
6 changed files with 308 additions and 5 deletions
--- a/README.md
+++ b/README.md
@ -525,6 +525,36 @@ Even with `restrict_to_workspace: false`, the `exec` tool blocks these dangerous
 * `shutdown`, `reboot`, `poweroff` — System shutdown
 * Fork bomb `:(){ :|:& };:`

+#### Shell Escape Sequence Protection
+
+When `restrict_to_workspace: true`, the `exec` tool also blocks shell escape sequences that can bypass metacharacter detection:
+
+| Pattern | Example | Risk |
+|---------|---------|------|
+| ANSI-C quoting `$'...'` | `$'\x24(id)'` | Embeds command substitution via hex escape |
+| Locale quoting `$"..."` | `$"$(cmd)"` | Alternative command substitution syntax |
+| Hex escapes `\xNN` | `\x24(id)` | Encodes `$` as `\x24` to bypass `$()` check |
+| Octal escapes `\NNN` | `\060` | Encodes characters via octal to bypass checks |
+| Escaped metacharacters | `` \` ``, `\$` | Bypasses backtick and dollar sign detection |
+
+#### Working Directory Validation
+
+When `restrict_to_workspace: true`, the `exec` tool validates the `working_dir` parameter to ensure it stays within the configured workspace. Passing `working_dir` pointing outside the workspace (e.g. `/etc`) is blocked:
+
+```
+Command blocked by safety guard (working directory outside workspace)
+```
+
+#### Symlink TOCTOU Protection
+
+All file tools (`read_file`, `write_file`, `edit_file`, `append_file`) re-verify symlink targets immediately before the actual I/O operation. This closes the time-of-check-to-time-of-use (TOCTOU) window where an attacker could swap a symlink between the initial `validatePath()` check and the subsequent file operation:
+
+1. **Check**: `validatePath()` resolves the symlink and verifies the target is inside the workspace
+2. **Re-check**: `safeReadFile` / `safeWriteFile` / `safeOpenFile` calls `Lstat` right before I/O — if the path is a symlink, it re-resolves and re-validates the target
+3. **Operate**: The file operation uses the resolved path
+
+If the symlink target has changed to a path outside the workspace between steps 1 and 2, the operation is denied.
+
 #### SSRF Protection (Web Fetch)

 The `web_fetch` tool blocks requests to internal and private network addresses to prevent Server-Side Request Forgery (SSRF) attacks. This protects against unauthorized access to cloud metadata endpoints (e.g. `169.254.169.254`), internal services, and local resources.
--- a/pkg/tools/edit.go
+++ b/pkg/tools/edit.go
@ -76,7 +76,7 @@ func (t *EditFileTool) Execute(ctx context.Context, args map[string]interface{})
 		return ErrorResult(fmt.Sprintf("file not found: %s", path))
 	}

-	content, err := os.ReadFile(resolvedPath)
+	content, err := safeReadFile(resolvedPath, t.allowedDir, t.restrict)
 	if err != nil {
 		return ErrorResult(fmt.Sprintf("failed to read file: %v", err))
 	}
@ -94,7 +94,7 @@ func (t *EditFileTool) Execute(ctx context.Context, args map[string]interface{})

 	newContent := strings.Replace(contentStr, oldText, newText, 1)

-	if err := os.WriteFile(resolvedPath, []byte(newContent), 0644); err != nil {
+	if err := safeWriteFile(resolvedPath, []byte(newContent), 0644, t.allowedDir, t.restrict); err != nil {
 		return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
 	}

@ -151,7 +151,7 @@ func (t *AppendFileTool) Execute(ctx context.Context, args map[string]interface{
 		return ErrorResult(err.Error())
 	}

-	f, err := os.OpenFile(resolvedPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+	f, err := safeOpenFile(resolvedPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644, t.workspace, t.restrict)
 	if err != nil {
 		return ErrorResult(fmt.Sprintf("failed to open file: %v", err))
 	}
--- a/pkg/tools/filesystem.go
+++ b/pkg/tools/filesystem.go
@ -77,6 +77,71 @@ func isWithinWorkspace(candidate, workspace string) bool {
 	return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator))
 }

+// recheckSymlink verifies that path does not resolve outside workspace via symlink.
+// This is called right before the actual I/O operation to close the TOCTOU window
+// between validatePath and the file operation.
+func recheckSymlink(path, workspace string, restrict bool) (string, error) {
+	if !restrict || workspace == "" {
+		return path, nil
+	}
+
+	info, err := os.Lstat(path)
+	if err != nil {
+		// File doesn't exist yet (e.g. new file write) — nothing to recheck
+		if os.IsNotExist(err) {
+			return path, nil
+		}
+		return "", fmt.Errorf("failed to stat path: %w", err)
+	}
+
+	if info.Mode()&os.ModeSymlink != 0 {
+		resolved, err := filepath.EvalSymlinks(path)
+		if err != nil {
+			return "", fmt.Errorf("failed to resolve symlink: %w", err)
+		}
+		absWorkspace, err := filepath.Abs(workspace)
+		if err != nil {
+			return "", fmt.Errorf("failed to resolve workspace: %w", err)
+		}
+		if wsResolved, err := filepath.EvalSymlinks(absWorkspace); err == nil {
+			absWorkspace = wsResolved
+		}
+		if !isWithinWorkspace(resolved, absWorkspace) {
+			return "", fmt.Errorf("access denied: symlink resolves outside workspace")
+		}
+		return resolved, nil
+	}
+
+	return path, nil
+}
+
+// safeReadFile re-checks symlinks right before reading to prevent TOCTOU attacks.
+func safeReadFile(path, workspace string, restrict bool) ([]byte, error) {
+	resolved, err := recheckSymlink(path, workspace, restrict)
+	if err != nil {
+		return nil, err
+	}
+	return os.ReadFile(resolved)
+}
+
+// safeWriteFile re-checks symlinks right before writing to prevent TOCTOU attacks.
+func safeWriteFile(path string, data []byte, perm os.FileMode, workspace string, restrict bool) error {
+	resolved, err := recheckSymlink(path, workspace, restrict)
+	if err != nil {
+		return err
+	}
+	return os.WriteFile(resolved, data, perm)
+}
+
+// safeOpenFile re-checks symlinks right before opening to prevent TOCTOU attacks.
+func safeOpenFile(path string, flag int, perm os.FileMode, workspace string, restrict bool) (*os.File, error) {
+	resolved, err := recheckSymlink(path, workspace, restrict)
+	if err != nil {
+		return nil, err
+	}
+	return os.OpenFile(resolved, flag, perm)
+}
+
 type ReadFileTool struct {
 	workspace string
 	restrict  bool
@ -118,7 +183,7 @@ func (t *ReadFileTool) Execute(ctx context.Context, args map[string]interface{})
 		return ErrorResult(err.Error())
 	}

-	content, err := os.ReadFile(resolvedPath)
+	content, err := safeReadFile(resolvedPath, t.workspace, t.restrict)
 	if err != nil {
 		return ErrorResult(fmt.Sprintf("failed to read file: %v", err))
 	}
@ -181,7 +246,7 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]interface{}
 		return ErrorResult(fmt.Sprintf("failed to create directory: %v", err))
 	}

-	if err := os.WriteFile(resolvedPath, []byte(content), 0644); err != nil {
+	if err := safeWriteFile(resolvedPath, []byte(content), 0644, t.workspace, t.restrict); err != nil {
 		return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
 	}

--- a/pkg/tools/filesystem_test.go
+++ b/pkg/tools/filesystem_test.go
@ -279,3 +279,115 @@ func TestFilesystemTool_ReadFile_RejectsSymlinkEscape(t *testing.T) {
 		t.Fatalf("expected symlink escape error, got: %s", result.ForLLM)
 	}
 }
+
+// TestFilesystemTool_WriteFile_RejectsSymlinkEscape verifies that writing via a symlink
+// that points outside the workspace is blocked (TOCTOU protection).
+func TestFilesystemTool_WriteFile_RejectsSymlinkEscape(t *testing.T) {
+	root := t.TempDir()
+	workspace := filepath.Join(root, "workspace")
+	if err := os.MkdirAll(workspace, 0755); err != nil {
+		t.Fatalf("failed to create workspace: %v", err)
+	}
+
+	target := filepath.Join(root, "outside.txt")
+	if err := os.WriteFile(target, []byte("original"), 0644); err != nil {
+		t.Fatalf("failed to write target file: %v", err)
+	}
+
+	link := filepath.Join(workspace, "link.txt")
+	if err := os.Symlink(target, link); err != nil {
+		t.Skipf("symlink not supported in this environment: %v", err)
+	}
+
+	tool := NewWriteFileTool(workspace, true)
+	result := tool.Execute(context.Background(), map[string]interface{}{
+		"path":    link,
+		"content": "hacked",
+	})
+
+	if !result.IsError {
+		t.Fatalf("expected symlink escape to be blocked for write")
+	}
+	if !strings.Contains(result.ForLLM, "symlink resolves outside workspace") {
+		t.Fatalf("expected symlink escape error, got: %s", result.ForLLM)
+	}
+
+	// Verify original content was not overwritten
+	content, _ := os.ReadFile(target)
+	if string(content) != "original" {
+		t.Fatalf("expected original content to be preserved, got: %s", string(content))
+	}
+}
+
+// TestFilesystemTool_EditFile_RejectsSymlinkEscape verifies that editing via a symlink
+// that points outside the workspace is blocked (TOCTOU protection).
+func TestFilesystemTool_EditFile_RejectsSymlinkEscape(t *testing.T) {
+	root := t.TempDir()
+	workspace := filepath.Join(root, "workspace")
+	if err := os.MkdirAll(workspace, 0755); err != nil {
+		t.Fatalf("failed to create workspace: %v", err)
+	}
+
+	target := filepath.Join(root, "outside.txt")
+	if err := os.WriteFile(target, []byte("original content"), 0644); err != nil {
+		t.Fatalf("failed to write target file: %v", err)
+	}
+
+	link := filepath.Join(workspace, "link.txt")
+	if err := os.Symlink(target, link); err != nil {
+		t.Skipf("symlink not supported in this environment: %v", err)
+	}
+
+	tool := NewEditFileTool(workspace, true)
+	result := tool.Execute(context.Background(), map[string]interface{}{
+		"path":     link,
+		"old_text": "original",
+		"new_text": "hacked",
+	})
+
+	if !result.IsError {
+		t.Fatalf("expected symlink escape to be blocked for edit")
+	}
+
+	// Verify original content was not modified
+	content, _ := os.ReadFile(target)
+	if string(content) != "original content" {
+		t.Fatalf("expected original content to be preserved, got: %s", string(content))
+	}
+}
+
+// TestFilesystemTool_AppendFile_RejectsSymlinkEscape verifies that appending via a symlink
+// that points outside the workspace is blocked (TOCTOU protection).
+func TestFilesystemTool_AppendFile_RejectsSymlinkEscape(t *testing.T) {
+	root := t.TempDir()
+	workspace := filepath.Join(root, "workspace")
+	if err := os.MkdirAll(workspace, 0755); err != nil {
+		t.Fatalf("failed to create workspace: %v", err)
+	}
+
+	target := filepath.Join(root, "outside.txt")
+	if err := os.WriteFile(target, []byte("original"), 0644); err != nil {
+		t.Fatalf("failed to write target file: %v", err)
+	}
+
+	link := filepath.Join(workspace, "link.txt")
+	if err := os.Symlink(target, link); err != nil {
+		t.Skipf("symlink not supported in this environment: %v", err)
+	}
+
+	tool := NewAppendFileTool(workspace, true)
+	result := tool.Execute(context.Background(), map[string]interface{}{
+		"path":    link,
+		"content": "appended",
+	})
+
+	if !result.IsError {
+		t.Fatalf("expected symlink escape to be blocked for append")
+	}
+
+	// Verify original content was not modified
+	content, _ := os.ReadFile(target)
+	if string(content) != "original" {
+		t.Fatalf("expected original content to be preserved, got: %s", string(content))
+	}
+}
--- a/pkg/tools/shell.go
+++ b/pkg/tools/shell.go
@ -20,6 +20,11 @@ var (
 	shellMetaRe    = regexp.MustCompile("`|\\$\\(|\\$\\{")
 	varReferenceRe = regexp.MustCompile(`\$[A-Za-z_][A-Za-z0-9_]*`)
 	cdAbsoluteRe   = regexp.MustCompile(`(?i)\bcd\s+/`)
+	ansiCQuoteRe   = regexp.MustCompile(`\$'`)
+	ansiDQuoteRe   = regexp.MustCompile(`\$"`)
+	hexEscapeRe    = regexp.MustCompile(`\\x[0-9a-fA-F]`)
+	octalEscapeRe  = regexp.MustCompile(`\\[0-7]{1,3}`)
+	escapedMetaRe  = regexp.MustCompile(`\\[` + "`" + `$]`)
 )

 type ExecTool struct {
@ -108,6 +113,20 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]interface{}) *To
 		cwd = wd
 	}

+	if t.restrictToWorkspace && cwd != t.workingDir {
+		absCwd, err := filepath.Abs(cwd)
+		if err != nil {
+			return ErrorResult("invalid working directory")
+		}
+		absWs, err := filepath.Abs(t.workingDir)
+		if err != nil {
+			return ErrorResult("invalid workspace directory")
+		}
+		if !isWithinWorkspace(absCwd, absWs) {
+			return ErrorResult("Command blocked by safety guard (working directory outside workspace)")
+		}
+	}
+
 	if cwd == "" {
 		wd, err := os.Getwd()
 		if err == nil {
@ -219,6 +238,18 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
 			return "Command blocked by safety guard (shell metacharacter in restricted mode)"
 		}

+		// Block escape sequences that can bypass shell metacharacter detection
+		escapePatterns := []*regexp.Regexp{ansiCQuoteRe, ansiDQuoteRe, hexEscapeRe, octalEscapeRe, escapedMetaRe}
+		for _, re := range escapePatterns {
+			if re.MatchString(cmd) {
+				logger.WarnCF("shell", "Command blocked (escape sequence in restricted mode)", map[string]interface{}{
+					"command_preview": truncateForLog(cmd),
+					"pattern":         re.String(),
+				})
+				return "Command blocked by safety guard (escape sequence in restricted mode)"
+			}
+		}
+
 		// Block variable expansion ($VAR) which can reference paths outside workspace
 		if varReferenceRe.MatchString(cmd) {
 			logger.WarnCF("shell", "Command blocked (variable expansion in restricted mode)", map[string]interface{}{
--- a/pkg/tools/shell_test.go
+++ b/pkg/tools/shell_test.go
@ -313,3 +313,68 @@ func TestShellTool_WorkspaceAllowedCommands(t *testing.T) {
 		})
 	}
 }
+
+// TestShellTool_EscapeSequenceBlocking verifies that escape sequences that bypass
+// shell metacharacter detection are blocked in restricted mode.
+func TestShellTool_EscapeSequenceBlocking(t *testing.T) {
+	tmpDir := t.TempDir()
+	tool := NewExecTool(tmpDir, true)
+	ctx := context.Background()
+
+	cases := []struct {
+		name    string
+		command string
+	}{
+		{"ANSI-C quoting", `echo $'\x24(id)'`},
+		{"locale quoting", `echo $"hello"`},
+		{"hex escape", `echo \x24(id)`},
+		{"octal escape", `echo \060`},
+		{"escaped dollar", `echo \$HOME`},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := tool.Execute(ctx, map[string]interface{}{"command": tc.command})
+			if !result.IsError {
+				t.Errorf("Expected command to be blocked: %q", tc.command)
+			}
+			if !strings.Contains(result.ForLLM, "escape sequence") {
+				t.Errorf("Expected 'escape sequence' in error for %q, got: %s", tc.command, result.ForLLM)
+			}
+		})
+	}
+}
+
+// TestShellTool_WorkingDirRestriction verifies that working_dir outside workspace is blocked.
+func TestShellTool_WorkingDirRestriction(t *testing.T) {
+	tmpDir := t.TempDir()
+	tool := NewExecTool(tmpDir, true)
+	ctx := context.Background()
+
+	// working_dir outside workspace should be blocked
+	t.Run("outside workspace blocked", func(t *testing.T) {
+		result := tool.Execute(ctx, map[string]interface{}{
+			"command":     "ls",
+			"working_dir": "/etc",
+		})
+		if !result.IsError {
+			t.Errorf("Expected working_dir outside workspace to be blocked")
+		}
+		if !strings.Contains(result.ForLLM, "working directory outside workspace") {
+			t.Errorf("Expected 'working directory outside workspace' error, got: %s", result.ForLLM)
+		}
+	})
+
+	// working_dir inside workspace should be allowed
+	t.Run("inside workspace allowed", func(t *testing.T) {
+		subDir := filepath.Join(tmpDir, "subdir")
+		os.MkdirAll(subDir, 0755)
+		result := tool.Execute(ctx, map[string]interface{}{
+			"command":     "pwd",
+			"working_dir": subDir,
+		})
+		if result.IsError && strings.Contains(result.ForLLM, "working directory outside workspace") {
+			t.Errorf("working_dir inside workspace should not be blocked, got: %s", result.ForLLM)
+		}
+	})
+}