From 0fcad3ba027d26bbe602d5281223164be6f4ba88 Mon Sep 17 00:00:00 2001 From: Kujtim Hoxha Date: Tue, 29 Jul 2025 15:13:51 +0200 Subject: [PATCH 1/3] chore: improve permissions --- 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 ++- .../dialogs/permissions/permissions.go | 77 ++++++++++++++++++ 9 files changed, 219 insertions(+), 61 deletions(-) diff --git a/internal/llm/agent/agent.go b/internal/llm/agent/agent.go index 17a67f810b335f1dad105321a0bb0a8b354c9bfc..b7c7464cb78e9cf3b2c094b0846d404fafc269d0 100644 --- a/internal/llm/agent/agent.go +++ b/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), } diff --git a/internal/llm/prompt/coder.go b/internal/llm/prompt/coder.go index ed879754c7c8c78debda98fb6b89c33d75fcab24..641405db1b2356a414f802cc7ab9edb14858acef 100644 --- a/internal/llm/prompt/coder.go +++ b/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: Working directory: %s @@ -52,7 +48,7 @@ Today's date: %s %s - `, cwd, boolToYesNo(isGit), platform, date, r.Content) + `, cwd, boolToYesNo(isGit), platform, date, output) } func isGitRepo(dir string) bool { diff --git a/internal/llm/tools/edit.go b/internal/llm/tools/edit.go index 77821b7119bcd3756bb531a031b0d99307361718..6011ea3846bb9bfd6de7d6a4dbc2ad2518b9c12b 100644 --- a/internal/llm/tools/edit.go +++ b/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", diff --git a/internal/llm/tools/ls.go b/internal/llm/tools/ls.go index 6526a57274c0ef6c169f17979d277a3031fe1f72..2a14c7b667a41ece209c11149254fa1a3da03e3e 100644 --- a/internal/llm/tools/ls.go +++ b/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) diff --git a/internal/llm/tools/multiedit.go b/internal/llm/tools/multiedit.go index 2038140e7a6bc33741772eb315a5cf69258b7c1e..61559138aff61749532bd638fe7b7235c191ccb1 100644 --- a/internal/llm/tools/multiedit.go +++ b/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", diff --git a/internal/llm/tools/view.go b/internal/llm/tools/view.go index d8ca7e9e8c7880a760e8eb2096c83914a9dc13b5..ae8681f63206696c942ab96e8a4df5870831058a 100644 --- a/internal/llm/tools/view.go +++ b/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 { diff --git a/internal/llm/tools/write.go b/internal/llm/tools/write.go index 7d8d6f567955ae69f35bb4ac38d1d8331dd375a3..256c936313af19f1477b7f09a4eed007412bd4bc 100644 --- a/internal/llm/tools/write.go +++ b/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", diff --git a/internal/permission/permission.go b/internal/permission/permission.go index 476f33598feea326c42630b1ad54e012fc867bf4..7e71eb1caa861d4d702d9f028560e0dd4825abb7 100644 --- a/internal/permission/permission.go +++ b/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 } diff --git a/internal/tui/components/dialogs/permissions/permissions.go b/internal/tui/components/dialogs/permissions/permissions.go index 2e7a04dc7416baf6fdfec90ab56d61f60dad81f1..720dc807ae9da9ed6951c3133043ecbbac14d8e7 100644 --- a/internal/tui/components/dialogs/permissions/permissions.go +++ b/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) From 4eaa13a7fb48425f11c8335f07244886b4cf6665 Mon Sep 17 00:00:00 2001 From: Kujtim Hoxha Date: Tue, 29 Jul 2025 16:05:24 +0200 Subject: [PATCH 2/3] chore: fix homedir --- internal/fsext/ls.go | 2 +- internal/llm/tools/ls.go | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/fsext/ls.go b/internal/fsext/ls.go index a9ff2a6645d6531fc0037b1fe1a87563abdc3a85..2c08fd1c294cd35e8dfc436e8272004b68098b68 100644 --- a/internal/fsext/ls.go +++ b/internal/fsext/ls.go @@ -156,7 +156,7 @@ func ListDirectory(initialPath string, ignorePatterns []string, limit int) ([]st return nil }) - if err != nil { + if err != nil && len(results) == 0 { return nil, truncated, err } diff --git a/internal/llm/tools/ls.go b/internal/llm/tools/ls.go index 2a14c7b667a41ece209c11149254fa1a3da03e3e..559758c05c97e6540b8c63053a58ba2ee682d9a4 100644 --- a/internal/llm/tools/ls.go +++ b/internal/llm/tools/ls.go @@ -121,6 +121,14 @@ func (l *lsTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error) { searchPath = l.workingDir } + if searchPath == "~" { + homeDir, err := os.UserHomeDir() + if err != nil { + return ToolResponse{}, fmt.Errorf("error resolving home directory: %w", err) + } + searchPath = homeDir + } + if !filepath.IsAbs(searchPath) { searchPath = filepath.Join(l.workingDir, searchPath) } From 0c1b71c9f0c73042d5987318e284796faf13fb85 Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Tue, 29 Jul 2025 11:24:54 -0300 Subject: [PATCH 3/3] fix(ls): fix path expand on ls tool --- internal/fsext/expand.go | 29 +++++++++++++++++++++++++++++ internal/llm/tools/ls.go | 10 ++++------ 2 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 internal/fsext/expand.go diff --git a/internal/fsext/expand.go b/internal/fsext/expand.go new file mode 100644 index 0000000000000000000000000000000000000000..d4265ca6381f43056032ffdf35ab61f137d1752f --- /dev/null +++ b/internal/fsext/expand.go @@ -0,0 +1,29 @@ +package fsext + +import ( + "os" + "strings" + + "mvdan.cc/sh/v3/expand" + "mvdan.cc/sh/v3/syntax" +) + +// Expand is a wrapper around [expand.Literal]. It will escape the input +// string, expand any shell symbols (such as '~') and resolve any environment +// variables. +func Expand(s string) (string, error) { + if s == "" { + return "", nil + } + p := syntax.NewParser() + word, err := p.Document(strings.NewReader(s)) + if err != nil { + return "", err + } + cfg := &expand.Config{ + Env: expand.FuncEnviron(os.Getenv), + ReadDir2: os.ReadDir, + GlobStar: true, + } + return expand.Literal(cfg, word) +} diff --git a/internal/llm/tools/ls.go b/internal/llm/tools/ls.go index 559758c05c97e6540b8c63053a58ba2ee682d9a4..9dfce8ef83570414f6f61078bead34804f83a2cc 100644 --- a/internal/llm/tools/ls.go +++ b/internal/llm/tools/ls.go @@ -121,12 +121,10 @@ func (l *lsTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error) { searchPath = l.workingDir } - if searchPath == "~" { - homeDir, err := os.UserHomeDir() - if err != nil { - return ToolResponse{}, fmt.Errorf("error resolving home directory: %w", err) - } - searchPath = homeDir + var err error + searchPath, err = fsext.Expand(searchPath) + if err != nil { + return ToolResponse{}, fmt.Errorf("error expanding path: %w", err) } if !filepath.IsAbs(searchPath) {