Files
multica/server/internal/cli/client_test.go
Bohan Jiang 2187b58292 feat(attachments): let callers opt out of pre-signed URLs in bulk responses (MUL-5372) (#6119)
* feat(attachments): let callers opt out of pre-signed URLs in bulk responses

A CloudFront-signed download_url is ~800 chars, ~630 of which are a
Policy+Signature pair re-minted on every request (the policy embeds
now+TTL at second granularity). Emitting one per attachment on every
list response costs three ways: raw payload, a fresh RSA sign per
attachment per request, and — because the bytes differ on every read —
it defeats any cache keyed on response content.

Agents pay all three and use none of it. `multica attachment download
<id>` fetches a fresh signature from the single-attachment endpoint, so
the id is the only part the CLI needs; measured on a 15-attachment
issue, the URL fields were 28.3% of the payload and ~10k chars of that
rotated on every read.

Callers now advertise `stable_attachment_urls` in X-Client-Capabilities
to receive the stable /api/attachments/{id}/download path instead. That
endpoint re-signs and 302s on every hit, so the value stays correct
forever at ~95 chars. The CLI advertises it unconditionally — it is a
protocol detail, not a user-visible flag.

The single-attachment endpoint keeps signing regardless of the
capability: it is the one source of fresh, natively-loadable URLs, and
stable-mode callers exchange a stable path for a signature there. That
carve-out is what makes the rest safe.

The server default never moves. A caller that advertises nothing — every
installed mobile build, every third-party script — gets byte-identical
responses, so this ships ahead of any client and each surface migrates
on its own release cadence. Web/desktop (Phase 2) and mobile (Phase 3)
are deliberately not touched here.

MUL-5372

Co-authored-by: multica-agent <github@multica.ai>

* test(attachments): pin the CLI capability header and the rotation premise

Two review nits from #6119.

The CLI's setHeaders call is the only place Phase 1 is switched on, and
nothing observed it: the server tests prove a request carrying
`X-Client-Capabilities: stable_attachment_urls` gets stable paths, but a
typo in the CLI token — or dropping the header — would have left every
test green while the CLI silently went back to ~800-char signed URLs.
Assert the header verbatim across GET, GET-with-headers, POST, and the
no-token client. Verified the assertion is load-bearing by mutating the
constant to `stable_attachment_url`: the test fails.

TestAttachmentToResponse_SignedModeRotatesButStableDoesNot only checked
the "stable does not" half, so its name overstated coverage. Add the
rotation half. It drives the signer with two explicit expiries rather
than calling attachmentToResponse twice: that function mints its expiry
from time.Now() at second granularity, so consecutive calls usually land
in the same second and asserting on them directly would be a flaky test
of a real property. Also pins that only the query rotates, and that a
fixed expiry re-signs deterministically — without which the rotation
assertion would prove nothing about the clock.

MUL-5372

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-29 18:25:14 +08:00

528 lines
17 KiB
Go

package cli
import (
"context"
"encoding/json"
"errors"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
)
func TestPostJSON(t *testing.T) {
type reqBody struct {
Name string `json:"name"`
Age int `json:"age"`
}
type respBody struct {
ID string `json:"id"`
}
t.Run("success", func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
t.Errorf("expected POST, got %s", r.Method)
}
if ct := r.Header.Get("Content-Type"); ct != "application/json" {
t.Errorf("expected Content-Type application/json, got %s", ct)
}
if auth := r.Header.Get("Authorization"); auth != "Bearer test-token" {
t.Errorf("expected Authorization Bearer test-token, got %s", auth)
}
var body reqBody
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
t.Fatalf("failed to decode request body: %v", err)
}
if body.Name != "alice" || body.Age != 30 {
t.Errorf("unexpected body: %+v", body)
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(respBody{ID: "123"})
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "", "test-token")
var out respBody
err := client.PostJSON(context.Background(), "/test", reqBody{Name: "alice", Age: 30}, &out)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if out.ID != "123" {
t.Errorf("expected ID 123, got %s", out.ID)
}
})
t.Run("error status", func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadRequest)
io.WriteString(w, "bad request")
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "", "test-token")
err := client.PostJSON(context.Background(), "/test", reqBody{Name: "bob"}, nil)
if err == nil {
t.Fatal("expected error, got nil")
}
if got := err.Error(); got != "POST /test returned 400: bad request" {
t.Errorf("unexpected error message: %s", got)
}
})
t.Run("nil output", func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusCreated)
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "", "test-token")
err := client.PostJSON(context.Background(), "/test", reqBody{Name: "charlie"}, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
})
t.Run("workspace and agent context headers", func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if ws := r.Header.Get("X-Workspace-ID"); ws != "ws-abc" {
t.Errorf("expected X-Workspace-ID ws-abc, got %s", ws)
}
if agent := r.Header.Get("X-Agent-ID"); agent != "agent-123" {
t.Errorf("expected X-Agent-ID agent-123, got %s", agent)
}
if task := r.Header.Get("X-Task-ID"); task != "task-456" {
t.Errorf("expected X-Task-ID task-456, got %s", task)
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(respBody{ID: "456"})
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "ws-abc", "test-token")
client.AgentID = "agent-123"
client.TaskID = "task-456"
var out respBody
err := client.PostJSON(context.Background(), "/test", reqBody{}, &out)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
})
t.Run("client identity headers", func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if got := r.Header.Get("X-Client-Platform"); got != "cli-test" {
t.Errorf("expected X-Client-Platform cli-test, got %s", got)
}
if got := r.Header.Get("X-Client-Version"); got != "9.9.9" {
t.Errorf("expected X-Client-Version 9.9.9, got %s", got)
}
if got := r.Header.Get("X-Client-OS"); got != "linux" {
t.Errorf("expected X-Client-OS linux, got %s", got)
}
w.WriteHeader(http.StatusNoContent)
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "", "")
client.Platform = "cli-test"
client.Version = "9.9.9"
client.OS = "linux"
if err := client.PostJSON(context.Background(), "/test", reqBody{}, nil); err != nil {
t.Fatalf("unexpected error: %v", err)
}
})
t.Run("client identity headers fall back to package defaults", func(t *testing.T) {
origPlatform, origVersion, origOS := ClientPlatform, ClientVersion, ClientOS
ClientPlatform = "cli"
ClientVersion = "1.2.3-test"
ClientOS = "macos"
t.Cleanup(func() {
ClientPlatform, ClientVersion, ClientOS = origPlatform, origVersion, origOS
})
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if got := r.Header.Get("X-Client-Platform"); got != "cli" {
t.Errorf("expected X-Client-Platform cli, got %s", got)
}
if got := r.Header.Get("X-Client-Version"); got != "1.2.3-test" {
t.Errorf("expected X-Client-Version 1.2.3-test, got %s", got)
}
if got := r.Header.Get("X-Client-OS"); got != "macos" {
t.Errorf("expected X-Client-OS macos, got %s", got)
}
w.WriteHeader(http.StatusNoContent)
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "", "")
if err := client.PostJSON(context.Background(), "/test", reqBody{}, nil); err != nil {
t.Fatalf("unexpected error: %v", err)
}
})
}
func TestDeleteJSONResponse(t *testing.T) {
type respBody struct {
ID string `json:"id"`
}
t.Run("success decodes response", func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodDelete {
t.Errorf("expected DELETE, got %s", r.Method)
}
if auth := r.Header.Get("Authorization"); auth != "Bearer test-token" {
t.Errorf("expected Authorization Bearer test-token, got %s", auth)
}
json.NewEncoder(w).Encode(respBody{ID: "comment-123"})
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "", "test-token")
var out respBody
if err := client.DeleteJSONResponse(context.Background(), "/test", &out); err != nil {
t.Fatalf("unexpected error: %v", err)
}
if out.ID != "comment-123" {
t.Errorf("expected ID comment-123, got %s", out.ID)
}
})
t.Run("error status", func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
io.WriteString(w, "missing")
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "", "test-token")
err := client.DeleteJSONResponse(context.Background(), "/test", nil)
if err == nil {
t.Fatal("expected error, got nil")
}
if got := err.Error(); got != "DELETE /test returned 404: missing" {
t.Errorf("unexpected error message: %s", got)
}
})
}
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 TestUploadFileWithURL(t *testing.T) {
t.Run("success", func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
t.Errorf("expected POST, got %s", r.Method)
}
if ct := r.Header.Get("Content-Type"); !strings.Contains(ct, "multipart/form-data") {
t.Errorf("expected multipart content-type, got %s", ct)
}
file, header, err := r.FormFile("file")
if err != nil {
t.Fatalf("missing file field: %v", err)
}
defer file.Close()
data, _ := io.ReadAll(file)
if string(data) != "hello" {
t.Errorf("unexpected file data: %q", string(data))
}
if header.Filename != "test.txt" {
t.Errorf("unexpected filename: %q", header.Filename)
}
// Verify no issue_id or comment_id fields are sent.
if r.FormValue("issue_id") != "" {
t.Errorf("unexpected issue_id field")
}
if r.FormValue("comment_id") != "" {
t.Errorf("unexpected comment_id field")
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(AttachmentResponse{
ID: "att-123",
URL: "https://cdn.example.com/file.txt",
Filename: "test.txt",
SizeBytes: 5,
})
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "ws-1", "test-token")
id, url, err := client.UploadFileWithURL(context.Background(), []byte("hello"), "test.txt")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if id != "att-123" {
t.Errorf("expected id att-123, got %s", id)
}
if url != "https://cdn.example.com/file.txt" {
t.Errorf("expected url https://cdn.example.com/file.txt, got %s", url)
}
})
t.Run("error status", func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadRequest)
io.WriteString(w, "bad request")
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "", "")
_, _, err := client.UploadFileWithURL(context.Background(), []byte("x"), "x.txt")
if err == nil {
t.Fatal("expected error, got nil")
}
var httpErr *HTTPError
if !errors.As(err, &httpErr) {
t.Fatalf("expected *HTTPError, got %T: %v", err, err)
}
if httpErr.StatusCode != 400 {
t.Errorf("expected status 400, got %d", httpErr.StatusCode)
}
})
t.Run("missing id in response succeeds (fallback path)", func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]string{"url": "https://example.com"})
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "", "")
id, url, err := client.UploadFileWithURL(context.Background(), []byte("x"), "x.txt")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if id != "" {
t.Errorf("expected empty id, got %s", id)
}
if url != "https://example.com" {
t.Errorf("expected url https://example.com, got %s", url)
}
})
t.Run("workspace header sent", func(t *testing.T) {
var gotWorkspace string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
gotWorkspace = r.Header.Get("X-Workspace-ID")
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(AttachmentResponse{ID: "att-1", URL: "https://example.com"})
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "ws-abc", "test-token")
_, _, err := client.UploadFileWithURL(context.Background(), []byte("x"), "x.txt")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if gotWorkspace != "ws-abc" {
t.Errorf("expected X-Workspace-ID ws-abc, got %s", gotWorkspace)
}
})
t.Run("missing url in response", func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(AttachmentResponse{ID: "att-123"})
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "", "")
_, _, err := client.UploadFileWithURL(context.Background(), []byte("x"), "x.txt")
if err == nil {
t.Fatal("expected error, got nil")
}
if !strings.Contains(err.Error(), "missing attachment url") {
t.Errorf("unexpected error message: %s", err.Error())
}
})
}
func TestNormalizeGOOS(t *testing.T) {
cases := map[string]string{
"darwin": "macos",
"windows": "windows",
"linux": "linux",
"freebsd": "freebsd",
}
for in, want := range cases {
if got := normalizeGOOS(in); got != want {
t.Errorf("normalizeGOOS(%q) = %q, want %q", in, got, want)
}
}
}
// TestSetHeaders_AdvertisesStableAttachmentURLs is the only test that proves
// Phase 1 of MUL-5372 is actually switched on.
//
// The server-side tests verify that a request carrying
// `X-Client-Capabilities: stable_attachment_urls` gets stable attachment paths,
// but nothing there observes what the CLI sends. Without this assertion a typo
// in the token, or dropping the header from setHeaders entirely, would leave
// every other test green while the CLI silently went back to receiving ~800-char
// signed URLs on every attachment of every list read.
//
// The exact string is load-bearing: the server matches the token literally
// (handler.requestHasClientCapability), so it is asserted verbatim rather than
// through the constant.
func TestSetHeaders_AdvertisesStableAttachmentURLs(t *testing.T) {
const wantCapability = "stable_attachment_urls"
// Every verb goes through setHeaders, and an agent's read path is not only
// GET (comment add posts, attachment upload multiparts). Cover the shapes
// that actually carry attachment payloads back.
t.Run("GET", func(t *testing.T) {
var got string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
got = r.Header.Get("X-Client-Capabilities")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{}`))
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "ws-1", "test-token")
var out map[string]any
if err := client.GetJSON(context.Background(), "/api/issues/x/comments", &out); err != nil {
t.Fatalf("GetJSON: %v", err)
}
if got != wantCapability {
t.Errorf("X-Client-Capabilities = %q, want %q — Phase 1 is off unless the CLI advertises this", got, wantCapability)
}
})
t.Run("GET with headers", func(t *testing.T) {
var got string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
got = r.Header.Get("X-Client-Capabilities")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`[]`))
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "ws-1", "test-token")
var out []map[string]any
if _, err := client.GetJSONWithHeaders(context.Background(), "/api/issues/x/comments", &out); err != nil {
t.Fatalf("GetJSONWithHeaders: %v", err)
}
if got != wantCapability {
t.Errorf("X-Client-Capabilities = %q, want %q", got, wantCapability)
}
})
t.Run("POST", func(t *testing.T) {
var got string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
got = r.Header.Get("X-Client-Capabilities")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{}`))
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "ws-1", "test-token")
var out map[string]any
if err := client.PostJSON(context.Background(), "/api/issues/x/comments", map[string]string{"content": "hi"}, &out); err != nil {
t.Fatalf("PostJSON: %v", err)
}
if got != wantCapability {
t.Errorf("X-Client-Capabilities = %q, want %q", got, wantCapability)
}
})
// An unauthenticated client (no token configured yet) must still advertise:
// the capability describes what the binary can parse, not who it is.
t.Run("without a token", func(t *testing.T) {
var got string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
got = r.Header.Get("X-Client-Capabilities")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{}`))
}))
defer srv.Close()
client := NewAPIClient(srv.URL, "", "")
var out map[string]any
if err := client.GetJSON(context.Background(), "/api/health", &out); err != nil {
t.Fatalf("GetJSON: %v", err)
}
if got != wantCapability {
t.Errorf("X-Client-Capabilities = %q, want %q", got, wantCapability)
}
})
}