mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-11 03:13:24 +00:00
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
This commit is contained in:
1 parent
6bfad07ed8
commit
8b5a05a4b7
5 files changed
+131
-5
No files matched your search
@@ -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{
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)"'<>]+)`)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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 ?? ""),
|
||||
}))
|
||||
|
||||
Reference in new issue
Block a user