diff --git a/core/storage/interface.go b/core/storage/interface.go index dc08ca00a..02c1d14d9 100644 --- a/core/storage/interface.go +++ b/core/storage/interface.go @@ -17,6 +17,14 @@ type MusicFS interface { ReadTags(path ...string) (map[string]metadata.Info, error) } +// SymlinkResolverFS is an optional interface for MusicFS implementations backed by a real +// filesystem. ResolveSymlink resolves the whole symlink chain of the named entry at the OS +// level and returns the final target's path — including targets outside the FS root, which +// fs.ReadLink-based resolution cannot follow. +type SymlinkResolverFS interface { + ResolveSymlink(name string) (string, error) +} + // Watcher is a storage with the ability watch the FS and notify changes type Watcher interface { // Start starts a watcher on the whole FS and returns a channel to send detected changes. diff --git a/core/storage/local/local.go b/core/storage/local/local.go index 5384581e0..32aff0955 100644 --- a/core/storage/local/local.go +++ b/core/storage/local/local.go @@ -54,12 +54,23 @@ func (s *localStorage) FS() (storage.MusicFS, error) { if _, err := os.Stat(path); err != nil { //nolint:gosec return nil, fmt.Errorf("%w: %s", err, path) } - return &localFS{FS: os.DirFS(path), extractor: s.extractor}, nil + return &localFS{FS: os.DirFS(path), extractor: s.extractor, root: path}, nil } type localFS struct { fs.FS extractor Extractor + root string +} + +// ResolveSymlink implements storage.SymlinkResolverFS. It resolves the whole chain at the +// OS level, so links whose targets live outside the library folder (not reachable through +// the fs.FS abstraction) still resolve to their final target. +func (lfs *localFS) ResolveSymlink(name string) (string, error) { + if !fs.ValidPath(name) { + return "", &fs.PathError{Op: "resolvesymlink", Path: name, Err: fs.ErrInvalid} + } + return filepath.EvalSymlinks(filepath.Join(lfs.root, filepath.FromSlash(name))) } func (lfs *localFS) ReadTags(path ...string) (map[string]metadata.Info, error) { diff --git a/core/storage/local/local_test.go b/core/storage/local/local_test.go index d65d8214a..90bdd4b5b 100644 --- a/core/storage/local/local_test.go +++ b/core/storage/local/local_test.go @@ -199,6 +199,78 @@ var _ = Describe("LocalStorage", func() { }) }) + Describe("localFS.ResolveSymlink", func() { + var musicFS storage.MusicFS + + BeforeEach(func() { + if runtime.GOOS == "windows" { + Skip("symlink semantics") + } + u, err := storage.LocalPathToURL(tempDir) + Expect(err).ToNot(HaveOccurred()) + musicFS, err = newLocalStorage(u).FS() + Expect(err).ToNot(HaveOccurred()) + }) + + It("implements storage.SymlinkResolverFS", func() { + _, ok := musicFS.(storage.SymlinkResolverFS) + Expect(ok).To(BeTrue()) + }) + + It("resolves a chain that leaves the library folder to its final target", func() { + outside, err := os.MkdirTemp("", "navidrome-symlink-outside-") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(func() { os.RemoveAll(outside) }) + + target := filepath.Join(outside, "final.txt") + Expect(os.WriteFile(target, []byte("data"), 0600)).To(Succeed()) + mid := filepath.Join(outside, "mid.wav") + Expect(os.Symlink(target, mid)).To(Succeed()) + Expect(os.Symlink(mid, filepath.Join(tempDir, "link.wav"))).To(Succeed()) + + resolved, err := musicFS.(storage.SymlinkResolverFS).ResolveSymlink("link.wav") + Expect(err).ToNot(HaveOccurred()) + expected, err := filepath.EvalSymlinks(target) + Expect(err).ToNot(HaveOccurred()) + Expect(resolved).To(Equal(expected)) + }) + + It("resolves entries in subfolders (slash-separated fs paths)", func() { + Expect(os.MkdirAll(filepath.Join(tempDir, "sub"), 0755)).To(Succeed()) + target := filepath.Join(tempDir, "real.mp3") + Expect(os.WriteFile(target, []byte("audio"), 0600)).To(Succeed()) + Expect(os.Symlink(target, filepath.Join(tempDir, "sub", "link.mp3"))).To(Succeed()) + + resolved, err := musicFS.(storage.SymlinkResolverFS).ResolveSymlink("sub/link.mp3") + Expect(err).ToNot(HaveOccurred()) + expected, err := filepath.EvalSymlinks(target) + Expect(err).ToNot(HaveOccurred()) + Expect(resolved).To(Equal(expected)) + }) + + It("returns an error for a broken symlink", func() { + Expect(os.Symlink(filepath.Join(tempDir, "missing.mp3"), filepath.Join(tempDir, "broken.mp3"))).To(Succeed()) + + _, err := musicFS.(storage.SymlinkResolverFS).ResolveSymlink("broken.mp3") + Expect(err).To(HaveOccurred()) + }) + + It("rejects names that are not valid fs paths", func() { + for _, name := range []string{"../outside.mp3", "/etc/hosts", "sub/../../outside.mp3", ""} { + _, err := musicFS.(storage.SymlinkResolverFS).ResolveSymlink(name) + Expect(err).To(MatchError(fs.ErrInvalid), name) + } + }) + + It("returns an error for a symlink loop", func() { + Expect(os.Symlink(filepath.Join(tempDir, "loop2.mp3"), filepath.Join(tempDir, "loop1.mp3"))).To(Succeed()) + Expect(os.Symlink(filepath.Join(tempDir, "loop1.mp3"), filepath.Join(tempDir, "loop2.mp3"))).To(Succeed()) + + _, err := musicFS.(storage.SymlinkResolverFS).ResolveSymlink("loop1.mp3") + Expect(err).To(HaveOccurred()) + }) + }) + Describe("localFS.ReadTags", func() { var testFile string diff --git a/scanner/scanner_suite_test.go b/scanner/scanner_suite_test.go index 9ee6fc89b..10be0401f 100644 --- a/scanner/scanner_suite_test.go +++ b/scanner/scanner_suite_test.go @@ -2,17 +2,34 @@ package scanner_test import ( "context" + "io/fs" "os" "testing" + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/core/storage/local" "github.com/navidrome/navidrome/db" "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model/metadata" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "go.uber.org/goleak" ) +// The local storage is registered in this test binary, so any spec (or background watcher) +// touching a file:// library needs a default extractor to avoid a startup fatal. +type noopSuiteExtractor struct{} + +func (noopSuiteExtractor) Parse(...string) (map[string]metadata.Info, error) { return nil, nil } +func (noopSuiteExtractor) Version() string { return "0" } + +func init() { + local.RegisterExtractor(consts.DefaultScannerExtractor, func(fs.FS, string) local.Extractor { + return noopSuiteExtractor{} + }) +} + func TestScanner(t *testing.T) { // Only run goleak checks when the GOLEAK env var is set if os.Getenv("GOLEAK") != "" { diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index 55bbab684..887344b1b 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -5,11 +5,13 @@ import ( "io/fs" "maps" "path" + "path/filepath" "slices" "sort" "strings" "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/core/storage" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils" @@ -232,6 +234,20 @@ func resolveEntryName(ctx context.Context, fsys fs.FS, dirPath string, entry fs. log.Trace(ctx, "Scanner: Skipping symlink, following is disabled", "path", linkPath) return "", false } + // OS-backed filesystems can resolve the whole chain, even when it leaves the FS root + // (e.g. a link into another folder/drive), so the final target is always what gets + // classified. The fs.ReadLink loop below can't see past the root: it classifies by the + // last in-chain name it can reach. + if resolver, ok := fsys.(storage.SymlinkResolverFS); ok { + target, err := resolver.ResolveSymlink(linkPath) + if err != nil { + log.Trace(ctx, "Scanner: Skipping symlink, cannot resolve target", "path", linkPath, err) + return "", false + } + resolved := filepath.Base(target) + log.Trace(ctx, "Scanner: Resolved symlink", "path", linkPath, "target", target, "name", resolved) + return resolved, true + } cur := linkPath for hop := 0; hop < maxSymlinkHops; hop++ { target, err := fs.ReadLink(fsys, cur) diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index f3b13a4ef..9fb650c4d 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -432,6 +432,79 @@ 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. + Context("production local storage FS", func() { + var libRoot string + var musicFS storage.MusicFS + + BeforeEach(func() { + conf.Server.Scanner.FollowSymlinks = true + + // Reproduces the reported layout: a "pool" with the real files and a + // library containing only symlinks into the pool. + base := GinkgoT().TempDir() + pool := filepath.Join(base, "pool") + libRoot = filepath.Join(base, "userlib") + Expect(os.MkdirAll(pool, 0755)).To(Succeed()) + Expect(os.MkdirAll(libRoot, 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(pool, "real.mp3"), []byte("AUDIO"), 0600)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(pool, "secrets.txt"), []byte("TOPSECRET"), 0600)).To(Succeed()) + // mid.wav lives OUTSIDE the library and has an audio name, but points at a + // non-audio file. A chain through it must be classified by the FINAL target. + Expect(os.Symlink(filepath.Join(pool, "secrets.txt"), filepath.Join(pool, "mid.wav"))).To(Succeed()) + + Expect(os.Symlink("../pool/real.mp3", filepath.Join(libRoot, "relative.mp3"))).To(Succeed()) + Expect(os.Symlink(filepath.Join(pool, "real.mp3"), filepath.Join(libRoot, "absolute.mp3"))).To(Succeed()) + 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()) + }) + + walkRoot := func() *folderEntry { + job := &scanJob{fs: musicFS, lib: model.Library{Path: libRoot}} + results, err := walkDirTree(GinkgoT().Context(), job) + Expect(err).ToNot(HaveOccurred()) + var root *folderEntry + for folder := range results { + if folder.path == "." { + root = folder + } + } + Expect(root).ToNot(BeNil()) + return root + } + + It("imports symlinks to out-of-library audio files", func() { + root := walkRoot() + Expect(root.audioFiles).To(HaveKey("relative.mp3")) + Expect(root.audioFiles).To(HaveKey("absolute.mp3")) + }) + + It("rejects a chain that ends in a non-audio file, even through an audio-named intermediate", func() { + root := walkRoot() + Expect(root.audioFiles).ToNot(HaveKey("evil.wav")) + }) + + It("skips broken symlinks", func() { + root := walkRoot() + Expect(root.audioFiles).ToNot(HaveKey("broken.mp3")) + }) + + It("skips all file symlinks when FollowSymlinks is disabled", func() { + conf.Server.Scanner.FollowSymlinks = false + root := walkRoot() + Expect(root.audioFiles).To(BeEmpty()) + }) + }) + Context("out-of-tree escape (temp dir)", func() { var root string BeforeEach(func() { diff --git a/scanner/watcher_test.go b/scanner/watcher_test.go index ffe9f8b15..15e49e195 100644 --- a/scanner/watcher_test.go +++ b/scanner/watcher_test.go @@ -9,6 +9,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/core/storage/storagetest" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" @@ -30,10 +31,14 @@ var _ = Describe("Watcher", func() { ctx, cancel = context.WithCancel(GinkgoT().Context()) DeferCleanup(cancel) + // Use a fake storage scheme: watchLibrary goroutines spawned by Run/Watch are not + // joined on spec teardown, and the real file:// storage reads conf.Server on + // construction, racing with the configtest cleanup that restores the config. + storagetest.Register("fake-watcher", &storagetest.FakeFS{}) lib = &model.Library{ ID: 1, Name: "Test Library", - Path: "/test/library", + Path: "fake-watcher:///test/library", } // Set up mocks @@ -234,7 +239,7 @@ var _ = Describe("Watcher", func() { lib2 = &model.Library{ ID: 2, Name: "Test Library 2", - Path: "/test/library2", + Path: "fake-watcher:///test/library2", } mockLibRepo := mockDS.MockedLibrary.(*tests.MockLibraryRepo)