Skip to content

Commit 9011539

Browse files
committed
fix(daemon): harden Claude initial image seeding
1 parent dcd9483 commit 9011539

4 files changed

Lines changed: 69 additions & 47 deletions

File tree

server/internal/daemon/client.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -679,18 +679,12 @@ type AttachmentResponse struct {
679679
}
680680

681681
func (c *Client) DownloadAttachment(ctx context.Context, attachmentID string) (*AttachmentResponse, []byte, error) {
682-
var att AttachmentResponse
683-
if err := c.getJSON(ctx, fmt.Sprintf("/api/attachments/%s", attachmentID), &att); err != nil {
684-
return nil, nil, err
685-
}
686-
if att.DownloadURL == "" {
687-
return nil, nil, fmt.Errorf("attachment %s has no download_url", attachmentID)
688-
}
689-
data, err := c.downloadFile(ctx, att.DownloadURL)
682+
path := fmt.Sprintf("/api/attachments/%s/download", attachmentID)
683+
data, err := c.downloadFile(ctx, path)
690684
if err != nil {
691685
return nil, nil, err
692686
}
693-
return &att, data, nil
687+
return &AttachmentResponse{ID: attachmentID}, data, nil
694688
}
695689

696690
func (c *Client) downloadFile(ctx context.Context, downloadURL string) ([]byte, error) {

server/internal/daemon/client_test.go

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -255,28 +255,19 @@ func TestDefaultTerminalRetrySchedule_MatchesAgreedPlan(t *testing.T) {
255255
}
256256
}
257257

258-
func TestClient_DownloadAttachmentDownloadsRelativeURLWithAuthHeaders(t *testing.T) {
258+
func TestClient_DownloadAttachmentUsesDownloadEndpointWithAuthHeaders(t *testing.T) {
259259
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
260260
switch r.URL.Path {
261-
case "/api/attachments/att-1":
262-
if got := r.Header.Get("Authorization"); got != "Bearer tok" {
263-
t.Errorf("metadata request Authorization = %q, want Bearer tok", got)
264-
}
265-
w.Header().Set("Content-Type", "application/json")
266-
json.NewEncoder(w).Encode(map[string]any{
267-
"id": "att-1",
268-
"download_url": "/download/att-1",
269-
"filename": "shot.png",
270-
"content_type": "image/png",
271-
})
272-
case "/download/att-1":
261+
case "/api/attachments/att-1/download":
273262
if got := r.Header.Get("Authorization"); got != "Bearer tok" {
274-
t.Errorf("download request Authorization = %q, want Bearer tok", got)
263+
t.Errorf("attachment download Authorization = %q, want Bearer tok", got)
275264
}
276265
if got := r.Header.Get("X-Client-Platform"); got != "daemon" {
277-
t.Errorf("download request X-Client-Platform = %q, want daemon", got)
266+
t.Errorf("attachment download X-Client-Platform = %q, want daemon", got)
278267
}
279268
w.Write([]byte("png-bytes"))
269+
case "/api/attachments/att-1":
270+
t.Fatal("DownloadAttachment must not fetch member-scoped attachment metadata")
280271
default:
281272
t.Errorf("unexpected path: %s", r.URL.Path)
282273
http.NotFound(w, r)
@@ -291,7 +282,7 @@ func TestClient_DownloadAttachmentDownloadsRelativeURLWithAuthHeaders(t *testing
291282
if err != nil {
292283
t.Fatalf("DownloadAttachment: %v", err)
293284
}
294-
if meta.Filename != "shot.png" || meta.ContentType != "image/png" {
285+
if meta.ID != "att-1" {
295286
t.Fatalf("unexpected attachment metadata: %+v", meta)
296287
}
297288
if string(data) != "png-bytes" {

server/internal/daemon/daemon.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3267,6 +3267,10 @@ const (
32673267
maxClaudeInitialImageBytes = 10 * 1024 * 1024
32683268
)
32693269

3270+
func shouldSeedClaudeInitialImages(provider string, task Task) bool {
3271+
return provider == "claude" && task.PriorSessionID == ""
3272+
}
3273+
32703274
func (d *Daemon) claudeInitialImages(ctx context.Context, task Task, taskLog *slog.Logger) []agent.InputImage {
32713275
attachments := make([]AttachmentMeta, 0, len(task.IssueAttachments)+len(task.ChatMessageAttachments))
32723276
attachments = append(attachments, task.IssueAttachments...)
@@ -3280,10 +3284,10 @@ func (d *Daemon) claudeInitialImages(ctx context.Context, task Task, taskLog *sl
32803284
if len(images) >= maxClaudeInitialImages {
32813285
break
32823286
}
3283-
if attachment.ID == "" || !strings.HasPrefix(strings.ToLower(attachment.ContentType), "image/") {
3287+
if attachment.ID == "" || !isClaudeInitialImageMediaType(attachment.ContentType) {
32843288
continue
32853289
}
3286-
meta, data, err := d.client.DownloadAttachment(ctx, attachment.ID)
3290+
_, data, err := d.client.DownloadAttachment(ctx, attachment.ID)
32873291
if err != nil {
32883292
taskLog.Warn("claude initial image download failed", "attachment_id", attachment.ID, "error", err)
32893293
continue
@@ -3293,16 +3297,7 @@ func (d *Daemon) claudeInitialImages(ctx context.Context, task Task, taskLog *sl
32933297
continue
32943298
}
32953299
mediaType := attachment.ContentType
3296-
if meta != nil && meta.ContentType != "" {
3297-
mediaType = meta.ContentType
3298-
}
3299-
if !strings.HasPrefix(strings.ToLower(mediaType), "image/") {
3300-
continue
3301-
}
33023300
filename := attachment.Filename
3303-
if meta != nil && meta.Filename != "" {
3304-
filename = meta.Filename
3305-
}
33063301
images = append(images, agent.InputImage{
33073302
Filename: filename,
33083303
MediaType: mediaType,
@@ -3312,6 +3307,15 @@ func (d *Daemon) claudeInitialImages(ctx context.Context, task Task, taskLog *sl
33123307
return images
33133308
}
33143309

3310+
func isClaudeInitialImageMediaType(mediaType string) bool {
3311+
switch strings.ToLower(strings.TrimSpace(mediaType)) {
3312+
case "image/jpeg", "image/png", "image/gif", "image/webp":
3313+
return true
3314+
default:
3315+
return false
3316+
}
3317+
}
3318+
33153319
// gateResumeToReusedWorkdir clears the task's prior session unless the task
33163320
// runs in the exact workdir the session was recorded against, and reports
33173321
// whether that workdir was reused. CLI backends key their session stores to
@@ -3972,7 +3976,7 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i
39723976
ThinkingLevel: thinkingLevel,
39733977
OpenclawMode: openclawMode,
39743978
}
3975-
if provider == "claude" {
3979+
if shouldSeedClaudeInitialImages(provider, task) {
39763980
execOpts.InitialImages = d.claudeInitialImages(ctx, task, taskLog)
39773981
}
39783982
// Some providers do not reliably load the per-task runtime config files we

server/internal/daemon/daemon_test.go

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -908,15 +908,7 @@ func TestClaudeInitialImagesDownloadsImageAttachments(t *testing.T) {
908908

909909
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
910910
switch r.URL.Path {
911-
case "/api/attachments/img-1":
912-
w.Header().Set("Content-Type", "application/json")
913-
json.NewEncoder(w).Encode(map[string]any{
914-
"id": "img-1",
915-
"download_url": "/files/img-1",
916-
"filename": "server-shot.png",
917-
"content_type": "image/png",
918-
})
919-
case "/files/img-1":
911+
case "/api/attachments/img-1/download":
920912
w.Write([]byte("image-bytes"))
921913
default:
922914
http.NotFound(w, r)
@@ -936,11 +928,52 @@ func TestClaudeInitialImagesDownloadsImageAttachments(t *testing.T) {
936928
t.Fatalf("initial images length = %d, want 1", len(images))
937929
}
938930
got := images[0]
939-
if got.Filename != "server-shot.png" || got.MediaType != "image/png" || string(got.Data) != "image-bytes" {
931+
if got.Filename != "client-shot.png" || got.MediaType != "image/png" || string(got.Data) != "image-bytes" {
940932
t.Fatalf("unexpected initial image: %+v data=%q", got, string(got.Data))
941933
}
942934
}
943935

936+
func TestClaudeInitialImagesSkipsUnsupportedImageTypes(t *testing.T) {
937+
t.Parallel()
938+
939+
var downloadCalled atomic.Bool
940+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
941+
downloadCalled.Store(true)
942+
w.Write([]byte("image-bytes"))
943+
}))
944+
t.Cleanup(srv.Close)
945+
946+
d := &Daemon{client: NewClient(srv.URL), logger: slog.Default()}
947+
images := d.claudeInitialImages(context.Background(), Task{
948+
IssueAttachments: []AttachmentMeta{
949+
{ID: "svg-1", Filename: "diagram.svg", ContentType: "image/svg+xml"},
950+
{ID: "bmp-1", Filename: "bitmap.bmp", ContentType: "image/bmp"},
951+
{ID: "ico-1", Filename: "favicon.ico", ContentType: "image/x-icon"},
952+
},
953+
}, slog.New(slog.NewTextHandler(io.Discard, nil)))
954+
955+
if len(images) != 0 {
956+
t.Fatalf("initial images length = %d, want 0", len(images))
957+
}
958+
if downloadCalled.Load() {
959+
t.Fatal("unsupported image attachments should be skipped before download")
960+
}
961+
}
962+
963+
func TestShouldSeedClaudeInitialImagesOnlyForColdClaudeTasks(t *testing.T) {
964+
t.Parallel()
965+
966+
if !shouldSeedClaudeInitialImages("claude", Task{}) {
967+
t.Fatal("cold Claude task should seed initial images")
968+
}
969+
if shouldSeedClaudeInitialImages("claude", Task{PriorSessionID: "sess-123"}) {
970+
t.Fatal("resumed Claude task should not seed initial images")
971+
}
972+
if shouldSeedClaudeInitialImages("codex", Task{}) {
973+
t.Fatal("non-Claude task should not seed initial images")
974+
}
975+
}
976+
944977
func TestMergeUsage(t *testing.T) {
945978
t.Parallel()
946979

0 commit comments

Comments
 (0)