fix(plugins): reject plugin IDs that are unusable as directory names (#5886)

The plugin ID is derived from the package filename and used verbatim as a
directory name under DataFolder/plugins by the kvstore and taskqueue host
services. A package installed as '..ndp' yields the ID '.', whose data
directory resolves to the parent of every other plugin's directory, so it
overlaps their private data. On Windows, trailing dots and spaces are dropped
during path normalization, so 'foo..ndp' and 'foo.ndp' yield distinct IDs that
resolve to the same directory and would share the same SQLite files.

Discovery and the file watcher now derive the ID through pluginIDFromPath,
which rejects '.', '..', empty names, separators, trailing dots or spaces, and
anything filepath.IsLocal refuses (Windows reserved names, drive-relative
paths). The loader repeats the check, since a sync failure is non-fatal and
could otherwise leave a stale row reaching the host services.

plugins/manager_loader.go

@@ -271,6 +271,10 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
 		return fmt.Errorf("manager is stopped")
 	}
 
+	if !validPluginID(p.ID) {
+		return fmt.Errorf("invalid plugin ID %q", p.ID)
+	}
+
 	// Track this operation
 	m.loadWg.Add(1)
 	defer m.loadWg.Done()

plugins/manager_loader_test.go

@@ -1,6 +1,7 @@
 package plugins
 
 import (
+	"github.com/navidrome/navidrome/model"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
@@ -27,6 +28,18 @@ var _ = Describe("buildExtismManifest", func() {
 	})
 })
 
+var _ = Describe("loadPluginWithConfig", func() {
+	// Discovery already rejects these, but a row predating that check, or one
+	// left behind by a failed sync, must not reach the mount setup
+	It("refuses a plugin whose ID is not usable as a directory name", func() {
+		m := &Manager{plugins: make(map[string]*plugin)}
+
+		err := m.loadPluginWithConfig(&model.Plugin{ID: "..", Path: "/does/not/matter.ndp"})
+
+		Expect(err).To(MatchError(ContainSubstring("invalid plugin ID")))
+	})
+})
+
 var _ = Describe("parsePluginConfig", func() {
 	It("returns nil for empty string", func() {
 		result, err := parsePluginConfig("")

plugins/manager_sync.go

@@ -152,7 +152,11 @@ func (m *Manager) syncPlugins(ctx context.Context, folder string) error {
 			log.Trace(ctx, "Skipping non-plugin entry", "name", entry.Name(), "isDir", entry.IsDir())
 			continue
 		}
-		name := strings.TrimSuffix(entry.Name(), PackageExtension)
+		name, ok := pluginIDFromPath(entry.Name())
+		if !ok {
+			log.Warn(ctx, "Skipping plugin with unusable name", "name", entry.Name())
+			continue
+		}
 		filesOnDisk[name] = filepath.Join(folder, entry.Name())
 	}
 	log.Debug(ctx, "Plugin sync: scanned folder", "folder", folder, "entriesTotal", len(entries), "pluginsFound", len(filesOnDisk))

plugins/manager_sync_test.go

@@ -12,6 +12,46 @@ import (
 	. "github.com/onsi/gomega"
 )
 
+var _ = Describe("syncPlugins", func() {
+	var m *Manager
+	var repo *tests.MockPluginRepo
+	var folder string
+
+	BeforeEach(func() {
+		folder = GinkgoT().TempDir()
+		repo = tests.CreateMockPluginRepo()
+		repo.SetData(model.Plugins{})
+		m = &Manager{ds: &tests.MockDataStore{MockedPlugin: repo}}
+	})
+
+	writePackage := func(name string) {
+		GinkgoHelper()
+		manifest := &Manifest{Name: "Test Plugin", Author: "Test Author", Version: "1.0.0"}
+		wasm := []byte{0x00, 0x61, 0x73, 0x6d} // Minimal wasm header
+		Expect(createTestPackage(filepath.Join(folder, name), manifest, wasm)).To(Succeed())
+	}
+
+	It("registers a plugin with a usable ID", func() {
+		writePackage("my-plugin" + PackageExtension)
+
+		Expect(m.syncPlugins(context.Background(), folder)).To(Succeed())
+
+		_, err := repo.Get("my-plugin")
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	It("skips packages whose name yields a path-like ID", func() {
+		writePackage("." + PackageExtension)
+		writePackage(".." + PackageExtension)
+
+		Expect(m.syncPlugins(context.Background(), folder)).To(Succeed())
+
+		all, err := repo.GetAll()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(all).To(BeEmpty())
+	})
+})
+
 var _ = Describe("removePluginFromDB", func() {
 	It("discards buffered scrobbles for the removed plugin", func() {
 		ctx := context.Background()

plugins/manager_watcher.go

@@ -89,7 +89,11 @@ func (m *Manager) handleWatcherEvent(event notify.EventInfo) {
 		return
 	}
 
-	pluginName := strings.TrimSuffix(filepath.Base(path), PackageExtension)
+	pluginName, ok := pluginIDFromPath(path)
+	if !ok {
+		log.Warn(m.ctx, "Ignoring plugin file with unusable name", "path", path)
+		return
+	}
 
 	log.Trace(m.ctx, "Plugin file event", "plugin", pluginName, "event", event.Event(), "path", path)
 

plugins/package.go

@@ -5,6 +5,8 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"path/filepath"
+	"strings"
 )
 
 const (
@@ -18,6 +20,26 @@ const (
 	wasmFileName = "plugin.wasm"
 )
 
+// validPluginID reports whether id is usable. It names a directory under
+// DataFolder/plugins, so a path-like one would point at another plugin's data.
+func validPluginID(id string) bool {
+	if id == "." || id == ".." || strings.ContainsAny(id, `/\`) || !filepath.IsLocal(id) {
+		return false
+	}
+	// Windows drops trailing dots and spaces, so "foo." and "foo" would end up
+	// sharing a directory
+	return strings.TrimRight(id, ". ") == id
+}
+
+// pluginIDFromPath derives the plugin ID from a package path.
+func pluginIDFromPath(path string) (string, bool) {
+	id := strings.TrimSuffix(filepath.Base(path), PackageExtension)
+	if !validPluginID(id) {
+		return "", false
+	}
+	return id, true
+}
+
 // ndpPackage represents a loaded .ndp plugin package.
 // It contains the manifest and wasm bytes read from the archive.
 type ndpPackage struct {

plugins/package_test.go

@@ -11,6 +11,36 @@ import (
 	. "github.com/onsi/gomega"
 )
 
+var _ = Describe("pluginIDFromPath", func() {
+	DescribeTable("derives the ID from the package filename",
+		func(path, expected string) {
+			id, ok := pluginIDFromPath(path)
+			Expect(ok).To(BeTrue())
+			Expect(id).To(Equal(expected))
+		},
+		Entry("plain name", "/plugins/discord-rich-presence.ndp", "discord-rich-presence"),
+		Entry("name with spaces", "/plugins/My Plugin.ndp", "My Plugin"),
+		Entry("leading dot", "/plugins/.hidden.ndp", ".hidden"),
+		Entry("dots inside", "/plugins/v1.2.3.ndp", "v1.2.3"),
+	)
+
+	// The ID becomes a directory name under DataFolder/plugins, so a path-like
+	// one would let a plugin reach another plugin's data
+	DescribeTable("rejects IDs that are unsafe as a directory name",
+		func(path string) {
+			_, ok := pluginIDFromPath(path)
+			Expect(ok).To(BeFalse())
+		},
+		Entry("empty", "/plugins/.ndp"),
+		Entry("current directory", "/plugins/..ndp"),
+		Entry("parent directory", "/plugins/...ndp"),
+		// Windows drops trailing dots and spaces, so these would share a
+		// directory with "foo"
+		Entry("trailing dot", "/plugins/foo..ndp"),
+		Entry("trailing space", "/plugins/foo .ndp"),
+	)
+})
+
 var _ = Describe("ndpPackage", func() {
 	var tmpDir string