From 2ab7925c7a61f52acfd905331d0d0eb817382f82 Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Mon, 3 Aug 2026 14:51:50 -0400
Subject: [PATCH] fix(artwork): ignore image candidates that cannot be fetched

bestImageURL only rejected URLs that url.Parse itself refuses, which is almost
nothing: relative paths and any scheme parse cleanly. A candidate like
images/big.jpg or ftp://host/big.jpg therefore won on size, and since the
function returns a single URL whose failure ends that agent's turn, the agent's
valid smaller images were never tried.

Two details made it easy to hit rather than theoretical. Size is frequently 0 for
every candidate, and only a strictly larger one replaces the first, so an
unfetchable entry in first position sticks. And these strings come straight from
plugins, where a relative path is an ordinary mistake.

Accept only absolute http/https URLs with a host.
---
 core/artwork/agent_images.go      |  6 +++---
 core/artwork/agent_images_test.go | 36 +++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/core/artwork/agent_images.go b/core/artwork/agent_images.go
index bc4d0735a..c7032b62b 100644
--- a/core/artwork/agent_images.go
+++ b/core/artwork/agent_images.go
@@ -21,8 +21,8 @@ func externalName(name string) string {
 	return str.Clear(name)
 }
 
-// bestImageURL returns the largest parseable image URL, so a malformed candidate never
-// shadows a smaller valid one.
+// bestImageURL returns the largest fetchable image URL. Only one is returned and its failure ends
+// the agent's turn, so an unfetchable candidate must never win: url.Parse alone accepts anything.
 func bestImageURL(imgs []agents.ExternalImage) *url.URL {
 	var best *url.URL
 	var bestSize int
@@ -31,7 +31,7 @@ func bestImageURL(imgs []agents.ExternalImage) *url.URL {
 			continue
 		}
 		u, err := url.Parse(imgs[i].URL)
-		if err != nil {
+		if err != nil || !u.IsAbs() || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" {
 			continue
 		}
 		if best == nil || imgs[i].Size > bestSize {
diff --git a/core/artwork/agent_images_test.go b/core/artwork/agent_images_test.go
index f57b3c19a..716245c93 100644
--- a/core/artwork/agent_images_test.go
+++ b/core/artwork/agent_images_test.go
@@ -110,6 +110,42 @@ var _ = Describe("agent images", func() {
 			Expect(bestImageURL(nil)).To(BeNil())
 			Expect(bestImageURL([]agents.ExternalImage{{URL: "", Size: 5}})).To(BeNil())
 		})
+
+		// Plugins hand these over as free-form strings, and url.Parse accepts them all. An
+		// unfetchable candidate that wins here ends the agent's turn before its valid images run.
+		DescribeTable("skips a candidate that cannot be fetched",
+			func(badURL string) {
+				u := bestImageURL([]agents.ExternalImage{
+					{URL: badURL, Size: 100}, // largest, and first
+					{URL: "https://cdn.example.com/ok.jpg", Size: 10},
+				})
+				Expect(u).ToNot(BeNil())
+				Expect(u.String()).To(Equal("https://cdn.example.com/ok.jpg"))
+			},
+			Entry("a relative path", "images/big.jpg"),
+			Entry("a root-relative path", "/images/big.jpg"),
+			Entry("a scheme we cannot fetch", "ftp://host/big.jpg"),
+			Entry("a scheme-relative URL", "//host/big.jpg"),
+			Entry("a URL with no host", "http:///big.jpg"),
+		)
+
+		// Size is often 0 for every candidate, and only a strictly larger one replaces the first,
+		// so an unfetchable entry in first position would otherwise stick.
+		It("skips an unfetchable first candidate when every Size is zero", func() {
+			u := bestImageURL([]agents.ExternalImage{
+				{URL: "images/rel.jpg"},
+				{URL: "https://cdn.example.com/ok.jpg"},
+			})
+			Expect(u).ToNot(BeNil())
+			Expect(u.String()).To(Equal("https://cdn.example.com/ok.jpg"))
+		})
+
+		It("returns nil when no candidate is fetchable", func() {
+			Expect(bestImageURL([]agents.ExternalImage{
+				{URL: "images/a.jpg", Size: 10},
+				{URL: "ftp://host/b.jpg", Size: 20},
+			})).To(BeNil())
+		})
 	})
 
 	Describe("fetchArtistImage", func() {