Skip to content

Commit 43a0965

Browse files
committed
fix(daemon): harden Claude initial image seeding
1 parent ab846d2 commit 43a0965

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
@@ -676,18 +676,12 @@ type AttachmentResponse struct {
676676
}
677677

678678
func (c *Client) DownloadAttachment(ctx context.Context, attachmentID string) (*AttachmentResponse, []byte, error) {
679-
var att AttachmentResponse
680-
if err := c.getJSON(ctx, fmt.Sprintf("/api/attachments/%s", attachmentID), &att); err != nil {
681-
return nil, nil, err
682-
}
683-
if att.DownloadURL == "" {
684-
return nil, nil, fmt.Errorf("attachment %s has no download_url", attachmentID)
685-
}
686-
data, err := c.downloadFile(ctx, att.DownloadURL)
679+
path := fmt.Sprintf("/api/attachments/%s/download", attachmentID)
680+
data, err := c.downloadFile(ctx, path)
687681
if err != nil {
688682
return nil, nil, err
689683
}
690-
return &att, data, nil
684+
return &AttachmentResponse{ID: attachmentID}, data, nil
691685
}
692686

693687
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
@@ -240,28 +240,19 @@ func TestDefaultTerminalRetrySchedule_MatchesAgreedPlan(t *testing.T) {
240240
}
241241
}
242242

