From 9ac94f32cf6df0d2e4d39c6d4811292cb167bdec Mon Sep 17 00:00:00 2001 From: anthrodjear Date: Tue, 5 May 2026 06:39:20 +0300 Subject: [PATCH] feat: modify ExecTool to check PermissionCache before executing outside-workspace commands --- pkg/tools/shell.go | 62 +++++++++++++++++++++++++++++++++++++++++ pkg/tools/shell_test.go | 37 ++++++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index a570ac9ec..4b6c338c0 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -21,6 +21,7 @@ import ( "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/constants" "github.com/sipeed/picoclaw/pkg/isolation" + "github.com/sipeed/picoclaw/pkg/logger" ) var ( @@ -44,6 +45,8 @@ type ExecTool struct { restrictToWorkspace bool allowRemote bool sessionManager *SessionManager + permissionCache *PermissionCache + askPermission bool } var ( @@ -235,12 +238,71 @@ func (t *ExecTool) Parameters() map[string]any { } } +func (t *ExecTool) checkPermission(command string) string { + if !t.askPermission { + return "granted" + } + + path := t.extractPathFromCommand(command) + if path == "" { + return "granted" + } + + if t.restrictToWorkspace && t.isOutsideWorkspace(path) { + if perm := t.permissionCache.Check(path); perm != "" { + if perm == "denied" { + return "denied" + } + return "granted" + } + return "needs_permission" + } + + return "granted" +} + +func (t *ExecTool) isOutsideWorkspace(path string) bool { + isAbs := filepath.IsAbs(path) || strings.HasPrefix(path, "/") + if isAbs { + absWorkspace, _ := filepath.Abs(t.workingDir) + absPath, _ := filepath.Abs(path) + return !strings.HasPrefix(absPath, absWorkspace) + } + return false +} + +func (t *ExecTool) extractPathFromCommand(command string) string { + parts := strings.Fields(command) + for _, part := range parts { + if filepath.IsAbs(part) || strings.HasPrefix(part, "/") { + return part + } + } + return "" +} + func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult { action, _ := args["action"].(string) if action == "" { return ErrorResult("action is required") } + if action == "run" { + command, _ := args["command"].(string) + switch t.checkPermission(command) { + case "needs_permission": + path := t.extractPathFromCommand(command) + logger.InfoCF("exec", "Permission needed", map[string]any{"command": command, "path": path}) + return &ToolResult{ + ForLLM: fmt.Sprintf("Permission needed for path: %s. Call request_permission tool with path='%s'.", path, path), + ForUser: fmt.Sprintf("⚠️ Permission required to access %s", path), + } + case "denied": + path := t.extractPathFromCommand(command) + return ErrorResult(fmt.Sprintf("Access to %s was denied", path)) + } + } + switch action { case "run": return t.executeRun(ctx, args) diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index a8de2f4c9..354e03dbe 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -1613,3 +1613,40 @@ func TestEncodeKeyTokenWithPtyKeyMode(t *testing.T) { }) } } + +func TestExecTool_CheckPermission(t *testing.T) { + pc := NewPermissionCache() + + tool := &ExecTool{ + permissionCache: pc, + askPermission: true, + workingDir: "/workspace", + restrictToWorkspace: true, + } + + // Test 1: command with outside-workspace path + result := tool.checkPermission("ls /desktop") + if result != "needs_permission" { + t.Errorf("Expected 'needs_permission', got %s", result) + } + + // Test 2: command with workspace path + result = tool.checkPermission("ls /workspace/folder") + if result != "granted" { + t.Errorf("Expected 'granted' for workspace path, got %s", result) + } + + // Test 3: permission denied + pc.Grant("/desktop", "denied") + result = tool.checkPermission("ls /desktop") + if result != "denied" { + t.Errorf("Expected 'denied', got %s", result) + } + + // Test 4: permission granted (session) + pc.Grant("/desktop", "session") + result = tool.checkPermission("ls /desktop") + if result != "granted" { + t.Errorf("Expected 'granted' for session permission, got %s", result) + } +}