From 582e33340079b184007fa64c5db8f4983a71b493 Mon Sep 17 00:00:00 2001 From: Junker der Provinz <133605895+junkerderprovinz@users.noreply.github.com> Date: Sat, 5 Sep 2026 05:44:00 +0200 Subject: [PATCH 1/3] fix(scanner): skip symlinks that point back into the folder being scanned - #5334 A symlink pointing at a folder that is already part of the current branch of the walk (e.g. `music/tracks/music -> ..`) made the scanner walk the same folders again and again, re-adding the same files until the OS hit its recursion or path limits. Each folder now carries its resolved path, with symlinks followed, and the walk keeps the resolved paths of the branch it is currently in. A child whose resolved path is already in that branch is a cycle and is skipped. The set is per branch rather than global, so two sibling folders linking to the same target are both walked: that is the same folder reached twice, not a loop. Resolution uses the storage's own resolver where the filesystem is backed by a real one, and falls back to following the link chain with fs.ReadLink, which keeps the comparison inside the FS-relative path space. The tests give the walk a deadline and a folder cap, because an undetected cycle does not fail, it runs forever. Verified both ways: green with the detection, and failing on the deadline without it. Fixes #5334 Signed-off-by: Junker der Provinz <133605895+junkerderprovinz@users.noreply.github.com> --- scanner/walk_dir_tree.go | 126 +++++++++++++++++++++++++++++----- scanner/walk_dir_tree_test.go | 126 ++++++++++++++++++++++++++++++++++ 2 files changed, 236 insertions(+), 16 deletions(-) diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index d39cad5f6..10c0a4c9d 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -59,7 +59,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, newDirRef(job.fs, folderPath), checker, results, map[string]struct{}{}) if utils.IsCtxDone(ctx) { return } @@ -73,28 +73,44 @@ 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 be walked: its path in the library's filesystem and its resolved +// path, with all symlinks followed. The resolved path is the folder's identity: two paths +// that resolve to the same one are the same folder on disk, which is how symlink cycles +// are detected while walking (see #5334). +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) + // Keep track of the folders in the current branch, so any symlink pointing back into + // one of them is recognized as a cycle and skipped. Removed again on the way out, so + // two sibling folders linking to the same target are both walked - that is not a + // cycle, just the same folder reached twice. + 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 +121,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 +135,22 @@ 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) + // Named dirFile rather than dir: dir is now the folder reference this function + // received, and shadowing it here would silently walk the wrong path. + 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 +170,16 @@ 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) + // A symlink whose target is already in the current branch points back into a + // folder being walked right now: following it would walk the same folders + // forever. Everything else is walked normally. + 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,72 @@ func isSymlinkedPath(fsys fs.FS, folderPath string) bool { const maxSymlinkHops = 40 +// newDirRef returns the reference for the folder a walk starts at, resolving it in case +// the folder itself is (or is reached through) 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 +} + +// newChildDirRef returns the reference for a subfolder of dir. Only symlinks need to be +// resolved: a real subfolder is always a new folder, it can never point back into one of +// 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 the path of the folder dir points to, with all symlinks +// resolved. Storages backed by a real filesystem resolve the whole path at the OS level. +// For any other filesystem the symlink chain is followed with fs.ReadLink, starting from +// the parent's already resolved path, so the walk keeps comparing paths in the same +// (FS-relative) space. If the path cannot be resolved it is returned as is, and the +// folder is walked as any other. +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 the symlink chain starting at linkPath using fs.ReadLink, +// and returns the last path it could reach, plus the number of hops it took to get there. +// Zero hops means linkPath is not a symlink this filesystem can read; maxSymlinkHops means +// the chain is too long to be resolved and is most likely a loop. +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..df5e62f1c 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,131 @@ var _ = Describe("walk_dir_tree", func() { ) }) + // Regression tests for #5334: a symlink pointing back into a folder that is already + // being walked made the scanner walk the same folders over and over, re-adding the + // same files until the OS hit its recursion/path limits. + Context("with symlink cycles", func() { + // A cycle that is not detected makes the walk run forever, so it gets a deadline + // and a hard cap on the number of folders. All layouts below are small enough to + // be walked instantly, and none of them has more folders than the cap. + const walkTimeout = 3 * time.Second + const maxFolders = 25 + + walkFS := func(musicFS storage.MusicFS, libPath 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) + 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) map[string]*folderEntry { + return walkFS(&mockMusicFS{FS: mapFS}, "/music") + } + + 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")) + }) + + // The production localFS resolves symlinks at the OS level, a different code path + // than the in-memory filesystem used by the specs above. + Context("production local storage FS", func() { + var musicFS storage.MusicFS + var libRoot string + + BeforeEach(func() { + tests.SkipOnWindows("symlink semantics") + + // The layout reported in #5334: a subfolder linking back to the library root + libRoot = filepath.Join(GinkgoT().TempDir(), "music") + Expect(os.MkdirAll(filepath.Join(libRoot, "tracks"), 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libRoot, "track1.mp3"), []byte("AUDIO"), 0600)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libRoot, "tracks", "track2.mp3"), []byte("AUDIO"), 0600)).To(Succeed()) + Expect(os.Symlink("..", filepath.Join(libRoot, "tracks", "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(".", "tracks")) + Expect(folders["."].audioFiles).To(HaveKey("track1.mp3")) + Expect(folders["tracks"].audioFiles).To(HaveKey("track2.mp3")) + }) + }) + }) + Context("with target folders", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) From 09249ed1c629e412317779fc231d45fff4557cc0 Mon Sep 17 00:00:00 2001 From: junkerderprovinz Date: Fri, 9 Oct 2026 20:55:05 +0200 Subject: [PATCH 2/3] fix(scanner): skip symlinks back into the ancestors of a targeted folder A scan of a single folder started its branch at that folder, so a link like playlists/music -> .. walked the whole library again. Seed the branch with the resolved paths of the folders above the target. Also shortens the comments around the cycle check. Signed-off-by: junkerderprovinz --- scanner/walk_dir_tree.go | 52 +++++++++++++++------------------ scanner/walk_dir_tree_test.go | 54 ++++++++++++++++++++++------------- 2 files changed, 57 insertions(+), 49 deletions(-) diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index 10c0a4c9d..b93438a82 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -59,7 +59,7 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<- } // Recursively walk this folder and all its children - err = walkFolder(ctx, job, newDirRef(job.fs, folderPath), checker, results, map[string]struct{}{}) + err = walkFolder(ctx, job, newDirRef(job.fs, folderPath), checker, results, ancestorsOf(job.fs, folderPath)) if utils.IsCtxDone(ctx) { return } @@ -73,10 +73,8 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<- return results, nil } -// dirRef is a folder to be walked: its path in the library's filesystem and its resolved -// path, with all symlinks followed. The resolved path is the folder's identity: two paths -// that resolve to the same one are the same folder on disk, which is how symlink cycles -// are detected while walking (see #5334). +// 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 @@ -87,10 +85,8 @@ func walkFolder(ctx context.Context, job *scanJob, dir dirRef, checker *IgnoreCh _ = checker.Push(ctx, dir.path) defer checker.Pop() // Pop patterns when leaving this folder - // Keep track of the folders in the current branch, so any symlink pointing back into - // one of them is recognized as a cycle and skipped. Removed again on the way out, so - // two sibling folders linking to the same target are both walked - that is not a - // cycle, just the same folder reached twice. + // 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) @@ -135,8 +131,6 @@ func loadDir(ctx context.Context, job *scanJob, dir dirRef, checker *IgnoreCheck folder = job.createFolderEntry(dirPath) folder.modTime = dirInfo.ModTime() - // Named dirFile rather than dir: dir is now the folder reference this function - // received, and shadowing it here would silently walk the wrong path. dirFile, err := job.fs.Open(dirPath) if err != nil { log.Warn(ctx, "Scanner: Error in Opening directory", "path", dirPath, err) @@ -171,9 +165,6 @@ func loadDir(ctx context.Context, job *scanJob, dir dirRef, checker *IgnoreCheck } if isDir && isDirReadable(ctx, job.fs, entryPath) { child := newChildDirRef(job.fs, dir, entry) - // A symlink whose target is already in the current branch points back into a - // folder being walked right now: following it would walk the same folders - // forever. Everything else is walked normally. if _, isCycle := branch[child.realPath]; isCycle { log.Debug(ctx, "Scanner: Skipping symlink pointing back into a folder being scanned", "path", entryPath, "target", child.realPath) @@ -274,17 +265,26 @@ func isSymlinkedPath(fsys fs.FS, folderPath string) bool { const maxSymlinkHops = 40 -// newDirRef returns the reference for the folder a walk starts at, resolving it in case -// the folder itself is (or is reached through) a symlink. +// 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 } -// newChildDirRef returns the reference for a subfolder of dir. Only symlinks need to be -// resolved: a real subfolder is always a new folder, it can never point back into one of -// its own ancestors. +// ancestorsOf returns the resolved paths of the folders above folderPath up to the library +// root, so walking only folderPath catches links back into them as a full walk would. +func ancestorsOf(fsys fs.FS, folderPath string) map[string]struct{} { + ancestors := map[string]struct{}{} + for p := path.Clean(folderPath); p != "." && p != "/"; { + p = path.Dir(p) + ancestors[newDirRef(fsys, p).realPath] = struct{}{} + } + return ancestors +} + +// 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()), @@ -296,12 +296,8 @@ func newChildDirRef(fsys fs.FS, dir dirRef, entry fs.DirEntry) dirRef { return child } -// resolveDirPath returns the path of the folder dir points to, with all symlinks -// resolved. Storages backed by a real filesystem resolve the whole path at the OS level. -// For any other filesystem the symlink chain is followed with fs.ReadLink, starting from -// the parent's already resolved path, so the walk keeps comparing paths in the same -// (FS-relative) space. If the path cannot be resolved it is returned as is, and the -// folder is walked as any other. +// 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) @@ -317,10 +313,8 @@ func resolveDirPath(fsys fs.FS, dir dirRef) string { return target } -// followSymlinkChain follows the symlink chain starting at linkPath using fs.ReadLink, -// and returns the last path it could reach, plus the number of hops it took to get there. -// Zero hops means linkPath is not a symlink this filesystem can read; maxSymlinkHops means -// the chain is too long to be resolved and is most likely a loop. +// 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 diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index df5e62f1c..8dcfb7f0a 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -153,22 +153,19 @@ var _ = Describe("walk_dir_tree", func() { ) }) - // Regression tests for #5334: a symlink pointing back into a folder that is already - // being walked made the scanner walk the same folders over and over, re-adding the - // same files until the OS hit its recursion/path limits. + // See #5334 Context("with symlink cycles", func() { - // A cycle that is not detected makes the walk run forever, so it gets a deadline - // and a hard cap on the number of folders. All layouts below are small enough to - // be walked instantly, and none of them has more folders than the cap. + // 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) map[string]*folderEntry { + 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) + results, err := walkDirTree(ctx, job, targetFolders...) Expect(err).ToNot(HaveOccurred()) folders := map[string]*folderEntry{} @@ -182,8 +179,8 @@ var _ = Describe("walk_dir_tree", func() { return folders } - walk := func(mapFS fstest.MapFS) map[string]*folderEntry { - return walkFS(&mockMusicFS{FS: mapFS}, "/music") + walk := func(mapFS fstest.MapFS, targetFolders ...string) map[string]*folderEntry { + return walkFS(&mockMusicFS{FS: mapFS}, "/music", targetFolders...) } BeforeEach(func() { @@ -244,8 +241,17 @@ var _ = Describe("walk_dir_tree", func() { Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "music", "music/tracks")) }) - // The production localFS resolves symlinks at the OS level, a different code path - // than the in-memory filesystem used by the specs above. + 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")) + }) + + // 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 @@ -253,12 +259,13 @@ var _ = Describe("walk_dir_tree", func() { BeforeEach(func() { tests.SkipOnWindows("symlink semantics") - // The layout reported in #5334: a subfolder linking back to the library root + // 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, "tracks"), 0755)).To(Succeed()) - Expect(os.WriteFile(filepath.Join(libRoot, "track1.mp3"), []byte("AUDIO"), 0600)).To(Succeed()) - Expect(os.WriteFile(filepath.Join(libRoot, "tracks", "track2.mp3"), []byte("AUDIO"), 0600)).To(Succeed()) - Expect(os.Symlink("..", filepath.Join(libRoot, "tracks", "music"))).To(Succeed()) + 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()) @@ -271,9 +278,16 @@ var _ = Describe("walk_dir_tree", func() { It("skips a symlink pointing back into the library", func() { folders := walkFS(musicFS, libRoot) - Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "tracks")) - Expect(folders["."].audioFiles).To(HaveKey("track1.mp3")) - Expect(folders["tracks"].audioFiles).To(HaveKey("track2.mp3")) + 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")) }) }) }) From f61d940d68799190e992fd817fc6666039d1075c Mon Sep 17 00:00:00 2001 From: junkerderprovinz Date: Fri, 9 Oct 2026 21:04:02 +0200 Subject: [PATCH 3/3] fix(scanner): skip targeted folders reached through a symlink cycle Creating playlists/music -> .. makes the watcher scan the link itself, which walked the whole library again under playlists/music. A full walk never enters such a link, so a targeted one skips it too. Signed-off-by: junkerderprovinz --- scanner/walk_dir_tree.go | 27 ++++++++++++++++++++------- scanner/walk_dir_tree_test.go | 13 +++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index b93438a82..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, newDirRef(job.fs, folderPath), checker, results, ancestorsOf(job.fs, folderPath)) + err = walkFolder(ctx, job, dir, checker, results, ancestors) if utils.IsCtxDone(ctx) { return } @@ -272,15 +281,19 @@ func newDirRef(fsys fs.FS, dirPath string) dirRef { return dir } -// ancestorsOf returns the resolved paths of the folders above folderPath up to the library -// root, so walking only folderPath catches links back into them as a full walk would. -func ancestorsOf(fsys fs.FS, folderPath string) map[string]struct{} { +// 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(folderPath); p != "." && p != "/"; { + for p := path.Clean(dir.path); p != "." && p != "/"; { p = path.Dir(p) - ancestors[newDirRef(fsys, p).realPath] = struct{}{} + realPath := newDirRef(fsys, p).realPath + if _, seen := ancestors[realPath]; seen || realPath == dir.realPath { + return nil, false + } + ancestors[realPath] = struct{}{} } - return ancestors + return ancestors, true } // newChildDirRef returns the reference for a subfolder of dir. Only symlinks need resolving, diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index 8dcfb7f0a..7a3f909b2 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -251,6 +251,15 @@ var _ = Describe("walk_dir_tree", func() { 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 @@ -289,6 +298,10 @@ var _ = Describe("walk_dir_tree", func() { 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()) + }) }) })