243-
func TestClient_DownloadAttachmentDownloadsRelativeURLWithAuthHeaders(t *testing.T) {
243+
func TestClient_DownloadAttachmentUsesDownloadEndpointWithAuthHeaders(t *testing.T) {
244244
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
245245
switch r.URL.Path {
246-
case "/api/attachments/att-1":
247-
if got := r.Header.Get("Authorization"); got != "Bearer tok" {
248-
t.Errorf("metadata request Authorization = %q, want Bearer tok", got)
249-
}
250-
w.Header().Set("Content-Type", "application/json")
251-
json.NewEncoder(w).Encode(map[string]any{
252-
"id": "att-1",
253-
"download_url": "/download/att-1",
254-
"filename": "shot.png",
255-
"content_type": "image/png",
256-
})
257-
case "/download/att-1":
246+
case "/api/attachments/att-1/download":
258247
if got := r.Header.Get("Authorization"); got != "Bearer tok" {
259-
t.Errorf("download request Authorization = %q, want Bearer tok", got)
248+
t.Errorf("attachment download Authorization = %q, want Bearer tok", got)
260249
}
261250
if got := r.Header.Get("X-Client-Platform"); got != "daemon" {
262-
t.Errorf("download request X-Client-Platform = %q, want daemon", got)
251+
t.Errorf("attachment download X-Client-Platform = %q, want daemon", got)
263252
}
264253
w.Write([]byte("png-bytes"))
254+
case "/api/attachments/att-1":
255+
t.Fatal("DownloadAttachment must not fetch member-scoped attachment metadata")
265256
default:
266257
t.Errorf("unexpected path: %s", r.URL.Path)
267258
http.NotFound(w, r)
@@ -276,7 +267,7 @@ func TestClient_DownloadAttachmentDownloadsRelativeURLWithAuthHeaders(t *testing
276267
if err != nil {
277268
t.Fatalf("DownloadAttachment: %v", err)
278269
}
279-
if meta.Filename != "shot.png" || meta.ContentType != "image/png" {
270+
if meta.ID != "att-1" {
280271
t.Fatalf("unexpected attachment metadata: %+v", meta)
281272
}
282273
if string(data) != "png-bytes" {

server/internal/daemon/daemon.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3440,6 +3440,10 @@ const (
34403440
maxClaudeInitialImageBytes = 10 * 1024 * 1024
34413441
)
34423442

3443+
func shouldSeedClaudeInitialImages(provider string, task Task) bool {
3444+
return provider == "claude" && task.PriorSessionID == ""
3445+
}
3446+
34433447
func (d *Daemon) claudeInitialImages(ctx context.Context, task Task, taskLog *slog.Logger) []agent.InputImage {
34443448
attachments := make([]AttachmentMeta, 0, len(task.IssueAttachments)+len(task.ChatMessageAttachments))
34453449
attachments = append(attachments, task.IssueAttachments...)
@@ -3453,10 +3457,10 @@ func (d *Daemon) claudeInitialImages(ctx context.Context, task Task, taskLog *sl
34533457
if len(images) >= maxClaudeInitialImages {
34543458
break
34553459
}
3456-
if attachment.ID == "" || !strings.HasPrefix(strings.ToLower(attachment.ContentType), "image/") {
3460+
if attachment.ID == "" || !isClaudeInitialImageMediaType(attachment.ContentType) {
34573461
continue
34583462
}
3459-
meta, data, err := d.client.DownloadAttachment(ctx, attachment.ID)
3463+
_, data, err := d.client.DownloadAttachment(ctx, attachment.ID)
34603464
if err != nil {
34613465
taskLog.Warn("claude initial image download failed", "attachment_id", attachment.ID, "error", err)
34623466
continue
@@ -3466,16 +3470,7 @@ func (d *Daemon) claudeInitialImages(ctx context.Context, task Task, taskLog *sl
34663470
continue
34673471
}
34683472
mediaType := attachment.ContentType
3469-
if meta != nil && meta.ContentType != "" {
3470-
mediaType = meta.ContentType
3471-
}
3472-
if !strings.HasPrefix(strings.ToLower(mediaType), "image/") {
3473-
continue
3474-
}
34753473
filename := attachment.Filename
3476-
if meta != nil && meta.Filename != "" {
3477-
filename = meta.Filename
3478-
}
34793474
images = append(images, agent.InputImage{
34803475
Filename: filename,
34813476
MediaType: mediaType,
@@ -3485,6 +3480,15 @@ func (d *Daemon) claudeInitialImages(ctx context.Context, task Task, taskLog *sl
34853480
return images
34863481
}
34873482

3483+
func isClaudeInitialImageMediaType(mediaType string) bool {
3484+
switch strings.ToLower(strings.TrimSpace(mediaType)) {
3485+
case "image/jpeg", "image/png", "image/gif", "image/webp":
3486+
return true
3487+
default:
3488+
return false
3489+
}
3490+
}
3491+
34883492
// gateResumeToReusedWorkdir clears the task's prior session unless the task
34893493
// runs in the exact workdir the session was recorded against, and reports
34903494
// whether that workdir was reused. CLI backends key their session stores to
@@ -4135,7 +4139,7 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i
41354139
ThinkingLevel: thinkingLevel,
41364140
OpenclawMode: openclawMode,
41374141
}
4138-
if provider == "claude" {
4142+
if shouldSeedClaudeInitialImages(provider, task) {
41394143
execOpts.InitialImages = d.claudeInitialImages(ctx, task, taskLog)
41404144
}
41414145
// 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
@@ -903,15 +903,7 @@ func TestClaudeInitialImagesDownloadsImageAttachments(t *testing.T) {
903903

904904
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
905905
switch r.URL.Path {
906-
case "/api/attachments/img-1":
907-
w.Header().Set("Content-Type", "application/json")
908-
json.NewEncoder(w).Encode(map[string]any{
909-
"id": "img-1",
910-
"download_url": "/files/img-1",
911-
"filename": "server-shot.png",
912-
"content_type": "image/png",
913-
})
914-
case "/files/img-1":
906+
case "/api/attachments/img-1/download":
915907
w.Write([]byte("image-bytes"))
916908
default:
917909
http.NotFound(w, r)
@@ -931,11 +923,52 @@ func TestClaudeInitialImagesDownloadsImageAttachments(t *testing.T) {
931923
t.Fatalf("initial images length = %d, want 1", len(images))
932924
}
933925
got := images[0]
934-
if got.Filename != "server-shot.png" || got.MediaType != "image/png" || string(got.Data) != "image-bytes" {
926+
if got.Filename != "client-shot.png" || got.MediaType != "image/png" || string(got.Data) != "image-bytes" {
935927
t.Fatalf("unexpected initial image: %+v data=%q", got, string(got.Data))
936928
}
937929
}
938930

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

0 commit comments

Comments
 (0)