on main help sign in
Merge pull request #229 from charmbracelet/prevent-binary-data

Prevent binary data in the fetch/view tool. Add a download tool

commit89bdfbf79bbcdc98cdd249c6c75ca424faaf2977 authorKujtim Hoxha <kujtimii.h@gmail.com> dateSat 19 Jul 2025, 12:54 (1 year ago) parent030f899eedd505
6 files changed+339−10
internal/llm/agent/agent.go+1 internal/llm/tools/download.go+223 internal/llm/tools/fetch.go+33−9 internal/llm/tools/view.go+6−1 internal/tui/components/chat/messages/renderer.go+29 internal/tui/components/dialogs/permissions/permissions.go+47
internal/llm/agent/agent.go+1
@@ -102,6 +102,7 @@ func NewAgent(
102102 cwd := cfg.WorkingDir()
103103 allTools := []tools.BaseTool{
104104 tools.NewBashTool(permissions, cwd),
105+ tools.NewDownloadTool(permissions, cwd),
105106 tools.NewEditTool(lspClients, permissions, history, cwd),
106107 tools.NewFetchTool(permissions, cwd),
107108 tools.NewGlobTool(cwd),
internal/llm/tools/download.gonew file+223
@@ -0,0 +1,223 @@
1+package tools
2+
3+import (
4+ "context"
5+ "encoding/json"
6+ "fmt"
7+ "io"
8+ "net/http"
9+ "os"
10+ "path/filepath"
11+ "strings"
12+ "time"
13+
14+ "github.com/charmbracelet/crush/internal/permission"
15+)
16+
17+type DownloadParams struct {
18+ URL string `json:"url"`
19+ FilePath string `json:"file_path"`
20+ Timeout int `json:"timeout,omitempty"`
21+}
22+
23+type DownloadPermissionsParams struct {
24+ URL string `json:"url"`
25+ FilePath string `json:"file_path"`
26+ Timeout int `json:"timeout,omitempty"`
27+}
28+
29+type downloadTool struct {
30+ client *http.Client
31+ permissions permission.Service
32+ workingDir string
33+}
34+
35+const (
36+ DownloadToolName = "download"
37+ downloadToolDescription = `Downloads binary data from a URL and saves it to a local file.
38+
39+WHEN TO USE THIS TOOL:
40+- Use when you need to download files, images, or other binary data from URLs
41+- Helpful for downloading assets, documents, or any file type
42+- Useful for saving remote content locally for processing or storage
43+
44+HOW TO USE:
45+- Provide the URL to download from
46+- Specify the local file path where the content should be saved
47+- Optionally set a timeout for the request
48+
49+FEATURES:
50+- Downloads any file type (binary or text)
51+- Automatically creates parent directories if they don't exist
52+- Handles large files efficiently with streaming
53+- Sets reasonable timeouts to prevent hanging
54+- Validates input parameters before making requests
55+
56+LIMITATIONS:
57+- Maximum file size is 100MB
58+- Only supports HTTP and HTTPS protocols
59+- Cannot handle authentication or cookies
60+- Some websites may block automated requests
61+- Will overwrite existing files without warning
62+
63+TIPS:
64+- Use absolute paths or paths relative to the working directory
65+- Set appropriate timeouts for large files or slow connections`
66+)
67+
68+func NewDownloadTool(permissions permission.Service, workingDir string) BaseTool {
69+ return &downloadTool{
70+ client: &http.Client{
71+ Timeout: 5 * time.Minute, // Default 5 minute timeout for downloads
72+ Transport: &http.Transport{
73+ MaxIdleConns: 100,
74+ MaxIdleConnsPerHost: 10,
75+ IdleConnTimeout: 90 * time.Second,
76+ },
77+ },
78+ permissions: permissions,
79+ workingDir: workingDir,
80+ }
81+}
82+
83+func (t *downloadTool) Name() string {
84+ return DownloadToolName
85+}
86+
87+func (t *downloadTool) Info() ToolInfo {
88+ return ToolInfo{
89+ Name: DownloadToolName,
90+ Description: downloadToolDescription,
91+ Parameters: map[string]any{
92+ "url": map[string]any{
93+ "type": "string",
94+ "description": "The URL to download from",
95+ },
96+ "file_path": map[string]any{
97+ "type": "string",
98+ "description": "The local file path where the downloaded content should be saved",
99+ },
100+ "timeout": map[string]any{
101+ "type": "number",
102+ "description": "Optional timeout in seconds (max 600)",
103+ },
104+ },
105+ Required: []string{"url", "file_path"},
106+ }
107+}
108+
109+func (t *downloadTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error) {
110+ var params DownloadParams
111+ if err := json.Unmarshal([]byte(call.Input), &params); err != nil {
112+ return NewTextErrorResponse("Failed to parse download parameters: " + err.Error()), nil
113+ }
114+
115+ if params.URL == "" {
116+ return NewTextErrorResponse("URL parameter is required"), nil
117+ }
118+
119+ if params.FilePath == "" {
120+ return NewTextErrorResponse("file_path parameter is required"), nil
121+ }
122+
123+ if !strings.HasPrefix(params.URL, "http://") && !strings.HasPrefix(params.URL, "https://") {
124+ return NewTextErrorResponse("URL must start with http:// or https://"), nil
125+ }
126+
127+ // Convert relative path to absolute path
128+ var filePath string
129+ if filepath.IsAbs(params.FilePath) {
130+ filePath = params.FilePath
131+ } else {
132+ filePath = filepath.Join(t.workingDir, params.FilePath)
133+ }
134+
135+ sessionID, messageID := GetContextValues(ctx)
136+ if sessionID == "" || messageID == "" {
137+ return ToolResponse{}, fmt.Errorf("session ID and message ID are required for downloading files")
138+ }
139+
140+ p := t.permissions.Request(
141+ permission.CreatePermissionRequest{
142+ SessionID: sessionID,
143+ Path: filePath,
144+ ToolName: DownloadToolName,
145+ Action: "download",
146+ Description: fmt.Sprintf("Download file from URL: %s to %s", params.URL, filePath),
147+ Params: DownloadPermissionsParams(params),
148+ },
149+ )
150+
151+ if !p {
152+ return ToolResponse{}, permission.ErrorPermissionDenied
153+ }
154+
155+ // Handle timeout with context
156+ requestCtx := ctx
157+ if params.Timeout > 0 {
158+ maxTimeout := 600 // 10 minutes
159+ if params.Timeout > maxTimeout {
160+ params.Timeout = maxTimeout
161+ }
162+ var cancel context.CancelFunc
163+ requestCtx, cancel = context.WithTimeout(ctx, time.Duration(params.Timeout)*time.Second)
164+ defer cancel()
165+ }
166+
167+ req, err := http.NewRequestWithContext(requestCtx, "GET", params.URL, nil)
168+ if err != nil {
169+ return ToolResponse{}, fmt.Errorf("failed to create request: %w", err)
170+ }
171+
172+ req.Header.Set("User-Agent", "crush/1.0")
173+
174+ resp, err := t.client.Do(req)
175+ if err != nil {
176+ return ToolResponse{}, fmt.Errorf("failed to download from URL: %w", err)
177+ }
178+ defer resp.Body.Close()
179+
180+ if resp.StatusCode != http.StatusOK {
181+ return NewTextErrorResponse(fmt.Sprintf("Request failed with status code: %d", resp.StatusCode)), nil
182+ }
183+
184+ // Check content length if available
185+ maxSize := int64(100 * 1024 * 1024) // 100MB
186+ if resp.ContentLength > maxSize {
187+ return NewTextErrorResponse(fmt.Sprintf("File too large: %d bytes (max %d bytes)", resp.ContentLength, maxSize)), nil
188+ }
189+
190+ // Create parent directories if they don't exist
191+ if err := os.MkdirAll(filepath.Dir(filePath), 0o755); err != nil {
192+ return ToolResponse{}, fmt.Errorf("failed to create parent directories: %w", err)
193+ }
194+
195+ // Create the output file
196+ outFile, err := os.Create(filePath)
197+ if err != nil {
198+ return ToolResponse{}, fmt.Errorf("failed to create output file: %w", err)
199+ }
200+ defer outFile.Close()
201+
202+ // Copy data with size limit
203+ limitedReader := io.LimitReader(resp.Body, maxSize)
204+ bytesWritten, err := io.Copy(outFile, limitedReader)
205+ if err != nil {
206+ return ToolResponse{}, fmt.Errorf("failed to write file: %w", err)
207+ }
208+
209+ // Check if we hit the size limit
210+ if bytesWritten == maxSize {
211+ // Clean up the file since it might be incomplete
212+ os.Remove(filePath)
213+ return NewTextErrorResponse(fmt.Sprintf("File too large: exceeded %d bytes limit", maxSize)), nil
214+ }
215+
216+ contentType := resp.Header.Get("Content-Type")
217+ responseMsg := fmt.Sprintf("Successfully downloaded %d bytes to %s", bytesWritten, filePath)
218+ if contentType != "" {
219+ responseMsg += fmt.Sprintf(" (Content-Type: %s)", contentType)
220+ }
221+
222+ return NewTextResponse(responseMsg), nil
223+}
internal/llm/tools/fetch.go+33−9
@@ -8,6 +8,7 @@ import (
88 "net/http"
99 "strings"
1010 "time"
11+ "unicode/utf8"
1112
1213 md "github.com/JohannesKaufmann/html-to-markdown"
1314 "github.com/PuerkitoBio/goquery"
@@ -182,6 +183,11 @@ func (t *fetchTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error
182183 }
183184
184185 content := string(body)
186+
187+ isValidUt8 := utf8.ValidString(content)
188+ if !isValidUt8 {
189+ return NewTextErrorResponse("Response content is not valid UTF-8"), nil
190+ }
185191 contentType := resp.Header.Get("Content-Type")
186192
187193 switch format {
@@ -191,9 +197,8 @@ func (t *fetchTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error
191197 if err != nil {
192198 return NewTextErrorResponse("Failed to extract text from HTML: " + err.Error()), nil
193199 }
194− return NewTextResponse(text), nil
200+ content = text
195201 }
196− return NewTextResponse(content), nil
197202
198203 case "markdown":
199204 if strings.Contains(contentType, "text/html") {
@@ -201,17 +206,36 @@ func (t *fetchTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error
201206 if err != nil {
202207 return NewTextErrorResponse("Failed to convert HTML to Markdown: " + err.Error()), nil
203208 }
204− return NewTextResponse(markdown), nil
209+ content = markdown
205210 }
206211
207− return NewTextResponse("```\n" + content + "\n```"), nil
212+ content = "```\n" + content + "\n```"
208213
209214 case "html":
210− return NewTextResponse(content), nil
211−
212− default:
213− return NewTextResponse(content), nil
215+ // return only the body of the HTML document
216+ if strings.Contains(contentType, "text/html") {
217+ doc, err := goquery.NewDocumentFromReader(strings.NewReader(content))
218+ if err != nil {
219+ return NewTextErrorResponse("Failed to parse HTML: " + err.Error()), nil
220+ }
221+ body, err := doc.Find("body").Html()
222+ if err != nil {
223+ return NewTextErrorResponse("Failed to extract body from HTML: " + err.Error()), nil
224+ }
225+ if body == "" {
226+ return NewTextErrorResponse("No body content found in HTML"), nil
227+ }
228+ content = "<html>\n<body>\n" + body + "\n</body>\n</html>"
229+ }
230+ }
231+ // calculate byte size of content
232+ contentSize := int64(len(content))
233+ if contentSize > MaxReadSize {
234+ content = content[:MaxReadSize]
235+ content += fmt.Sprintf("\n\n[Content truncated to %d bytes]", MaxReadSize)
214236 }
237+
238+ return NewTextResponse(content), nil
215239}
216240
217241func extractTextFromHTML(html string) (string, error) {
@@ -220,7 +244,7 @@ func extractTextFromHTML(html string) (string, error) {
220244 return "", err
221245 }
222246
223− text := doc.Text()
247+ text := doc.Find("body").Text()
224248 text = strings.Join(strings.Fields(text), " ")
225249
226250 return text, nil
internal/llm/tools/view.go+6−1
@@ -9,6 +9,7 @@ import (
99 "os"
1010 "path/filepath"
1111 "strings"
12+ "unicode/utf8"
1213
1314 "github.com/charmbracelet/crush/internal/lsp"
1415)
@@ -173,11 +174,15 @@ func (v *viewTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error)
173174 isImage, imageType := isImageFile(filePath)
174175 // TODO: handle images
175176 if isImage {
176− return NewTextErrorResponse(fmt.Sprintf("This is an image file of type: %s\nUse a different tool to process images", imageType)), nil
177+ return NewTextErrorResponse(fmt.Sprintf("This is an image file of type: %s\n", imageType)), nil
177178 }
178179
179180 // Read the file content
180181 content, lineCount, err := readTextFile(filePath, params.Offset, params.Limit)
182+ isValidUt8 := utf8.ValidString(content)
183+ if !isValidUt8 {
184+ return NewTextErrorResponse("File content is not valid UTF-8"), nil
185+ }
181186 if err != nil {
182187 return ToolResponse{}, fmt.Errorf("error reading file: %w", err)
183188 }
internal/tui/components/chat/messages/renderer.go+29
@@ -162,6 +162,7 @@ func (br baseRenderer) renderError(v *toolCallCmp, message string) string {
162162// Register tool renderers
163163func init() {
164164 registry.register(tools.BashToolName, func() renderer { return bashRenderer{} })
165+ registry.register(tools.DownloadToolName, func() renderer { return downloadRenderer{} })
165166 registry.register(tools.ViewToolName, func() renderer { return viewRenderer{} })
166167 registry.register(tools.EditToolName, func() renderer { return editRenderer{} })
167168 registry.register(tools.WriteToolName, func() renderer { return writeRenderer{} })
@@ -376,6 +377,32 @@ func formatTimeout(timeout int) string {
376377 return (time.Duration(timeout) * time.Second).String()
377378}
378379
380+// -----------------------------------------------------------------------------
381+// Download renderer
382+// -----------------------------------------------------------------------------
383+
384+// downloadRenderer handles file downloading with URL and file path display
385+type downloadRenderer struct {
386+ baseRenderer
387+}
388+
389+// Render displays the download URL and destination file path with timeout parameter
390+func (dr downloadRenderer) Render(v *toolCallCmp) string {
391+ var params tools.DownloadParams
392+ var args []string
393+ if err := dr.unmarshalParams(v.call.Input, &params); err == nil {
394+ args = newParamBuilder().
395+ addMain(params.URL).
396+ addKeyValue("file_path", fsext.PrettyPath(params.FilePath)).
397+ addKeyValue("timeout", formatTimeout(params.Timeout)).
398+ build()
399+ }
400+
401+ return dr.renderWithParams(v, "Download", args, func() string {
402+ return renderPlainContent(v, v.result.Content)
403+ })
404+}
405+
379406// -----------------------------------------------------------------------------
380407// Glob renderer
381408// -----------------------------------------------------------------------------
@@ -758,6 +785,8 @@ func prettifyToolName(name string) string {
758785 return "Agent"
759786 case tools.BashToolName:
760787 return "Bash"
788+ case tools.DownloadToolName:
789+ return "Download"
761790 case tools.EditToolName:
762791 return "Edit"
763792 case tools.FetchToolName:
internal/tui/components/dialogs/permissions/permissions.go+47
@@ -252,6 +252,30 @@ func (p *permissionDialogCmp) renderHeader() string {
252252 switch p.permission.ToolName {
253253 case tools.BashToolName:
254254 headerParts = append(headerParts, t.S().Muted.Width(p.width).Render("Command"))
255+ case tools.DownloadToolName:
256+ params := p.permission.Params.(tools.DownloadPermissionsParams)
257+ urlKey := t.S().Muted.Render("URL")
258+ urlValue := t.S().Text.
259+ Width(p.width - lipgloss.Width(urlKey)).
260+ Render(fmt.Sprintf(" %s", params.URL))
261+ fileKey := t.S().Muted.Render("File")
262+ filePath := t.S().Text.
263+ Width(p.width - lipgloss.Width(fileKey)).
264+ Render(fmt.Sprintf(" %s", fsext.PrettyPath(params.FilePath)))
265+ headerParts = append(headerParts,
266+ lipgloss.JoinHorizontal(
267+ lipgloss.Left,
268+ urlKey,
269+ urlValue,
270+ ),
271+ baseStyle.Render(strings.Repeat(" ", p.width)),
272+ lipgloss.JoinHorizontal(
273+ lipgloss.Left,
274+ fileKey,
275+ filePath,
276+ ),
277+ baseStyle.Render(strings.Repeat(" ", p.width)),
278+ )
255279 case tools.EditToolName:
256280 params := p.permission.Params.(tools.EditPermissionsParams)
257281 fileKey := t.S().Muted.Render("File")
@@ -299,6 +323,8 @@ func (p *permissionDialogCmp) getOrGenerateContent() string {
299323 switch p.permission.ToolName {
300324 case tools.BashToolName:
301325 content = p.generateBashContent()
326+ case tools.DownloadToolName:
327+ content = p.generateDownloadContent()
302328 case tools.EditToolName:
303329 content = p.generateEditContent()
304330 case tools.WriteToolName:
@@ -391,6 +417,24 @@ func (p *permissionDialogCmp) generateWriteContent() string {
391417 return ""
392418}
393419
420+func (p *permissionDialogCmp) generateDownloadContent() string {
421+ t := styles.CurrentTheme()
422+ baseStyle := t.S().Base.Background(t.BgSubtle)
423+ if pr, ok := p.permission.Params.(tools.DownloadPermissionsParams); ok {
424+ content := fmt.Sprintf("URL: %s\nFile: %s", pr.URL, fsext.PrettyPath(pr.FilePath))
425+ if pr.Timeout > 0 {
426+ content += fmt.Sprintf("\nTimeout: %ds", pr.Timeout)
427+ }
428+
429+ finalContent := baseStyle.
430+ Padding(1, 2).
431+ Width(p.contentViewPort.Width()).
432+ Render(content)
433+ return finalContent
434+ }
435+ return ""
436+}
437+
394438func (p *permissionDialogCmp) generateFetchContent() string {
395439 t := styles.CurrentTheme()
396440 baseStyle := t.S().Base.Background(t.BgSubtle)
@@ -526,6 +570,9 @@ func (p *permissionDialogCmp) SetSize() tea.Cmd {
526570 case tools.BashToolName:
527571 p.width = int(float64(p.wWidth) * 0.8)
528572 p.height = int(float64(p.wHeight) * 0.3)
573+ case tools.DownloadToolName:
574+ p.width = int(float64(p.wWidth) * 0.8)
575+ p.height = int(float64(p.wHeight) * 0.4)
529576 case tools.EditToolName:
530577 p.width = int(float64(p.wWidth) * 0.8)
531578 p.height = int(float64(p.wHeight) * 0.8)