feat: add permission system core — PermissionStore, PermissionFunc, and tool integration
Add PermissibleTool interface and validatePathWithPermission for filesystem tools (read_file, write_file, list_dir, edit_file, append_file) and guardCommandWithPermission for exec tool. When a path outside the workspace is detected, the tool checks cached approvals in PermissionStore, then calls PermissionFunc to ask the user. Deny patterns and path traversal checks remain non-bypassable. Includes CLI stdin prompt implementation.
This commit is contained in:
parent
cfbb9fa9fe
commit
e3300b199d
8 changed files with 722 additions and 6 deletions
|
|
@ -12,6 +12,13 @@ import (
|
||||||
type EditFileTool struct {
|
type EditFileTool struct {
|
||||||
allowedDir string
|
allowedDir string
|
||||||
restrict bool
|
restrict bool
|
||||||
|
permStore *PermissionStore
|
||||||
|
permFn PermissionFunc
|
||||||
|
}
|
||||||
|
|
||||||
|
func (t *EditFileTool) SetPermission(store *PermissionStore, fn PermissionFunc) {
|
||||||
|
t.permStore = store
|
||||||
|
t.permFn = fn
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewEditFileTool creates a new EditFileTool with optional directory restriction.
|
// NewEditFileTool creates a new EditFileTool with optional directory restriction.
|
||||||
|
|
@ -67,7 +74,7 @@ func (t *EditFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
|
||||||
return ErrorResult("new_text is required")
|
return ErrorResult("new_text is required")
|
||||||
}
|
}
|
||||||
|
|
||||||
resolvedPath, err := validatePath(path, t.allowedDir, t.restrict)
|
resolvedPath, err := validatePathWithPermission(ctx, path, t.allowedDir, t.restrict, t.permStore, t.permFn)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return ErrorResult(err.Error())
|
return ErrorResult(err.Error())
|
||||||
}
|
}
|
||||||
|
|
@ -106,6 +113,13 @@ func (t *EditFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
|
||||||
type AppendFileTool struct {
|
type AppendFileTool struct {
|
||||||
workspace string
|
workspace string
|
||||||
restrict bool
|
restrict bool
|
||||||
|
permStore *PermissionStore
|
||||||
|
permFn PermissionFunc
|
||||||
|
}
|
||||||
|
|
||||||
|
func (t *AppendFileTool) SetPermission(store *PermissionStore, fn PermissionFunc) {
|
||||||
|
t.permStore = store
|
||||||
|
t.permFn = fn
|
||||||
}
|
}
|
||||||
|
|
||||||
func NewAppendFileTool(workspace string, restrict bool) *AppendFileTool {
|
func NewAppendFileTool(workspace string, restrict bool) *AppendFileTool {
|
||||||
|
|
@ -148,7 +162,7 @@ func (t *AppendFileTool) Execute(ctx context.Context, args map[string]any) *Tool
|
||||||
return ErrorResult("content is required")
|
return ErrorResult("content is required")
|
||||||
}
|
}
|
||||||
|
|
||||||
resolvedPath, err := validatePath(path, t.workspace, t.restrict)
|
resolvedPath, err := validatePathWithPermission(ctx, path, t.workspace, t.restrict, t.permStore, t.permFn)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return ErrorResult(err.Error())
|
return ErrorResult(err.Error())
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -8,6 +8,82 @@ import (
|
||||||
"strings"
|
"strings"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// validatePathWithPermission is like validatePath but instead of immediately
|
||||||
|
// erroring on outside-workspace paths, it checks the PermissionStore and
|
||||||
|
// optionally calls PermissionFunc to request access.
|
||||||
|
func validatePathWithPermission(ctx context.Context, path, workspace string, restrict bool, store *PermissionStore, permFn PermissionFunc) (string, error) {
|
||||||
|
if workspace == "" {
|
||||||
|
return path, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
absWorkspace, err := filepath.Abs(workspace)
|
||||||
|
if err != nil {
|
||||||
|
return "", fmt.Errorf("failed to resolve workspace path: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
var absPath string
|
||||||
|
if filepath.IsAbs(path) {
|
||||||
|
absPath = filepath.Clean(path)
|
||||||
|
} else {
|
||||||
|
absPath, err = filepath.Abs(filepath.Join(absWorkspace, path))
|
||||||
|
if err != nil {
|
||||||
|
return "", fmt.Errorf("failed to resolve file path: %w", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if restrict {
|
||||||
|
if isWithinWorkspace(absPath, absWorkspace) {
|
||||||
|
// Path is inside workspace — do symlink checks
|
||||||
|
workspaceReal := absWorkspace
|
||||||
|
if resolved, err := filepath.EvalSymlinks(absWorkspace); err == nil {
|
||||||
|
workspaceReal = resolved
|
||||||
|
}
|
||||||
|
|
||||||
|
if resolved, err := filepath.EvalSymlinks(absPath); err == nil {
|
||||||
|
if !isWithinWorkspace(resolved, workspaceReal) {
|
||||||
|
return "", fmt.Errorf("access denied: symlink resolves outside workspace")
|
||||||
|
}
|
||||||
|
} else if os.IsNotExist(err) {
|
||||||
|
if parentResolved, err := resolveExistingAncestor(filepath.Dir(absPath)); err == nil {
|
||||||
|
if !isWithinWorkspace(parentResolved, workspaceReal) {
|
||||||
|
return "", fmt.Errorf("access denied: symlink resolves outside workspace")
|
||||||
|
}
|
||||||
|
} else if !os.IsNotExist(err) {
|
||||||
|
return "", fmt.Errorf("failed to resolve path: %w", err)
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
return "", fmt.Errorf("failed to resolve path: %w", err)
|
||||||
|
}
|
||||||
|
return absPath, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Path is outside workspace — check permission store then ask
|
||||||
|
dir := filepath.Dir(absPath)
|
||||||
|
if store != nil && store.IsApproved(dir) {
|
||||||
|
return absPath, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
if permFn == nil {
|
||||||
|
return "", fmt.Errorf("access denied: path %s is outside the workspace. Ask the user for permission to access directory %s, then retry", absPath, dir)
|
||||||
|
}
|
||||||
|
|
||||||
|
approved, err := permFn(ctx, dir)
|
||||||
|
if err != nil {
|
||||||
|
return "", fmt.Errorf("permission check failed: %w", err)
|
||||||
|
}
|
||||||
|
if !approved {
|
||||||
|
return "", fmt.Errorf("access denied: user denied permission to access %s", dir)
|
||||||
|
}
|
||||||
|
|
||||||
|
if store != nil {
|
||||||
|
store.Approve(dir)
|
||||||
|
}
|
||||||
|
return absPath, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
return absPath, nil
|
||||||
|
}
|
||||||
|
|
||||||
// validatePath ensures the given path is within the workspace if restrict is true.
|
// validatePath ensures the given path is within the workspace if restrict is true.
|
||||||
func validatePath(path, workspace string, restrict bool) (string, error) {
|
func validatePath(path, workspace string, restrict bool) (string, error) {
|
||||||
if workspace == "" {
|
if workspace == "" {
|
||||||
|
|
@ -82,6 +158,13 @@ func isWithinWorkspace(candidate, workspace string) bool {
|
||||||
type ReadFileTool struct {
|
type ReadFileTool struct {
|
||||||
workspace string
|
workspace string
|
||||||
restrict bool
|
restrict bool
|
||||||
|
permStore *PermissionStore
|
||||||
|
permFn PermissionFunc
|
||||||
|
}
|
||||||
|
|
||||||
|
func (t *ReadFileTool) SetPermission(store *PermissionStore, fn PermissionFunc) {
|
||||||
|
t.permStore = store
|
||||||
|
t.permFn = fn
|
||||||
}
|
}
|
||||||
|
|
||||||
func NewReadFileTool(workspace string, restrict bool) *ReadFileTool {
|
func NewReadFileTool(workspace string, restrict bool) *ReadFileTool {
|
||||||
|
|
@ -115,7 +198,7 @@ func (t *ReadFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
|
||||||
return ErrorResult("path is required")
|
return ErrorResult("path is required")
|
||||||
}
|
}
|
||||||
|
|
||||||
resolvedPath, err := validatePath(path, t.workspace, t.restrict)
|
resolvedPath, err := validatePathWithPermission(ctx, path, t.workspace, t.restrict, t.permStore, t.permFn)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return ErrorResult(err.Error())
|
return ErrorResult(err.Error())
|
||||||
}
|
}
|
||||||
|
|
@ -131,6 +214,13 @@ func (t *ReadFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
|
||||||
type WriteFileTool struct {
|
type WriteFileTool struct {
|
||||||
workspace string
|
workspace string
|
||||||
restrict bool
|
restrict bool
|
||||||
|
permStore *PermissionStore
|
||||||
|
permFn PermissionFunc
|
||||||
|
}
|
||||||
|
|
||||||
|
func (t *WriteFileTool) SetPermission(store *PermissionStore, fn PermissionFunc) {
|
||||||
|
t.permStore = store
|
||||||
|
t.permFn = fn
|
||||||
}
|
}
|
||||||
|
|
||||||
func NewWriteFileTool(workspace string, restrict bool) *WriteFileTool {
|
func NewWriteFileTool(workspace string, restrict bool) *WriteFileTool {
|
||||||
|
|
@ -173,7 +263,7 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *ToolR
|
||||||
return ErrorResult("content is required")
|
return ErrorResult("content is required")
|
||||||
}
|
}
|
||||||
|
|
||||||
resolvedPath, err := validatePath(path, t.workspace, t.restrict)
|
resolvedPath, err := validatePathWithPermission(ctx, path, t.workspace, t.restrict, t.permStore, t.permFn)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return ErrorResult(err.Error())
|
return ErrorResult(err.Error())
|
||||||
}
|
}
|
||||||
|
|
@ -193,6 +283,13 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *ToolR
|
||||||
type ListDirTool struct {
|
type ListDirTool struct {
|
||||||
workspace string
|
workspace string
|
||||||
restrict bool
|
restrict bool
|
||||||
|
permStore *PermissionStore
|
||||||
|
permFn PermissionFunc
|
||||||
|
}
|
||||||
|
|
||||||
|
func (t *ListDirTool) SetPermission(store *PermissionStore, fn PermissionFunc) {
|
||||||
|
t.permStore = store
|
||||||
|
t.permFn = fn
|
||||||
}
|
}
|
||||||
|
|
||||||
func NewListDirTool(workspace string, restrict bool) *ListDirTool {
|
func NewListDirTool(workspace string, restrict bool) *ListDirTool {
|
||||||
|
|
@ -226,7 +323,7 @@ func (t *ListDirTool) Execute(ctx context.Context, args map[string]any) *ToolRes
|
||||||
path = "."
|
path = "."
|
||||||
}
|
}
|
||||||
|
|
||||||
resolvedPath, err := validatePath(path, t.workspace, t.restrict)
|
resolvedPath, err := validatePathWithPermission(ctx, path, t.workspace, t.restrict, t.permStore, t.permFn)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return ErrorResult(err.Error())
|
return ErrorResult(err.Error())
|
||||||
}
|
}
|
||||||
|
|
|
||||||
25
pkg/tools/permission_cli.go
Normal file
25
pkg/tools/permission_cli.go
Normal file
|
|
@ -0,0 +1,25 @@
|
||||||
|
package tools
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bufio"
|
||||||
|
"context"
|
||||||
|
"fmt"
|
||||||
|
"io"
|
||||||
|
"strings"
|
||||||
|
)
|
||||||
|
|
||||||
|
// NewCLIPermissionFunc creates a PermissionFunc that prompts the user on a terminal.
|
||||||
|
func NewCLIPermissionFunc(reader io.Reader, writer io.Writer) PermissionFunc {
|
||||||
|
scanner := bufio.NewScanner(reader)
|
||||||
|
return func(ctx context.Context, path string) (bool, error) {
|
||||||
|
fmt.Fprintf(writer, "\n⚠ Agent wants to access: %s\nAllow access to this directory? [y/N]: ", path)
|
||||||
|
if !scanner.Scan() {
|
||||||
|
if err := scanner.Err(); err != nil {
|
||||||
|
return false, err
|
||||||
|
}
|
||||||
|
return false, nil
|
||||||
|
}
|
||||||
|
answer := strings.TrimSpace(strings.ToLower(scanner.Text()))
|
||||||
|
return answer == "y" || answer == "yes", nil
|
||||||
|
}
|
||||||
|
}
|
||||||
42
pkg/tools/permission_cli_test.go
Normal file
42
pkg/tools/permission_cli_test.go
Normal file
|
|
@ -0,0 +1,42 @@
|
||||||
|
package tools
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"context"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestCLIPermissionFunc(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
input string
|
||||||
|
wantOK bool
|
||||||
|
}{
|
||||||
|
{name: "approve y", input: "y\n", wantOK: true},
|
||||||
|
{name: "approve yes", input: "yes\n", wantOK: true},
|
||||||
|
{name: "approve Y", input: "Y\n", wantOK: true},
|
||||||
|
{name: "approve YES", input: "YES\n", wantOK: true},
|
||||||
|
{name: "deny n", input: "n\n", wantOK: false},
|
||||||
|
{name: "deny empty", input: "\n", wantOK: false},
|
||||||
|
{name: "deny other", input: "maybe\n", wantOK: false},
|
||||||
|
}
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
reader := strings.NewReader(tt.input)
|
||||||
|
var output bytes.Buffer
|
||||||
|
fn := NewCLIPermissionFunc(reader, &output)
|
||||||
|
|
||||||
|
got, err := fn(context.Background(), "/Volumes/Code/project")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if got != tt.wantOK {
|
||||||
|
t.Errorf("got %v, want %v", got, tt.wantOK)
|
||||||
|
}
|
||||||
|
if !strings.Contains(output.String(), "/Volumes/Code/project") {
|
||||||
|
t.Errorf("output should mention the path, got: %s", output.String())
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
54
pkg/tools/permissions.go
Normal file
54
pkg/tools/permissions.go
Normal file
|
|
@ -0,0 +1,54 @@
|
||||||
|
package tools
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
|
"sync"
|
||||||
|
)
|
||||||
|
|
||||||
|
// PermissionFunc asks the user for permission to access a directory outside the workspace.
|
||||||
|
// Returns true if approved, false if denied. Implementations should block until the user responds.
|
||||||
|
type PermissionFunc func(ctx context.Context, path string) (bool, error)
|
||||||
|
|
||||||
|
// PermissionFuncFactory creates a PermissionFunc for a given channel and chatID.
|
||||||
|
// This allows channel-specific permission implementations (CLI stdin, Telegram buttons, etc.)
|
||||||
|
type PermissionFuncFactory func(channel, chatID string) PermissionFunc
|
||||||
|
|
||||||
|
// PermissibleTool is an optional interface that tools can implement
|
||||||
|
// to support permission-based access to paths outside the workspace.
|
||||||
|
type PermissibleTool interface {
|
||||||
|
Tool
|
||||||
|
SetPermission(store *PermissionStore, fn PermissionFunc)
|
||||||
|
}
|
||||||
|
|
||||||
|
// PermissionStore tracks approved directories for a session.
|
||||||
|
type PermissionStore struct {
|
||||||
|
mu sync.RWMutex
|
||||||
|
approved map[string]struct{}
|
||||||
|
}
|
||||||
|
|
||||||
|
func NewPermissionStore() *PermissionStore {
|
||||||
|
return &PermissionStore{
|
||||||
|
approved: make(map[string]struct{}),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func (ps *PermissionStore) Approve(dir string) {
|
||||||
|
ps.mu.Lock()
|
||||||
|
defer ps.mu.Unlock()
|
||||||
|
ps.approved[filepath.Clean(dir)] = struct{}{}
|
||||||
|
}
|
||||||
|
|
||||||
|
func (ps *PermissionStore) IsApproved(path string) bool {
|
||||||
|
ps.mu.RLock()
|
||||||
|
defer ps.mu.RUnlock()
|
||||||
|
|
||||||
|
cleanPath := filepath.Clean(path)
|
||||||
|
for dir := range ps.approved {
|
||||||
|
if cleanPath == dir || strings.HasPrefix(cleanPath, dir+string(filepath.Separator)) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
239
pkg/tools/permissions_test.go
Normal file
239
pkg/tools/permissions_test.go
Normal file
|
|
@ -0,0 +1,239 @@
|
||||||
|
package tools
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"fmt"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
|
"sync"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestPermissionStore_ApproveAndCheck(t *testing.T) {
|
||||||
|
store := NewPermissionStore()
|
||||||
|
store.Approve("/home/user/projects")
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
path string
|
||||||
|
want bool
|
||||||
|
}{
|
||||||
|
{name: "exact dir", path: "/home/user/projects", want: true},
|
||||||
|
{name: "child file", path: "/home/user/projects/main.go", want: true},
|
||||||
|
{name: "nested child", path: "/home/user/projects/pkg/tools/foo.go", want: true},
|
||||||
|
{name: "parent dir", path: "/home/user", want: false},
|
||||||
|
{name: "sibling dir", path: "/home/user/documents", want: false},
|
||||||
|
{name: "prefix overlap", path: "/home/user/projects-other/foo.go", want: false},
|
||||||
|
{name: "unrelated path", path: "/tmp/foo", want: false},
|
||||||
|
}
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
got := store.IsApproved(tt.path)
|
||||||
|
if got != tt.want {
|
||||||
|
t.Errorf("IsApproved(%q) = %v, want %v", tt.path, got, tt.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestPermissionStore_MultipleApprovals(t *testing.T) {
|
||||||
|
store := NewPermissionStore()
|
||||||
|
store.Approve("/home/user/projects")
|
||||||
|
store.Approve("/tmp/data")
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
path string
|
||||||
|
want bool
|
||||||
|
}{
|
||||||
|
{name: "first approval child", path: "/home/user/projects/main.go", want: true},
|
||||||
|
{name: "second approval child", path: "/tmp/data/file.csv", want: true},
|
||||||
|
{name: "neither approved", path: "/var/log/syslog", want: false},
|
||||||
|
}
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
got := store.IsApproved(tt.path)
|
||||||
|
if got != tt.want {
|
||||||
|
t.Errorf("IsApproved(%q) = %v, want %v", tt.path, got, tt.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestPermissionStore_ConcurrentAccess(t *testing.T) {
|
||||||
|
store := NewPermissionStore()
|
||||||
|
var wg sync.WaitGroup
|
||||||
|
|
||||||
|
// Concurrent writes
|
||||||
|
for i := 0; i < 100; i++ {
|
||||||
|
wg.Add(1)
|
||||||
|
go func(n int) {
|
||||||
|
defer wg.Done()
|
||||||
|
store.Approve(fmt.Sprintf("/dir/%d", n))
|
||||||
|
}(i)
|
||||||
|
}
|
||||||
|
wg.Wait()
|
||||||
|
|
||||||
|
// Concurrent reads
|
||||||
|
for i := 0; i < 100; i++ {
|
||||||
|
wg.Add(1)
|
||||||
|
go func(n int) {
|
||||||
|
defer wg.Done()
|
||||||
|
got := store.IsApproved(fmt.Sprintf("/dir/%d/file.txt", n))
|
||||||
|
if !got {
|
||||||
|
t.Errorf("expected /dir/%d/file.txt to be approved", n)
|
||||||
|
}
|
||||||
|
}(i)
|
||||||
|
}
|
||||||
|
wg.Wait()
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidatePath_WithPermission(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)
|
||||||
|
}
|
||||||
|
outside := filepath.Join(root, "outside")
|
||||||
|
if err := os.MkdirAll(outside, 0755); err != nil {
|
||||||
|
t.Fatalf("failed to create outside dir: %v", err)
|
||||||
|
}
|
||||||
|
outsideFile := filepath.Join(outside, "secret.txt")
|
||||||
|
if err := os.WriteFile(outsideFile, []byte("secret"), 0644); err != nil {
|
||||||
|
t.Fatalf("failed to write secret file: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
t.Run("approved by permFn", func(t *testing.T) {
|
||||||
|
store := NewPermissionStore()
|
||||||
|
approver := func(_ context.Context, _ string) (bool, error) {
|
||||||
|
return true, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
path, err := validatePathWithPermission(ctx, outsideFile, workspace, true, store, approver)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected success, got error: %v", err)
|
||||||
|
}
|
||||||
|
if path != outsideFile {
|
||||||
|
t.Errorf("expected path %s, got %s", outsideFile, path)
|
||||||
|
}
|
||||||
|
// Verify it was cached
|
||||||
|
if !store.IsApproved(outside) {
|
||||||
|
t.Errorf("expected directory %s to be approved in store", outside)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("denied by permFn", func(t *testing.T) {
|
||||||
|
store := NewPermissionStore()
|
||||||
|
denier := func(_ context.Context, _ string) (bool, error) {
|
||||||
|
return false, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err := validatePathWithPermission(ctx, outsideFile, workspace, true, store, denier)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected error, got nil")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "denied permission") {
|
||||||
|
t.Errorf("expected denied message, got: %v", err)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("nil permFn returns descriptive error", func(t *testing.T) {
|
||||||
|
store := NewPermissionStore()
|
||||||
|
|
||||||
|
_, err := validatePathWithPermission(ctx, outsideFile, workspace, true, store, nil)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected error, got nil")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "outside the workspace") {
|
||||||
|
t.Errorf("expected 'outside the workspace' message, got: %v", err)
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "Ask the user") {
|
||||||
|
t.Errorf("expected 'Ask the user' hint, got: %v", err)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("inside workspace still works", func(t *testing.T) {
|
||||||
|
insideFile := filepath.Join(workspace, "hello.txt")
|
||||||
|
if err := os.WriteFile(insideFile, []byte("hello"), 0644); err != nil {
|
||||||
|
t.Fatalf("failed to write inside file: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
path, err := validatePathWithPermission(ctx, insideFile, workspace, true, nil, nil)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected success for inside-workspace path, got error: %v", err)
|
||||||
|
}
|
||||||
|
if path != insideFile {
|
||||||
|
t.Errorf("expected path %s, got %s", insideFile, path)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("permFn error propagates", func(t *testing.T) {
|
||||||
|
store := NewPermissionStore()
|
||||||
|
errFn := func(_ context.Context, _ string) (bool, error) {
|
||||||
|
return false, fmt.Errorf("connection lost")
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err := validatePathWithPermission(ctx, outsideFile, workspace, true, store, errFn)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("expected error, got nil")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "connection lost") {
|
||||||
|
t.Errorf("expected 'connection lost' in error, got: %v", err)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidatePath_WithPermission_CachedApproval(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)
|
||||||
|
}
|
||||||
|
outside := filepath.Join(root, "outside")
|
||||||
|
if err := os.MkdirAll(outside, 0755); err != nil {
|
||||||
|
t.Fatalf("failed to create outside dir: %v", err)
|
||||||
|
}
|
||||||
|
outsideFile := filepath.Join(outside, "data.txt")
|
||||||
|
if err := os.WriteFile(outsideFile, []byte("data"), 0644); err != nil {
|
||||||
|
t.Fatalf("failed to write data file: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
store := NewPermissionStore()
|
||||||
|
callCount := 0
|
||||||
|
permFn := func(_ context.Context, _ string) (bool, error) {
|
||||||
|
callCount++
|
||||||
|
return true, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// First call — should invoke permFn
|
||||||
|
_, err := validatePathWithPermission(ctx, outsideFile, workspace, true, store, permFn)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("first call: unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if callCount != 1 {
|
||||||
|
t.Fatalf("expected permFn called once, got %d", callCount)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Second call — should use cached approval, not invoke permFn again
|
||||||
|
_, err = validatePathWithPermission(ctx, outsideFile, workspace, true, store, permFn)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("second call: unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if callCount != 1 {
|
||||||
|
t.Errorf("expected permFn still called once (cached), got %d", callCount)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Different file in same directory — should also use cache
|
||||||
|
otherFile := filepath.Join(outside, "other.txt")
|
||||||
|
_, err = validatePathWithPermission(ctx, otherFile, workspace, true, store, permFn)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("third call: unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
if callCount != 1 {
|
||||||
|
t.Errorf("expected permFn still called once (same dir cached), got %d", callCount)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -22,6 +22,13 @@ type ExecTool struct {
|
||||||
denyPatterns []*regexp.Regexp
|
denyPatterns []*regexp.Regexp
|
||||||
allowPatterns []*regexp.Regexp
|
allowPatterns []*regexp.Regexp
|
||||||
restrictToWorkspace bool
|
restrictToWorkspace bool
|
||||||
|
permStore *PermissionStore
|
||||||
|
permFn PermissionFunc
|
||||||
|
}
|
||||||
|
|
||||||
|
func (t *ExecTool) SetPermission(store *PermissionStore, fn PermissionFunc) {
|
||||||
|
t.permStore = store
|
||||||
|
t.permFn = fn
|
||||||
}
|
}
|
||||||
|
|
||||||
var defaultDenyPatterns = []*regexp.Regexp{
|
var defaultDenyPatterns = []*regexp.Regexp{
|
||||||
|
|
@ -162,7 +169,7 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if guardError := t.guardCommand(command, cwd); guardError != "" {
|
if guardError := t.guardCommandWithPermission(ctx, command, cwd); guardError != "" {
|
||||||
return ErrorResult(guardError)
|
return ErrorResult(guardError)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -313,6 +320,89 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// guardCommandWithPermission is like guardCommand but instead of blocking
|
||||||
|
// absolute paths outside the workspace, it checks the PermissionStore and
|
||||||
|
// optionally calls PermissionFunc to request access.
|
||||||
|
func (t *ExecTool) guardCommandWithPermission(ctx context.Context, command, cwd string) string {
|
||||||
|
cmd := strings.TrimSpace(command)
|
||||||
|
lower := strings.ToLower(cmd)
|
||||||
|
|
||||||
|
// 1. Deny pattern checks — no bypass
|
||||||
|
for _, pattern := range t.denyPatterns {
|
||||||
|
if pattern.MatchString(lower) {
|
||||||
|
return "Command blocked by safety guard (dangerous pattern detected)"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// 2. Allowlist checks — no bypass
|
||||||
|
if len(t.allowPatterns) > 0 {
|
||||||
|
allowed := false
|
||||||
|
for _, pattern := range t.allowPatterns {
|
||||||
|
if pattern.MatchString(lower) {
|
||||||
|
allowed = true
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !allowed {
|
||||||
|
return "Command blocked by safety guard (not in allowlist)"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if t.restrictToWorkspace {
|
||||||
|
// 3. Path traversal check — no bypass, security critical
|
||||||
|
if strings.Contains(cmd, "..\\") || strings.Contains(cmd, "../") {
|
||||||
|
return "Command blocked by safety guard (path traversal detected)"
|
||||||
|
}
|
||||||
|
|
||||||
|
cwdPath, err := filepath.Abs(cwd)
|
||||||
|
if err != nil {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
|
// 4. Absolute path outside workspace — check permission
|
||||||
|
pathPattern := regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`)
|
||||||
|
matches := pathPattern.FindAllString(cmd, -1)
|
||||||
|
|
||||||
|
for _, raw := range matches {
|
||||||
|
p, err := filepath.Abs(raw)
|
||||||
|
if err != nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
rel, err := filepath.Rel(cwdPath, p)
|
||||||
|
if err != nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
if strings.HasPrefix(rel, "..") {
|
||||||
|
dir := filepath.Dir(p)
|
||||||
|
|
||||||
|
if t.permStore != nil && t.permStore.IsApproved(dir) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
if t.permFn == nil {
|
||||||
|
return "Command blocked by safety guard (path outside working dir)"
|
||||||
|
}
|
||||||
|
|
||||||
|
approved, err := t.permFn(ctx, dir)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Sprintf("permission check failed: %v", err)
|
||||||
|
}
|
||||||
|
if !approved {
|
||||||
|
return fmt.Sprintf("access denied: user denied permission to access %s", dir)
|
||||||
|
}
|
||||||
|
|
||||||
|
if t.permStore != nil {
|
||||||
|
t.permStore.Approve(dir)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
func (t *ExecTool) SetTimeout(timeout time.Duration) {
|
func (t *ExecTool) SetTimeout(timeout time.Duration) {
|
||||||
t.timeout = timeout
|
t.timeout = timeout
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -272,3 +272,158 @@ func TestShellTool_RestrictToWorkspace(t *testing.T) {
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestGuardCommand_WithPermission(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
outsideDir := t.TempDir()
|
||||||
|
outsideFile := filepath.Join(outsideDir, "data.txt")
|
||||||
|
|
||||||
|
t.Run("nil permFn blocks outside path", func(t *testing.T) {
|
||||||
|
tool := NewExecTool(tmpDir, true)
|
||||||
|
// no SetPermission — permFn is nil
|
||||||
|
|
||||||
|
result := tool.guardCommandWithPermission(
|
||||||
|
context.Background(),
|
||||||
|
"cat "+outsideFile,
|
||||||
|
tmpDir,
|
||||||
|
)
|
||||||
|
if result == "" {
|
||||||
|
t.Fatalf("expected block, got empty string (allowed)")
|
||||||
|
}
|
||||||
|
if !strings.Contains(result, "path outside working dir") {
|
||||||
|
t.Errorf("expected 'path outside working dir' message, got: %s", result)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("permFn approves outside path", func(t *testing.T) {
|
||||||
|
tool := NewExecTool(tmpDir, true)
|
||||||
|
store := NewPermissionStore()
|
||||||
|
tool.SetPermission(store, func(_ context.Context, _ string) (bool, error) {
|
||||||
|
return true, nil
|
||||||
|
})
|
||||||
|
|
||||||
|
result := tool.guardCommandWithPermission(
|
||||||
|
context.Background(),
|
||||||
|
"cat "+outsideFile,
|
||||||
|
tmpDir,
|
||||||
|
)
|
||||||
|
if result != "" {
|
||||||
|
t.Fatalf("expected allowed, got: %s", result)
|
||||||
|
}
|
||||||
|
// Verify cached
|
||||||
|
if !store.IsApproved(outsideDir) {
|
||||||
|
t.Errorf("expected directory to be cached in store")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("permFn denies outside path", func(t *testing.T) {
|
||||||
|
tool := NewExecTool(tmpDir, true)
|
||||||
|
store := NewPermissionStore()
|
||||||
|
tool.SetPermission(store, func(_ context.Context, _ string) (bool, error) {
|
||||||
|
return false, nil
|
||||||
|
})
|
||||||
|
|
||||||
|
result := tool.guardCommandWithPermission(
|
||||||
|
context.Background(),
|
||||||
|
"cat "+outsideFile,
|
||||||
|
tmpDir,
|
||||||
|
)
|
||||||
|
if result == "" {
|
||||||
|
t.Fatalf("expected denial, got allowed")
|
||||||
|
}
|
||||||
|
if !strings.Contains(result, "denied permission") {
|
||||||
|
t.Errorf("expected 'denied permission' message, got: %s", result)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("cached approval skips permFn", func(t *testing.T) {
|
||||||
|
tool := NewExecTool(tmpDir, true)
|
||||||
|
store := NewPermissionStore()
|
||||||
|
callCount := 0
|
||||||
|
tool.SetPermission(store, func(_ context.Context, _ string) (bool, error) {
|
||||||
|
callCount++
|
||||||
|
return true, nil
|
||||||
|
})
|
||||||
|
|
||||||
|
// First call
|
||||||
|
result := tool.guardCommandWithPermission(
|
||||||
|
context.Background(),
|
||||||
|
"cat "+outsideFile,
|
||||||
|
tmpDir,
|
||||||
|
)
|
||||||
|
if result != "" {
|
||||||
|
t.Fatalf("first call: expected allowed, got: %s", result)
|
||||||
|
}
|
||||||
|
if callCount != 1 {
|
||||||
|
t.Fatalf("expected permFn called once, got %d", callCount)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Second call — should use cache
|
||||||
|
result = tool.guardCommandWithPermission(
|
||||||
|
context.Background(),
|
||||||
|
"cat "+outsideFile,
|
||||||
|
tmpDir,
|
||||||
|
)
|
||||||
|
if result != "" {
|
||||||
|
t.Fatalf("second call: expected allowed, got: %s", result)
|
||||||
|
}
|
||||||
|
if callCount != 1 {
|
||||||
|
t.Errorf("expected permFn still called once (cached), got %d", callCount)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("path traversal still blocked with permission", func(t *testing.T) {
|
||||||
|
tool := NewExecTool(tmpDir, true)
|
||||||
|
store := NewPermissionStore()
|
||||||
|
tool.SetPermission(store, func(_ context.Context, _ string) (bool, error) {
|
||||||
|
return true, nil
|
||||||
|
})
|
||||||
|
|
||||||
|
result := tool.guardCommandWithPermission(
|
||||||
|
context.Background(),
|
||||||
|
"cat ../../etc/passwd",
|
||||||
|
tmpDir,
|
||||||
|
)
|
||||||
|
if result == "" {
|
||||||
|
t.Fatalf("expected path traversal to be blocked even with permission")
|
||||||
|
}
|
||||||
|
if !strings.Contains(result, "path traversal") {
|
||||||
|
t.Errorf("expected 'path traversal' message, got: %s", result)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("deny pattern still blocked with permission", func(t *testing.T) {
|
||||||
|
tool := NewExecTool(tmpDir, true)
|
||||||
|
store := NewPermissionStore()
|
||||||
|
tool.SetPermission(store, func(_ context.Context, _ string) (bool, error) {
|
||||||
|
return true, nil
|
||||||
|
})
|
||||||
|
|
||||||
|
result := tool.guardCommandWithPermission(
|
||||||
|
context.Background(),
|
||||||
|
"rm -rf /",
|
||||||
|
tmpDir,
|
||||||
|
)
|
||||||
|
if result == "" {
|
||||||
|
t.Fatalf("expected deny pattern to be blocked even with permission")
|
||||||
|
}
|
||||||
|
if !strings.Contains(result, "dangerous pattern") {
|
||||||
|
t.Errorf("expected 'dangerous pattern' message, got: %s", result)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("inside workspace still allowed", func(t *testing.T) {
|
||||||
|
tool := NewExecTool(tmpDir, true)
|
||||||
|
// No permission set — inside workspace should still work
|
||||||
|
insideFile := filepath.Join(tmpDir, "hello.txt")
|
||||||
|
|
||||||
|
result := tool.guardCommandWithPermission(
|
||||||
|
context.Background(),
|
||||||
|
"cat "+insideFile,
|
||||||
|
tmpDir,
|
||||||
|
)
|
||||||
|
if result != "" {
|
||||||
|
t.Errorf("expected inside-workspace path to be allowed, got: %s", result)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue