fix(scanner): resolve file symlinks with the production local storage FS (#5755)

* fix(scanner): resolve file symlinks with the production local storage FS

The symlink classification added for GHSA-r5qr-m328-qcf4 relied on
fs.ReadLink, but the local storage FS wraps os.DirFS behind the fs.FS
interface, hiding its ReadLinkFS implementation. Every resolution failed
at the first hop, so the scanner silently skipped ALL file symlinks,
regardless of target or the FollowSymlinks setting. Libraries made of
symlinks (e.g. shared-pool setups) lost all their tracks after
upgrading to 0.63.

The local storage now exposes full OS-level resolution (EvalSymlinks)
through a new optional storage.SymlinkResolverFS interface, which the
scanner prefers over the fs.ReadLink hop loop. This also classifies a
chain by its FINAL target even when it passes through an audio-named
intermediate outside the library, closing a bypass the hop loop had.

Regular (non-symlink) entries keep the same early-return path, so scan
performance is unaffected for normal libraries.

Fixes #5752

* fix(test): keep watcher specs off the real local storage

The watcher specs spawn watchLibrary goroutines that are not joined on
spec teardown. Now that the scanner test binary registers the file://
storage, those leaked goroutines reached newLocalStorage, which reads
conf.Server on construction, racing with the configtest cleanup that
restores the config snapshot (caught by CI's race detector). Point the
mock libraries at a fake storage scheme, which never touches the config
and does not support watching, so the goroutine exits immediately.

* fix(storage): reject invalid fs paths in ResolveSymlink

Defense-in-depth for the SymlinkResolverFS contract: names must be valid
fs.FS paths. A lexical ".." in the name would otherwise escape the
library root via filepath.Join cleaning. No current caller can produce
such a name (they come from ReadDir walks), but the guard enforces the
documented contract at the boundary.

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.

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) {

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
 

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") != "" {

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)

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() {

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)