chore: improve permissions

Kujtim Hoxha created

Change summary

internal/llm/agent/agent.go                                |  4 
internal/llm/prompt/coder.go                               |  8 
internal/llm/tools/edit.go                                 | 21 -
internal/llm/tools/ls.go                                   | 81 ++++++-
internal/llm/tools/multiedit.go                            | 14 -
internal/llm/tools/view.go                                 | 56 +++++
internal/llm/tools/write.go                                |  7 
internal/permission/permission.go                          | 12 +
internal/tui/components/dialogs/permissions/permissions.go | 77 +++++++
9 files changed, 219 insertions(+), 61 deletions(-)

Detailed changes

internal/llm/agent/agent.go 🔗

@@ -181,9 +181,9 @@ func NewAgent(
 			tools.NewFetchTool(permissions, cwd),
 			tools.NewGlobTool(cwd),
 			tools.NewGrepTool(cwd),
-			tools.NewLsTool(cwd),
+			tools.NewLsTool(permissions, cwd),
 			tools.NewSourcegraphTool(),
-			tools.NewViewTool(lspClients, cwd),
+			tools.NewViewTool(lspClients, permissions, cwd),
 			tools.NewWriteTool(lspClients, permissions, history, cwd),
 		}
 

internal/llm/prompt/coder.go 🔗

