diff --git a/core/storage/local/local.go b/core/storage/local/local.go index 686838565..e2ce00a1b 100644 --- a/core/storage/local/local.go +++ b/core/storage/local/local.go @@ -76,6 +76,17 @@ func (lfs *localFS) ResolveSymlink(name string) (string, error) { return filepath.EvalSymlinks(filepath.Join(lfs.root, filepath.FromSlash(name))) } +// ReadLink and Lstat implement fs.ReadLinkFS, so callers can detect symlinks without following them. +var _ fs.ReadLinkFS = (*localFS)(nil) + +func (lfs *localFS) ReadLink(name string) (string, error) { + return fs.ReadLink(lfs.FS, name) +} + +func (lfs *localFS) Lstat(name string) (fs.FileInfo, error) { + return fs.Lstat(lfs.FS, name) +} + func (lfs *localFS) ReadTags(path ...string) (map[string]metadata.Info, error) { res, err := lfs.extractor.Parse(path...) if err != nil { diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index 1864a6a44..d39cad5f6 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -43,6 +43,13 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<- continue } + // A full walk never descends into symlinked folders when following is disabled, so a + // target reached through one (e.g. a watcher event for a new link) is skipped too. + if !conf.Server.Scanner.FollowSymlinks && isSymlinkedPath(job.fs, folderPath) { + log.Debug(ctx, "Scanner: Skipping symlinked target folder, following is disabled", "path", folderPath) + continue + } + // Create checker and push patterns from root to this folder checker := newIgnoreChecker(job.fs) err = checker.PushAllParents(ctx, folderPath) @@ -225,6 +232,18 @@ func isDirOrSymlinkToDir(fsys fs.FS, baseDir string, dirEnt fs.DirEntry) (bool, return fileInfo.IsDir(), nil } +// isSymlinkedPath returns true if folderPath, or any of its parent folders, is a symbolic link. +// It needs fsys to implement fs.ReadLinkFS, otherwise links are followed and never detected. +func isSymlinkedPath(fsys fs.FS, folderPath string) bool { + for p := path.Clean(folderPath); p != "." && p != "/"; p = path.Dir(p) { + info, err := fs.Lstat(fsys, p) + if err == nil && info.Mode()&fs.ModeSymlink != 0 { + return true + } + } + return false +} + const maxSymlinkHops = 40 // resolveEntryName returns the name to classify the entry by, and whether to diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index 43939e5c2..0c2aba3e0 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -4,8 +4,10 @@ import ( "context" "fmt" "io/fs" + "maps" "os" "path/filepath" + "slices" "testing/fstest" "github.com/navidrome/navidrome/conf" @@ -260,6 +262,44 @@ var _ = Describe("walk_dir_tree", func() { // Folders not in targets should remain in lastUpdates Expect(job.lastUpdates).To(HaveKey(model.FolderID(job.lib, "OtherArtist/Album3"))) }) + + // #6292: a watcher event for a new folder symlink makes the link itself a scan target + Context("symlinked target folders (production local storage FS)", func() { + BeforeEach(func() { + libRoot := GinkgoT().TempDir() + Expect(os.MkdirAll(filepath.Join(libRoot, "Mozart", "Album1"), 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libRoot, "Mozart", "Album1", "track.mp3"), []byte("AUDIO"), 0600)).To(Succeed()) + Expect(os.Symlink("Mozart", filepath.Join(libRoot, "Wolfgang Amadeus Mozart"))).To(Succeed()) + job = &scanJob{fs: newLocalMusicFS(libRoot), lib: model.Library{Path: libRoot}} + }) + + walkTargets := func(targets ...string) map[string]*folderEntry { + results, err := walkDirTree(ctx, job, targets...) + Expect(err).ToNot(HaveOccurred()) + folders := map[string]*folderEntry{} + for folder := range results { + folders[folder.path] = folder + } + return folders + } + + DescribeTable("with FollowSymlinks disabled", + func(target string, expected ...string) { + conf.Server.Scanner.FollowSymlinks = false + Expect(slices.Collect(maps.Keys(walkTargets(target)))).To(ConsistOf(expected)) + }, + Entry("skips a target that is a symlink", "Wolfgang Amadeus Mozart"), + Entry("skips a target under a symlinked folder", "Wolfgang Amadeus Mozart/Album1"), + Entry("walks a regular target", "Mozart", "Mozart", "Mozart/Album1"), + ) + + It("walks a symlinked target when FollowSymlinks is enabled", func() { + conf.Server.Scanner.FollowSymlinks = true + folders := walkTargets("Wolfgang Amadeus Mozart") + Expect(folders).To(HaveKey("Wolfgang Amadeus Mozart/Album1")) + Expect(folders["Wolfgang Amadeus Mozart/Album1"].audioFiles).To(HaveKey("track.mp3")) + }) + }) }) }) @@ -433,8 +473,8 @@ var _ = Describe("walk_dir_tree", func() { }) // Regression for #5752: the production localFS must resolve file symlinks. - // It wraps os.DirFS behind the fs.FS interface, so fs.ReadLink-based - // resolution is not available and full OS-level resolution is required. + // fs.ReadLink-based resolution can't follow targets outside the library + // root, so full OS-level resolution is required. Context("production local storage FS", func() { var libRoot string var musicFS storage.MusicFS @@ -460,12 +500,7 @@ var _ = Describe("walk_dir_tree", func() { Expect(os.Symlink(filepath.Join(pool, "mid.wav"), filepath.Join(libRoot, "evil.wav"))).To(Succeed()) Expect(os.Symlink(filepath.Join(pool, "missing.mp3"), filepath.Join(libRoot, "broken.mp3"))).To(Succeed()) - u, err := storage.LocalPathToURL(libRoot) - Expect(err).ToNot(HaveOccurred()) - s, err := storage.For(u.String()) - Expect(err).ToNot(HaveOccurred()) - musicFS, err = s.FS() - Expect(err).ToNot(HaveOccurred()) + musicFS = newLocalMusicFS(libRoot) }) walkRoot := func() *folderEntry { @@ -700,6 +735,17 @@ func getDirEntry(baseDir, name string) os.DirEntry { panic(fmt.Sprintf("Could not find %s in %s", name, baseDir)) } +// newLocalMusicFS returns the production local storage MusicFS rooted at libRoot +func newLocalMusicFS(libRoot string) storage.MusicFS { + u, err := storage.LocalPathToURL(libRoot) + Expect(err).ToNot(HaveOccurred()) + s, err := storage.For(u.String()) + Expect(err).ToNot(HaveOccurred()) + musicFS, err := s.FS() + Expect(err).ToNot(HaveOccurred()) + return musicFS +} + // mockMusicFS is a mock implementation of the MusicFS interface that supports symlinks type mockMusicFS struct { storage.MusicFS