Skip to content
Merged
2 changes: 1 addition & 1 deletion internal/app/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,5 @@ func buildToolManager(registry *tools.Registry) (tools.Manager, error) {
if err != nil {
return nil, err
}
return tools.NewManager(registry, engine, nil)
return tools.NewManager(registry, engine, security.NewWorkspaceSandbox())
}
11 changes: 11 additions & 0 deletions internal/app/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ func TestBuildToolManagerWrapsRegistry(t *testing.T) {

registry := tools.NewRegistry()
registry.Register(stubToolForBootstrap{name: "bash", content: "ok"})
workdir := t.TempDir()
manager, err := buildToolManager(registry)
if err != nil {
t.Fatalf("buildToolManager() error = %v", err)
Expand All @@ -95,13 +96,23 @@ func TestBuildToolManagerWrapsRegistry(t *testing.T) {
result, execErr := manager.Execute(context.Background(), tools.ToolCallInput{
Name: "bash",
Arguments: []byte(`{"command":"echo hi"}`),
Workdir: workdir,
})
if execErr != nil {
t.Fatalf("Execute() error = %v", execErr)
}
if result.Content != "ok" {
t.Fatalf("expected ok result, got %+v", result)
}

_, execErr = manager.Execute(context.Background(), tools.ToolCallInput{
Name: "bash",
Arguments: []byte(`{"command":"echo hi","workdir":"../outside"}`),
Workdir: workdir,
})
if execErr == nil {
t.Fatalf("expected sandbox rejection for outside workdir")
}
}

type stubToolForBootstrap struct {
Expand Down
8 changes: 8 additions & 0 deletions internal/security/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ type ActionPayload struct {
Workdir string
TargetType TargetType
Target string
// SandboxTargetType is the target kind used specifically for workspace boundary
// checks. It falls back to TargetType when unset so callers can keep permission
// display metadata separate from the path actually validated by the sandbox.
SandboxTargetType TargetType
Comment thread
Cai-Tang-www marked this conversation as resolved.
// SandboxTarget is the concrete path/value used specifically for workspace
// validation. It falls back to Target when unset. For example, bash validates
// its requested workdir here while Target still contains the shell command.
SandboxTarget string
}

// Action is the unified security input for one tool execution request.
Expand Down
318 changes: 318 additions & 0 deletions internal/security/workspace.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,318 @@
package security

import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
"runtime"
"strings"
"sync"
)

// WorkspaceSandbox enforces workspace-relative path boundaries for tool actions.
type WorkspaceSandbox struct {
canonicalRoots sync.Map
}

// NewWorkspaceSandbox creates a sandbox that blocks traversal and symlink escape.
func NewWorkspaceSandbox() *WorkspaceSandbox {
return &WorkspaceSandbox{}
}

// Check validates that the action stays within the configured workspace root.
func (s *WorkspaceSandbox) Check(ctx context.Context, action Action) error {
if err := ctx.Err(); err != nil {
return err
}
if err := action.Validate(); err != nil {
return err
}

plan, ok, err := buildWorkspacePlan(action)
if err != nil {
return err
}
if !ok {
return nil
}

return s.validateWorkspacePlan(plan)
}

type workspacePlan struct {
root string
target string
}

// buildWorkspacePlan extracts the workspace root and the sandbox-specific target
// that should be validated. Actions that do not touch local workspace paths are
// ignored here and continue through the permission pipeline unchanged.
func buildWorkspacePlan(action Action) (workspacePlan, bool, error) {
if !needsWorkspaceSandbox(action) {
return workspacePlan{}, false, nil
}

root := strings.TrimSpace(action.Payload.Workdir)
if root == "" {
return workspacePlan{}, false, errors.New("security: workspace root is empty")
}

target, ok := sandboxTarget(action)
if !ok {
return workspacePlan{}, false, nil
}

return workspacePlan{
root: root,
target: target,
}, true, nil
}

func needsWorkspaceSandbox(action Action) bool {
switch action.Type {
case ActionTypeRead, ActionTypeWrite, ActionTypeBash:
return true
default:
return false
}
}

// sandboxTarget returns the path-like value that should be checked against the
// workspace boundary. This is intentionally separate from ActionPayload.Target,
// because some tools expose one value to policy while validating another one for
// local filesystem access.
func sandboxTarget(action Action) (string, bool) {
if action.Type == ActionTypeBash {
target := strings.TrimSpace(action.Payload.SandboxTarget)
if target == "" {
return ".", true
}
return target, true
}

targetType := action.Payload.SandboxTargetType
if targetType == "" {
targetType = action.Payload.TargetType
}

target := strings.TrimSpace(action.Payload.SandboxTarget)
if target == "" {
target = strings.TrimSpace(action.Payload.Target)
}

switch targetType {
case TargetTypeDirectory:
if target == "" {
return ".", true
}
return target, true
case TargetTypePath:
if target == "" {
return "", false
}
return target, true
default:
return "", false
}
}

// validateWorkspacePlan resolves the canonical workspace root, expands the
// requested target to an absolute path, verifies it stays inside the workspace,
// and then checks the nearest existing path ancestor for symlink escape.
func (s *WorkspaceSandbox) validateWorkspacePlan(plan workspacePlan) error {
root, err := s.canonicalWorkspaceRoot(plan.root)
if err != nil {
return err
}

target, err := absoluteWorkspaceTarget(root, plan.target)
if err != nil {
return err
}
if !isWithinWorkspace(root, target) {
return fmt.Errorf("security: path %q escapes workspace root", plan.target)
}

return ensureNoSymlinkEscape(root, target, plan.target)
}

// canonicalWorkspaceRoot resolves the configured workspace root to a canonical
// directory path and caches non-symlink workspace roots for repeated tool calls.
func (s *WorkspaceSandbox) canonicalWorkspaceRoot(root string) (string, error) {
absoluteRoot, err := filepath.Abs(strings.TrimSpace(root))
if err != nil {
return "", fmt.Errorf("security: resolve workspace root: %w", err)
}
cacheKey := cleanedPathKey(absoluteRoot)
if cached, ok := s.canonicalRoots.Load(cacheKey); ok {
return cached.(string), nil
}

canonicalRoot, cacheable, err := resolveCanonicalWorkspaceRoot(cacheKey)
if err != nil {
return "", err
}
if cacheable {
s.canonicalRoots.Store(cacheKey, canonicalRoot)
}
return canonicalRoot, nil
}

// resolveCanonicalWorkspaceRoot resolves the configured workspace root to a
// canonical directory path. The root must already exist so validation cannot
// silently rely on a string-only path that may later resolve through a
// symlinked parent.
func resolveCanonicalWorkspaceRoot(absoluteRoot string) (string, bool, error) {
info, err := os.Stat(absoluteRoot)
if err != nil {
return "", false, fmt.Errorf("security: resolve workspace root: %w", err)
}
if !info.IsDir() {
return "", false, fmt.Errorf("security: workspace root %q is not a directory", absoluteRoot)
}

canonicalRoot, err := filepath.EvalSymlinks(absoluteRoot)
if err != nil {
return "", false, fmt.Errorf("security: resolve workspace root: %w", err)
}

cleanedCanonical := cleanedPathKey(canonicalRoot)
return cleanedCanonical, samePathKey(absoluteRoot, cleanedCanonical), nil
}

// absoluteWorkspaceTarget expands a workspace-relative target into an absolute
// path and rejects Windows volume changes up front so later boundary checks do
// not depend on platform-specific filepath.Rel behavior.
func absoluteWorkspaceTarget(root string, target string) (string, error) {
trimmedTarget := strings.TrimSpace(target)
if trimmedTarget == "" {
trimmedTarget = "."
}
if !filepath.IsAbs(trimmedTarget) {
Comment thread
Cai-Tang-www marked this conversation as resolved.
trimmedTarget = filepath.Join(root, trimmedTarget)
}

absoluteTarget, err := filepath.Abs(trimmedTarget)
if err != nil {
return "", fmt.Errorf("security: resolve workspace target %q: %w", target, err)
}

if err := validateTargetVolume(root, absoluteTarget); err != nil {
return "", err
}

return filepath.Clean(absoluteTarget), nil
}

// ensureNoSymlinkEscape resolves the nearest existing path on the way to target
// and ensures that any symlink in the existing prefix still resolves inside the
// workspace.
//
// This blocks common symlink escape attempts such as placing a link inside the
// workspace that points to a file or directory outside the workspace tree while
// avoiding a path-by-path stat on every component.
//
// Known limitation: this validation is still subject to TOCTOU races between the
// sandbox check and the later filesystem operation in the executor. Eliminating
// that window requires executor-level changes such as descriptor-based access or
// no-follow file opening, which are outside this lightweight sandbox layer.
func ensureNoSymlinkEscape(root string, target string, original string) error {
Comment thread
Cai-Tang-www marked this conversation as resolved.
existingPath, err := nearestExistingPath(root, target)
if err != nil {
return err
}
if existingPath == root {
return nil
}

resolved, err := filepath.EvalSymlinks(existingPath)
if err != nil {
return fmt.Errorf("security: resolve symlink %q: %w", existingPath, err)
}
resolved, err = filepath.Abs(resolved)
if err != nil {
return fmt.Errorf("security: resolve symlink %q: %w", existingPath, err)
}
if !isWithinWorkspace(root, resolved) {
return fmt.Errorf("security: path %q escapes workspace root via symlink", original)
}
return nil
}

func validateTargetVolume(root string, target string) error {
rootVolume := normalizeVolumeName(root)
targetVolume := normalizeVolumeName(target)
if rootVolume == "" || targetVolume == "" {
return nil
}
if !strings.EqualFold(rootVolume, targetVolume) {
return fmt.Errorf("security: path %q is on different volume than workspace root", target)
}
return nil
}

func normalizeVolumeName(path string) string {
volume := strings.TrimSpace(filepath.VolumeName(cleanedPathKey(path)))
volume = strings.TrimPrefix(volume, `\\?\`)
return strings.ToLower(volume)
}

func nearestExistingPath(root string, target string) (string, error) {
current := cleanedPathKey(target)
root = cleanedPathKey(root)
for {
info, err := os.Lstat(current)
if err == nil {
if info.Mode()&os.ModeSymlink != 0 || current != root {
return current, nil
}
return root, nil
}
if !errors.Is(err, os.ErrNotExist) {
return "", fmt.Errorf("security: inspect path %q: %w", current, err)
}
if samePathKey(current, root) {
return root, nil
}

parent := filepath.Dir(current)
if samePathKey(parent, current) {
return "", fmt.Errorf("security: compare workspace target %q: reached filesystem root", target)
}
current = parent
}
}

func cleanedPathKey(path string) string {
cleaned := filepath.Clean(path)
if runtime.GOOS == "windows" {
return strings.ToLower(cleaned)
}
return cleaned
}

func samePathKey(a string, b string) bool {
if runtime.GOOS == "windows" {
return strings.EqualFold(cleanedPathKey(a), cleanedPathKey(b))
}
return cleanedPathKey(a) == cleanedPathKey(b)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL SECURITY: TOCTOU Race Condition Vulnerability

This function has a Time-of-Check-Time-of-Use (TOCTOU) vulnerability. The validation happens here, but the actual file operation occurs later in the tool executor. An attacker can:

  1. Pass validation with a safe symlink pointing inside workspace
  2. Between validation and file access, atomically replace the symlink to point outside workspace
  3. Tool executor accesses the malicious path (sandbox bypassed)

Attack scenario:

# During validation: workspace/link -> workspace/safe.txt (PASSES)
# Attacker replaces: workspace/link -> /etc/passwd
# Tool reads workspace/link (BYPASSED)

Recommendations:

  1. Open file descriptor during validation and pass to executor (requires executor refactor)
  2. Use O_NOFOLLOW flag on Linux or FILE_FLAG_OPEN_REPARSE_POINT on Windows
  3. At minimum, document this limitation and consider adding re-validation in critical paths
  4. Add mutex/locking around validation + execution for high-security scenarios

Impact: Complete sandbox bypass, arbitrary file read/write outside workspace.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这不是 workspace sandbox 这一层单独能解决的,得进执行器层,走 descriptor/no-follow/原子访问
我会留到后面修改


func splitRelativePath(path string) []string {
cleanPath := filepath.Clean(path)
if cleanPath == "." {
return nil
}
return strings.Split(cleanPath, string(os.PathSeparator))
}

func isWithinWorkspace(root string, target string) bool {
relativePath, err := filepath.Rel(root, target)
if err != nil {
return false
}
return relativePath == "." ||
(relativePath != ".." && !strings.HasPrefix(relativePath, ".."+string(os.PathSeparator)))
}
Loading
Loading