From 8b5a05a4b7072bf12eebd2a8b223e87280bb8487 Mon Sep 17 00:00:00 2001 From: Duc Nguyen Date: Sat, 28 Mar 2026 13:33:25 +0700 Subject: [PATCH] fix(web): sign media_refs paths in session history delivery (#519) (#520) File downloads in web chat returned 401 when loading session history because MediaRefs paths were not signed with ?ft= HMAC tokens. - Add SignMediaPath() to clean legacy corrupted paths (stacked /v1/files/ prefixes, stale ?ft= tokens) and produce fresh signed URLs - Sign MediaRefs in chat.history and sessions.get WS handlers - Strip ?ft= from filename display in frontend - Add path traversal defense-in-depth check - Add unit tests for SignMediaPath legacy data healing Closes #519 --- internal/gateway/methods/chat.go | 6 +- internal/gateway/methods/sessions.go | 8 +- internal/http/file_token.go | 28 ++++++ internal/http/file_token_test.go | 90 +++++++++++++++++++ .../src/pages/chat/hooks/use-chat-messages.ts | 4 +- 5 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 internal/http/file_token_test.go diff --git a/internal/gateway/methods/chat.go b/internal/gateway/methods/chat.go index a4c05688..26d63ec9 100644 --- a/internal/gateway/methods/chat.go +++ b/internal/gateway/methods/chat.go @@ -310,8 +310,12 @@ func (m *ChatMethods) handleHistory(ctx context.Context, client *gateway.Client, history := m.sessions.GetHistory(ctx, sessionKey) // Sign file URLs before delivery — sessions store clean paths. + secret := httpapi.FileSigningKey() for i := range history { - history[i].Content = httpapi.SignFileURLs(history[i].Content, httpapi.FileSigningKey()) + history[i].Content = httpapi.SignFileURLs(history[i].Content, secret) + for j := range history[i].MediaRefs { + history[i].MediaRefs[j].Path = httpapi.SignMediaPath(history[i].MediaRefs[j].Path, secret) + } } client.SendResponse(protocol.NewOKResponse(req.ID, map[string]any{ diff --git a/internal/gateway/methods/sessions.go b/internal/gateway/methods/sessions.go index 02124e43..2f66b286 100644 --- a/internal/gateway/methods/sessions.go +++ b/internal/gateway/methods/sessions.go @@ -99,10 +99,14 @@ func (m *SessionsMethods) handlePreview(ctx context.Context, client *gateway.Cli summary := m.sessions.GetSummary(ctx, params.Key) // Sign file URLs before delivery — sessions store clean paths. + secret := httpapi.FileSigningKey() for i := range history { - history[i].Content = httpapi.SignFileURLs(history[i].Content, httpapi.FileSigningKey()) + history[i].Content = httpapi.SignFileURLs(history[i].Content, secret) + for j := range history[i].MediaRefs { + history[i].MediaRefs[j].Path = httpapi.SignMediaPath(history[i].MediaRefs[j].Path, secret) + } } - summary = httpapi.SignFileURLs(summary, httpapi.FileSigningKey()) + summary = httpapi.SignFileURLs(summary, secret) client.SendResponse(protocol.NewOKResponse(req.ID, map[string]any{ "key": params.Key, diff --git a/internal/http/file_token.go b/internal/http/file_token.go index cca2a8ff..29c7e8ec 100644 --- a/internal/http/file_token.go +++ b/internal/http/file_token.go @@ -6,6 +6,7 @@ import ( "crypto/sha256" "encoding/base64" "fmt" + "path/filepath" "regexp" "strconv" "strings" @@ -64,6 +65,33 @@ func fileTokenHMAC(path, secret string, expiry int64) string { return base64.RawURLEncoding.EncodeToString(mac.Sum(nil)[:16]) } +// SignMediaPath converts a media ref path to a signed /v1/files/ URL. +// Handles legacy data where paths may already contain /v1/files/ prefixes +// and stale ?ft= tokens from prior signing bugs. +func SignMediaPath(rawPath, secret string) string { + if rawPath == "" { + return "" + } + // Defense-in-depth: reject path traversal (also blocked by handleServe) + if strings.Contains(rawPath, "..") { + return "" + } + // Strip stale ?ft= tokens + path := staleTokenRe.ReplaceAllString(rawPath, "") + path = strings.TrimRight(path, "?&") + // Strip all /v1/files/ and /v1/media/ prefixes (may be stacked from legacy bugs) + for strings.Contains(path, "/v1/files/") { + path = strings.Replace(path, "/v1/files/", "/", 1) + } + for strings.Contains(path, "/v1/media/") { + path = strings.Replace(path, "/v1/media/", "/", 1) + } + path = filepath.Clean(path) + urlPath := "/v1/files/" + strings.TrimPrefix(path, "/") + ft := SignFileToken(urlPath, secret, FileTokenTTL) + return urlPath + "?ft=" + ft +} + // fileURLRe matches /v1/files/... and /v1/media/... URLs in markdown and plain text. // Captures the full URL path (stops at whitespace, closing paren, quote, or angle bracket). var fileURLRe = regexp.MustCompile(`(/v1/(?:files|media)/[^\s)"'<>]+)`) diff --git a/internal/http/file_token_test.go b/internal/http/file_token_test.go new file mode 100644 index 00000000..8626e06e --- /dev/null +++ b/internal/http/file_token_test.go @@ -0,0 +1,90 @@ +package http + +import ( + "strings" + "testing" +) + +func TestSignMediaPath(t *testing.T) { + secret := "test-secret-key" + + tests := []struct { + name string + rawPath string + wantBase string // expected URL path prefix (before ?ft=) + wantEmpty bool + }{ + { + name: "clean absolute path", + rawPath: "/app/workspace/teams/abc/login.html", + wantBase: "/v1/files/app/workspace/teams/abc/login.html", + }, + { + name: "single /v1/files/ prefix", + rawPath: "/v1/files/app/workspace/login.html", + wantBase: "/v1/files/app/workspace/login.html", + }, + { + name: "double /v1/files/ prefix (legacy bug)", + rawPath: "/v1/files/v1/files/app/workspace/login.html", + wantBase: "/v1/files/app/workspace/login.html", + }, + { + name: "triple /v1/files/ prefix (legacy bug)", + rawPath: "/v1/files/v1/files/v1/files/app/workspace/login.html", + wantBase: "/v1/files/app/workspace/login.html", + }, + { + name: "stale ?ft= token stripped", + rawPath: "/v1/files/app/workspace/login.html?ft=old.123", + wantBase: "/v1/files/app/workspace/login.html", + }, + { + name: "stacked prefixes and tokens (legacy corruption)", + rawPath: "/v1/files/v1/files/app/workspace/login.html?ft=old1.1?ft=old2.2", + wantBase: "/v1/files/app/workspace/login.html", + }, + { + name: "/v1/media/ prefix stripped", + rawPath: "/v1/media/app/workspace/image.png", + wantBase: "/v1/files/app/workspace/image.png", + }, + { + name: "empty path returns empty", + rawPath: "", + wantEmpty: true, + }, + { + name: "path traversal rejected", + rawPath: "/app/workspace/../../etc/passwd", + wantEmpty: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := SignMediaPath(tt.rawPath, secret) + + if tt.wantEmpty { + if result != "" { + t.Errorf("expected empty, got %q", result) + } + return + } + + if !strings.HasPrefix(result, tt.wantBase+"?ft=") { + t.Errorf("expected prefix %q?ft=..., got %q", tt.wantBase, result) + } + + // Must have exactly one ?ft= token + if strings.Count(result, "?ft=") != 1 { + t.Errorf("expected exactly one ?ft= token, got %d in %q", strings.Count(result, "?ft="), result) + } + + // Must have exactly one /v1/files/ prefix + if strings.Count(result, "/v1/files/") != 1 { + t.Errorf("expected exactly one /v1/files/ prefix, got %d in %q", strings.Count(result, "/v1/files/"), result) + } + }) + } +} diff --git a/ui/web/src/pages/chat/hooks/use-chat-messages.ts b/ui/web/src/pages/chat/hooks/use-chat-messages.ts index f7f994fb..cc671e0c 100644 --- a/ui/web/src/pages/chat/hooks/use-chat-messages.ts +++ b/ui/web/src/pages/chat/hooks/use-chat-messages.ts @@ -100,7 +100,7 @@ export function useChatMessages(sessionKey: string, agentId: string) { chatMsg.mediaItems = m.media_refs.map((ref) => ({ path: toFileUrl(ref.path || ref.id), mimeType: ref.mime_type, - fileName: ref.path?.split("/").pop() ?? ref.id, + fileName: (ref.path?.split("?")[0]?.split("/").pop()) ?? ref.id, kind: (ref.kind as MediaItem["kind"]) || "document", })); } @@ -339,7 +339,7 @@ export function useChatMessages(sessionKey: string, agentId: string) { ? rawMedia.map((m) => ({ path: toFileUrl(m.path), mimeType: m.content_type ?? "application/octet-stream", - fileName: m.path.split("/").pop() ?? "file", + fileName: m.path.split("?")[0]?.split("/").pop() ?? "file", size: m.size, kind: mediaKindFromMime(m.content_type ?? ""), }))