diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index d39cad5f6..119edf7a4 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -50,6 +50,15 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<- continue } + // Likewise a full walk never enters a link back into the folders above it, so a target + // reached through one (e.g. a watcher event for playlists/music -> ..) is skipped. + dir := newDirRef(job.fs, folderPath) + ancestors, ok := ancestorsOf(job.fs, dir) + if !ok { + log.Debug(ctx, "Scanner: Skipping target folder reached through a symlink cycle", "path", folderPath) + continue + } + // Create checker and push patterns from root to this folder checker := newIgnoreChecker(job.fs) err = checker.PushAllParents(ctx, folderPath) @@ -59,7 +68,7 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<- } // Recursively walk this folder and all its children - err = walkFolder(ctx, job, folderPath, checker, results) + err = walkFolder(ctx, job, dir, checker, results, ancestors) if utils.IsCtxDone(ctx) { return } @@ -73,28 +82,40 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<- return results, nil } -func walkFolder(ctx context.Context, job *scanJob, currentFolder string, checker *IgnoreChecker, results chan<- *folderEntry) error { +// dirRef is a folder to walk. realPath has all symlinks resolved and identifies the folder +// on disk, so a link back into one of its ancestors can be recognized as a cycle. +type dirRef struct { + path string + realPath string +} + +func walkFolder(ctx context.Context, job *scanJob, dir dirRef, checker *IgnoreChecker, results chan<- *folderEntry, branch map[string]struct{}) error { // Push patterns for this folder onto the stack - _ = checker.Push(ctx, currentFolder) + _ = checker.Push(ctx, dir.path) defer checker.Pop() // Pop patterns when leaving this folder - folder, children, err := loadDir(ctx, job, currentFolder, checker) + // branch holds this folder and the ones above it. Only a link back into it is a cycle, two + // sibling links to the same folder are both walked. + branch[dir.realPath] = struct{}{} + defer delete(branch, dir.realPath) + + folder, children, err := loadDir(ctx, job, dir, checker, branch) if err != nil { - log.Warn(ctx, "Scanner: Error loading dir. Skipping", "path", currentFolder, err) + log.Warn(ctx, "Scanner: Error loading dir. Skipping", "path", dir.path, err) return nil } for _, c := range children { - err := walkFolder(ctx, job, c, checker, results) + err := walkFolder(ctx, job, c, checker, results, branch) if err != nil { return err } } - dir := path.Clean(currentFolder) - log.Trace(ctx, "Scanner: Found directory", " path", dir, "audioFiles", maps.Keys(folder.audioFiles), + cleanPath := path.Clean(dir.path) + log.Trace(ctx, "Scanner: Found directory", " path", cleanPath, "audioFiles", maps.Keys(folder.audioFiles), "images", maps.Keys(folder.imageFiles), "playlists", len(folder.playlistFiles), "imagesUpdatedAt", folder.imagesUpdatedAt, "updTime", folder.updTime, "modTime", folder.modTime, "numChildren", len(children)) - folder.path = dir + folder.path = cleanPath folder.elapsed.Start() select { @@ -105,7 +126,8 @@ func walkFolder(ctx context.Context, job *scanJob, currentFolder string, checker } } -func loadDir(ctx context.Context, job *scanJob, dirPath string, checker *IgnoreChecker) (folder *folderEntry, children []string, err error) { +func loadDir(ctx context.Context, job *scanJob, dir dirRef, checker *IgnoreChecker, branch map[string]struct{}) (folder *folderEntry, children []dirRef, err error) { + dirPath := dir.path // Check if directory exists before creating the folder entry // This is important to avoid removing the folder from lastUpdates if it doesn't exist dirInfo, err := fs.Stat(job.fs, dirPath) @@ -118,20 +140,20 @@ func loadDir(ctx context.Context, job *scanJob, dirPath string, checker *IgnoreC folder = job.createFolderEntry(dirPath) folder.modTime = dirInfo.ModTime() - dir, err := job.fs.Open(dirPath) + dirFile, err := job.fs.Open(dirPath) if err != nil { log.Warn(ctx, "Scanner: Error in Opening directory", "path", dirPath, err) return folder, children, err } - defer dir.Close() - dirFile, ok := dir.(fs.ReadDirFile) + defer dirFile.Close() + readDirFile, ok := dirFile.(fs.ReadDirFile) if !ok { log.Error(ctx, "Not a directory", "path", dirPath) return folder, children, err } - entries := fullReadDir(ctx, dirFile) - children = make([]string, 0, len(entries)) + entries := fullReadDir(ctx, readDirFile) + children = make([]dirRef, 0, len(entries)) for _, entry := range entries { entryPath := path.Join(dirPath, entry.Name()) if checker.ShouldIgnore(ctx, entryPath) { @@ -151,7 +173,13 @@ func loadDir(ctx context.Context, job *scanJob, dirPath string, checker *IgnoreC continue } if isDir && isDirReadable(ctx, job.fs, entryPath) { - children = append(children, entryPath) + child := newChildDirRef(job.fs, dir, entry) + if _, isCycle := branch[child.realPath]; isCycle { + log.Debug(ctx, "Scanner: Skipping symlink pointing back into a folder being scanned", + "path", entryPath, "target", child.realPath) + continue + } + children = append(children, child) folder.numSubFolders++ } else { fileInfo, err := entry.Info() @@ -246,6 +274,79 @@ func isSymlinkedPath(fsys fs.FS, folderPath string) bool { const maxSymlinkHops = 40 +// newDirRef returns the reference for a folder a walk starts at, which may itself be a symlink. +func newDirRef(fsys fs.FS, dirPath string) dirRef { + dir := dirRef{path: dirPath, realPath: path.Clean(dirPath)} + dir.realPath = resolveDirPath(fsys, dir) + return dir +} + +// ancestorsOf returns the resolved paths of the folders above dir up to the library root, or +// false if dir is only reachable through a link back into one of them. +func ancestorsOf(fsys fs.FS, dir dirRef) (map[string]struct{}, bool) { + ancestors := map[string]struct{}{} + for p := path.Clean(dir.path); p != "." && p != "/"; { + p = path.Dir(p) + realPath := newDirRef(fsys, p).realPath + if _, seen := ancestors[realPath]; seen || realPath == dir.realPath { + return nil, false + } + ancestors[realPath] = struct{}{} + } + return ancestors, true +} + +// newChildDirRef returns the reference for a subfolder of dir. Only symlinks need resolving, +// a real subfolder can never point back into its own ancestors. +func newChildDirRef(fsys fs.FS, dir dirRef, entry fs.DirEntry) dirRef { + child := dirRef{ + path: path.Join(dir.path, entry.Name()), + realPath: path.Join(dir.realPath, entry.Name()), + } + if entry.Type()&fs.ModeSymlink != 0 { + child.realPath = resolveDirPath(fsys, child) + } + return child +} + +// resolveDirPath returns dir's path with all symlinks resolved, or dir.realPath if it can't be +// resolved. Without OS-level resolution the chain is followed inside the FS from dir.realPath. +func resolveDirPath(fsys fs.FS, dir dirRef) string { + if resolver, ok := fsys.(storage.SymlinkResolverFS); ok { + target, err := resolver.ResolveSymlink(dir.path) + if err != nil { + return dir.realPath + } + return filepath.ToSlash(target) + } + target, hops := followSymlinkChain(fsys, dir.realPath) + if hops >= maxSymlinkHops { + return dir.realPath + } + return target +} + +// followSymlinkChain follows linkPath with fs.ReadLink and returns the last path reached and +// the hops taken. maxSymlinkHops hops means the chain most likely loops. +func followSymlinkChain(fsys fs.FS, linkPath string) (string, int) { + cur := linkPath + hop := 0 + for ; hop < maxSymlinkHops; hop++ { + target, err := fs.ReadLink(fsys, cur) + if err != nil { + break + } + if path.IsAbs(target) { + // Absolute targets are not valid fs.FS paths, so the next ReadLink fails and + // resolution stops here, leaving cur as the final target. + cur = target + } else { + cur = path.Join(path.Dir(cur), target) + } + } + return cur, hop +} + // resolveEntryName returns the name to classify the entry by, and whether to // consider it at all. Symlinks are resolved to their final target so the caller // classifies by the target's extension, not the link's name. Returns ok=false diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index 0c2aba3e0..7a3f909b2 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -9,6 +9,7 @@ import ( "path/filepath" "slices" "testing/fstest" + "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" @@ -152,6 +153,158 @@ var _ = Describe("walk_dir_tree", func() { ) }) + // See #5334 + Context("with symlink cycles", func() { + // An undetected cycle walks forever, so each walk gets a deadline and a folder cap + // well above what any of these layouts contains. + const walkTimeout = 3 * time.Second + const maxFolders = 25 + + walkFS := func(musicFS storage.MusicFS, libPath string, targetFolders ...string) map[string]*folderEntry { + job := &scanJob{fs: musicFS, lib: model.Library{Path: libPath}} + ctx, cancel := context.WithTimeout(GinkgoT().Context(), walkTimeout) + defer cancel() + + results, err := walkDirTree(ctx, job, targetFolders...) + Expect(err).ToNot(HaveOccurred()) + + folders := map[string]*folderEntry{} + for folder := range results { + folders[folder.path] = folder + if len(folders) >= maxFolders { + cancel() + } + } + Expect(ctx.Err()).ToNot(Equal(context.DeadlineExceeded), "the walk never finished: symlink cycle not detected") + return folders + } + + walk := func(mapFS fstest.MapFS, targetFolders ...string) map[string]*folderEntry { + return walkFS(&mockMusicFS{FS: mapFS}, "/music", targetFolders...) + } + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.Scanner.FollowSymlinks = true + }) + + It("skips a symlink pointing to the parent folder", func() { + folders := walk(fstest.MapFS{ + "music/track1.mp3": {}, + "music/tracks/track2.mp3": {}, + "music/tracks/music": {Mode: fs.ModeSymlink, Data: []byte("..")}, + }) + + Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "music", "music/tracks")) + Expect(folders["music/tracks"].audioFiles).To(HaveLen(1)) + }) + + It("skips a symlink pointing to its own folder", func() { + folders := walk(fstest.MapFS{ + "music/track1.mp3": {}, + "music/music": {Mode: fs.ModeSymlink, Data: []byte(".")}, + }) + + Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "music")) + Expect(folders["music"].audioFiles).To(HaveLen(1)) + }) + + It("skips a symlink closing a cycle between two folders", func() { + folders := walk(fstest.MapFS{ + "lib/a/track1.mp3": {}, + "lib/a/toB": {Mode: fs.ModeSymlink, Data: []byte("../b")}, + "lib/b/track2.mp3": {}, + "lib/b/toA": {Mode: fs.ModeSymlink, Data: []byte("../a")}, + }) + + Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "lib", "lib/a", "lib/a/toB", "lib/b", "lib/b/toA")) + }) + + It("still follows a symlink to a folder outside the current branch", func() { + folders := walk(fstest.MapFS{ + "lib/real/track1.mp3": {}, + "lib/alias": {Mode: fs.ModeSymlink, Data: []byte("real")}, + }) + + Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "lib", "lib/real", "lib/alias")) + Expect(folders["lib/alias"].audioFiles).To(HaveKey("track1.mp3")) + }) + + It("does not follow the cycle when symlinks are disabled", func() { + conf.Server.Scanner.FollowSymlinks = false + folders := walk(fstest.MapFS{ + "music/track1.mp3": {}, + "music/tracks/track2.mp3": {}, + "music/tracks/music": {Mode: fs.ModeSymlink, Data: []byte("..")}, + }) + + Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "music", "music/tracks")) + }) + + It("skips a symlink back to the library root when walking only the folder holding it", func() { + folders := walk(fstest.MapFS{ + "artist/album/track1.mp3": {}, + "playlists/best.m3u": {}, + "playlists/music": {Mode: fs.ModeSymlink, Data: []byte("..")}, + }, "playlists") + + Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf("playlists")) + }) + + It("skips a target folder that is a symlink back to the library root", func() { + folders := walk(fstest.MapFS{ + "artist/album/track1.mp3": {}, + "playlists/music": {Mode: fs.ModeSymlink, Data: []byte("..")}, + }, "playlists/music") + + Expect(folders).To(BeEmpty()) + }) + + // localFS resolves symlinks at the OS level instead of through fs.ReadLink + Context("production local storage FS", func() { + var musicFS storage.MusicFS + var libRoot string + + BeforeEach(func() { + tests.SkipOnWindows("symlink semantics") + + // The layout reported in #5334: playlists/music links back to the library root + libRoot = filepath.Join(GinkgoT().TempDir(), "music") + Expect(os.MkdirAll(filepath.Join(libRoot, "artist", "album"), 0755)).To(Succeed()) + Expect(os.MkdirAll(filepath.Join(libRoot, "playlists"), 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libRoot, "artist", "album", "track1.mp3"), []byte("AUDIO"), 0600)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libRoot, "playlists", "best.m3u"), []byte("#EXTM3U"), 0600)).To(Succeed()) + Expect(os.Symlink("..", filepath.Join(libRoot, "playlists", "music"))).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()) + }) + + It("skips a symlink pointing back into the library", func() { + folders := walkFS(musicFS, libRoot) + + Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "artist", "artist/album", "playlists")) + Expect(folders["artist/album"].audioFiles).To(HaveKey("track1.mp3")) + Expect(folders["playlists"].playlistFiles).To(HaveKey("best.m3u")) + }) + + It("skips the same symlink when walking only the folder holding it", func() { + folders := walkFS(musicFS, libRoot, "playlists") + + Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf("playlists")) + Expect(folders["playlists"].playlistFiles).To(HaveKey("best.m3u")) + }) + + It("skips target folders reached through the symlink", func() { + Expect(walkFS(musicFS, libRoot, "playlists/music", "playlists/music/artist")).To(BeEmpty()) + }) + }) + }) + Context("with target folders", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig())