From 4381366e663b4647cfec1eb5597c39c3c42e9d7a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= <deluan@navidrome.org>
Date: Thu, 9 Jul 2026 19:43:04 -0400
Subject: [PATCH 1/6] ci: validate DB migration order on pull requests (#5750)

* ci: add DB migration ordering/naming validation script

* ci: run DB migration validation on pull requests

* ci: don't reject non-migration .go files

* ci: fetch latest master before validating migration order

* ci: annotate the offending migration file on validation failure

* ci: gate Build on the migration check so a bad migration fails fast

* ci: reject migrations placed in a subdirectory of db/migrations

* ci: reword fetch-step comment to not hard-code the base branch name
---
 .github/workflows/pipeline.yml           |  23 +++-
 .github/workflows/validate-migrations.sh | 150 +++++++++++++++++++++++
 2 files changed, 172 insertions(+), 1 deletion(-)
 create mode 100755 .github/workflows/validate-migrations.sh

diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml
index 8c714945e..d21d0a681 100644
--- a/.github/workflows/pipeline.yml
+++ b/.github/workflows/pipeline.yml
@@ -96,6 +96,23 @@ jobs:
             exit 1
           fi
 
+  validate-migrations:
+    name: Validate DB migrations
+    runs-on: ubuntu-latest
+    if: github.event_name == 'pull_request'
+    steps:
+      - uses: actions/checkout@v7
+        with:
+          fetch-depth: 0
+      # Refresh the base branch so the check compares against its CURRENT tip,
+      # not the (possibly stale) commit the PR was opened against.
+      - name: Fetch latest base branch
+        run: git fetch --no-tags origin "+refs/heads/${{ github.event.pull_request.base.ref }}:refs/remotes/origin/${{ github.event.pull_request.base.ref }}"
+      - name: Validate migration ordering and naming
+        env:
+          BASE_REF: origin/${{ github.event.pull_request.base.ref }}
+        run: ./.github/workflows/validate-migrations.sh
+
   go:
     name: Test Go code
     runs-on: ubuntu-latest
@@ -257,7 +274,11 @@ jobs:
 
   build:
     name: Build
-    needs: [js, go, go-windows, go-lint, i18n-lint, git-version, check-push-enabled]
+    needs: [js, go, go-windows, go-lint, i18n-lint, git-version, check-push-enabled, validate-migrations]
+    # validate-migrations only runs on pull_request, so it is "skipped" on push/tag
+    # builds. Run Build unless a dependency actually failed — a *skipped* dependency
+    # (the migration check on non-PR events) must not block release builds.
+    if: ${{ !cancelled() && !failure() }}
     strategy:
       matrix:
         platform: [ linux/amd64, linux/arm64, linux/arm/v5, linux/arm/v6, linux/arm/v7, linux/386, linux/riscv64, darwin/amd64, darwin/arm64, windows/amd64, windows/386 ]
diff --git a/.github/workflows/validate-migrations.sh b/.github/workflows/validate-migrations.sh
new file mode 100755
index 000000000..07d05c3a6
--- /dev/null
+++ b/.github/workflows/validate-migrations.sh
@@ -0,0 +1,150 @@
+#!/usr/bin/env bash
+#
+# Validates DB migrations added by a pull request:
+#   1. Ordering  - an added migration must be NEWER than the latest migration
+#                  already on the base branch. Goose applies migrations in
+#                  timestamp order, so an older-timestamped migration would be
+#                  silently skipped on databases already upgraded past it.
+#   2. Uniqueness - no two migration files may share a timestamp.
+#   3. Naming     - files must match YYYYMMDDHHMMSS_lower_snake_name.(sql|go).
+#
+# On failure it prints a human-readable message and, when running in GitHub
+# Actions, emits an error annotation bound to the offending file so the message
+# also renders inline in the PR "Files changed" tab.
+#
+# Compares HEAD against $BASE_REF (default origin/master). Requires full history
+# (fetch-depth: 0 in CI).
+# -e is intentionally omitted: the script accumulates violations into $status
+# and must not exit on the first non-zero command (grep no-match, a false [[ ]]
+# in an if, `is_migration || continue`).
+set -uo pipefail
+export LC_ALL=C
+
+MIGRATIONS_DIR="db/migrations"
+BASE_REF="${BASE_REF:-origin/master}"
+NAME_RE='^[0-9]{14}_[a-z0-9_]+\.(sql|go)$'
+
+status=0
+
+# Log a message to stderr and mark the run as failed.
+fail() {
+  printf '%s\n' "$1" >&2
+  status=1
+}
+
+# Emit a GitHub Actions error annotation bound to a file, so the message renders
+# inline on the offending migration in the PR "Files changed" tab. No-op outside
+# CI. `%`, newline and CR are encoded as required by the workflow-command syntax
+# (the `%` replacement must run first so the encodings we add aren't re-escaped).
+annotate() { # $1=file  $2=message
+  [ "${GITHUB_ACTIONS:-}" = "true" ] || return 0
+  local msg="$2"
+  msg="${msg//'%'/%25}"
+  msg="${msg//$'\n'/%0A}"
+  msg="${msg//$'\r'/%0D}"
+  printf '::error file=%s,line=1::%s\n' "$1" "$msg"
+}
+
+# Report a migration problem: log it, annotate the offending file, mark failed.
+report() { # $1=file  $2=message
+  fail "$2"
+  printf '\n' >&2
+  annotate "$1" "$2"
+}
+
+human_ts() {
+  local t="$1"
+  printf '%s-%s-%s %s:%s:%s' "${t:0:4}" "${t:4:2}" "${t:6:2}" "${t:8:2}" "${t:10:2}" "${t:12:2}"
+}
+
+is_migration() { # $1=basename -> 0 if a .sql/.go file with a 14-digit prefix
+  local b="$1"
+  case "$b" in
+    *.sql | *.go) ;;
+    *) return 1 ;;
+  esac
+  [[ "${b%%_*}" =~ ^[0-9]{14}$ ]]
+}
+
+if ! git rev-parse --verify --quiet "$BASE_REF" >/dev/null; then
+  printf '❌ Cannot resolve base ref "%s". In CI, check out with fetch-depth: 0.\n' "$BASE_REF" >&2
+  exit 1
+fi
+
+# --- Newest timestamp already on the base branch ---
+base_max=""
+base_max_file=""
+while IFS= read -r f; do
+  [ -z "$f" ] && continue
+  b="$(basename "$f")"
+  is_migration "$b" || continue
+  ts="${b%%_*}"
+  if [[ "$ts" > "$base_max" ]]; then
+    base_max="$ts"
+    base_max_file="$f"
+  fi
+done < <(git ls-tree -r --name-only "$BASE_REF" -- "$MIGRATIONS_DIR" 2>/dev/null)
+
+# --- Ordering + naming on files added by this PR ---
+while IFS= read -r f; do
+  [ -z "$f" ] && continue
+  b="$(basename "$f")"
+  case "$b" in
+    *.sql) ;;                                  # any .sql in this dir must be a migration
+    *.go) [[ "$b" == [0-9]* ]] || continue ;;  # non-timestamped .go = helper (e.g. migration.go), skip
+    *) continue ;;
+  esac
+  if [ "${f%/*}" != "$MIGRATIONS_DIR" ]; then
+    report "$f" "❌ Migration file in a subdirectory: $f
+   Migrations must live directly in $MIGRATIONS_DIR/ — only $MIGRATIONS_DIR/*.sql (and
+   top-level .go migrations) are embedded, so a nested file would be SILENTLY SKIPPED.
+   Move it to $MIGRATIONS_DIR/$b."
+    continue
+  fi
+  if ! [[ "$b" =~ $NAME_RE ]]; then
+    report "$f" "❌ Malformed migration filename: $f
+   Expected YYYYMMDDHHMMSS_lower_snake_name.(sql|go); the name segment must be lowercase.
+   Regenerate with: make migration-sql name=<description>  (or make migration-go name=<description>)"
+    continue
+  fi
+  ts="${b%%_*}"
+  if [[ -n "$base_max" ]] && ! [[ "$ts" > "$base_max" ]]; then
+    report "$f" "❌ Migration ordering error: $f ($(human_ts "$ts"))
+   is older than (or equal to) the newest migration already on ${BASE_REF#origin/}:
+   $base_max_file ($(human_ts "$base_max"))
+
+   Goose applies migrations in timestamp order, so databases already upgraded
+   past that point would SILENTLY SKIP your migration.
+
+   Fix: regenerate it with a current timestamp:
+     make migration-sql name=<description>   (or make migration-go name=<description>)
+   then move your SQL/Go body into the new file and delete the old one."
+  fi
+done < <(git diff --diff-filter=A --name-only "$BASE_REF"...HEAD -- "$MIGRATIONS_DIR" 2>/dev/null)
+
+# --- Duplicate timestamps across the merged set (HEAD) ---
+all_migs="$(git ls-tree -r --name-only HEAD -- "$MIGRATIONS_DIR" 2>/dev/null)"
+dups="$(printf '%s\n' "$all_migs" | while IFS= read -r f; do
+  b="$(basename "$f")"
+  is_migration "$b" || continue
+  printf '%s\n' "${b%%_*}"
+done | sort | uniq -d)"
+if [ -n "$dups" ]; then
+  while IFS= read -r ts; do
+    [ -z "$ts" ] && continue
+    colliding="$(printf '%s\n' "$all_migs" | grep "/${ts}_" || true)"
+    printf '❌ Duplicate migration timestamp %s used by multiple files:\n' "$ts" >&2
+    while IFS= read -r cf; do
+      [ -z "$cf" ] && continue
+      printf '   %s\n' "$cf" >&2
+      annotate "$cf" "Duplicate migration timestamp $ts — shared by another migration. Timestamps must be unique; regenerate one with make migration-*."
+    done <<< "$colliding"
+    printf '   Every migration needs a unique timestamp. Regenerate one with make migration-*.\n' >&2
+    status=1
+  done <<< "$dups"
+fi
+
+if [ "$status" -eq 0 ]; then
+  echo "✅ DB migrations OK (ordering, uniqueness, naming)."
+fi
+exit "$status"

From 7fa13761d734159e2f0a502a31affacc88e66d37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= <deluan@navidrome.org>
Date: Fri, 10 Jul 2026 10:52:05 -0400
Subject: [PATCH 2/6] 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        |  8 ++++
 core/storage/local/local.go      | 13 +++++-
 core/storage/local/local_test.go | 72 +++++++++++++++++++++++++++++++
 scanner/scanner_suite_test.go    | 17 ++++++++
 scanner/walk_dir_tree.go         | 16 +++++++
 scanner/walk_dir_tree_test.go    | 73 ++++++++++++++++++++++++++++++++
 scanner/watcher_test.go          |  9 +++-
 7 files changed, 205 insertions(+), 3 deletions(-)

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)

From 116a4407184b6f21221a6aa7702fb738a3915773 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= <deluan@navidrome.org>
Date: Fri, 10 Jul 2026 18:43:31 -0400
Subject: [PATCH 3/6] test(scanner): fix flaky Windows search_normalized rescan
 test (#5758)

The 'repopulates a stale search_normalized on a full rescan' spec runs
two full scans back-to-back. Whether the second scan refreshes the
unchanged artist depends on folderEntry.isOutdated(), which compares
folder.updated_at (written during the first scan) against the second
scan's library.last_scan_started_at using a strict time.Before(). Both
are time.Now() values captured milliseconds apart.

On Linux's fine-grained clock they are always distinct, so the test
passes. On Windows the coarse wall-clock granularity frequently makes
the two timestamps land in the same tick and compare equal, so
Before() returns false, the folder is treated as up-to-date and
skipped, the artist is never re-persisted, and search_normalized stays
empty -- failing the assertion intermittently across unrelated PRs.

Backdate the folder's updated_at an hour before the second scan so the
comparison is unambiguous on every platform. This is a test-only
timing artifact (real rescans never run milliseconds apart on an
unchanged library), so no production code changes are needed.
---
 scanner/scanner_test.go | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go
index 7f3dca775..cc3720717 100644
--- a/scanner/scanner_test.go
+++ b/scanner/scanner_test.go
@@ -6,6 +6,7 @@ import (
 	"errors"
 	"path/filepath"
 	"testing/fstest"
+	"time"
 
 	"github.com/Masterminds/squirrel"
 	"github.com/google/uuid"
@@ -212,6 +213,15 @@ var _ = Describe("Scanner", Ordered, func() {
 			_, err := db.Db().ExecContext(ctx, "UPDATE artist SET search_normalized = '' WHERE name = 'GØGGS'")
 			Expect(err).ToNot(HaveOccurred())
 
+			// Backdate the folder so the next full scan reliably sees it as outdated.
+			// isOutdated() compares folder.updated_at (written by this scan) against the
+			// next scan's last_scan_started_at with a strict Before(); back-to-back scans
+			// can capture both within one clock tick on Windows (coarse wall-clock), making
+			// the refresh flaky. Backdating forces the comparison to be unambiguous.
+			_, err = db.Db().ExecContext(ctx,
+				"UPDATE folder SET updated_at = ?", time.Now().Add(-time.Hour))
+			Expect(err).ToNot(HaveOccurred())
+
 			Expect(runScanner(ctx, true)).To(Succeed())
 			Expect(searchNormalized()).To(Equal("GOGGS"))
 		})

From 205c85da5523338807808202d8877ccaa6e82776 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= <deluan@navidrome.org>
Date: Fri, 10 Jul 2026 19:15:49 -0400
Subject: [PATCH 4/6] fix(plugins): surface host service failures when loading
 plugins (#5756)

* fix(plugins): surface host service failures when loading plugins

When a host service failed to initialize during plugin load (e.g. the
taskqueue database could not be created because the data folder is not
writable), the error was logged and swallowed, and its host functions were
silently omitted. Instantiation then failed with a misleading error such as
'"task_createqueue" is not exported in module "extism:host/user"', which
reads as a plugin/host API mismatch and gets wrongly blamed on plugin
authors (see kgarner7/navidrome-listenbrainz-daily-playlist#26).

Host service factories now return an error, and loadPluginWithConfig fails
fast with the actual cause (e.g. 'creating Task service: creating plugin
data directory: ...'), which is also stored in the plugin's last_error.
Closers accumulated before a load failure are now closed (via a deferred
guard covering all failure paths), so partially-created services no longer
leak goroutines or database handles.

Also fix a panic in 'navidrome plugin enable': CLI commands use the plugin
Manager without calling Start, so manager.ctx was nil and
newTaskQueueService panicked in context.WithCancel. Long-lived host
services (taskqueue, kvstore, websocket) now receive their lifecycle
context explicitly in the constructor, sourced from serviceContext.baseCtx(),
which falls back to context.Background() for the unstarted-manager case.

* docs(plugins): correct websocket readLoop lifecycle comment

The comment claimed the read loop's context is always cancelled during
application shutdown, which is not true when the manager was never started
(one-shot CLI runs, where baseCtx falls back to context.Background()).
Clarify that connection closure via Close() on plugin unload is what ends
the read loop, with context cancellation as a server-shutdown backstop.

Addresses review feedback on #5756.

* test(plugins): exclude plugin-loading specs from Windows builds

The new loadPluginWithConfig specs reference test suite helpers
(testdataDir, noopMetricsRecorder) defined in plugins_suite_test.go, which
is excluded on Windows, breaking test compilation there. Move the specs to
their own file with the same build constraint, keeping the pure-function
specs in manager_loader_test.go running on Windows as before.
---
 plugins/host_taskqueue.go           |  5 +-
 plugins/host_taskqueue_test.go      | 16 +----
 plugins/host_websocket.go           | 15 +++--
 plugins/manager_loader.go           | 98 +++++++++++++++++++----------
 plugins/manager_loader_load_test.go | 78 +++++++++++++++++++++++
 5 files changed, 158 insertions(+), 54 deletions(-)
 create mode 100644 plugins/manager_loader_load_test.go

diff --git a/plugins/host_taskqueue.go b/plugins/host_taskqueue.go
index a5db3344f..2f74c0aa4 100644
--- a/plugins/host_taskqueue.go
+++ b/plugins/host_taskqueue.go
@@ -82,7 +82,8 @@ type taskQueueServiceImpl struct {
 }
 
 // newTaskQueueService creates a new taskQueueServiceImpl with its own SQLite database.
-func newTaskQueueService(pluginName string, manager *Manager, maxConcurrency int32) (*taskQueueServiceImpl, error) {
+// The given ctx bounds the service's background work (queue workers, cleanup loop).
+func newTaskQueueService(ctx context.Context, pluginName string, manager *Manager, maxConcurrency int32) (*taskQueueServiceImpl, error) {
 	dataDir := filepath.Join(conf.Server.DataFolder.String(), "plugins", pluginName)
 	if err := os.MkdirAll(dataDir, 0700); err != nil {
 		return nil, fmt.Errorf("creating plugin data directory: %w", err)
@@ -102,7 +103,7 @@ func newTaskQueueService(pluginName string, manager *Manager, maxConcurrency int
 		return nil, fmt.Errorf("creating taskqueue schema: %w", err)
 	}
 
-	ctx, cancel := context.WithCancel(manager.ctx) //nolint:gosec // cancel is stored in struct and called in Close()
+	ctx, cancel := context.WithCancel(ctx) //nolint:gosec // cancel is stored in struct and called in Close()
 
 	s := &taskQueueServiceImpl{
 		pluginName:     pluginName,
diff --git a/plugins/host_taskqueue_test.go b/plugins/host_taskqueue_test.go
index faff79c8e..d459fd69b 100644
--- a/plugins/host_taskqueue_test.go
+++ b/plugins/host_taskqueue_test.go
@@ -42,15 +42,11 @@ var _ = Describe("TaskQueueService", func() {
 		DeferCleanup(configtest.SetupConfig())
 		conf.Server.DataFolder = conf.NewDir(tmpDir)
 
-		// Create a mock manager with context
-		managerCtx, cancel := context.WithCancel(ctx)
 		manager = &Manager{
 			plugins: make(map[string]*plugin),
-			ctx:     managerCtx,
 		}
-		DeferCleanup(cancel)
 
-		service, err = newTaskQueueService("test_plugin", manager, 5)
+		service, err = newTaskQueueService(ctx, "test_plugin", manager, 5)
 		Expect(err).ToNot(HaveOccurred())
 	})
 
@@ -730,14 +726,11 @@ var _ = Describe("TaskQueueService", func() {
 			service.Close()
 
 			// Create a new service pointing to the same DB
-			managerCtx2, cancel2 := context.WithCancel(ctx)
-			DeferCleanup(cancel2)
 			manager2 := &Manager{
 				plugins: make(map[string]*plugin),
-				ctx:     managerCtx2,
 			}
 
-			service, err = newTaskQueueService("test_plugin", manager2, 5)
+			service, err = newTaskQueueService(ctx, "test_plugin", manager2, 5)
 			Expect(err).ToNot(HaveOccurred())
 
 			// Override callback to succeed
@@ -775,14 +768,11 @@ var _ = Describe("TaskQueueService", func() {
 
 	Describe("Plugin isolation", func() {
 		It("uses separate databases for different plugins", func() {
-			managerCtx2, cancel2 := context.WithCancel(ctx)
-			DeferCleanup(cancel2)
 			manager2 := &Manager{
 				plugins: make(map[string]*plugin),
-				ctx:     managerCtx2,
 			}
 
-			service2, err := newTaskQueueService("other_plugin", manager2, 5)
+			service2, err := newTaskQueueService(ctx, "other_plugin", manager2, 5)
 			Expect(err).ToNot(HaveOccurred())
 			defer service2.Close()
 
diff --git a/plugins/host_websocket.go b/plugins/host_websocket.go
index eef1e6236..90403f4c0 100644
--- a/plugins/host_websocket.go
+++ b/plugins/host_websocket.go
@@ -54,6 +54,7 @@ type wsConnection struct {
 // webSocketServiceImpl implements host.WebSocketService.
 // It provides plugins with WebSocket communication capabilities.
 type webSocketServiceImpl struct {
+	baseCtx       context.Context // bounds the read loops, which outlive the Connect() call
 	pluginName    string
 	manager       *Manager
 	requiredHosts []string
@@ -63,8 +64,9 @@ type webSocketServiceImpl struct {
 }
 
 // newWebSocketService creates a new WebSocketService for a plugin.
-func newWebSocketService(pluginName string, manager *Manager, permission *WebSocketPermission) *webSocketServiceImpl {
+func newWebSocketService(ctx context.Context, pluginName string, manager *Manager, permission *WebSocketPermission) *webSocketServiceImpl {
 	return &webSocketServiceImpl{
+		baseCtx:       ctx,
 		pluginName:    pluginName,
 		manager:       manager,
 		requiredHosts: permission.RequiredHosts,
@@ -129,11 +131,12 @@ func (s *webSocketServiceImpl) Connect(ctx context.Context, urlStr string, heade
 	s.connections[connectionID] = wsConn
 	s.mu.Unlock()
 
-	// Start read goroutine with manager's context.
-	// We use manager.ctx instead of the caller's ctx because the readLoop must
-	// outlive the Connect() call. The manager's context is cancelled during
-	// application shutdown, ensuring graceful cleanup.
-	go s.readLoop(s.manager.ctx, connectionID, wsConn)
+	// Start read goroutine with the service's base context instead of the
+	// caller's ctx, because the readLoop must outlive the Connect() call.
+	// Connections are closed by Close() when the plugin is unloaded, which ends
+	// the readLoop; the base context is a backstop that also ends it on server
+	// shutdown (it is never cancelled in one-shot CLI runs).
+	go s.readLoop(s.baseCtx, connectionID, wsConn)
 
 	log.Debug(ctx, "WebSocket connected", "plugin", s.pluginName, "connectionID", connectionID, "url", urlStr)
 	return connectionID, nil
diff --git a/plugins/manager_loader.go b/plugins/manager_loader.go
index 604fba3a7..757ededb5 100644
--- a/plugins/manager_loader.go
+++ b/plugins/manager_loader.go
@@ -30,11 +30,23 @@ type serviceContext struct {
 	allLibraries     bool     // If true, plugin can access all libraries
 }
 
+// baseCtx returns the manager's lifecycle context, for host services that
+// outlive the plugin call that created them. It falls back to
+// context.Background() when the manager was never started, which is the case
+// for CLI commands (e.g. `navidrome plugin enable`) that load plugins without
+// calling Start.
+func (c *serviceContext) baseCtx() context.Context {
+	if c.manager.ctx == nil {
+		return context.Background()
+	}
+	return c.manager.ctx
+}
+
 // hostServiceEntry defines a host service for table-driven registration.
 type hostServiceEntry struct {
 	name          string
 	hasPermission func(*Permissions) bool
-	create        func(*serviceContext) ([]extism.HostFunction, io.Closer)
+	create        func(*serviceContext) ([]extism.HostFunction, io.Closer, error)
 }
 
 // hostServices defines all available host services.
@@ -43,119 +55,117 @@ var hostServices = []hostServiceEntry{
 	{
 		name:          "Config",
 		hasPermission: func(p *Permissions) bool { return true }, // Always available, no permission required
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			service := newConfigService(ctx.pluginName, ctx.config)
-			return host.RegisterConfigHostFunctions(service), nil
+			return host.RegisterConfigHostFunctions(service), nil, nil
 		},
 	},
 	{
 		name:          "SubsonicAPI",
 		hasPermission: func(p *Permissions) bool { return p != nil && p.Subsonicapi != nil },
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			service := newSubsonicAPIService(ctx.pluginName, ctx.manager.subsonicRouter, ctx.manager.ds, newUserAccess(ctx.allowedUsers, ctx.allUsers))
-			return host.RegisterSubsonicAPIHostFunctions(service), nil
+			return host.RegisterSubsonicAPIHostFunctions(service), nil, nil
 		},
 	},
 	{
 		name:          "Scheduler",
 		hasPermission: func(p *Permissions) bool { return p != nil && p.Scheduler != nil },
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			service := newSchedulerService(ctx.pluginName, ctx.manager, scheduler.GetInstance())
-			return host.RegisterSchedulerHostFunctions(service), service
+			return host.RegisterSchedulerHostFunctions(service), service, nil
 		},
 	},
 	{
 		name:          "WebSocket",
 		hasPermission: func(p *Permissions) bool { return p != nil && p.Websocket != nil },
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			perm := ctx.permissions.Websocket
-			service := newWebSocketService(ctx.pluginName, ctx.manager, perm)
-			return host.RegisterWebSocketHostFunctions(service), service
+			service := newWebSocketService(ctx.baseCtx(), ctx.pluginName, ctx.manager, perm)
+			return host.RegisterWebSocketHostFunctions(service), service, nil
 		},
 	},
 	{
 		name:          "Artwork",
 		hasPermission: func(p *Permissions) bool { return p != nil && p.Artwork != nil },
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			service := newArtworkService()
-			return host.RegisterArtworkHostFunctions(service), nil
+			return host.RegisterArtworkHostFunctions(service), nil, nil
 		},
 	},
 	{
 		name:          "Cache",
 		hasPermission: func(p *Permissions) bool { return p != nil && p.Cache != nil },
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			service := newCacheService(ctx.pluginName)
-			return host.RegisterCacheHostFunctions(service), service
+			return host.RegisterCacheHostFunctions(service), service, nil
 		},
 	},
 	{
 		name:          "Library",
 		hasPermission: func(p *Permissions) bool { return p != nil && p.Library != nil },
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			perm := ctx.permissions.Library
 			service := newLibraryService(ctx.manager.ds, perm, ctx.allowedLibraries, ctx.allLibraries)
-			return host.RegisterLibraryHostFunctions(service), nil
+			return host.RegisterLibraryHostFunctions(service), nil, nil
 		},
 	},
 	{
 		name:          "KVStore",
 		hasPermission: func(p *Permissions) bool { return p != nil && p.Kvstore != nil },
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			perm := ctx.permissions.Kvstore
-			service, err := newKVStoreService(ctx.manager.ctx, ctx.pluginName, perm)
+			service, err := newKVStoreService(ctx.baseCtx(), ctx.pluginName, perm)
 			if err != nil {
-				log.Error("Failed to create KVStore service", "plugin", ctx.pluginName, err)
-				return nil, nil
+				return nil, nil, err
 			}
-			return host.RegisterKVStoreHostFunctions(service), service
+			return host.RegisterKVStoreHostFunctions(service), service, nil
 		},
 	},
 	{
 		name:          "Users",
 		hasPermission: func(p *Permissions) bool { return p != nil && p.Users != nil },
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			service := newUsersService(ctx.manager.ds, ctx.allowedUsers, ctx.allUsers)
-			return host.RegisterUsersHostFunctions(service), nil
+			return host.RegisterUsersHostFunctions(service), nil, nil
 		},
 	},
 	{
 		name:          "Matcher",
 		hasPermission: func(p *Permissions) bool { return p != nil && p.Matcher != nil },
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			hasFilesystemPerm := ctx.permissions.Library != nil && ctx.permissions.Library.Filesystem
 			service := newMatcherService(
 				ctx.manager.ds, hasFilesystemPerm,
 				newUserAccess(ctx.allowedUsers, ctx.allUsers),
 				newLibraryAccess(ctx.allowedLibraries, ctx.allLibraries),
 			)
-			return host.RegisterMatcherHostFunctions(service), nil
+			return host.RegisterMatcherHostFunctions(service), nil, nil
 		},
 	},
 	{
 		name:          "HTTP",
 		hasPermission: func(p *Permissions) bool { return p != nil && p.Http != nil },
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			perm := ctx.permissions.Http
 			service := newHTTPService(ctx.pluginName, perm)
-			return host.RegisterHTTPHostFunctions(service), nil
+			return host.RegisterHTTPHostFunctions(service), nil, nil
 		},
 	},
 	{
 		name:          "Task",
 		hasPermission: func(p *Permissions) bool { return p != nil && p.Taskqueue != nil },
-		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
+		create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
 			perm := ctx.permissions.Taskqueue
 			maxConcurrency := int32(1)
 			if perm.MaxConcurrency > 0 {
 				maxConcurrency = int32(perm.MaxConcurrency)
 			}
-			service, err := newTaskQueueService(ctx.pluginName, ctx.manager, maxConcurrency)
+			service, err := newTaskQueueService(ctx.baseCtx(), ctx.pluginName, ctx.manager, maxConcurrency)
 			if err != nil {
-				log.Error("Failed to create Task service", "plugin", ctx.pluginName, err)
-				return nil, nil
+				return nil, nil, err
 			}
-			return host.RegisterTaskHostFunctions(service), service
+			return host.RegisterTaskHostFunctions(service), service, nil
 		},
 	},
 }
@@ -256,6 +266,7 @@ func (m *Manager) loadEnabledPlugins(ctx context.Context) error {
 // loadPluginWithConfig loads a plugin with configuration from DB.
 // The p.Path should point to an .ndp package file.
 func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
+	// NewContext falls back to context.Background() when m.ctx is nil (unstarted manager)
 	ctx := log.NewContext(m.ctx, "plugin", p.ID)
 
 	if m.stopped.Load() {
@@ -328,6 +339,15 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
 	// Build host functions based on permissions from manifest
 	var hostFunctions []extism.HostFunction
 	var closers []io.Closer
+	loaded := false
+	// On success the closers are owned by the registered plugin; on any
+	// failure past this point, close them so partially-created services
+	// don't leak goroutines or file handles.
+	defer func() {
+		if !loaded {
+			closeAll(closers)
+		}
+	}()
 
 	svcCtx := &serviceContext{
 		pluginName:       p.ID,
@@ -341,7 +361,10 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
 	}
 	for _, entry := range hostServices {
 		if entry.hasPermission(pkg.Manifest.Permissions) {
-			funcs, closer := entry.create(svcCtx)
+			funcs, closer, err := entry.create(svcCtx)
+			if err != nil {
+				return fmt.Errorf("creating %s service: %w", entry.name, err)
+			}
 			hostFunctions = append(hostFunctions, funcs...)
 			if closer != nil {
 				closers = append(closers, closer)
@@ -400,6 +423,7 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
 		libraries:      newLibraryAccess(allowedLibraries, p.AllLibraries),
 	}
 	m.mu.Unlock()
+	loaded = true
 
 	// Call plugin init function
 	callPluginInit(ctx, m.plugins[p.ID])
@@ -407,6 +431,14 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
 	return nil
 }
 
+// closeAll closes host service closers accumulated before a load failure,
+// so partially-created services don't leak goroutines or file handles.
+func closeAll(closers []io.Closer) {
+	for _, c := range closers {
+		_ = c.Close()
+	}
+}
+
 // parsePluginConfig parses a JSON config string into a map of string values.
 // For Extism, all config values must be strings, so non-string values are serialized as JSON.
 func parsePluginConfig(configJSON string) (map[string]string, error) {
diff --git a/plugins/manager_loader_load_test.go b/plugins/manager_loader_load_test.go
new file mode 100644
index 000000000..8f35548af
--- /dev/null
+++ b/plugins/manager_loader_load_test.go
@@ -0,0 +1,78 @@
+//go:build !windows
+
+package plugins
+
+import (
+	"crypto/sha256"
+	"encoding/hex"
+	"net/http"
+	"os"
+	"path/filepath"
+
+	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/conf/configtest"
+	"github.com/navidrome/navidrome/model"
+	"github.com/navidrome/navidrome/tests"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("loadPluginWithConfig", func() {
+	var manager *Manager
+	var dataDir string
+
+	BeforeEach(func() {
+		pluginsDir := GinkgoT().TempDir()
+		dataDir = GinkgoT().TempDir()
+
+		src := filepath.Join(testdataDir, "test-taskqueue"+PackageExtension)
+		data, err := os.ReadFile(src)
+		Expect(err).ToNot(HaveOccurred())
+		dest := filepath.Join(pluginsDir, "test-taskqueue"+PackageExtension)
+		Expect(os.WriteFile(dest, data, 0600)).To(Succeed())
+		hash := sha256.Sum256(data)
+
+		DeferCleanup(configtest.SetupConfig())
+		conf.Server.Plugins.Enabled = true
+		conf.Server.Plugins.Folder = conf.NewDir(pluginsDir)
+		conf.Server.Plugins.AutoReload = false
+		conf.Server.DataFolder = conf.NewDir(dataDir)
+
+		repo := tests.CreateMockPluginRepo()
+		repo.Permitted = true
+		repo.SetData(model.Plugins{{
+			ID:      "test-taskqueue",
+			Path:    dest,
+			SHA256:  hex.EncodeToString(hash[:]),
+			Enabled: false,
+		}})
+		manager = &Manager{
+			plugins:        make(map[string]*plugin),
+			ds:             &tests.MockDataStore{MockedPlugin: repo},
+			metrics:        noopMetricsRecorder{},
+			subsonicRouter: http.NotFoundHandler(),
+		}
+	})
+
+	Describe("host service creation failures", func() {
+		It("reports the Task service creation error instead of a missing host function", func() {
+			Expect(manager.Start(GinkgoT().Context())).To(Succeed())
+			DeferCleanup(func() { _ = manager.Stop() })
+
+			// Block the taskqueue data dir by creating a file where the directory should be
+			Expect(os.WriteFile(filepath.Join(dataDir, "plugins"), nil, 0600)).To(Succeed())
+
+			err := manager.EnablePlugin(GinkgoT().Context(), "test-taskqueue")
+			Expect(err).To(MatchError(ContainSubstring("creating Task service")))
+			Expect(err).ToNot(MatchError(ContainSubstring("not exported")))
+		})
+	})
+
+	Describe("unstarted manager", func() {
+		It("enables a taskqueue plugin on a manager that was never started", func() {
+			// CLI commands (navidrome plugin enable) use the manager without calling Start
+			Expect(manager.EnablePlugin(GinkgoT().Context(), "test-taskqueue")).To(Succeed())
+			DeferCleanup(func() { _ = manager.unloadPlugin("test-taskqueue") })
+		})
+	})
+})

From e91687e760a8b4e19dc74b6a9b1ff7f3674565ce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= <deluan@navidrome.org>
Date: Fri, 10 Jul 2026 20:27:29 -0400
Subject: [PATCH 5/6] fix(smartplaylist): reject NSP mixing top-level 'any' and
 'all' (#5759)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* test(scanner): fix flaky Windows search_normalized rescan test

The 'repopulates a stale search_normalized on a full rescan' spec runs
two full scans back-to-back. Whether the second scan refreshes the
unchanged artist depends on folderEntry.isOutdated(), which compares
folder.updated_at (written during the first scan) against the second
scan's library.last_scan_started_at using a strict time.Before(). Both
are time.Now() values captured milliseconds apart.

On Linux's fine-grained clock they are always distinct, so the test
passes. On Windows the coarse wall-clock granularity frequently makes
the two timestamps land in the same tick and compare equal, so
Before() returns false, the folder is treated as up-to-date and
skipped, the artist is never re-persisted, and search_normalized stays
empty -- failing the assertion intermittently across unrelated PRs.

Backdate the folder's updated_at an hour before the second scan so the
comparison is unambiguous on every platform. This is a test-only
timing artifact (real rescans never run milliseconds apart on an
unchanged library), so no production code changes are needed.

* fix(smartplaylist): reject NSP mixing top-level 'any' and 'all'

A smart playlist (.nsp) that specified both a top-level "any" and a
top-level "all" group was imported by silently keeping only "any" and
discarding "all", regardless of key order. The Criteria model holds a
single top-level Expression, so it cannot represent both groups, and the
parser picked "any" without reporting the dropped rules.

Make Criteria.UnmarshalJSON return an error when both keys are present at
the top level, so the scanner fails loudly (logging the playlist as
invalid) instead of silently losing rules. Users should nest one group
inside the other, as shown in the documented examples.

Fixes #5757

* fix(smartplaylist): reject top-level any+all by key presence

Address code review feedback: the previous guard checked decoded slice
lengths, so it only rejected the mixed top-level any/all form when both
groups were non-empty. An input like {"any":[],"all":[...]} (or a
null group) slipped past and silently used just one group — the same
class of silent drop this change set out to prevent.

Decode the two keys as json.RawMessage and detect presence by key rather
than length, so any file that provides both top-level keys is rejected
regardless of whether one group is empty or null.

* refactor(smartplaylist): detect top-level any+all via presence type

Replace the json.RawMessage + manual double-unmarshal in
Criteria.UnmarshalJSON with a small optionalConjunction wrapper whose
UnmarshalJSON records that its key was present. Because encoding/json
invokes UnmarshalJSON even for a JSON null, this keeps the exact
behavior (a present-but-empty or null group still counts, so mixing
both top-level keys is rejected) while decoding in a single pass — no
raw-message capture, no re-decode, no shadow variables.

No behavior change; existing tests pass unchanged.
---
 core/playlists/parse_nsp_test.go | 14 ++++++++++++++
 model/criteria/criteria.go       | 27 ++++++++++++++++-----------
 model/criteria/criteria_test.go  | 22 ++++++++++++++++++++++
 model/criteria/json.go           | 14 ++++++++++++++
 4 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/core/playlists/parse_nsp_test.go b/core/playlists/parse_nsp_test.go
index 516a5355d..d6d69866f 100644
--- a/core/playlists/parse_nsp_test.go
+++ b/core/playlists/parse_nsp_test.go
@@ -113,6 +113,20 @@ var _ = Describe("parseNSP", func() {
 		Expect(err.Error()).To(ContainSubstring("SmartPlaylist"))
 	})
 
+	It("rejects a NSP that mixes top-level 'any' and 'all' instead of silently dropping a group", func() {
+		nsp := `{
+			"name": "Overplayed Favorites",
+			"any": [{"inPlaylist": {"path": "most-played-favorites.nsp"}}],
+			"all": [{"notInPlaylist": {"path": "favorites-not-played-in-4-yrs.nsp"}}],
+			"sort": "playCount, lastPlayed"
+		}`
+		pls := &model.Playlist{}
+		err := s.parseNSP(ctx, pls, strings.NewReader(nsp))
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("SmartPlaylist"))
+		Expect(err.Error()).To(And(ContainSubstring("all"), ContainSubstring("any")))
+	})
+
 	It("gracefully handles non-string name field", func() {
 		nsp := `{"name": 123, "all": [{"is": {"loved": true}}]}`
 		pls := &model.Playlist{Name: "Original"}
diff --git a/model/criteria/criteria.go b/model/criteria/criteria.go
index 31d208d08..8c3d183a9 100644
--- a/model/criteria/criteria.go
+++ b/model/criteria/criteria.go
@@ -103,21 +103,26 @@ func (c Criteria) MarshalJSON() ([]byte, error) {
 
 func (c *Criteria) UnmarshalJSON(data []byte) error {
 	var aux struct {
-		All          unmarshalConjunctionType `json:"all"`
-		Any          unmarshalConjunctionType `json:"any"`
-		Sort         string                   `json:"sort"`
-		Order        string                   `json:"order"`
-		Limit        int                      `json:"limit"`
-		LimitPercent int                      `json:"limitPercent"`
-		Offset       int                      `json:"offset"`
+		All          optionalConjunction `json:"all"`
+		Any          optionalConjunction `json:"any"`
+		Sort         string              `json:"sort"`
+		Order        string              `json:"order"`
+		Limit        int                 `json:"limit"`
+		LimitPercent int                 `json:"limitPercent"`
+		Offset       int                 `json:"offset"`
 	}
 	if err := json.Unmarshal(data, &aux); err != nil {
 		return err
 	}
-	if len(aux.Any) > 0 {
-		c.Expression = Any(aux.Any)
-	} else if len(aux.All) > 0 {
-		c.Expression = All(aux.All)
+	// A Criteria has a single top-level group. Reject files that provide both keys
+	// (even when one is [] or null) rather than silently dropping one of them.
+	if aux.All.present && aux.Any.present {
+		return errors.New("invalid criteria json: 'all' and 'any' cannot both be used at the top level; nest one inside the other instead")
+	}
+	if len(aux.Any.rules) > 0 {
+		c.Expression = Any(aux.Any.rules)
+	} else if len(aux.All.rules) > 0 {
+		c.Expression = All(aux.All.rules)
 	} else {
 		return errors.New("invalid criteria json. missing rules (key 'all' or 'any')")
 	}
diff --git a/model/criteria/criteria_test.go b/model/criteria/criteria_test.go
index 092cfd36a..7f214e703 100644
--- a/model/criteria/criteria_test.go
+++ b/model/criteria/criteria_test.go
@@ -80,6 +80,28 @@ var _ = Describe("Criteria", func() {
 		})
 	})
 
+	Context("with both top-level 'all' and 'any'", func() {
+		It("returns an error instead of silently dropping one of the groups", func() {
+			jsonStr := `{"any":[{"inPlaylist":{"path":"a.nsp"}}],"all":[{"notInPlaylist":{"path":"b.nsp"}}]}`
+			var c Criteria
+			err := json.Unmarshal([]byte(jsonStr), &c)
+			gomega.Expect(err).To(gomega.HaveOccurred())
+			gomega.Expect(err.Error()).To(gomega.And(gomega.ContainSubstring("all"), gomega.ContainSubstring("any")))
+		})
+
+		DescribeTable("rejects both keys even when one group is present but empty",
+			func(jsonStr string) {
+				var c Criteria
+				err := json.Unmarshal([]byte(jsonStr), &c)
+				gomega.Expect(err).To(gomega.HaveOccurred())
+				gomega.Expect(err.Error()).To(gomega.And(gomega.ContainSubstring("all"), gomega.ContainSubstring("any")))
+			},
+			Entry("empty any", `{"any":[],"all":[{"is":{"loved":true}}]}`),
+			Entry("empty all", `{"all":[],"any":[{"is":{"loved":true}}]}`),
+			Entry("null any", `{"any":null,"all":[{"is":{"loved":true}}]}`),
+		)
+	})
+
 	Describe("LimitPercent", func() {
 		Describe("JSON round-trip", func() {
 			It("marshals and unmarshals limitPercent", func() {
diff --git a/model/criteria/json.go b/model/criteria/json.go
index beded9d1f..d0f453524 100644
--- a/model/criteria/json.go
+++ b/model/criteria/json.go
@@ -33,6 +33,20 @@ func (uc *unmarshalConjunctionType) UnmarshalJSON(data []byte) error {
 	return nil
 }
 
+// optionalConjunction is a top-level "all"/"any" value that remembers whether its
+// key was present at all, so a Criteria providing both can be rejected. encoding/json
+// calls UnmarshalJSON even for a JSON null, so present is set whenever the key appears
+// — including as [] or null — while an absent key leaves it false.
+type optionalConjunction struct {
+	present bool
+	rules   unmarshalConjunctionType
+}
+
+func (o *optionalConjunction) UnmarshalJSON(data []byte) error {
+	o.present = true
+	return json.Unmarshal(data, &o.rules)
+}
+
 func unmarshalExpression(opName string, rawValue json.RawMessage) Expression {
 	m := make(map[string]any)
 	err := json.Unmarshal(rawValue, &m)

From be10f89c117925fabf10394b8d2962a370108b97 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= <deluan@navidrome.org>
Date: Sat, 11 Jul 2026 09:18:14 -0400
Subject: [PATCH 6/6] ci: don't skip release jobs after the DB migration check
 on tag pushes (#5760)

* ci: don't skip release jobs after the DB migration check on tag pushes

The validate-migrations job added in #5750 was gated at job level with
if: github.event_name == 'pull_request', so it concluded "skipped" on tag
pushes. GitHub Actions propagates a skipped job transitively through the
needs chain (actions/runner#491): even though Build overrode its own gate
with !cancelled() && !failure() and ran successfully, every job downstream
of Build (msi, release, push-manifest-*, PKG uploads) still failed the
implicit success() check and was skipped, which broke the v0.63.2 release.

Move the pull_request gate from the job to its steps. On non-PR events all
steps are skipped and the job concludes "success", so downstream jobs run
normally. This also restores the default success() gate on Build, keeping
the fail-fast behavior on PRs with a bad migration.

* ci: trim workflow comment

Condense the explanation of the step-level pull_request gate on the
validate-migrations job to the essential rationale.
---
 .github/workflows/pipeline.yml | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml
index d21d0a681..86a1055f8 100644
--- a/.github/workflows/pipeline.yml
+++ b/.github/workflows/pipeline.yml
@@ -99,16 +99,20 @@ jobs:
   validate-migrations:
     name: Validate DB migrations
     runs-on: ubuntu-latest
-    if: github.event_name == 'pull_request'
+    # PR-only gate is at step level: a job-level skip would propagate through
+    # the needs chain (actions/runner#491) and skip all release jobs on tag pushes.
     steps:
       - uses: actions/checkout@v7
+        if: github.event_name == 'pull_request'
         with:
           fetch-depth: 0
       # Refresh the base branch so the check compares against its CURRENT tip,
       # not the (possibly stale) commit the PR was opened against.
       - name: Fetch latest base branch
+        if: github.event_name == 'pull_request'
         run: git fetch --no-tags origin "+refs/heads/${{ github.event.pull_request.base.ref }}:refs/remotes/origin/${{ github.event.pull_request.base.ref }}"
       - name: Validate migration ordering and naming
+        if: github.event_name == 'pull_request'
         env:
           BASE_REF: origin/${{ github.event.pull_request.base.ref }}
         run: ./.github/workflows/validate-migrations.sh
@@ -275,10 +279,6 @@ jobs:
   build:
     name: Build
     needs: [js, go, go-windows, go-lint, i18n-lint, git-version, check-push-enabled, validate-migrations]
-    # validate-migrations only runs on pull_request, so it is "skipped" on push/tag
-    # builds. Run Build unless a dependency actually failed — a *skipped* dependency
-    # (the migration check on non-PR events) must not block release builds.
-    if: ${{ !cancelled() && !failure() }}
     strategy:
       matrix:
         platform: [ linux/amd64, linux/arm64, linux/arm/v5, linux/arm/v6, linux/arm/v7, linux/386, linux/riscv64, darwin/amd64, darwin/arm64, windows/amd64, windows/386 ]