From 1344e6e6601c6820b57cab455de707b64422a6d5 Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Sat, 1 Aug 2026 20:54:13 -0400
Subject: [PATCH] fix(artwork): keep sweeping past a store file that cannot be
 removed

Sweep returned the os.Remove error straight out of WalkDir, so a single
unremovable file abandoned the rest of the walk. Every later orphan, stale mime
variant and abandoned temp file then survived until the next prune, which would
abort at the same place.

Warn and carry on instead. The orphan-file loop in prune already had this
resilience; Sweep is where it belongs, since it is the only step that reaches
stray files and superseded variants.
---
 core/artwork/image_store.go      |  5 ++++-
 core/artwork/image_store_test.go | 30 ++++++++++++++++++++++++++++++
 core/artwork/prune_test.go       |  6 ++----
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/core/artwork/image_store.go b/core/artwork/image_store.go
index 72cb34ceb..5ec2df294 100644
--- a/core/artwork/image_store.go
+++ b/core/artwork/image_store.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/consts"
+	"github.com/navidrome/navidrome/log"
 	"github.com/zeebo/xxh3"
 )
 
@@ -157,7 +158,9 @@ func (s *ImageStore) Sweep(ctx context.Context, cutoff time.Time, keep func(hash
 		if remove {
 			// #nosec G122 -- path comes from WalkDir over our own store root, no attacker-controlled symlinks
 			if err := os.Remove(path); err != nil {
-				return err
+				// One unremovable file must not strand the rest of the store until the next prune.
+				log.Warn(ctx, "Artwork: Could not remove store file", "path", path, err)
+				return nil
 			}
 			removed++
 		}
diff --git a/core/artwork/image_store_test.go b/core/artwork/image_store_test.go
index c2cca399d..975194c6d 100644
--- a/core/artwork/image_store_test.go
+++ b/core/artwork/image_store_test.go
@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/navidrome/navidrome/tests"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
@@ -205,6 +206,35 @@ var _ = Describe("ImageStore", func() {
 		Expect(freshTmp).To(BeAnExistingFile())
 	})
 
+	It("keeps sweeping past a file it cannot remove", func() {
+		tests.SkipOnWindows("uses Unix file permission bits")
+		if os.Geteuid() == 0 {
+			Skip("read-only dir cannot block root (e.g. tests in a container)")
+		}
+		old := time.Now().Add(-2 * time.Hour)
+		// "blocked" sorts before "ok", so the walk hits the unremovable file first.
+		blockedDir := filepath.Join(root, "blocked")
+		Expect(os.MkdirAll(blockedDir, 0755)).To(Succeed())
+		blocked := filepath.Join(blockedDir, "a.jpg")
+		Expect(os.WriteFile(blocked, []byte("x"), 0600)).To(Succeed())
+		Expect(os.Chtimes(blocked, old, old)).To(Succeed())
+
+		okDir := filepath.Join(root, "ok")
+		Expect(os.MkdirAll(okDir, 0755)).To(Succeed())
+		reachable := filepath.Join(okDir, "b.jpg")
+		Expect(os.WriteFile(reachable, []byte("y"), 0600)).To(Succeed())
+		Expect(os.Chtimes(reachable, old, old)).To(Succeed())
+
+		Expect(os.Chmod(blockedDir, 0500)).To(Succeed())
+		DeferCleanup(func() { _ = os.Chmod(blockedDir, 0755) })
+
+		removed, err := store.Sweep(ctx, time.Now().Add(-time.Hour), func(string, string) bool { return false })
+		Expect(err).ToNot(HaveOccurred())
+		Expect(removed).To(Equal(1))
+		Expect(blocked).To(BeAnExistingFile())
+		Expect(reachable).ToNot(BeAnExistingFile())
+	})
+
 	// Prune holds the worker's write lock for the whole sweep, and shutdown waits on the
 	// worker, so an uncancellable walk over a large store stalls it until SIGKILL.
 	It("abandons the walk when the context is cancelled", func() {
diff --git a/core/artwork/prune_test.go b/core/artwork/prune_test.go
index 348c7d15e..e65d4b1de 100644
--- a/core/artwork/prune_test.go
+++ b/core/artwork/prune_test.go
@@ -214,11 +214,9 @@ var _ = Describe("Prune", func() {
 		// hb is processed first: aborting on its Remove failure would leave hg unreached.
 		awRepo.OrphanHashes = []string{hb, hg}
 
-		// Prune still errors: Sweep revisits hb's leftover file with no warn-and-continue of its own.
-		err := prune(context.Background(), ds, store)
-		Expect(err).To(HaveOccurred())
+		Expect(prune(context.Background(), ds, store)).To(Succeed())
 
-		_, err = awRepo.GetImage(hg)
+		_, err := awRepo.GetImage(hg)
 		Expect(err).To(MatchError(model.ErrNotFound))
 		_, err = store.Open(hg, "image/jpeg")
 		Expect(os.IsNotExist(err)).To(BeTrue())