fix(attachments): preserve workspace in web download URLs

Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
Naiyuan Qing
2026-06-04 15:35:19 +08:00
parent d102fb8e48
commit c092700de8
10 changed files with 391 additions and 23 deletions

View File

@@ -5,11 +5,18 @@ import { act, renderHook, waitFor } from "@testing-library/react";
// outside-of-scope vars, but vi.hoisted runs before the import graph.
const getAttachmentMock = vi.hoisted(() => vi.fn());
const getBaseUrlMock = vi.hoisted(() => vi.fn(() => ""));
const useWorkspaceSlugMock = vi.hoisted(() =>
vi.fn<() => string | null>(() => "acme"),
);
vi.mock("@multica/core/api", () => ({
api: { getAttachment: getAttachmentMock, getBaseUrl: getBaseUrlMock },
}));
vi.mock("@multica/core/paths", () => ({
useWorkspaceSlug: useWorkspaceSlugMock,
}));
vi.mock("sonner", () => ({
toast: { error: vi.fn(), success: vi.fn() },
}));
@@ -30,6 +37,7 @@ beforeEach(() => {
// a non-empty base (desktop standalone, server-relative download URL)
// overrides via getBaseUrlMock.mockReturnValue(...).
getBaseUrlMock.mockReturnValue("");
useWorkspaceSlugMock.mockReturnValue("acme");
});
afterEach(() => {
@@ -72,7 +80,12 @@ describe("useDownloadAttachment (web)", () => {
node instanceof HTMLAnchorElement,
);
expect(anchor).toBeDefined();
expect(anchor!.getAttribute("href")).toBe("/api/attachments/att-1/download");
expect(anchor!.getAttribute("href")).toBe(
"/api/attachments/att-1/download?workspace_slug=acme",
);
expect(anchor!.href).toBe(
"http://localhost:3000/api/attachments/att-1/download?workspace_slug=acme",
);
// Empty download attribute intentionally defers the final filename to the
// endpoint / redirected object Content-Disposition header.
expect(anchor!.getAttribute("download")).toBe("");
@@ -106,10 +119,63 @@ describe("useDownloadAttachment (web)", () => {
);
expect(anchor).toBeDefined();
expect(anchor!.href).toBe(
"https://api.example.test/api/attachments/att%201%2Fslash/download",
"https://api.example.test/api/attachments/att%201%2Fslash/download?workspace_slug=acme",
);
});
it("encodes the workspace slug into the bare navigation URL instead of relying on custom headers", async () => {
useWorkspaceSlugMock.mockReturnValueOnce("team/space");
getAttachmentMock.mockResolvedValueOnce({
id: "att-1",
url: "https://static.example.test/file.md",
download_url: SIGNED_URL,
filename: "file.md",
});
const clickSpy = vi
.spyOn(HTMLAnchorElement.prototype, "click")
.mockImplementation(() => {});
const appendSpy = vi.spyOn(document.body, "appendChild");
const { result } = renderHook(() => useDownloadAttachment());
await act(async () => {
await result.current("att-1");
});
expect(clickSpy).toHaveBeenCalledOnce();
const anchor = appendSpy.mock.calls
.map(([node]) => node)
.find((node): node is HTMLAnchorElement =>
node instanceof HTMLAnchorElement,
);
expect(anchor).toBeDefined();
expect(anchor!.getAttribute("href")).toBe(
"/api/attachments/att-1/download?workspace_slug=team%2Fspace",
);
});
it("shows a toast and does not click a download link when the workspace slug is missing", async () => {
useWorkspaceSlugMock.mockReturnValueOnce(null);
getAttachmentMock.mockResolvedValueOnce({
id: "att-1",
url: "https://static.example.test/file.md",
download_url: SIGNED_URL,
filename: "file.md",
});
const clickSpy = vi
.spyOn(HTMLAnchorElement.prototype, "click")
.mockImplementation(() => {});
const { result } = renderHook(() => useDownloadAttachment());
await act(async () => {
await result.current("att-1");
});
expect(clickSpy).not.toHaveBeenCalled();
await waitFor(() => expect(toast.error).toHaveBeenCalled());
});
it("shows a toast and does not click a download link when the metadata preflight fails", async () => {
getAttachmentMock.mockRejectedValueOnce(new Error("boom"));
const openSpy = vi.spyOn(window, "open");

View File

@@ -3,6 +3,7 @@
import { useCallback } from "react";
import { toast } from "sonner";
import { api } from "@multica/core/api";
import { useWorkspaceSlug } from "@multica/core/paths";
import { resolvePublicFileUrl } from "@multica/core/workspace/avatar-url";
import { useT } from "../i18n";
@@ -10,9 +11,14 @@ interface DesktopBridge {
downloadURL?: (u: string) => Promise<void> | void;
}
function attachmentDownloadEndpoint(attachmentId: string): string {
function attachmentDownloadEndpoint(
attachmentId: string,
workspaceSlug: string,
): string {
const params = new URLSearchParams({ workspace_slug: workspaceSlug });
const path = `/api/attachments/${encodeURIComponent(attachmentId)}/download`;
return resolvePublicFileUrl(path) ?? path;
const endpoint = `${path}?${params.toString()}`;
return resolvePublicFileUrl(endpoint) ?? endpoint;
}
function triggerBrowserDownload(url: string): void {
@@ -21,9 +27,9 @@ function triggerBrowserDownload(url: string): void {
// Keep the click in the current browsing context. For same-origin API
// downloads this hint lets Chromium/Safari use Content-Disposition's
// filename without opening a blank tab. If the endpoint later 302s to
// CloudFront/S3, the object metadata / signed response still controls the
// final Content-Disposition; the browser follows that redirect natively
// without buffering the file into JS memory.
// CloudFront/S3, the server signs that redirect with an attachment
// disposition; the browser follows it natively without buffering the file
// into JS memory.
anchor.download = "";
anchor.rel = "noopener";
anchor.style.display = "none";
@@ -50,9 +56,10 @@ function hasDesktopDownloadBridge(): boolean {
*
* - **Web**: first refreshes attachment metadata for the existing error
* feedback path, then clicks a temporary same-origin
* `/api/attachments/{id}/download` anchor. The backend endpoint owns
* CloudFront / S3 presign / proxy selection and Content-Disposition, so
* large files stay in the browser's native download pipeline.
* `/api/attachments/{id}/download?workspace_slug=...` anchor. The backend
* endpoint owns CloudFront / S3 presign / proxy selection and download
* Content-Disposition, so large files stay in the browser's native download
* pipeline.
*
* - **Desktop**: uses `desktopAPI.downloadURL()` which invokes Electron's
* native `webContents.downloadURL()`, showing a save dialog and saving
@@ -62,6 +69,7 @@ function hasDesktopDownloadBridge(): boolean {
*/
export function useDownloadAttachment(): (attachmentId: string) => Promise<void> {
const { t } = useT("editor");
const workspaceSlug = useWorkspaceSlug();
return useCallback(
async (attachmentId: string) => {
const failed = () => toast.error(t(($) => $.attachment.download_failed));
@@ -103,11 +111,17 @@ export function useDownloadAttachment(): (attachmentId: string) => Promise<void>
failed();
return;
}
triggerBrowserDownload(attachmentDownloadEndpoint(attachmentId));
if (!workspaceSlug) {
failed();
return;
}
triggerBrowserDownload(
attachmentDownloadEndpoint(attachmentId, workspaceSlug),
);
} catch {
failed();
}
},
[t],
[t, workspaceSlug],
);
}

View File

@@ -12,6 +12,7 @@ import (
"fmt"
"log/slog"
"net/http"
"net/url"
"os"
"strings"
"time"
@@ -195,6 +196,20 @@ func (s *CloudFrontSigner) SignedURL(rawURL string, expiry time.Time) string {
return fmt.Sprintf("%s%sPolicy=%s&Signature=%s&Key-Pair-Id=%s", rawURL, separator, encodedPolicy, encodedSig, s.keyPairID)
}
func (s *CloudFrontSigner) SignedURLWithContentDisposition(rawURL string, contentDisposition string, expiry time.Time) string {
if contentDisposition == "" {
return s.SignedURL(rawURL, expiry)
}
u, err := url.Parse(rawURL)
if err != nil {
return s.SignedURL(rawURL, expiry)
}
q := u.Query()
q.Set("response-content-disposition", contentDisposition)
u.RawQuery = q.Encode()
return s.SignedURL(u.String(), expiry)
}
// cfBase64Encode applies CloudFront's URL-safe base64 encoding.
func cfBase64Encode(data []byte) string {
encoded := base64.StdEncoding.EncodeToString(data)

View File

@@ -0,0 +1,57 @@
package auth
import (
"crypto/rand"
"crypto/rsa"
"encoding/base64"
"net/url"
"strings"
"testing"
"time"
)
func decodeCloudFrontBase64(t *testing.T, encoded string) string {
t.Helper()
standard := strings.NewReplacer("-", "+", "_", "=", "~", "/").Replace(encoded)
decoded, err := base64.StdEncoding.DecodeString(standard)
if err != nil {
t.Fatalf("decode CloudFront base64: %v", err)
}
return string(decoded)
}
func TestCloudFrontSignedURLWithContentDisposition(t *testing.T) {
key, err := rsa.GenerateKey(rand.Reader, 2048)
if err != nil {
t.Fatalf("generate key: %v", err)
}
signer := &CloudFrontSigner{
keyPairID: "K123",
privateKey: key,
}
got := signer.SignedURLWithContentDisposition(
"https://static.example.test/uploads/report.md?existing=1",
`attachment; filename="report.md"`,
time.Unix(1893456000, 0),
)
u, err := url.Parse(got)
if err != nil {
t.Fatalf("parse signed URL: %v", err)
}
q := u.Query()
if got := q.Get("response-content-disposition"); got != `attachment; filename="report.md"` {
t.Fatalf("response-content-disposition = %q", got)
}
if got := q.Get("Key-Pair-Id"); got != "K123" {
t.Fatalf("Key-Pair-Id = %q", got)
}
if q.Get("Signature") == "" {
t.Fatalf("missing Signature in %q", got)
}
policy := decodeCloudFrontBase64(t, q.Get("Policy"))
if !strings.Contains(policy, "response-content-disposition=attachment%3B+filename%3D%22report.md%22") {
t.Fatalf("policy did not include signed response-content-disposition: %s", policy)
}
}

View File

@@ -442,14 +442,28 @@ func (h *Handler) DownloadAttachment(w http.ResponseWriter, r *http.Request) {
writeError(w, http.StatusInternalServerError, "cloudfront attachment downloads are not configured")
return
}
http.Redirect(w, r, h.CFSigner.SignedURL(att.Url, time.Now().Add(h.attachmentDownloadURLTTL())), http.StatusFound)
http.Redirect(
w,
r,
h.CFSigner.SignedURLWithContentDisposition(
att.Url,
storage.AttachmentContentDisposition(att.Filename),
time.Now().Add(h.attachmentDownloadURLTTL()),
),
http.StatusFound,
)
case attachmentDownloadModePresign:
presigner, ok := h.Storage.(storage.Presigner)
presigner, ok := h.Storage.(storage.DownloadPresigner)
if !ok {
writeError(w, http.StatusInternalServerError, "attachment storage does not support presigned downloads")
return
}
signedURL, err := presigner.PresignGet(r.Context(), key, h.attachmentDownloadURLTTL())
signedURL, err := presigner.PresignGetWithContentDisposition(
r.Context(),
key,
h.attachmentDownloadURLTTL(),
storage.AttachmentContentDisposition(att.Filename),
)
if err != nil {
slog.Error("failed to presign attachment download", "id", uuidToString(att.ID), "key", key, "error", err)
writeError(w, http.StatusBadGateway, "failed to create download URL")
@@ -478,7 +492,7 @@ func (h *Handler) resolveAttachmentDownloadMode(rawURL string) attachmentDownloa
if shouldProxyAttachmentURL(rawURL) {
return attachmentDownloadModeProxy
}
if _, ok := h.Storage.(storage.Presigner); ok {
if _, ok := h.Storage.(storage.DownloadPresigner); ok {
return attachmentDownloadModePresign
}
return attachmentDownloadModeProxy
@@ -547,8 +561,9 @@ func (h *Handler) proxyAttachmentDownload(w http.ResponseWriter, r *http.Request
// Exists to (a) bypass CloudFront CORS (not configured) and (b) bypass
// Content-Disposition: attachment which Chromium honors for iframe document
// loads. Media types (image/video/audio/pdf) intentionally use download_url
// instead; /download signs or streams them with Content-Disposition: inline
// using the same media-type policy as storage uploads.
// instead. Metadata download_url keeps CloudFront/S3's media preview behavior;
// the explicit /download route signs redirects as attachment downloads and
// proxy mode streams with the same media-type policy as storage uploads.
//
// Hard cap: 2 MB. Larger files return 413. Anything outside the text
// whitelist returns 415.

View File

@@ -3,18 +3,26 @@ package handler
import (
"bytes"
"context"
"crypto/rand"
"crypto/rsa"
"crypto/x509"
"encoding/base64"
"encoding/json"
"encoding/pem"
"fmt"
"io"
"mime/multipart"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"sync"
"testing"
"time"
"github.com/go-chi/chi/v5"
"github.com/multica-ai/multica/server/internal/auth"
"github.com/multica-ai/multica/server/internal/middleware"
db "github.com/multica-ai/multica/server/pkg/db/generated"
)
@@ -43,9 +51,10 @@ func createHandlerTestChatSession(t *testing.T, agentID string) string {
// strips the synthetic CDN host so consumers can pass either the URL or the
// raw key.
type mockStorage struct {
mu sync.Mutex
files map[string][]byte
presignCalls []string
mu sync.Mutex
files map[string][]byte
presignCalls []string
presignDispositions []string
}
func (m *mockStorage) Upload(_ context.Context, key string, data []byte, _ string, _ string) (string, error) {
@@ -91,6 +100,24 @@ func (m *mockStorage) PresignGet(_ context.Context, key string, _ time.Duration)
m.presignCalls = append(m.presignCalls, key)
return "https://signed.example.com/" + key + "?X-Amz-Signature=mock", nil
}
func (m *mockStorage) PresignGetWithContentDisposition(_ context.Context, key string, _ time.Duration, contentDisposition string) (string, error) {
m.mu.Lock()
defer m.mu.Unlock()
m.presignCalls = append(m.presignCalls, key)
m.presignDispositions = append(m.presignDispositions, contentDisposition)
u := url.URL{
Scheme: "https",
Host: "signed.example.com",
Path: "/" + key,
}
q := u.Query()
q.Set("X-Amz-Signature", "mock")
if contentDisposition != "" {
q.Set("response-content-disposition", contentDisposition)
}
u.RawQuery = q.Encode()
return u.String(), nil
}
func (m *mockStorage) put(key string, data []byte) {
m.mu.Lock()
defer m.mu.Unlock()
@@ -407,6 +434,35 @@ func newDownloadRequest(t *testing.T, attachmentID, workspaceID string) (*http.R
return req, httptest.NewRecorder()
}
func newDownloadRouter() http.Handler {
r := chi.NewRouter()
r.Use(middleware.RequireWorkspaceMember(testHandler.Queries))
r.Get("/api/attachments/{id}/download", testHandler.DownloadAttachment)
return r
}
func testCloudFrontSigner(t *testing.T) *auth.CloudFrontSigner {
t.Helper()
key, err := rsa.GenerateKey(rand.Reader, 2048)
if err != nil {
t.Fatalf("generate CloudFront test key: %v", err)
}
pemBytes := pem.EncodeToMemory(&pem.Block{
Type: "RSA PRIVATE KEY",
Bytes: x509.MarshalPKCS1PrivateKey(key),
})
t.Setenv("CLOUDFRONT_KEY_PAIR_ID", "KTEST")
t.Setenv("CLOUDFRONT_DOMAIN", "static.example.test")
t.Setenv("COOKIE_DOMAIN", ".example.test")
t.Setenv("CLOUDFRONT_PRIVATE_KEY", base64.StdEncoding.EncodeToString(pemBytes))
t.Setenv("CLOUDFRONT_PRIVATE_KEY_SECRET", "")
signer := auth.NewCloudFrontSignerFromEnv()
if signer == nil {
t.Fatal("expected CloudFront signer")
}
return signer
}
func TestAttachmentToResponse_NonCloudFrontUsesDownloadEndpoint(t *testing.T) {
origSigner := testHandler.CFSigner
testHandler.CFSigner = nil
@@ -430,6 +486,94 @@ func TestAttachmentToResponse_NonCloudFrontUsesDownloadEndpoint(t *testing.T) {
}
}
func TestDownloadAttachment_CloudFrontRedirectSignsAttachmentDisposition(t *testing.T) {
origStorage := testHandler.Storage
origCfg := testHandler.cfg
origSigner := testHandler.CFSigner
testHandler.Storage = &mockStorage{}
testHandler.cfg.AttachmentDownloadMode = "cloudfront"
testHandler.CFSigner = testCloudFrontSigner(t)
t.Cleanup(func() {
testHandler.Storage = origStorage
testHandler.cfg = origCfg
testHandler.CFSigner = origSigner
})
id := seedAttachmentURL(t, "https://static.example.test/downloads/cloudfront.md", "cloud front.md", "text/markdown", 10)
req, w := newDownloadRequest(t, id, testWorkspaceID)
testHandler.DownloadAttachment(w, req)
if w.Code != http.StatusFound {
t.Fatalf("status = %d, want 302; body=%s", w.Code, w.Body.String())
}
loc := w.Header().Get("Location")
parsed, err := url.Parse(loc)
if err != nil {
t.Fatalf("parse Location: %v", err)
}
if got := parsed.Query().Get("response-content-disposition"); got != `attachment; filename="cloud front.md"` {
t.Fatalf("response-content-disposition = %q", got)
}
if got := parsed.Query().Get("Key-Pair-Id"); got != "KTEST" {
t.Fatalf("Key-Pair-Id = %q", got)
}
}
func TestDownloadAttachment_BareNavigationWithWorkspaceSlugQueryPassesMiddleware(t *testing.T) {
store := &mockStorage{}
origStorage := testHandler.Storage
origCfg := testHandler.cfg
origSigner := testHandler.CFSigner
testHandler.Storage = store
testHandler.cfg.AttachmentDownloadMode = "proxy"
testHandler.CFSigner = nil
t.Cleanup(func() {
testHandler.Storage = origStorage
testHandler.cfg = origCfg
testHandler.CFSigner = origSigner
})
key := "downloads/bare-nav.txt"
body := []byte("download body")
store.put(key, body)
id := seedAttachmentURL(t, "https://s3.example.com/test-bucket/"+key, "bare-nav.txt", "text/plain", int64(len(body)))
req := httptest.NewRequest("GET", "/api/attachments/"+id+"/download?workspace_slug="+url.QueryEscape(handlerTestWorkspaceSlug), nil)
req.Header.Set("X-User-ID", testUserID)
w := httptest.NewRecorder()
newDownloadRouter().ServeHTTP(w, req)
if w.Code != http.StatusOK {
t.Fatalf("status = %d, want 200; body=%s", w.Code, w.Body.String())
}
if got := w.Body.String(); got != string(body) {
t.Fatalf("body = %q, want %q", got, body)
}
if req.Header.Get("X-Workspace-ID") != "" || req.Header.Get("X-Workspace-Slug") != "" {
t.Fatalf("bare navigation test must not set custom workspace headers")
}
}
func TestDownloadAttachment_BareNavigationWithoutWorkspaceQueryFailsMiddleware(t *testing.T) {
req := httptest.NewRequest("GET", "/api/attachments/00000000-0000-0000-0000-000000000001/download", nil)
req.Header.Set("X-User-ID", testUserID)
w := httptest.NewRecorder()
newDownloadRouter().ServeHTTP(w, req)
if w.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want 400; body=%s", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), "workspace_id or workspace_slug is required") {
t.Fatalf("body = %q, want workspace identifier error", w.Body.String())
}
if req.Header.Get("X-Workspace-ID") != "" || req.Header.Get("X-Workspace-Slug") != "" {
t.Fatalf("bare navigation test must not set custom workspace headers")
}
}
func TestDownloadAttachment_AutoInternalEndpointProxies(t *testing.T) {
store := &mockStorage{}
origStorage := testHandler.Storage
@@ -502,9 +646,19 @@ func TestDownloadAttachment_AutoPublicEndpointPresigns(t *testing.T) {
if !strings.Contains(loc, "X-Amz-Signature=mock") {
t.Fatalf("Location = %q, want fake S3 signature", loc)
}
parsed, err := url.Parse(loc)
if err != nil {
t.Fatalf("parse Location: %v", err)
}
if got := parsed.Query().Get("response-content-disposition"); got != `attachment; filename="public.txt"` {
t.Fatalf("response-content-disposition = %q", got)
}
if len(store.presignCalls) != 1 || store.presignCalls[0] != key {
t.Fatalf("presign calls = %v, want [%s]", store.presignCalls, key)
}
if len(store.presignDispositions) != 1 || store.presignDispositions[0] != `attachment; filename="public.txt"` {
t.Fatalf("presign dispositions = %v", store.presignDispositions)
}
}
func TestDownloadAttachment_ExplicitProxyStreamsPublicEndpoint(t *testing.T) {

View File

@@ -174,16 +174,24 @@ func (s *S3Storage) GetReader(ctx context.Context, key string) (io.ReadCloser, e
}
func (s *S3Storage) PresignGet(ctx context.Context, key string, ttl time.Duration) (string, error) {
return s.PresignGetWithContentDisposition(ctx, key, ttl, "")
}
func (s *S3Storage) PresignGetWithContentDisposition(ctx context.Context, key string, ttl time.Duration, contentDisposition string) (string, error) {
if key == "" {
return "", fmt.Errorf("s3 PresignGet: empty key")
}
if ttl <= 0 {
ttl = 30 * time.Minute
}
out, err := s3.NewPresignClient(s.client).PresignGetObject(ctx, &s3.GetObjectInput{
input := &s3.GetObjectInput{
Bucket: aws.String(s.bucket),
Key: aws.String(key),
}, func(opts *s3.PresignOptions) {
}
if contentDisposition != "" {
input.ResponseContentDisposition = aws.String(contentDisposition)
}
out, err := s3.NewPresignClient(s.client).PresignGetObject(ctx, input, func(opts *s3.PresignOptions) {
opts.Expires = ttl
})
if err != nil {

View File

@@ -2,6 +2,7 @@ package storage
import (
"context"
"net/url"
"strings"
"testing"
"time"
@@ -48,6 +49,36 @@ func TestS3StoragePresignGet(t *testing.T) {
}
}
func TestS3StoragePresignGetWithContentDisposition(t *testing.T) {
store := &S3Storage{
client: s3.New(s3.Options{
Region: "us-east-1",
Credentials: aws.NewCredentialsCache(credentials.NewStaticCredentialsProvider("AKID", "SECRET", "")),
}),
bucket: "test-bucket",
}
got, err := store.PresignGetWithContentDisposition(
context.Background(),
"uploads/abc/file.txt",
5*time.Minute,
`attachment; filename="report.txt"`,
)
if err != nil {
t.Fatalf("PresignGetWithContentDisposition: %v", err)
}
u, err := url.Parse(got)
if err != nil {
t.Fatalf("parse presigned URL: %v", err)
}
if got := u.Query().Get("response-content-disposition"); got != `attachment; filename="report.txt"` {
t.Fatalf("response-content-disposition = %q", got)
}
if sig := u.Query().Get("X-Amz-Signature"); sig == "" {
t.Fatalf("missing X-Amz-Signature in %q", got)
}
}
func TestS3StorageKeyFromURL_CustomEndpointWithTrailingSlash(t *testing.T) {
s := &S3Storage{
bucket: "test-bucket",

View File

@@ -22,3 +22,7 @@ type Storage interface {
type Presigner interface {
PresignGet(ctx context.Context, key string, ttl time.Duration) (string, error)
}
type DownloadPresigner interface {
PresignGetWithContentDisposition(ctx context.Context, key string, ttl time.Duration, contentDisposition string) (string, error)
}

View File

@@ -27,6 +27,10 @@ func ContentDisposition(contentType, filename string) string {
return disposition + `; filename="` + sanitizeFilename(filename) + `"`
}
func AttachmentContentDisposition(filename string) string {
return `attachment; filename="` + sanitizeFilename(filename) + `"`
}
// isInlineContentType returns true for media types that browsers should
// display inline (images, video, audio, PDF). Everything else triggers a
// download via Content-Disposition: attachment.