From 67f6d8aee810301e471a09bd7c5c5633266edf13 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 22 Jul 2026 12:38:40 -0400 Subject: [PATCH] fix(artwork): cap resolved image reads A user-editable ExternalImageURL can point at an arbitrarily large endpoint; a fast server could make the worker buffer hundreds of MB inside the 5s HTTP timeout. Bound the read to a fixed 20MB cap (no config knob) via io.LimitReader and fail the item if it is exceeded. --- core/artwork/processor.go | 10 +++++++++- core/artwork/processor_test.go | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/core/artwork/processor.go b/core/artwork/processor.go index 8852f70b8..262d06518 100644 --- a/core/artwork/processor.go +++ b/core/artwork/processor.go @@ -31,6 +31,10 @@ const ( // thumbnailSize is the max dimension fed to blurhash. const thumbnailSize = 128 +// maxImageBytes caps a resolved image read: a user-editable ExternalImageURL could +// point at an arbitrarily large endpoint, and 20MB is generous for any real cover. +const maxImageBytes = 20 << 20 + // workerDeps are the collaborators processItem needs; extGate is set by NewWorker in // production and nil only in tests, where resolveItem falls back to a plain passthrough. type workerDeps struct { @@ -60,11 +64,15 @@ func processItem(ctx context.Context, deps *workerDeps, item model.ArtworkQueueI } defer res.reader.Close() - data, err := io.ReadAll(res.reader) + data, err := io.ReadAll(io.LimitReader(res.reader, maxImageBytes+1)) if err != nil { log.Warn(ctx, "artwork: failed to read resolved image", "kind", item.ItemKind, "id", item.ItemID, err) return outcomeFailed } + if len(data) > maxImageBytes { + log.Warn(ctx, "artwork: resolved image exceeds size cap", "kind", item.ItemKind, "id", item.ItemID, "source", res.source, "cap", maxImageBytes) + return outcomeFailed + } log.Debug(ctx, "artwork: read resolved image", "kind", item.ItemKind, "id", item.ItemID, "source", res.source, "bytes", len(data)) hash, err := HashImage(bytes.NewReader(data)) diff --git a/core/artwork/processor_test.go b/core/artwork/processor_test.go index a73cd4fca..64b0181bd 100644 --- a/core/artwork/processor_test.go +++ b/core/artwork/processor_test.go @@ -186,6 +186,27 @@ var _ = Describe("processItem", func() { Expect(err).To(MatchError(model.ErrNotFound)) }) + It("oversized read: a resolved image larger than the cap fails without writing state", func() { + tmpDir := GinkgoT().TempDir() + conf.Server.DataFolder = conf.NewDir(tmpDir) + Expect(os.MkdirAll(filepath.Join(tmpDir, "artwork", "radio"), 0755)).To(Succeed()) + imgPath := filepath.Join(tmpDir, "artwork", "radio", "big_test.jpg") + f, err := os.Create(imgPath) + Expect(err).ToNot(HaveOccurred()) + Expect(f.Truncate(maxImageBytes + 1)).To(Succeed()) + Expect(f.Close()).To(Succeed()) + + radioRepo := tests.CreateMockedRadioRepo() + radioRepo.Data = map[string]*model.Radio{"big": {ID: "big", Name: "Radio", UploadedImage: "big_test.jpg"}} + ds.MockedRadio = radioRepo + + out := processItem(ctx, deps, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "big"}) + Expect(out).To(Equal(outcomeFailed)) + + _, err = artRepo.GetItemArtwork("ra", "big", model.ImageTypePrimary) + Expect(err).To(MatchError(model.ErrNotFound)) + }) + It("store write failure: fails without writing state", func() { ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{ {ID: "al7", Name: "Album", EmbedArtPath: "tests/fixtures/artist/an-album/test.mp3", FolderIDs: []string{"f1"}},