fix(server): enhance artist folder detection with directory traversal (#4151)

* fix: enhance artist folder detection with directory traversal

Enhanced fromArtistFolder function to implement directory traversal fallback for finding artist images. The original implementation only searched in the calculated artist folder, which failed for single album artists where artist.jpg files were not detected.

Changes: Modified fromArtistFolder to search up to 3 directory levels (artist folder + 2 parent levels), extracted findImageInFolder helper function for cleaner code organization, added proper boundary checks to prevent infinite traversal, maintained backward compatibility with existing functionality.

This fix ensures artist.jpg files are properly detected for single album artists while preserving all existing behavior for multi-album artists.

* refactor: address PR review suggestions

Applied review suggestions from gemini-code-assist bot:

- Added maxArtistFolderTraversalDepth constant instead of hardcoded value 3

- Updated error message to mention that parent directories were also searched

- Enhanced test assertion to verify the improved error message

* fix: improve artist folder traversal logic and enhance error logging

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: remove test for special glob characters in artist folder detection

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: add logging for artist image search in folder

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>

core/artwork/reader_artist.go

@@ -20,6 +20,12 @@ import (
 	"github.com/navidrome/navidrome/utils/str"
 )
 
+const (
+	// maxArtistFolderTraversalDepth defines how many directory levels to search
+	// when looking for artist images (artist folder + parent directories)
+	maxArtistFolderTraversalDepth = 3
+)
+
 type artistReader struct {
 	cacheKey
 	a            *artwork
@@ -108,36 +114,52 @@ func (a *artistReader) fromArtistArtPriority(ctx context.Context, priority strin
 
 func fromArtistFolder(ctx context.Context, artistFolder string, pattern string) sourceFunc {
 	return func() (io.ReadCloser, string, error) {
-		fsys := os.DirFS(artistFolder)
+		current := artistFolder
+		for i := 0; i < maxArtistFolderTraversalDepth; i++ {
+			if reader, path, err := findImageInFolder(ctx, current, pattern); err == nil {
+				return reader, path, nil
+			}
+
+			parent := filepath.Dir(current)
+			if parent == current {
+				break
+			}
+			current = parent
+		}
+		return nil, "", fmt.Errorf(`no matches for '%s' in '%s' or its parent directories`, pattern, artistFolder)
+	}
+}
+
+func findImageInFolder(ctx context.Context, folder, pattern string) (io.ReadCloser, string, error) {
+	log.Trace(ctx, "looking for artist image", "pattern", pattern, "folder", folder)
+	fsys := os.DirFS(folder)
 	matches, err := fs.Glob(fsys, pattern)
 	if err != nil {
-			log.Warn(ctx, "Error matching artist image pattern", "pattern", pattern, "folder", artistFolder)
+		log.Warn(ctx, "Error matching artist image pattern", "pattern", pattern, "folder", folder, err)
 		return nil, "", err
 	}
-		if len(matches) == 0 {
+
-			return nil, "", fmt.Errorf(`no matches for '%s' in '%s'`, pattern, artistFolder)
-		}
 	for _, m := range matches {
-			filePath := filepath.Join(artistFolder, m)
 		if !model.IsImageFile(m) {
 			continue
 		}
+		filePath := filepath.Join(folder, m)
 		f, err := os.Open(filePath)
 		if err != nil {
 			log.Warn(ctx, "Could not open cover art file", "file", filePath, err)
-				return nil, "", err
+			continue
 		}
 		return f, filePath, nil
 	}
-		return nil, "", nil
+
-	}
+	return nil, "", fmt.Errorf(`no matches for '%s' in '%s'`, pattern, folder)
 }
 
 func loadArtistFolder(ctx context.Context, ds model.DataStore, albums model.Albums, paths []string) (string, time.Time, error) {
 	if len(albums) == 0 {
 		return "", time.Time{}, nil
 	}
-	libID := albums[0].LibraryID // Just need one of the albums, as they should all be in the same Library
+	libID := albums[0].LibraryID // Just need one of the albums, as they should all be in the same Library - for now! TODO: Support multiple libraries
 
 	folderPath := str.LongestCommonPrefix(paths)
 	if !strings.HasSuffix(folderPath, string(filepath.Separator)) {

core/artwork/reader_artist_test.go

@@ -3,6 +3,8 @@ package artwork
 import (
 	"context"
 	"errors"
+	"io"
+	"os"
 	"path/filepath"
 	"time"
 
@@ -108,6 +110,254 @@ var _ = Describe("artistArtworkReader", func() {
 			})
 		})
 	})
+
+	var _ = Describe("fromArtistFolder", func() {
+		var (
+			ctx      context.Context
+			tempDir  string
+			testFunc sourceFunc
+		)
+
+		BeforeEach(func() {
+			ctx = context.Background()
+			tempDir = GinkgoT().TempDir()
+		})
+
+		When("artist folder contains matching image", func() {
+			BeforeEach(func() {
+				// Create test structure: /temp/artist/artist.jpg
+				artistDir := filepath.Join(tempDir, "artist")
+				Expect(os.MkdirAll(artistDir, 0755)).To(Succeed())
+
+				artistImagePath := filepath.Join(artistDir, "artist.jpg")
+				Expect(os.WriteFile(artistImagePath, []byte("fake image data"), 0600)).To(Succeed())
+
+				testFunc = fromArtistFolder(ctx, artistDir, "artist.*")
+			})
+
+			It("finds and returns the image", func() {
+				reader, path, err := testFunc()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reader).ToNot(BeNil())
+				Expect(path).To(ContainSubstring("artist.jpg"))
+
+				// Verify we can read the content
+				data, err := io.ReadAll(reader)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(string(data)).To(Equal("fake image data"))
+				reader.Close()
+			})
+		})
+
+		When("artist folder is empty but parent contains image", func() {
+			BeforeEach(func() {
+				// Create test structure: /temp/parent/artist.jpg and /temp/parent/artist/album/
+				parentDir := filepath.Join(tempDir, "parent")
+				artistDir := filepath.Join(parentDir, "artist")
+				albumDir := filepath.Join(artistDir, "album")
+				Expect(os.MkdirAll(albumDir, 0755)).To(Succeed())
+
+				// Put artist image in parent directory
+				artistImagePath := filepath.Join(parentDir, "artist.jpg")
+				Expect(os.WriteFile(artistImagePath, []byte("parent image"), 0600)).To(Succeed())
+
+				testFunc = fromArtistFolder(ctx, artistDir, "artist.*")
+			})
+
+			It("finds image in parent directory", func() {
+				reader, path, err := testFunc()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reader).ToNot(BeNil())
+				Expect(path).To(ContainSubstring("parent" + string(filepath.Separator) + "artist.jpg"))
+
+				data, err := io.ReadAll(reader)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(string(data)).To(Equal("parent image"))
+				reader.Close()
+			})
+		})
+
+		When("image is two levels up", func() {
+			BeforeEach(func() {
+				// Create test structure: /temp/grandparent/artist.jpg and /temp/grandparent/parent/artist/
+				grandparentDir := filepath.Join(tempDir, "grandparent")
+				parentDir := filepath.Join(grandparentDir, "parent")
+				artistDir := filepath.Join(parentDir, "artist")
+				Expect(os.MkdirAll(artistDir, 0755)).To(Succeed())
+
+				// Put artist image in grandparent directory
+				artistImagePath := filepath.Join(grandparentDir, "artist.jpg")
+				Expect(os.WriteFile(artistImagePath, []byte("grandparent image"), 0600)).To(Succeed())
+
+				testFunc = fromArtistFolder(ctx, artistDir, "artist.*")
+			})
+
+			It("finds image in grandparent directory", func() {
+				reader, path, err := testFunc()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reader).ToNot(BeNil())
+				Expect(path).To(ContainSubstring("grandparent" + string(filepath.Separator) + "artist.jpg"))
+
+				data, err := io.ReadAll(reader)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(string(data)).To(Equal("grandparent image"))
+				reader.Close()
+			})
+		})
+
+		When("images exist at multiple levels", func() {
+			BeforeEach(func() {
+				// Create test structure with images at multiple levels
+				grandparentDir := filepath.Join(tempDir, "grandparent")
+				parentDir := filepath.Join(grandparentDir, "parent")
+				artistDir := filepath.Join(parentDir, "artist")
+				Expect(os.MkdirAll(artistDir, 0755)).To(Succeed())
+
+				// Put artist images at all levels
+				Expect(os.WriteFile(filepath.Join(artistDir, "artist.jpg"), []byte("artist level"), 0600)).To(Succeed())
+				Expect(os.WriteFile(filepath.Join(parentDir, "artist.jpg"), []byte("parent level"), 0600)).To(Succeed())
+				Expect(os.WriteFile(filepath.Join(grandparentDir, "artist.jpg"), []byte("grandparent level"), 0600)).To(Succeed())
+
+				testFunc = fromArtistFolder(ctx, artistDir, "artist.*")
+			})
+
+			It("prioritizes the closest (artist folder) image", func() {
+				reader, path, err := testFunc()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reader).ToNot(BeNil())
+				Expect(path).To(ContainSubstring("artist" + string(filepath.Separator) + "artist.jpg"))
+
+				data, err := io.ReadAll(reader)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(string(data)).To(Equal("artist level"))
+				reader.Close()
+			})
+		})
+
+		When("pattern matches multiple files", func() {
+			BeforeEach(func() {
+				artistDir := filepath.Join(tempDir, "artist")
+				Expect(os.MkdirAll(artistDir, 0755)).To(Succeed())
+
+				// Create multiple matching files
+				Expect(os.WriteFile(filepath.Join(artistDir, "artist.jpg"), []byte("jpg image"), 0600)).To(Succeed())
+				Expect(os.WriteFile(filepath.Join(artistDir, "artist.png"), []byte("png image"), 0600)).To(Succeed())
+				Expect(os.WriteFile(filepath.Join(artistDir, "artist.txt"), []byte("text file"), 0600)).To(Succeed())
+
+				testFunc = fromArtistFolder(ctx, artistDir, "artist.*")
+			})
+
+			It("returns the first valid image file", func() {
+				reader, path, err := testFunc()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reader).ToNot(BeNil())
+
+				// Should return an image file, not the text file
+				Expect(path).To(SatisfyAny(
+					ContainSubstring("artist.jpg"),
+					ContainSubstring("artist.png"),
+				))
+				Expect(path).ToNot(ContainSubstring("artist.txt"))
+				reader.Close()
+			})
+		})
+
+		When("no matching files exist anywhere", func() {
+			BeforeEach(func() {
+				artistDir := filepath.Join(tempDir, "artist")
+				Expect(os.MkdirAll(artistDir, 0755)).To(Succeed())
+
+				// Create non-matching files
+				Expect(os.WriteFile(filepath.Join(artistDir, "cover.jpg"), []byte("cover image"), 0600)).To(Succeed())
+
+				testFunc = fromArtistFolder(ctx, artistDir, "artist.*")
+			})
+
+			It("returns an error", func() {
+				reader, path, err := testFunc()
+				Expect(err).To(HaveOccurred())
+				Expect(reader).To(BeNil())
+				Expect(path).To(BeEmpty())
+				Expect(err.Error()).To(ContainSubstring("no matches for 'artist.*'"))
+				Expect(err.Error()).To(ContainSubstring("parent directories"))
+			})
+		})
+
+		When("directory traversal reaches filesystem root", func() {
+			BeforeEach(func() {
+				// Start from a shallow directory to test root boundary
+				artistDir := filepath.Join(tempDir, "artist")
+				Expect(os.MkdirAll(artistDir, 0755)).To(Succeed())
+
+				testFunc = fromArtistFolder(ctx, artistDir, "artist.*")
+			})
+
+			It("handles root boundary gracefully", func() {
+				reader, path, err := testFunc()
+				Expect(err).To(HaveOccurred())
+				Expect(reader).To(BeNil())
+				Expect(path).To(BeEmpty())
+				// Should not panic or cause infinite loop
+			})
+		})
+
+		When("file exists but cannot be opened", func() {
+			BeforeEach(func() {
+				artistDir := filepath.Join(tempDir, "artist")
+				Expect(os.MkdirAll(artistDir, 0755)).To(Succeed())
+
+				// Create a file that cannot be opened (permission denied)
+				restrictedFile := filepath.Join(artistDir, "artist.jpg")
+				Expect(os.WriteFile(restrictedFile, []byte("restricted"), 0600)).To(Succeed())
+
+				testFunc = fromArtistFolder(ctx, artistDir, "artist.*")
+			})
+
+			It("logs warning and continues searching", func() {
+				// This test depends on the ability to restrict file permissions
+				// For now, we'll just ensure it doesn't panic and returns appropriate error
+				reader, _, err := testFunc()
+				// The file should be readable in test environment, so this will succeed
+				// In a real scenario with permission issues, it would continue searching
+				if err == nil {
+					Expect(reader).ToNot(BeNil())
+					reader.Close()
+				}
+			})
+		})
+
+		When("single album artist scenario (original issue)", func() {
+			BeforeEach(func() {
+				// Simulate the exact folder structure from the issue:
+				// /music/artist/album1/ (single album)
+				// /music/artist/artist.jpg (artist image that should be found)
+				artistDir := filepath.Join(tempDir, "music", "artist")
+				albumDir := filepath.Join(artistDir, "album1")
+				Expect(os.MkdirAll(albumDir, 0755)).To(Succeed())
+
+				// Create artist.jpg in the artist folder (this was not being found before)
+				artistImagePath := filepath.Join(artistDir, "artist.jpg")
+				Expect(os.WriteFile(artistImagePath, []byte("single album artist image"), 0600)).To(Succeed())
+
+				// The fromArtistFolder is called with the artist folder path
+				testFunc = fromArtistFolder(ctx, artistDir, "artist.*")
+			})
+
+			It("finds artist.jpg in artist folder for single album artist", func() {
+				reader, path, err := testFunc()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reader).ToNot(BeNil())
+				Expect(path).To(ContainSubstring("artist.jpg"))
+				Expect(path).To(ContainSubstring("artist"))
+
+				// Verify the content
+				data, err := io.ReadAll(reader)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(string(data)).To(Equal("single album artist image"))
+				reader.Close()
+			})
+		})
+	})
 })
 
 type fakeFolderRepo struct {