feat(scanner): add Scanner.FollowSymlinks option (#4061)

* Add Scanner.FollowSymlinks option (default: true) - Fixes #4060

* fix(mockMusicFS): improve symlink handling in Open, Stat, and ReadDir methods

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

* refactor(tests): enhance walkDirTree tests with symlink handling and cleanup

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

---------

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

conf/configuration.go

@@ -132,6 +132,7 @@ type scannerOptions struct {
 	ArtistJoiner       string
 	GenreSeparators    string // Deprecated: Use Tags.genre.Split instead
 	GroupAlbumReleases bool   // Deprecated: Use PID.Album instead
+	FollowSymlinks     bool   // Whether to follow symlinks when scanning directories
 }
 
 type subsonicOptions struct {
@@ -499,6 +500,7 @@ func init() {
 	viper.SetDefault("scanner.artistjoiner", consts.ArtistJoiner)
 	viper.SetDefault("scanner.genreseparators", "")
 	viper.SetDefault("scanner.groupalbumreleases", false)
+	viper.SetDefault("scanner.followsymlinks", true)
 
 	viper.SetDefault("subsonic.appendsubtitle", true)
 	viper.SetDefault("subsonic.artistparticipations", false)

scanner/walk_dir_tree.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/core"
 	"github.com/navidrome/navidrome/log"
@@ -266,6 +267,10 @@ func isDirOrSymlinkToDir(fsys fs.FS, baseDir string, dirEnt fs.DirEntry) (bool,
 	if dirEnt.Type()&fs.ModeSymlink == 0 {
 		return false, nil
 	}
+	// If symlinks are disabled, return false for symlinks
+	if !conf.Server.Scanner.FollowSymlinks {
+		return false, nil
+	}
 	// Does this symlink point to a directory?
 	fileInfo, err := fs.Stat(fsys, path.Join(baseDir, dirEnt.Name()))
 	if err != nil {

scanner/walk_dir_tree_test.go

@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"testing/fstest"
 
+	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/conf/configtest"
 	"github.com/navidrome/navidrome/core/storage"
 	"github.com/navidrome/navidrome/model"
 	. "github.com/onsi/ginkgo/v2"
@@ -17,8 +19,15 @@ import (
 
 var _ = Describe("walk_dir_tree", func() {
 	Describe("walkDirTree", func() {
-		var fsys storage.MusicFS
+		var (
+			fsys storage.MusicFS
+			job  *scanJob
+			ctx  context.Context
+		)
+
 		BeforeEach(func() {
+			DeferCleanup(configtest.SetupConfig())
+			ctx = GinkgoT().Context()
 			fsys = &mockMusicFS{
 				FS: fstest.MapFS{
 					"root/a/.ndignore":       {Data: []byte("ignored/*")},
@@ -32,21 +41,22 @@ var _ = Describe("walk_dir_tree", func() {
 					"root/d/f1.mp3":          {},
 					"root/d/f2.mp3":          {},
 					"root/d/f3.mp3":          {},
+					"root/e/original/f1.mp3": {},
+					"root/e/symlink":         {Mode: fs.ModeSymlink, Data: []byte("root/e/original")},
 				},
 			}
-		})
+			job = &scanJob{
-
-		It("walks all directories", func() {
-			job := &scanJob{
 				fs:  fsys,
 				lib: model.Library{Path: "/music"},
 			}
-			ctx := context.Background()
+		})
+
+		// Helper function to call walkDirTree and collect folders from the results channel
+		getFolders := func() map[string]*folderEntry {
 			results, err := walkDirTree(ctx, job)
 			Expect(err).ToNot(HaveOccurred())
 
 			folders := map[string]*folderEntry{}
-
 			g := errgroup.Group{}
 			g.Go(func() error {
 				for folder := range results {
@@ -55,9 +65,17 @@ var _ = Describe("walk_dir_tree", func() {
 				return nil
 			})
 			_ = g.Wait()
+			return folders
+		}
 
-			Expect(folders).To(HaveLen(6))
+		DescribeTable("symlink handling",
-			Expect(folders["root/a/ignored"].audioFiles).To(BeEmpty())
+			func(followSymlinks bool, expectedFolderCount int) {
+				conf.Server.Scanner.FollowSymlinks = followSymlinks
+				folders := getFolders()
+
+				Expect(folders).To(HaveLen(expectedFolderCount + 2)) // +2 for `.` and `root`
+
+				// Basic folder structure checks
 				Expect(folders["root/a"].audioFiles).To(SatisfyAll(
 					HaveLen(2),
 					HaveKey("f1.mp3"),
@@ -72,7 +90,17 @@ var _ = Describe("walk_dir_tree", func() {
 				Expect(folders["root/c"].audioFiles).To(BeEmpty())
 				Expect(folders["root/c"].imageFiles).To(BeEmpty())
 				Expect(folders).ToNot(HaveKey("root/d"))
-		})
+
+				// Symlink specific checks
+				if followSymlinks {
+					Expect(folders["root/e/symlink"].audioFiles).To(HaveLen(1))
+				} else {
+					Expect(folders).ToNot(HaveKey("root/e/symlink"))
+				}
+			},
+			Entry("with symlinks enabled", true, 7),
+			Entry("with symlinks disabled", false, 6),
+		)
 	})
 
 	Describe("helper functions", func() {
@@ -81,74 +109,88 @@ var _ = Describe("walk_dir_tree", func() {
 		baseDir := filepath.Join("tests", "fixtures")
 
 		Describe("isDirOrSymlinkToDir", func() {
-			It("returns true for normal dirs", func() {
+			BeforeEach(func() {
-				dirEntry := getDirEntry("tests", "fixtures")
+				DeferCleanup(configtest.SetupConfig())
-				Expect(isDirOrSymlinkToDir(fsys, baseDir, dirEntry)).To(BeTrue())
 			})
-			It("returns true for symlinks to dirs", func() {
+
-				dirEntry := getDirEntry(baseDir, "symlink2dir")
+			Context("with symlinks enabled", func() {
-				Expect(isDirOrSymlinkToDir(fsys, baseDir, dirEntry)).To(BeTrue())
+				BeforeEach(func() {
+					conf.Server.Scanner.FollowSymlinks = true
 				})
-			It("returns false for files", func() {
+
-				dirEntry := getDirEntry(baseDir, "test.mp3")
+				DescribeTable("returns expected result",
-				Expect(isDirOrSymlinkToDir(fsys, baseDir, dirEntry)).To(BeFalse())
+					func(dirName string, expected bool) {
+						dirEntry := getDirEntry("tests/fixtures", dirName)
+						Expect(isDirOrSymlinkToDir(fsys, baseDir, dirEntry)).To(Equal(expected))
+					},
+					Entry("normal dir", "empty_folder", true),
+					Entry("symlink to dir", "symlink2dir", true),
+					Entry("regular file", "test.mp3", false),
+					Entry("symlink to file", "symlink", false),
+				)
 			})
-			It("returns false for symlinks to files", func() {
+
-				dirEntry := getDirEntry(baseDir, "symlink")
+			Context("with symlinks disabled", func() {
-				Expect(isDirOrSymlinkToDir(fsys, baseDir, dirEntry)).To(BeFalse())
+				BeforeEach(func() {
-			})
+					conf.Server.Scanner.FollowSymlinks = false
-		})
-		Describe("isDirIgnored", func() {
-			It("returns false for normal dirs", func() {
-				Expect(isDirIgnored("empty_folder")).To(BeFalse())
-			})
-			It("returns true when folder name starts with a `.`", func() {
-				Expect(isDirIgnored(".hidden_folder")).To(BeTrue())
-			})
-			It("returns false when folder name starts with ellipses", func() {
-				Expect(isDirIgnored("...unhidden_folder")).To(BeFalse())
-			})
-			It("returns true when folder name is $Recycle.Bin", func() {
-				Expect(isDirIgnored("$Recycle.Bin")).To(BeTrue())
-			})
-			It("returns true when folder name is #snapshot", func() {
-				Expect(isDirIgnored("#snapshot")).To(BeTrue())
 				})
+
+				DescribeTable("returns expected result",
+					func(dirName string, expected bool) {
+						dirEntry := getDirEntry("tests/fixtures", dirName)
+						Expect(isDirOrSymlinkToDir(fsys, baseDir, dirEntry)).To(Equal(expected))
+					},
+					Entry("normal dir", "empty_folder", true),
+					Entry("symlink to dir", "symlink2dir", false),
+					Entry("regular file", "test.mp3", false),
+					Entry("symlink to file", "symlink", false),
+				)
 			})
 		})
 
+		Describe("isDirIgnored", func() {
+			DescribeTable("returns expected result",
+				func(dirName string, expected bool) {
+					Expect(isDirIgnored(dirName)).To(Equal(expected))
+				},
+				Entry("normal dir", "empty_folder", false),
+				Entry("hidden dir", ".hidden_folder", true),
+				Entry("dir starting with ellipsis", "...unhidden_folder", false),
+				Entry("recycle bin", "$Recycle.Bin", true),
+				Entry("snapshot dir", "#snapshot", true),
+			)
+		})
+
 		Describe("fullReadDir", func() {
-		var fsys fakeFS
+			var (
-		var ctx context.Context
+				fsys fakeFS
+				ctx  context.Context
+			)
+
 			BeforeEach(func() {
-			ctx = context.Background()
+				ctx = GinkgoT().Context()
 				fsys = fakeFS{MapFS: fstest.MapFS{
 					"root/a/f1": {},
 					"root/b/f2": {},
 					"root/c/f3": {},
 				}}
 			})
-		It("reads all entries", func() {
+
+			DescribeTable("reading directory entries",
+				func(failOn string, expectedErr error, expectedNames []string) {
+					fsys.failOn = failOn
+					fsys.err = expectedErr
 					dir, _ := fsys.Open("root")
 					entries := fullReadDir(ctx, dir.(fs.ReadDirFile))
-			Expect(entries).To(HaveLen(3))
+					Expect(entries).To(HaveLen(len(expectedNames)))
-			Expect(entries[0].Name()).To(Equal("a"))
+					for i, name := range expectedNames {
-			Expect(entries[1].Name()).To(Equal("b"))
+						Expect(entries[i].Name()).To(Equal(name))
-			Expect(entries[2].Name()).To(Equal("c"))
+					}
-		})
+				},
-		It("skips entries with permission error", func() {
+				Entry("reads all entries", "", nil, []string{"a", "b", "c"}),
-			fsys.failOn = "b"
+				Entry("skips entries with permission error", "b", nil, []string{"a", "c"}),
-			dir, _ := fsys.Open("root")
+				Entry("aborts on fs.ErrNotExist", "", fs.ErrNotExist, []string{}),
-			entries := fullReadDir(ctx, dir.(fs.ReadDirFile))
+			)
-			Expect(entries).To(HaveLen(2))
-			Expect(entries[0].Name()).To(Equal("a"))
-			Expect(entries[1].Name()).To(Equal("c"))
-		})
-		It("aborts if it keeps getting 'readdirent: no such file or directory'", func() {
-			fsys.err = fs.ErrNotExist
-			dir, _ := fsys.Open("root")
-			entries := fullReadDir(ctx, dir.(fs.ReadDirFile))
-			Expect(entries).To(BeEmpty())
 		})
 	})
 })
@@ -205,11 +247,54 @@ func getDirEntry(baseDir, name string) os.DirEntry {
 	panic(fmt.Sprintf("Could not find %s in %s", name, baseDir))
 }
 
+// mockMusicFS is a mock implementation of the MusicFS interface that supports symlinks
 type mockMusicFS struct {
 	storage.MusicFS
 	fs.FS
 }
 
+// Open resolves symlinks
 func (m *mockMusicFS) Open(name string) (fs.File, error) {
-	return m.FS.Open(name)
+	f, err := m.FS.Open(name)
+	if err != nil {
+		return nil, err
+	}
+
+	info, err := f.Stat()
+	if err != nil {
+		f.Close()
+		return nil, err
+	}
+
+	if info.Mode()&fs.ModeSymlink != 0 {
+		// For symlinks, read the target path from the Data field
+		target := string(m.FS.(fstest.MapFS)[name].Data)
+		f.Close()
+		return m.FS.Open(target)
+	}
+
+	return f, nil
+}
+
+// Stat uses Open to resolve symlinks
+func (m *mockMusicFS) Stat(name string) (fs.FileInfo, error) {
+	f, err := m.Open(name)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	return f.Stat()
+}
+
+// ReadDir uses Open to resolve symlinks
+func (m *mockMusicFS) ReadDir(name string) ([]fs.DirEntry, error) {
+	f, err := m.Open(name)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	if dirFile, ok := f.(fs.ReadDirFile); ok {
+		return dirFile.ReadDir(-1)
+	}
+	return nil, fmt.Errorf("not a directory")
 }