diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go index 38967832c..a61f56413 100644 --- a/scanner/phase_1_folders.go +++ b/scanner/phase_1_folders.go @@ -50,13 +50,14 @@ func createPhaseFolders(ctx context.Context, state *scanState, ds model.DataStor } type scanJob struct { - lib model.Library - fs storage.MusicFS - cw artwork.CacheWarmer - lastUpdates map[string]model.FolderUpdateInfo // Holds last update info for all (DB) folders in this library - targetFolders []string // Specific folders to scan (including all descendants) - lock sync.Mutex - numFolders atomic.Int64 + lib model.Library + fs storage.MusicFS + cw artwork.CacheWarmer + lastUpdates map[string]model.FolderUpdateInfo // Holds last update info for all (DB) folders in this library + targetFolders []string // Specific folders to scan (including all descendants) + lock sync.Mutex + numFolders atomic.Int64 + visitedRealPaths map[string]struct{} // Real paths visited, used to detect symlink cycles } func newScanJob(ctx context.Context, ds model.DataStore, cw artwork.CacheWarmer, lib model.Library, fullScan bool, targetFolders []string) (*scanJob, error) { @@ -83,11 +84,12 @@ func newScanJob(ctx context.Context, ds model.DataStore, cw artwork.CacheWarmer, lib.FullScanInProgress = lib.FullScanInProgress || fullScan return &scanJob{ - lib: lib, - fs: fsys, - cw: cw, - lastUpdates: lastUpdates, - targetFolders: targetFolders, + lib: lib, + fs: fsys, + cw: cw, + lastUpdates: lastUpdates, + targetFolders: targetFolders, + visitedRealPaths: make(map[string]struct{}), }, nil } diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index e6a694f2b..dbe511ffb 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -5,6 +5,7 @@ import ( "io/fs" "maps" "path" + "path/filepath" "slices" "sort" "strings" @@ -115,6 +116,13 @@ func loadDir(ctx context.Context, job *scanJob, dirPath string, checker *IgnoreC return folder, children, err } + // Mark current dir's real path as visited to detect symlink cycles + if conf.Server.Scanner.FollowSymlinks && job.visitedRealPaths != nil { + if realPath, e := filepath.EvalSymlinks(filepath.Join(job.lib.Path, dirPath)); e == nil { + job.visitedRealPaths[realPath] = struct{}{} + } + } + entries := fullReadDir(ctx, dirFile) children = make([]string, 0, len(entries)) for _, entry := range entries { @@ -136,6 +144,15 @@ func loadDir(ctx context.Context, job *scanJob, dirPath string, checker *IgnoreC continue } if isDir && !isDirIgnored(entry.Name()) && isDirReadable(ctx, job.fs, entryPath) { + if entry.Type()&fs.ModeSymlink != 0 && job.visitedRealPaths != nil { + realPath, e := filepath.EvalSymlinks(filepath.Join(job.lib.Path, entryPath)) + if e == nil { + if _, seen := job.visitedRealPaths[realPath]; seen { + log.Warn(ctx, "Scanner: Skipping symlink cycle", "path", entryPath) + continue + } + } + } children = append(children, entryPath) folder.numSubFolders++ } else { diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index 42b7af7ba..84063bbdf 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -48,8 +48,9 @@ var _ = Describe("walk_dir_tree", func() { }, } job = &scanJob{ - fs: fsys, - lib: model.Library{Path: "/music"}, + fs: fsys, + lib: model.Library{Path: "/music"}, + visitedRealPaths: make(map[string]struct{}), } }) @@ -120,8 +121,9 @@ var _ = Describe("walk_dir_tree", func() { }, } job = &scanJob{ - fs: fsys, - lib: model.Library{Path: "/music"}, + fs: fsys, + lib: model.Library{Path: "/music"}, + visitedRealPaths: make(map[string]struct{}), } }) @@ -216,6 +218,47 @@ var _ = Describe("walk_dir_tree", func() { Expect(job.lastUpdates).To(HaveKey(model.FolderID(job.lib, "OtherArtist/Album3"))) }) }) + + Context("with symlink cycles", func() { + var tmpDir string + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.Scanner.FollowSymlinks = true + ctx = GinkgoT().Context() + + var err error + tmpDir, err = os.MkdirTemp("", "navidrome-test-*") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(func() { os.RemoveAll(tmpDir) }) + + // Structure: tmpDir/tracks/track.mp3, tmpDir/tracks/loop -> tmpDir (cycle back to root) + tracksDir := filepath.Join(tmpDir, "tracks") + Expect(os.MkdirAll(tracksDir, 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(tracksDir, "track.mp3"), []byte{}, 0600)).To(Succeed()) + Expect(os.Symlink(tmpDir, filepath.Join(tracksDir, "loop"))).To(Succeed()) + + job = &scanJob{ + fs: &localMusicFS{base: os.DirFS(tmpDir)}, + lib: model.Library{Path: tmpDir}, + visitedRealPaths: make(map[string]struct{}), + } + }) + + It("should not follow cyclic symlinks", func() { + results, err := walkDirTree(ctx, job) + Expect(err).ToNot(HaveOccurred()) + + folders := map[string]*folderEntry{} + for folder := range results { + folders[folder.path] = folder + } + + Expect(folders).To(HaveKey(".")) + Expect(folders).To(HaveKey("tracks")) + Expect(folders).ToNot(HaveKey("tracks/loop")) + }) + }) }) Describe("helper functions", func() { @@ -353,6 +396,16 @@ func (fd *fakeDirFile) ReadDir(int) ([]fs.DirEntry, error) { return dirs, nil } +// localMusicFS wraps an os.DirFS for tests that need real OS symlinks +type localMusicFS struct { + storage.MusicFS // zero value; ReadTags not called in these tests + base fs.FS +} + +func (l *localMusicFS) Open(name string) (fs.File, error) { + return l.base.Open(name) +} + func getDirEntry(baseDir, name string) os.DirEntry { dirEntries, _ := os.ReadDir(baseDir) for _, entry := range dirEntries {