diff --git a/server/internal/cli/client.go b/server/internal/cli/client.go index 349bfbcbd..fd39acf2e 100644 --- a/server/internal/cli/client.go +++ b/server/internal/cli/client.go @@ -326,11 +326,28 @@ func (c *APIClient) UploadFile(ctx context.Context, fileData []byte, filename st // DownloadFile downloads a file from the given URL and returns the response body. // This is used for downloading attachments via their signed download_url. // Downloads are limited to 100 MB to match the upload size limit. +// +// The URL may be absolute (a signed CloudFront/S3 URL) or relative +// (a server-relative path like "/uploads/...") depending on how the +// server is configured. Relative URLs are resolved against the client's +// BaseURL and sent with the standard auth headers; absolute URLs are +// used as-is so that their query-string signatures are not disturbed. func (c *APIClient) DownloadFile(ctx context.Context, downloadURL string) ([]byte, error) { + isRelative := !strings.HasPrefix(downloadURL, "http://") && !strings.HasPrefix(downloadURL, "https://") + if isRelative { + if c.BaseURL == "" { + return nil, fmt.Errorf("download URL %q is relative but client has no BaseURL", downloadURL) + } + downloadURL = c.BaseURL + downloadURL + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) if err != nil { return nil, err } + if isRelative { + c.setHeaders(req) + } resp, err := c.HTTPClient.Do(req) if err != nil { diff --git a/server/internal/cli/client_test.go b/server/internal/cli/client_test.go index c49276f0d..6248f4455 100644 --- a/server/internal/cli/client_test.go +++ b/server/internal/cli/client_test.go @@ -164,6 +164,76 @@ func TestPostJSON(t *testing.T) { }) } +func TestDownloadFile(t *testing.T) { + t.Run("relative URL is resolved against BaseURL and sent with auth", func(t *testing.T) { + var gotPath, gotAuth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotAuth = r.Header.Get("Authorization") + w.Write([]byte("hello")) + })) + defer srv.Close() + + client := NewAPIClient(srv.URL, "", "test-token") + data, err := client.DownloadFile(context.Background(), "/uploads/workspaces/abc/file.md") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(data) != "hello" { + t.Errorf("unexpected body: %q", string(data)) + } + if gotPath != "/uploads/workspaces/abc/file.md" { + t.Errorf("unexpected path: %q", gotPath) + } + if gotAuth != "Bearer test-token" { + t.Errorf("expected Authorization Bearer test-token, got %q", gotAuth) + } + }) + + t.Run("absolute URL is used as-is without auth headers", func(t *testing.T) { + var gotAuth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + w.Write([]byte("signed-payload")) + })) + defer srv.Close() + + client := NewAPIClient("https://api.example.test", "", "test-token") + data, err := client.DownloadFile(context.Background(), srv.URL+"/signed?sig=abc") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(data) != "signed-payload" { + t.Errorf("unexpected body: %q", string(data)) + } + if gotAuth != "" { + t.Errorf("expected no Authorization header on signed URL, got %q", gotAuth) + } + }) + + t.Run("relative URL with empty BaseURL returns a helpful error", func(t *testing.T) { + client := NewAPIClient("", "", "test-token") + _, err := client.DownloadFile(context.Background(), "/uploads/x.md") + if err == nil { + t.Fatal("expected error, got nil") + } + }) + + t.Run("non-2xx status returns an error with the response body", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + io.WriteString(w, "not found") + })) + defer srv.Close() + + client := NewAPIClient(srv.URL, "", "test-token") + _, err := client.DownloadFile(context.Background(), "/uploads/missing") + if err == nil { + t.Fatal("expected error, got nil") + } + }) +} + func TestNormalizeGOOS(t *testing.T) { cases := map[string]string{ "darwin": "macos",