@@ -1,7 +1,6 @@
 package prompt
 
 import (
-	"context"
 	_ "embed"
 	"fmt"
 	"log/slog"
@@ -38,10 +37,7 @@ func getEnvironmentInfo() string {
 	isGit := isGitRepo(cwd)
 	platform := runtime.GOOS
 	date := time.Now().Format("1/2/2006")
-	ls := tools.NewLsTool(cwd)
-	r, _ := ls.Run(context.Background(), tools.ToolCall{
-		Input: `{"path":"."}`,
-	})
+	output, _ := tools.ListDirectoryTree(cwd, nil)
 	return fmt.Sprintf(`Here is useful information about the environment you are running in:
 <env>
 Working directory: %s
@@ -52,7 +48,7 @@ Today's date: %s
 <project>
 %s
 </project>
-		`, cwd, boolToYesNo(isGit), platform, date, r.Content)
+		`, cwd, boolToYesNo(isGit), platform, date, output)
 }
 
 func isGitRepo(dir string) bool {

internal/llm/tools/edit.go 🔗

@@ -215,15 +215,10 @@ func (e *editTool) createNewFile(ctx context.Context, filePath, content string,
 		content,
 		strings.TrimPrefix(filePath, e.workingDir),
 	)
-	rootDir := e.workingDir
-	permissionPath := filepath.Dir(filePath)
-	if strings.HasPrefix(filePath, rootDir) {
-		permissionPath = rootDir
-	}
 	p := e.permissions.Request(
 		permission.CreatePermissionRequest{
 			SessionID:   sessionID,
-			Path:        permissionPath,
+			Path:        filePath,
 			ToolCallID:  call.ID,
 			ToolName:    EditToolName,
 			Action:      "write",
@@ -341,15 +336,10 @@ func (e *editTool) deleteContent(ctx context.Context, filePath, oldString string
 		strings.TrimPrefix(filePath, e.workingDir),
 	)
 
-	rootDir := e.workingDir
-	permissionPath := filepath.Dir(filePath)
-	if strings.HasPrefix(filePath, rootDir) {
-		permissionPath = rootDir
-	}
 	p := e.permissions.Request(
 		permission.CreatePermissionRequest{
 			SessionID:   sessionID,
-			Path:        permissionPath,
+			Path:        filePath,
 			ToolCallID:  call.ID,
 			ToolName:    EditToolName,
 			Action:      "write",
@@ -476,15 +466,10 @@ func (e *editTool) replaceContent(ctx context.Context, filePath, oldString, newS
 		newContent,
 		strings.TrimPrefix(filePath, e.workingDir),
 	)
-	rootDir := e.workingDir
-	permissionPath := filepath.Dir(filePath)
-	if strings.HasPrefix(filePath, rootDir) {
-		permissionPath = rootDir
-	}
 	p := e.permissions.Request(
 		permission.CreatePermissionRequest{
 			SessionID:   sessionID,
-			Path:        permissionPath,
+			Path:        filePath,
 			ToolCallID:  call.ID,
 			ToolName:    EditToolName,
 			Action:      "write",

internal/llm/tools/ls.go 🔗

@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	"github.com/charmbracelet/crush/internal/fsext"
+	"github.com/charmbracelet/crush/internal/permission"
 )
 
 type LSParams struct {
@@ -16,6 +17,11 @@ type LSParams struct {
 	Ignore []string `json:"ignore"`
 }
 
+type LSPermissionsParams struct {
+	Path   string   `json:"path"`
+	Ignore []string `json:"ignore"`
+}
+
 type TreeNode struct {
 	Name     string      `json:"name"`
 	Path     string      `json:"path"`
@@ -29,7 +35,8 @@ type LSResponseMetadata struct {
 }
 
 type lsTool struct {
-	workingDir string
+	workingDir  string
+	permissions permission.Service
 }
 
 const (
@@ -71,9 +78,10 @@ TIPS:
 - Combine with other tools for more effective exploration`
 )
 
-func NewLsTool(workingDir string) BaseTool {
+func NewLsTool(permissions permission.Service, workingDir string) BaseTool {
 	return &lsTool{
-		workingDir: workingDir,
+		workingDir:  workingDir,
+		permissions: permissions,
 	}
 }
 
@@ -117,20 +125,51 @@ func (l *lsTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error) {
 		searchPath = filepath.Join(l.workingDir, searchPath)
 	}
 
-	if _, err := os.Stat(searchPath); os.IsNotExist(err) {
-		return NewTextErrorResponse(fmt.Sprintf("path does not exist: %s", searchPath)), nil
+	// Check if directory is outside working directory and request permission if needed
+	absWorkingDir, err := filepath.Abs(l.workingDir)
+	if err != nil {
+		return ToolResponse{}, fmt.Errorf("error resolving working directory: %w", err)
 	}
 
-	files, truncated, err := fsext.ListDirectory(searchPath, params.Ignore, MaxLSFiles)
+	absSearchPath, err := filepath.Abs(searchPath)
 	if err != nil {
-		return ToolResponse{}, fmt.Errorf("error listing directory: %w", err)
+		return ToolResponse{}, fmt.Errorf("error resolving search path: %w", err)
 	}
 
-	tree := createFileTree(files)
-	output := printTree(tree, searchPath)
+	relPath, err := filepath.Rel(absWorkingDir, absSearchPath)
+	if err != nil || strings.HasPrefix(relPath, "..") {
+		// Directory is outside working directory, request permission
+		sessionID, messageID := GetContextValues(ctx)
+		if sessionID == "" || messageID == "" {
+			return ToolResponse{}, fmt.Errorf("session ID and message ID are required for accessing directories outside working directory")
+		}
 
-	if truncated {
-		output = fmt.Sprintf("There are more than %d files in the directory. Use a more specific path or use the Glob tool to find specific files. The first %d files and directories are included below:\n\n%s", MaxLSFiles, MaxLSFiles, output)
+		granted := l.permissions.Request(
+			permission.CreatePermissionRequest{
+				SessionID:   sessionID,
+				Path:        absSearchPath,
+				ToolCallID:  call.ID,
+				ToolName:    LSToolName,
+				Action:      "list",
+				Description: fmt.Sprintf("List directory outside working directory: %s", absSearchPath),
+				Params:      LSPermissionsParams(params),
+			},
+		)
+
+		if !granted {
+			return ToolResponse{}, permission.ErrorPermissionDenied
+		}
+	}
+
+	output, err := ListDirectoryTree(searchPath, params.Ignore)
+	if err != nil {
+		return ToolResponse{}, err
+	}
+
+	// Get file count for metadata
+	files, truncated, err := fsext.ListDirectory(searchPath, params.Ignore, MaxLSFiles)
+	if err != nil {
+		return ToolResponse{}, fmt.Errorf("error listing directory for metadata: %w", err)
 	}
 
 	return WithResponseMetadata(
@@ -142,6 +181,26 @@ func (l *lsTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error) {
 	), nil
 }
 
+func ListDirectoryTree(searchPath string, ignore []string) (string, error) {
+	if _, err := os.Stat(searchPath); os.IsNotExist(err) {
+		return "", fmt.Errorf("path does not exist: %s", searchPath)
+	}
+
+	files, truncated, err := fsext.ListDirectory(searchPath, ignore, MaxLSFiles)
+	if err != nil {
+		return "", fmt.Errorf("error listing directory: %w", err)
+	}
+
+	tree := createFileTree(files)
+	output := printTree(tree, searchPath)
+
+	if truncated {
+		output = fmt.Sprintf("There are more than %d files in the directory. Use a more specific path or use the Glob tool to find specific files. The first %d files and directories are included below:\n\n%s", MaxLSFiles, MaxLSFiles, output)
+	}
+
+	return output, nil
+}
+
 func createFileTree(sortedPaths []string) []*TreeNode {
 	root := []*TreeNode{}
 	pathMap := make(map[string]*TreeNode)

internal/llm/tools/multiedit.go 🔗

@@ -249,15 +249,10 @@ func (m *multiEditTool) processMultiEditWithCreation(ctx context.Context, params
 
 	// Check permissions
 	_, additions, removals := diff.GenerateDiff("", currentContent, strings.TrimPrefix(params.FilePath, m.workingDir))
-	rootDir := m.workingDir
-	permissionPath := filepath.Dir(params.FilePath)
-	if strings.HasPrefix(params.FilePath, rootDir) {
-		permissionPath = rootDir
-	}
 
 	p := m.permissions.Request(permission.CreatePermissionRequest{
 		SessionID:   sessionID,
-		Path:        permissionPath,
+		Path:        params.FilePath,
 		ToolCallID:  call.ID,
 		ToolName:    MultiEditToolName,
 		Action:      "write",
@@ -364,15 +359,10 @@ func (m *multiEditTool) processMultiEditExistingFile(ctx context.Context, params
 
 	// Generate diff and check permissions
 	_, additions, removals := diff.GenerateDiff(oldContent, currentContent, strings.TrimPrefix(params.FilePath, m.workingDir))
-	rootDir := m.workingDir
-	permissionPath := filepath.Dir(params.FilePath)
-	if strings.HasPrefix(params.FilePath, rootDir) {
-		permissionPath = rootDir
-	}
 
 	p := m.permissions.Request(permission.CreatePermissionRequest{
 		SessionID:   sessionID,
-		Path:        permissionPath,
+		Path:        params.FilePath,
 		ToolCallID:  call.ID,
 		ToolName:    MultiEditToolName,
 		Action:      "write",

internal/llm/tools/view.go 🔗

@@ -12,6 +12,7 @@ import (
 	"unicode/utf8"
 
 	"github.com/charmbracelet/crush/internal/lsp"
+	"github.com/charmbracelet/crush/internal/permission"
 )
 
 type ViewParams struct {
@@ -20,9 +21,16 @@ type ViewParams struct {
 	Limit    int    `json:"limit"`
 }
 
+type ViewPermissionsParams struct {
+	FilePath string `json:"file_path"`
+	Offset   int    `json:"offset"`
+	Limit    int    `json:"limit"`
+}
+
 type viewTool struct {
-	lspClients map[string]*lsp.Client
-	workingDir string
+	lspClients  map[string]*lsp.Client
+	workingDir  string
+	permissions permission.Service
 }
 
 type ViewResponseMetadata struct {
@@ -46,6 +54,7 @@ HOW TO USE:
 - Provide the path to the file you want to view
 - Optionally specify an offset to start reading from a specific line
 - Optionally specify a limit to control how many lines are read
+- Do not use this for directories use the ls tool instead
 
 FEATURES:
 - Displays file contents with line numbers for easy reference
@@ -72,10 +81,11 @@ TIPS:
 - When viewing large files, use the offset parameter to read specific sections`
 )
 
-func NewViewTool(lspClients map[string]*lsp.Client, workingDir string) BaseTool {
+func NewViewTool(lspClients map[string]*lsp.Client, permissions permission.Service, workingDir string) BaseTool {
 	return &viewTool{
-		lspClients: lspClients,
-		workingDir: workingDir,
+		lspClients:  lspClients,
+		workingDir:  workingDir,
+		permissions: permissions,
 	}
 }
 
@@ -122,6 +132,42 @@ func (v *viewTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error)
 		filePath = filepath.Join(v.workingDir, filePath)
 	}
 
+	// Check if file is outside working directory and request permission if needed
+	absWorkingDir, err := filepath.Abs(v.workingDir)
+	if err != nil {
+		return ToolResponse{}, fmt.Errorf("error resolving working directory: %w", err)
+	}
+
+	absFilePath, err := filepath.Abs(filePath)
+	if err != nil {
+		return ToolResponse{}, fmt.Errorf("error resolving file path: %w", err)
+	}
+
+	relPath, err := filepath.Rel(absWorkingDir, absFilePath)
+	if err != nil || strings.HasPrefix(relPath, "..") {
+		// File is outside working directory, request permission
+		sessionID, messageID := GetContextValues(ctx)
+		if sessionID == "" || messageID == "" {
+			return ToolResponse{}, fmt.Errorf("session ID and message ID are required for accessing files outside working directory")
+		}
+
+		granted := v.permissions.Request(
+			permission.CreatePermissionRequest{
+				SessionID:   sessionID,
+				Path:        absFilePath,
+				ToolCallID:  call.ID,
+				ToolName:    ViewToolName,
+				Action:      "read",
+				Description: fmt.Sprintf("Read file outside working directory: %s", absFilePath),
+				Params:      ViewPermissionsParams(params),
+			},
+		)
+
+		if !granted {
+			return ToolResponse{}, permission.ErrorPermissionDenied
+		}
+	}
+
 	// Check if file exists
 	fileInfo, err := os.Stat(filePath)
 	if err != nil {

internal/llm/tools/write.go 🔗

@@ -172,15 +172,10 @@ func (w *writeTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error
 		strings.TrimPrefix(filePath, w.workingDir),
 	)
 
-	rootDir := w.workingDir
-	permissionPath := filepath.Dir(filePath)
-	if strings.HasPrefix(filePath, rootDir) {
-		permissionPath = rootDir
-	}
 	p := w.permissions.Request(
 		permission.CreatePermissionRequest{
 			SessionID:   sessionID,
-			Path:        permissionPath,
+			Path:        filePath,
 			ToolCallID:  call.ID,
 			ToolName:    WriteToolName,
 			Action:      "write",

internal/permission/permission.go 🔗

@@ -3,6 +3,7 @@ package permission
 import (
 	"context"
 	"errors"
+	"os"
 	"path/filepath"
 	"slices"
 	"sync"
@@ -145,7 +146,16 @@ func (s *permissionService) Request(opts CreatePermissionRequest) bool {
 		return true
 	}
 
-	dir := filepath.Dir(opts.Path)
+	fileInfo, err := os.Stat(opts.Path)
+	dir := opts.Path
+	if err == nil {
+		if fileInfo.IsDir() {
+			dir = opts.Path
+		} else {
+			dir = filepath.Dir(opts.Path)
+		}
+	}
+
 	if dir == "." {
 		dir = s.workingDir
 	}

internal/tui/components/dialogs/permissions/permissions.go 🔗

@@ -321,6 +321,34 @@ func (p *permissionDialogCmp) renderHeader() string {
 		)
 	case tools.FetchToolName:
 		headerParts = append(headerParts, t.S().Muted.Width(p.width).Bold(true).Render("URL"))
+	case tools.ViewToolName:
+		params := p.permission.Params.(tools.ViewPermissionsParams)
+		fileKey := t.S().Muted.Render("File")
+		filePath := t.S().Text.
+			Width(p.width - lipgloss.Width(fileKey)).
+			Render(fmt.Sprintf(" %s", fsext.PrettyPath(params.FilePath)))
+		headerParts = append(headerParts,
+			lipgloss.JoinHorizontal(
+				lipgloss.Left,
+				fileKey,
+				filePath,
+			),
+			baseStyle.Render(strings.Repeat(" ", p.width)),
+		)
+	case tools.LSToolName:
+		params := p.permission.Params.(tools.LSPermissionsParams)
+		pathKey := t.S().Muted.Render("Directory")
+		pathValue := t.S().Text.
+			Width(p.width - lipgloss.Width(pathKey)).
+			Render(fmt.Sprintf(" %s", fsext.PrettyPath(params.Path)))
+		headerParts = append(headerParts,
+			lipgloss.JoinHorizontal(
+				lipgloss.Left,
+				pathKey,
+				pathValue,
+			),
+			baseStyle.Render(strings.Repeat(" ", p.width)),
+		)
 	}
 
 	return baseStyle.Render(lipgloss.JoinVertical(lipgloss.Left, headerParts...))
@@ -347,6 +375,10 @@ func (p *permissionDialogCmp) getOrGenerateContent() string {
 		content = p.generateMultiEditContent()
 	case tools.FetchToolName:
 		content = p.generateFetchContent()
+	case tools.ViewToolName:
+		content = p.generateViewContent()
+	case tools.LSToolName:
+		content = p.generateLSContent()
 	default:
 		content = p.generateDefaultContent()
 	}
@@ -486,6 +518,45 @@ func (p *permissionDialogCmp) generateFetchContent() string {
 	return ""
 }
 
+func (p *permissionDialogCmp) generateViewContent() string {
+	t := styles.CurrentTheme()
+	baseStyle := t.S().Base.Background(t.BgSubtle)
+	if pr, ok := p.permission.Params.(tools.ViewPermissionsParams); ok {
+		content := fmt.Sprintf("File: %s", fsext.PrettyPath(pr.FilePath))
+		if pr.Offset > 0 {
+			content += fmt.Sprintf("\nStarting from line: %d", pr.Offset+1)
+		}
+		if pr.Limit > 0 && pr.Limit != 2000 { // 2000 is the default limit
+			content += fmt.Sprintf("\nLines to read: %d", pr.Limit)
+		}
+
+		finalContent := baseStyle.
+			Padding(1, 2).
+			Width(p.contentViewPort.Width()).
+			Render(content)
+		return finalContent
+	}
+	return ""
+}
+
+func (p *permissionDialogCmp) generateLSContent() string {
+	t := styles.CurrentTheme()
+	baseStyle := t.S().Base.Background(t.BgSubtle)
+	if pr, ok := p.permission.Params.(tools.LSPermissionsParams); ok {
+		content := fmt.Sprintf("Directory: %s", fsext.PrettyPath(pr.Path))
+		if len(pr.Ignore) > 0 {
+			content += fmt.Sprintf("\nIgnore patterns: %s", strings.Join(pr.Ignore, ", "))
+		}
+
+		finalContent := baseStyle.
+			Padding(1, 2).
+			Width(p.contentViewPort.Width()).
+			Render(content)
+		return finalContent
+	}
+	return ""
+}
+
 func (p *permissionDialogCmp) generateDefaultContent() string {
 	t := styles.CurrentTheme()
 	baseStyle := t.S().Base.Background(t.BgSubtle)
@@ -623,6 +694,12 @@ func (p *permissionDialogCmp) SetSize() tea.Cmd {
 	case tools.FetchToolName:
 		p.width = int(float64(p.wWidth) * 0.8)
 		p.height = int(float64(p.wHeight) * 0.3)
+	case tools.ViewToolName:
+		p.width = int(float64(p.wWidth) * 0.8)
+		p.height = int(float64(p.wHeight) * 0.4)
+	case tools.LSToolName:
+		p.width = int(float64(p.wWidth) * 0.8)
+		p.height = int(float64(p.wHeight) * 0.4)
 	default:
 		p.width = int(float64(p.wWidth) * 0.7)
 		p.height = int(float64(p.wHeight) * 0.5)