diff --git a/internal/app/bootstrap.go b/internal/app/bootstrap.go index 3578c1da..3acc7af7 100644 --- a/internal/app/bootstrap.go +++ b/internal/app/bootstrap.go @@ -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()) } diff --git a/internal/app/bootstrap_test.go b/internal/app/bootstrap_test.go index 14281848..8b2194bb 100644 --- a/internal/app/bootstrap_test.go +++ b/internal/app/bootstrap_test.go @@ -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) @@ -95,6 +96,7 @@ 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) @@ -102,6 +104,15 @@ func TestBuildToolManagerWrapsRegistry(t *testing.T) { 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 { diff --git a/internal/security/types.go b/internal/security/types.go index 95a32e6a..50da521b 100644 --- a/internal/security/types.go +++ b/internal/security/types.go @@ -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 + // 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. diff --git a/internal/security/workspace.go b/internal/security/workspace.go new file mode 100644 index 00000000..a0c6198a --- /dev/null +++ b/internal/security/workspace.go @@ -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) { + 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 { + 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) +} + +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))) +} diff --git a/internal/security/workspace_test.go b/internal/security/workspace_test.go new file mode 100644 index 00000000..4cb75c14 --- /dev/null +++ b/internal/security/workspace_test.go @@ -0,0 +1,864 @@ +package security + +import ( + "context" + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +func TestWorkspaceSandboxCheck(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + prepare func(t *testing.T, root string, outside string) + action func(root string, outside string) Action + expectErr string + }{ + { + name: "read path inside workspace is allowed", + prepare: func(t *testing.T, root string, outside string) { + t.Helper() + mustWriteWorkspaceFile(t, filepath.Join(root, "notes.txt"), "hello") + }, + action: func(root string, outside string) Action { + return fileAction(ActionTypeRead, "filesystem_read_file", "read_file", root, "notes.txt") + }, + }, + { + name: "read traversal is rejected", + action: func(root string, outside string) Action { + return fileAction(ActionTypeRead, "filesystem_read_file", "read_file", root, filepath.Join("..", "outside.txt")) + }, + expectErr: "escapes workspace root", + }, + { + name: "absolute path outside workspace is rejected", + action: func(root string, outside string) Action { + return fileAction(ActionTypeRead, "filesystem_read_file", "read_file", root, outside) + }, + expectErr: "escapes workspace root", + }, + { + name: "symlinked file outside workspace is rejected", + prepare: func(t *testing.T, root string, outside string) { + t.Helper() + mustWriteWorkspaceFile(t, outside, "secret") + mustSymlinkOrSkip(t, outside, filepath.Join(root, "linked.txt")) + }, + action: func(root string, outside string) Action { + return fileAction(ActionTypeRead, "filesystem_read_file", "read_file", root, "linked.txt") + }, + expectErr: "via symlink", + }, + { + name: "symlinked parent directory outside workspace is rejected", + prepare: func(t *testing.T, root string, outside string) { + t.Helper() + outsideDir := filepath.Dir(outside) + if err := os.MkdirAll(outsideDir, 0o755); err != nil { + t.Fatalf("mkdir outside dir: %v", err) + } + mustSymlinkOrSkip(t, outsideDir, filepath.Join(root, "linked-dir")) + }, + action: func(root string, outside string) Action { + return fileAction(ActionTypeWrite, "filesystem_write_file", "write_file", root, filepath.Join("linked-dir", "new.txt")) + }, + expectErr: "via symlink", + }, + { + name: "missing nested write path inside workspace is allowed", + action: func(root string, outside string) Action { + return fileAction(ActionTypeWrite, "filesystem_write_file", "write_file", root, filepath.Join("new", "nested.txt")) + }, + }, + { + name: "grep defaults to workspace root when dir is empty", + action: func(root string, outside string) Action { + return Action{ + Type: ActionTypeRead, + Payload: ActionPayload{ + ToolName: "filesystem_grep", + Resource: "filesystem_grep", + Operation: "grep", + Workdir: root, + TargetType: TargetTypeDirectory, + SandboxTargetType: TargetTypeDirectory, + }, + } + }, + }, + { + name: "bash workdir inside workspace is allowed", + prepare: func(t *testing.T, root string, outside string) { + t.Helper() + if err := os.MkdirAll(filepath.Join(root, "scripts"), 0o755); err != nil { + t.Fatalf("mkdir scripts: %v", err) + } + }, + action: func(root string, outside string) Action { + return bashAction(root, "pwd", "scripts") + }, + }, + { + name: "bash workdir traversal is rejected", + action: func(root string, outside string) Action { + return bashAction(root, "pwd", filepath.Join("..", "outside")) + }, + expectErr: "escapes workspace root", + }, + { + name: "webfetch does not trigger workspace checks", + action: func(root string, outside string) Action { + return Action{ + Type: ActionTypeRead, + Payload: ActionPayload{ + ToolName: "webfetch", + Resource: "webfetch", + Operation: "fetch", + Workdir: root, + TargetType: TargetTypeURL, + Target: "https://example.com", + }, + } + }, + }, + { + name: "missing workspace root is rejected for path action", + action: func(root string, outside string) Action { + return fileAction(ActionTypeRead, "filesystem_read_file", "read_file", "", "notes.txt") + }, + expectErr: "workspace root is empty", + }, + { + name: "empty file target is deferred to tool validation", + action: func(root string, outside string) Action { + return fileAction(ActionTypeRead, "filesystem_read_file", "read_file", root, "") + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + root := t.TempDir() + outsideRoot := t.TempDir() + outsideFile := filepath.Join(outsideRoot, "outside.txt") + if tt.prepare != nil { + tt.prepare(t, root, outsideFile) + } + + sandbox := NewWorkspaceSandbox() + err := sandbox.Check(context.Background(), tt.action(root, outsideFile)) + if tt.expectErr != "" { + if err == nil || !strings.Contains(err.Error(), tt.expectErr) { + t.Fatalf("expected error containing %q, got %v", tt.expectErr, err) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} + +func TestWorkspaceSandboxCheckShortCircuits(t *testing.T) { + t.Parallel() + + sandbox := NewWorkspaceSandbox() + root := t.TempDir() + action := fileAction(ActionTypeRead, "filesystem_read_file", "read_file", root, "notes.txt") + + canceledCtx, cancel := context.WithCancel(context.Background()) + cancel() + + err := sandbox.Check(canceledCtx, action) + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected context canceled error, got %v", err) + } + + err = sandbox.Check(context.Background(), Action{ + Type: ActionTypeRead, + Payload: ActionPayload{ + Resource: "filesystem_read_file", + Workdir: root, + }, + }) + if err == nil || !strings.Contains(err.Error(), "tool_name is empty") { + t.Fatalf("expected action validation error, got %v", err) + } +} + +func TestBuildWorkspacePlan(t *testing.T) { + t.Parallel() + + root := t.TempDir() + tests := []struct { + name string + action Action + wantOK bool + wantTarget string + wantErr string + }{ + { + name: "mcp action bypasses workspace sandbox", + action: Action{ + Type: ActionTypeMCP, + Payload: ActionPayload{ + ToolName: "mcp.call", + Resource: "mcp", + Workdir: root, + TargetType: TargetTypeMCP, + }, + }, + wantOK: false, + }, + { + name: "missing root is rejected when sandbox is needed", + action: Action{ + Type: ActionTypeRead, + Payload: ActionPayload{ + ToolName: "filesystem_read_file", + Resource: "filesystem_read_file", + TargetType: TargetTypePath, + Target: "notes.txt", + SandboxTargetType: TargetTypePath, + }, + }, + wantErr: "workspace root is empty", + }, + { + name: "empty path target falls back to tool validation", + action: Action{ + Type: ActionTypeRead, + Payload: ActionPayload{ + ToolName: "filesystem_read_file", + Resource: "filesystem_read_file", + Workdir: root, + TargetType: TargetTypePath, + SandboxTargetType: TargetTypePath, + }, + }, + wantOK: false, + }, + { + name: "directory target defaults to current workspace", + action: Action{ + Type: ActionTypeRead, + Payload: ActionPayload{ + ToolName: "filesystem_grep", + Resource: "filesystem_grep", + Workdir: root, + TargetType: TargetTypeDirectory, + SandboxTargetType: TargetTypeDirectory, + }, + }, + wantOK: true, + wantTarget: ".", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + plan, ok, err := buildWorkspacePlan(tt.action) + if tt.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("expected error containing %q, got %v", tt.wantErr, err) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ok != tt.wantOK { + t.Fatalf("ok = %v, want %v", ok, tt.wantOK) + } + if tt.wantTarget != "" && plan.target != tt.wantTarget { + t.Fatalf("target = %q, want %q", plan.target, tt.wantTarget) + } + }) + } +} + +func TestNeedsWorkspaceSandbox(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + typ ActionType + expect bool + }{ + {name: "read is sandboxed", typ: ActionTypeRead, expect: true}, + {name: "write is sandboxed", typ: ActionTypeWrite, expect: true}, + {name: "bash is sandboxed", typ: ActionTypeBash, expect: true}, + {name: "mcp is not sandboxed", typ: ActionTypeMCP, expect: false}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := needsWorkspaceSandbox(Action{Type: tt.typ}); got != tt.expect { + t.Fatalf("needsWorkspaceSandbox(%q) = %v, want %v", tt.typ, got, tt.expect) + } + }) + } +} + +func TestSandboxTarget(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + action Action + wantTarget string + wantOK bool + }{ + { + name: "bash defaults to current directory", + action: Action{ + Type: ActionTypeBash, + Payload: ActionPayload{ + SandboxTarget: "", + }, + }, + wantTarget: ".", + wantOK: true, + }, + { + name: "bash uses explicit sandbox target", + action: Action{ + Type: ActionTypeBash, + Payload: ActionPayload{ + SandboxTarget: "scripts", + }, + }, + wantTarget: "scripts", + wantOK: true, + }, + { + name: "fallback to target when sandbox target is empty", + action: Action{ + Type: ActionTypeRead, + Payload: ActionPayload{ + TargetType: TargetTypePath, + Target: "main.go", + }, + }, + wantTarget: "main.go", + wantOK: true, + }, + { + name: "directory target defaults to dot", + action: Action{ + Type: ActionTypeRead, + Payload: ActionPayload{ + TargetType: TargetTypeDirectory, + }, + }, + wantTarget: ".", + wantOK: true, + }, + { + name: "empty path target is ignored", + action: Action{ + Type: ActionTypeRead, + Payload: ActionPayload{ + TargetType: TargetTypePath, + }, + }, + wantOK: false, + }, + { + name: "unsupported target type is ignored", + action: Action{ + Type: ActionTypeRead, + Payload: ActionPayload{ + TargetType: TargetTypeURL, + Target: "https://example.com", + }, + }, + wantOK: false, + }, + { + name: "sandbox target takes precedence", + action: Action{ + Type: ActionTypeRead, + Payload: ActionPayload{ + TargetType: TargetTypePath, + Target: "main.go", + SandboxTargetType: TargetTypeDirectory, + SandboxTarget: "docs", + }, + }, + wantTarget: "docs", + wantOK: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + gotTarget, gotOK := sandboxTarget(tt.action) + if gotOK != tt.wantOK { + t.Fatalf("ok = %v, want %v", gotOK, tt.wantOK) + } + if gotTarget != tt.wantTarget { + t.Fatalf("target = %q, want %q", gotTarget, tt.wantTarget) + } + }) + } +} + +func TestCanonicalWorkspaceRoot(t *testing.T) { + t.Parallel() + + sandbox := NewWorkspaceSandbox() + existing := t.TempDir() + got, err := sandbox.canonicalWorkspaceRoot(existing) + if err != nil { + t.Fatalf("canonicalWorkspaceRoot(existing) error: %v", err) + } + expectedExisting, err := filepath.Abs(existing) + if err != nil { + t.Fatalf("filepath.Abs(existing): %v", err) + } + if !samePathKey(got, expectedExisting) { + t.Fatalf("canonicalWorkspaceRoot(existing) = %q, want %q", got, filepath.Clean(expectedExisting)) + } + if _, ok := sandbox.canonicalRoots.Load(cleanedPathKey(existing)); !ok { + t.Fatalf("expected canonical root cache entry for %q", existing) + } + + missing := filepath.Join(t.TempDir(), "missing", "dir") + _, err = sandbox.canonicalWorkspaceRoot(missing) + if err == nil || !strings.Contains(err.Error(), "resolve workspace root") { + t.Fatalf("expected missing root error, got %v", err) + } + + notDirRoot := filepath.Join(t.TempDir(), "file.txt") + mustWriteWorkspaceFile(t, notDirRoot, "content") + _, err = sandbox.canonicalWorkspaceRoot(notDirRoot) + if err == nil || !strings.Contains(err.Error(), "is not a directory") { + t.Fatalf("expected not-a-directory error, got %v", err) + } + + symlinkRoot := t.TempDir() + targetRoot := filepath.Join(symlinkRoot, "target") + linkRoot := filepath.Join(symlinkRoot, "link") + if err := os.MkdirAll(targetRoot, 0o755); err != nil { + t.Fatalf("mkdir target root: %v", err) + } + if err := os.Symlink(targetRoot, linkRoot); err != nil { + t.Skipf("symlink not supported in this environment: %v", err) + } + got, err = sandbox.canonicalWorkspaceRoot(linkRoot) + if err != nil { + t.Fatalf("canonicalWorkspaceRoot(link) error: %v", err) + } + expectedLink, err := filepath.Abs(targetRoot) + if err != nil { + t.Fatalf("filepath.Abs(targetRoot): %v", err) + } + if !samePathKey(got, expectedLink) { + t.Fatalf("canonicalWorkspaceRoot(link) = %q, want %q", got, filepath.Clean(expectedLink)) + } + if _, ok := sandbox.canonicalRoots.Load(cleanedPathKey(linkRoot)); ok { + t.Fatalf("did not expect symlinked root %q to be cached", linkRoot) + } +} + +func TestAbsoluteWorkspaceTarget(t *testing.T) { + t.Parallel() + + root := t.TempDir() + absoluteInside := filepath.Join(root, "abs.txt") + + tests := []struct { + name string + target string + want string + }{ + { + name: "empty target resolves to root", + target: "", + want: root, + }, + { + name: "relative target resolves from root", + target: filepath.Join("dir", "file.txt"), + want: filepath.Join(root, "dir", "file.txt"), + }, + { + name: "absolute target keeps absolute form", + target: absoluteInside, + want: absoluteInside, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := absoluteWorkspaceTarget(root, tt.target) + if err != nil { + t.Fatalf("absoluteWorkspaceTarget() error: %v", err) + } + wantAbs, err := filepath.Abs(tt.want) + if err != nil { + t.Fatalf("filepath.Abs(%q): %v", tt.want, err) + } + if got != filepath.Clean(wantAbs) { + t.Fatalf("absoluteWorkspaceTarget() = %q, want %q", got, filepath.Clean(wantAbs)) + } + }) + } +} + +func TestValidateTargetVolume(t *testing.T) { + t.Parallel() + + if runtime.GOOS != "windows" { + t.Skip("volume validation is Windows-specific") + } + + root := `C:\workspace` + inside := `C:\workspace\file.txt` + outside := `D:\secret.txt` + + if err := validateTargetVolume(root, inside); err != nil { + t.Fatalf("validateTargetVolume(inside) error: %v", err) + } + if err := validateTargetVolume(root, outside); err == nil || !strings.Contains(err.Error(), "different volume") { + t.Fatalf("expected different volume error, got %v", err) + } +} + +func TestNormalizeVolumeName(t *testing.T) { + t.Parallel() + + if runtime.GOOS != "windows" { + t.Skip("volume normalization is Windows-specific") + } + + tests := []struct { + name string + path string + want string + }{ + { + name: "drive letter is normalized", + path: `C:\workspace`, + want: `c:`, + }, + { + name: "extended path prefix is removed", + path: `\\?\C:\workspace`, + want: `c:`, + }, + { + name: "unc share is normalized", + path: `\\server\share\dir`, + want: `\\server\share`, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := normalizeVolumeName(tt.path); got != tt.want { + t.Fatalf("normalizeVolumeName(%q) = %q, want %q", tt.path, got, tt.want) + } + }) + } +} + +func TestEnsureNoSymlinkEscape(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setup func(t *testing.T) (root string, target string, original string) + expectErr string + }{ + { + name: "root target is allowed", + setup: func(t *testing.T) (string, string, string) { + t.Helper() + root := t.TempDir() + return root, root, "." + }, + }, + { + name: "symlink inside workspace is allowed", + setup: func(t *testing.T) (string, string, string) { + t.Helper() + root := t.TempDir() + realDir := filepath.Join(root, "real") + if err := os.MkdirAll(realDir, 0o755); err != nil { + t.Fatalf("mkdir real dir: %v", err) + } + linkDir := filepath.Join(root, "link") + if err := os.Symlink(realDir, linkDir); err != nil { + t.Skipf("symlink not supported in this environment: %v", err) + } + return root, filepath.Join(root, "link", "new.txt"), filepath.Join("link", "new.txt") + }, + }, + { + name: "broken symlink is rejected", + setup: func(t *testing.T) (string, string, string) { + t.Helper() + root := t.TempDir() + linkDir := filepath.Join(root, "broken") + if err := os.Symlink(filepath.Join(root, "missing"), linkDir); err != nil { + t.Skipf("symlink not supported in this environment: %v", err) + } + return root, filepath.Join(root, "broken", "new.txt"), filepath.Join("broken", "new.txt") + }, + expectErr: "resolve symlink", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + root, target, original := tt.setup(t) + err := ensureNoSymlinkEscape(root, target, original) + if tt.expectErr != "" { + if err == nil || !strings.Contains(err.Error(), tt.expectErr) { + t.Fatalf("expected error containing %q, got %v", tt.expectErr, err) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} + +func TestNearestExistingPath(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setup func(t *testing.T) (root string, target string) + expect func(root string, target string) string + expectErr string + }{ + { + name: "returns root for missing nested path", + setup: func(t *testing.T) (string, string) { + t.Helper() + root := t.TempDir() + return root, filepath.Join(root, "missing", "file.txt") + }, + expect: func(root string, target string) string { + return cleanedPathKey(root) + }, + }, + { + name: "returns nearest existing ancestor", + setup: func(t *testing.T) (string, string) { + t.Helper() + root := t.TempDir() + dir := filepath.Join(root, "dir") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatalf("mkdir dir: %v", err) + } + return root, filepath.Join(dir, "missing", "file.txt") + }, + expect: func(root string, target string) string { + return cleanedPathKey(filepath.Join(root, "dir")) + }, + }, + { + name: "returns broken symlink ancestor", + setup: func(t *testing.T) (string, string) { + t.Helper() + root := t.TempDir() + link := filepath.Join(root, "broken") + if err := os.Symlink(filepath.Join(root, "missing"), link); err != nil { + t.Skipf("symlink not supported in this environment: %v", err) + } + return root, filepath.Join(link, "child.txt") + }, + expect: func(root string, target string) string { + return cleanedPathKey(filepath.Join(root, "broken")) + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + root, target := tt.setup(t) + got, err := nearestExistingPath(root, target) + if tt.expectErr != "" { + if err == nil || !strings.Contains(err.Error(), tt.expectErr) { + t.Fatalf("expected error containing %q, got %v", tt.expectErr, err) + } + return + } + if err != nil { + t.Fatalf("nearestExistingPath() error: %v", err) + } + want := tt.expect(root, target) + if got != want { + t.Fatalf("nearestExistingPath() = %q, want %q", got, want) + } + }) + } +} + +func TestSplitRelativePath(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path string + want []string + }{ + { + name: "dot path returns nil", + path: ".", + want: nil, + }, + { + name: "nested path is split by separator", + path: filepath.Join("a", "b", "c"), + want: []string{"a", "b", "c"}, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := splitRelativePath(tt.path) + if len(got) != len(tt.want) { + t.Fatalf("splitRelativePath(%q) len = %d, want %d", tt.path, len(got), len(tt.want)) + } + for i := range got { + if got[i] != tt.want[i] { + t.Fatalf("splitRelativePath(%q)[%d] = %q, want %q", tt.path, i, got[i], tt.want[i]) + } + } + }) + } +} + +func TestIsWithinWorkspace(t *testing.T) { + t.Parallel() + + root := t.TempDir() + inside := filepath.Join(root, "sub", "file.txt") + outside := filepath.Join(t.TempDir(), "outside.txt") + + tests := []struct { + name string + root string + target string + want bool + }{ + { + name: "root itself is inside", + root: root, + target: root, + want: true, + }, + { + name: "child path is inside", + root: root, + target: inside, + want: true, + }, + { + name: "outside path is rejected", + root: root, + target: outside, + want: false, + }, + { + name: "invalid root returns false", + root: "", + target: root, + want: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := isWithinWorkspace(tt.root, tt.target); got != tt.want { + t.Fatalf("isWithinWorkspace(%q, %q) = %v, want %v", tt.root, tt.target, got, tt.want) + } + }) + } +} + +func fileAction(actionType ActionType, toolName string, operation string, workdir string, target string) Action { + return Action{ + Type: actionType, + Payload: ActionPayload{ + ToolName: toolName, + Resource: toolName, + Operation: operation, + Workdir: workdir, + TargetType: TargetTypePath, + Target: target, + SandboxTargetType: TargetTypePath, + SandboxTarget: target, + }, + } +} + +func bashAction(workdir string, command string, requestedWorkdir string) Action { + return Action{ + Type: ActionTypeBash, + Payload: ActionPayload{ + ToolName: "bash", + Resource: "bash", + Operation: "command", + Workdir: workdir, + TargetType: TargetTypeCommand, + Target: command, + SandboxTargetType: TargetTypeDirectory, + SandboxTarget: requestedWorkdir, + }, + } +} + +func mustWriteWorkspaceFile(t *testing.T, path string, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + +func mustSymlinkOrSkip(t *testing.T, target string, link string) { + t.Helper() + if err := os.Symlink(target, link); err != nil { + t.Skipf("symlink not supported in this environment: %v", err) + } +} diff --git a/internal/tools/manager.go b/internal/tools/manager.go index d7535f1e..fea81eb7 100644 --- a/internal/tools/manager.go +++ b/internal/tools/manager.go @@ -208,6 +208,12 @@ func actionMetadata(action security.Action) map[string]any { if action.Payload.Target != "" { metadata["permission_target"] = action.Payload.Target } + if action.Payload.SandboxTargetType != "" { + metadata["permission_sandbox_target_type"] = string(action.Payload.SandboxTargetType) + } + if action.Payload.SandboxTarget != "" { + metadata["permission_sandbox_target"] = action.Payload.SandboxTarget + } return metadata } diff --git a/internal/tools/manager_test.go b/internal/tools/manager_test.go index 7dcda95c..9f2615ec 100644 --- a/internal/tools/manager_test.go +++ b/internal/tools/manager_test.go @@ -3,6 +3,8 @@ package tools import ( "context" "errors" + "os" + "path/filepath" "strings" "testing" @@ -304,6 +306,41 @@ func TestDefaultManagerExecuteBoundaries(t *testing.T) { } } +func TestDefaultManagerExecuteWithWorkspaceSandbox(t *testing.T) { + t.Parallel() + + registry := NewRegistry() + tool := &managerStubTool{name: "filesystem_write_file", content: "ok"} + registry.Register(tool) + + engine, err := security.NewStaticGateway(security.DecisionAllow, nil) + if err != nil { + t.Fatalf("new engine: %v", err) + } + manager, err := NewManager(registry, engine, security.NewWorkspaceSandbox()) + if err != nil { + t.Fatalf("new manager: %v", err) + } + + workdir := t.TempDir() + outsideDir := t.TempDir() + if err := os.Symlink(outsideDir, filepath.Join(workdir, "link")); err != nil { + t.Skipf("symlink not supported in this environment: %v", err) + } + + _, execErr := manager.Execute(context.Background(), ToolCallInput{ + Name: "filesystem_write_file", + Arguments: []byte(`{"path":"link/outside.txt","content":"hello"}`), + Workdir: workdir, + }) + if execErr == nil || !strings.Contains(execErr.Error(), "escapes workspace root via symlink") { + t.Fatalf("expected sandbox escape error, got %v", execErr) + } + if tool.callCount != 0 { + t.Fatalf("expected blocked tool not to execute, got %d calls", tool.callCount) + } +} + func TestPermissionDecisionError(t *testing.T) { t.Parallel() @@ -358,17 +395,19 @@ func TestBuildPermissionAction(t *testing.T) { wantType security.ActionType wantResource string wantTarget string + wantSandbox string wantErr string }{ { name: "bash maps to bash action", input: ToolCallInput{ Name: "bash", - Arguments: []byte(`{"command":"echo hi"}`), + Arguments: []byte(`{"command":"echo hi","workdir":"scripts"}`), }, wantType: security.ActionTypeBash, wantResource: "bash", wantTarget: "echo hi", + wantSandbox: "scripts", }, { name: "read file maps to read action", @@ -379,6 +418,7 @@ func TestBuildPermissionAction(t *testing.T) { wantType: security.ActionTypeRead, wantResource: "filesystem_read_file", wantTarget: "main.go", + wantSandbox: "main.go", }, { name: "grep maps to read action", @@ -389,6 +429,7 @@ func TestBuildPermissionAction(t *testing.T) { wantType: security.ActionTypeRead, wantResource: "filesystem_grep", wantTarget: "internal", + wantSandbox: "internal", }, { name: "glob maps to read action", @@ -399,6 +440,7 @@ func TestBuildPermissionAction(t *testing.T) { wantType: security.ActionTypeRead, wantResource: "filesystem_glob", wantTarget: "cmd", + wantSandbox: "cmd", }, { name: "write file maps to write action", @@ -409,6 +451,7 @@ func TestBuildPermissionAction(t *testing.T) { wantType: security.ActionTypeWrite, wantResource: "filesystem_write_file", wantTarget: "main.go", + wantSandbox: "main.go", }, { name: "webfetch maps to read action", @@ -429,6 +472,7 @@ func TestBuildPermissionAction(t *testing.T) { wantType: security.ActionTypeWrite, wantResource: "filesystem_edit", wantTarget: "main.go", + wantSandbox: "main.go", }, { name: "mcp tool maps to mcp action", @@ -478,6 +522,9 @@ func TestBuildPermissionAction(t *testing.T) { if action.Payload.Target != tt.wantTarget { t.Fatalf("expected target %q, got %q", tt.wantTarget, action.Payload.Target) } + if action.Payload.SandboxTarget != tt.wantSandbox { + t.Fatalf("expected sandbox target %q, got %q", tt.wantSandbox, action.Payload.SandboxTarget) + } }) } } diff --git a/internal/tools/permission_mapper.go b/internal/tools/permission_mapper.go index 04bdf09a..946d1084 100644 --- a/internal/tools/permission_mapper.go +++ b/internal/tools/permission_mapper.go @@ -30,21 +30,29 @@ func buildPermissionAction(input ToolCallInput) (security.Action, error) { action.Payload.Operation = "command" action.Payload.TargetType = security.TargetTypeCommand action.Payload.Target = extractStringArgument(input.Arguments, "command") + action.Payload.SandboxTargetType = security.TargetTypeDirectory + action.Payload.SandboxTarget = extractStringArgument(input.Arguments, "workdir") case "filesystem_read_file": action.Type = security.ActionTypeRead action.Payload.Operation = "read_file" action.Payload.TargetType = security.TargetTypePath action.Payload.Target = extractStringArgument(input.Arguments, "path") + action.Payload.SandboxTargetType = security.TargetTypePath + action.Payload.SandboxTarget = action.Payload.Target case "filesystem_grep": action.Type = security.ActionTypeRead action.Payload.Operation = "grep" action.Payload.TargetType = security.TargetTypeDirectory action.Payload.Target = extractStringArgument(input.Arguments, "dir") + action.Payload.SandboxTargetType = security.TargetTypeDirectory + action.Payload.SandboxTarget = action.Payload.Target case "filesystem_glob": action.Type = security.ActionTypeRead action.Payload.Operation = "glob" action.Payload.TargetType = security.TargetTypeDirectory action.Payload.Target = extractStringArgument(input.Arguments, "dir") + action.Payload.SandboxTargetType = security.TargetTypeDirectory + action.Payload.SandboxTarget = action.Payload.Target case "webfetch": action.Type = security.ActionTypeRead action.Payload.Operation = "fetch" @@ -55,11 +63,15 @@ func buildPermissionAction(input ToolCallInput) (security.Action, error) { action.Payload.Operation = "write_file" action.Payload.TargetType = security.TargetTypePath action.Payload.Target = extractStringArgument(input.Arguments, "path") + action.Payload.SandboxTargetType = security.TargetTypePath + action.Payload.SandboxTarget = action.Payload.Target case "filesystem_edit": action.Type = security.ActionTypeWrite action.Payload.Operation = "edit" action.Payload.TargetType = security.TargetTypePath action.Payload.Target = extractStringArgument(input.Arguments, "path") + action.Payload.SandboxTargetType = security.TargetTypePath + action.Payload.SandboxTarget = action.Payload.Target default: if strings.HasPrefix(strings.ToLower(toolName), "mcp.") { action.Type = security.ActionTypeMCP