From a53f3e092f39d2c3d1f064e4bc0c216048619d4e Mon Sep 17 00:00:00 2001 From: viettranx Date: Sun, 8 Mar 2026 00:41:38 +0700 Subject: [PATCH] fix(media): use absolute paths and relative URLs for WS media delivery - FilesHandler now serves files by absolute path (auth-token protected) instead of requiring a workspace root, supporting multiple agent workspaces - mediaToMarkdown generates relative URLs (/v1/files/...) instead of absolute http://host:port URLs so images work from any client origin - Deduplicate media collection in agent loop: prefer result.Media over MEDIA: prefix parsing to prevent duplicate images --- cmd/gateway.go | 5 +++-- cmd/gateway_consumer.go | 30 ++++++++++++------------------ internal/agent/loop.go | 24 ++++++++++++++---------- internal/http/files.go | 38 +++++++++++++++----------------------- 4 files changed, 44 insertions(+), 53 deletions(-) diff --git a/cmd/gateway.go b/cmd/gateway.go index 9b9d51df..e25b74d3 100644 --- a/cmd/gateway.go +++ b/cmd/gateway.go @@ -626,8 +626,9 @@ func runGateway() { server.SetBuiltinToolsHandler(builtinToolsH) } - // Workspace file serving endpoint - server.SetFilesHandler(httpapi.NewFilesHandler(workspace, cfg.Gateway.Token)) + // Workspace file serving endpoint — serves files by absolute path, auth-token protected. + // Supports media from any agent workspace (each agent has its own workspace from DB). + server.SetFilesHandler(httpapi.NewFilesHandler(cfg.Gateway.Token)) // Seed + apply builtin tool disables if pgStores.BuiltinTools != nil { diff --git a/cmd/gateway_consumer.go b/cmd/gateway_consumer.go index b64161b3..5ce7b512 100644 --- a/cmd/gateway_consumer.go +++ b/cmd/gateway_consumer.go @@ -431,7 +431,7 @@ func consumeInboundMessages(ctx context.Context, msgBus *bus.MessageBus, agents fwdMedia := msg.Media contentSuffix := "" if origChannel == "ws" && len(msg.Media) > 0 { - contentSuffix = mediaToMarkdownFromPaths(msg.Media, cfg.WorkspacePath(), cfg) + contentSuffix = mediaToMarkdownFromPaths(msg.Media, cfg) fwdMedia = nil // WS: images delivered via ContentSuffix, not ForwardMedia } @@ -565,7 +565,7 @@ func consumeInboundMessages(ctx context.Context, msgBus *bus.MessageBus, agents fwdMedia := msg.Media contentSuffix := "" if origChannel == "ws" && len(msg.Media) > 0 { - contentSuffix = mediaToMarkdownFromPaths(msg.Media, cfg.WorkspacePath(), cfg) + contentSuffix = mediaToMarkdownFromPaths(msg.Media, cfg) fwdMedia = nil // WS: images delivered via ContentSuffix, not ForwardMedia } @@ -897,34 +897,28 @@ func overrideSessionKeyFromLocalKey(sessionKey, localKey, agentID, channel, chat // mediaToMarkdown converts media results to markdown image/link syntax using the // /v1/files/ HTTP endpoint. Used for WS channel where outbound media attachments // are not supported (no channel handler). Returns empty string if no media. -func mediaToMarkdown(media []agent.MediaResult, workspace string, cfg *config.Config) string { +// Uses absolute file paths with the /v1/files endpoint (auth-token protected). +// Generates relative URLs (/v1/files/...) so they work regardless of the server's +// external hostname — the browser resolves them from the current origin. +func mediaToMarkdown(media []agent.MediaResult, cfg *config.Config) string { if len(media) == 0 { return "" } - scheme := "http" - if cfg.Tailscale.EnableTLS { - scheme = "https" - } - host := cfg.Gateway.Host - if host == "" || host == "0.0.0.0" { - host = "localhost" - } - baseURL := fmt.Sprintf("%s://%s:%d/v1/files", scheme, host, cfg.Gateway.Port) tokenQuery := "" if cfg.Gateway.Token != "" { tokenQuery = "?token=" + cfg.Gateway.Token } - cleanRoot := filepath.Clean(workspace) var parts []string for _, mr := range media { cleanPath := filepath.Clean(mr.Path) - relPath, err := filepath.Rel(cleanRoot, cleanPath) - if err != nil || strings.HasPrefix(relPath, "..") { + // Strip leading "/" so URL path is /v1/files/app/.goclaw/... + urlPath := strings.TrimPrefix(cleanPath, "/") + if urlPath == "" { continue } - fileURL := baseURL + "/" + relPath + tokenQuery + fileURL := "/v1/files/" + urlPath + tokenQuery if strings.HasPrefix(mr.ContentType, "image/") { parts = append(parts, fmt.Sprintf("![image](%s)", fileURL)) } else { @@ -939,7 +933,7 @@ func mediaToMarkdown(media []agent.MediaResult, workspace string, cfg *config.Co // mediaToMarkdownFromPaths is like mediaToMarkdown but accepts raw file paths // ([]string from bus.InboundMessage.Media) instead of []agent.MediaResult. -func mediaToMarkdownFromPaths(paths []string, workspace string, cfg *config.Config) string { +func mediaToMarkdownFromPaths(paths []string, cfg *config.Config) string { if len(paths) == 0 { return "" } @@ -954,7 +948,7 @@ func mediaToMarkdownFromPaths(paths []string, workspace string, cfg *config.Conf ContentType: ct, }) } - return mediaToMarkdown(media, workspace, cfg) + return mediaToMarkdown(media, cfg) } func buildAnnounceOutMeta(localKey string) map[string]string { diff --git a/internal/agent/loop.go b/internal/agent/loop.go index f8436dc5..1acfac35 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -687,13 +687,15 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error) l.scanWebToolResult(tc.Name, result) - // Collect MEDIA: paths from tool results - if mr := parseMediaResult(result.ForLLM); mr != nil { + // Collect MEDIA: paths from tool results. + // Prefer result.Media (explicit) over ForLLM MEDIA: prefix (legacy) to avoid duplicates. + if len(result.Media) > 0 { + for _, p := range result.Media { + mediaResults = append(mediaResults, MediaResult{Path: p, ContentType: mimeFromExt(filepath.Ext(p))}) + } + } else if mr := parseMediaResult(result.ForLLM); mr != nil { mediaResults = append(mediaResults, *mr) } - for _, p := range result.Media { - mediaResults = append(mediaResults, MediaResult{Path: p, ContentType: mimeFromExt(filepath.Ext(p))}) - } if result.Deliverable != "" { deliverables = append(deliverables, result.Deliverable) } @@ -820,13 +822,15 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error) l.scanWebToolResult(r.tc.Name, r.result) - // Collect MEDIA: paths from tool results - if mr := parseMediaResult(r.result.ForLLM); mr != nil { + // Collect MEDIA: paths from tool results. + // Prefer result.Media (explicit) over ForLLM MEDIA: prefix (legacy) to avoid duplicates. + if len(r.result.Media) > 0 { + for _, p := range r.result.Media { + mediaResults = append(mediaResults, MediaResult{Path: p, ContentType: mimeFromExt(filepath.Ext(p))}) + } + } else if mr := parseMediaResult(r.result.ForLLM); mr != nil { mediaResults = append(mediaResults, *mr) } - for _, p := range r.result.Media { - mediaResults = append(mediaResults, MediaResult{Path: p, ContentType: mimeFromExt(filepath.Ext(p))}) - } if r.result.Deliverable != "" { deliverables = append(deliverables, r.result.Deliverable) } diff --git a/internal/http/files.go b/internal/http/files.go index 4f84b781..32ceadb4 100644 --- a/internal/http/files.go +++ b/internal/http/files.go @@ -9,15 +9,15 @@ import ( "strings" ) -// FilesHandler serves workspace files over HTTP with Bearer token auth. +// FilesHandler serves files over HTTP with Bearer token auth. +// Accepts absolute paths — the auth token protects against unauthorized access. type FilesHandler struct { - workspaceRoot string - token string + token string } -// NewFilesHandler creates a handler that serves files from the workspace root. -func NewFilesHandler(workspaceRoot, token string) *FilesHandler { - return &FilesHandler{workspaceRoot: workspaceRoot, token: token} +// NewFilesHandler creates a handler that serves files by absolute path. +func NewFilesHandler(token string) *FilesHandler { + return &FilesHandler{token: token} } // RegisterRoutes registers the file serving route. @@ -43,42 +43,34 @@ func (h *FilesHandler) auth(next http.HandlerFunc) http.HandlerFunc { } func (h *FilesHandler) handleServe(w http.ResponseWriter, r *http.Request) { - relPath := r.PathValue("path") - if relPath == "" { + urlPath := r.PathValue("path") + if urlPath == "" { http.Error(w, "path is required", http.StatusBadRequest) return } // Prevent path traversal - if strings.Contains(relPath, "..") { - slog.Warn("security.files_traversal", "path", relPath) + if strings.Contains(urlPath, "..") { + slog.Warn("security.files_traversal", "path", urlPath) http.Error(w, "invalid path", http.StatusBadRequest) return } - absPath := filepath.Join(h.workspaceRoot, relPath) + // URL path is the absolute path with leading "/" stripped (e.g. "app/.goclaw/workspace/file.png") + absPath := filepath.Clean("/" + urlPath) - // Double-check the resolved path is within the workspace - cleanAbs := filepath.Clean(absPath) - cleanRoot := filepath.Clean(h.workspaceRoot) - if !strings.HasPrefix(cleanAbs, cleanRoot+string(filepath.Separator)) && cleanAbs != cleanRoot { - slog.Warn("security.files_escape", "resolved", cleanAbs, "root", cleanRoot) - http.Error(w, "invalid path", http.StatusBadRequest) - return - } - - info, err := os.Stat(cleanAbs) + info, err := os.Stat(absPath) if err != nil || info.IsDir() { http.NotFound(w, r) return } // Set Content-Type from extension - ext := filepath.Ext(cleanAbs) + ext := filepath.Ext(absPath) ct := mime.TypeByExtension(ext) if ct != "" { w.Header().Set("Content-Type", ct) } - http.ServeFile(w, r, cleanAbs) + http.ServeFile(w, r, absPath) }