fix(playlist/share): apply user library access to import and sharing paths (#5640)

* fix(playlist): respect the user's library access when resolving M3U paths

FindByPaths looked up paths across all libraries, so importing an M3U
could add tracks from libraries the importing user has no access to.
Apply the user's library filter to the lookup, matching every other
media_file read. Admins and the (admin-context) scanner are unaffected.

* fix(share): scope shared playlist tracks to the owner's libraries

loadMedia loaded playlist tracks with a fake-admin context, so a shared
playlist could include tracks from libraries the owner has no access to.
Load as the share owner instead, so the library filter applies.
Admin-owned shares are unchanged.

* fix(share): only serve shared tracks the owner can access

A shared stream fetched the media file by id without checking the share
owner's library access, so it could serve tracks from libraries the
owner has no access to. Gate share-scoped streams on the owner's library
access. Non-share streams are unaffected.

* test(share): tidy library-access test setup

Consolidate the repeated share-owner test fixture in handleStream into a
helper, assert on track fields with HaveField instead of building an id
slice, and delete the scratch media file through the public repository
method.

* style: trim verbose comments in library-access checks

* fix(share): guard against nil owner and clean up test users

Add a nil check after loading the share owner so a missing user yields a
clear error instead of a possible nil dereference, and delete the users
created by the new tests in their AfterEach blocks.

* fix(share): avoid panic when a shared playlist is no longer visible to its owner

Tracks() returns nil when the playlist can't be loaded under the owner's
context (e.g. a public playlist shared by a non-owner that was later made
private). Capture the result and return early instead of chaining GetAll
on a nil repository, leaving the share with no tracks.

persistence/mediafile_repository.go

@@ -306,7 +306,7 @@ func (r *mediaFileRepository) FindByPaths(paths []string) (model.MediaFiles, err
 		return model.MediaFiles{}, nil
 	}
 
-	sel := r.newSelect().Columns("*").Where(query)
+	sel := r.applyLibraryFilter(r.newSelect().Columns("*").Where(query))
 	var res dbMediaFiles
 	if err := r.queryAll(sel, &res); err != nil {
 		return nil, err

persistence/mediafile_repository_test.go

@@ -880,6 +880,58 @@ var _ = Describe("MediaRepository", func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(results).To(BeEmpty())
 		})
+
+		Context("when the user has restricted library access", func() {
+			var otherLib model.Library
+			var restrictedUser model.User
+
+			BeforeEach(func() {
+				adminCtx := request.WithUser(GinkgoT().Context(), adminUser)
+				lr := NewLibraryRepository(adminCtx, GetDBXBuilder())
+
+				// A second library the restricted user has no access to
+				otherLib = model.Library{ID: 0, Name: "Other Library", Path: "/other/lib"}
+				Expect(lr.Put(&otherLib)).To(Succeed())
+
+				// A track that lives only in the other library (created as admin)
+				adminMr := NewMediaFileRepository(adminCtx, GetDBXBuilder())
+				Expect(adminMr.Put(&model.MediaFile{
+					ID: "otherlib-track", LibraryID: otherLib.ID,
+					Path: "hidden/test.mp3", Title: "Hidden",
+				})).To(Succeed())
+
+				// Non-admin user with access to library 1 ONLY
+				restrictedUser = createUserWithLibraries("restricted-finder", []int{1})
+				ur := NewUserRepository(adminCtx, GetDBXBuilder())
+				Expect(ur.Put(&restrictedUser)).To(Succeed())
+				Expect(ur.SetUserLibraries(restrictedUser.ID, []int{1})).To(Succeed())
+			})
+
+			AfterEach(func() {
+				adminCtx := request.WithUser(GinkgoT().Context(), adminUser)
+				_ = NewMediaFileRepository(adminCtx, GetDBXBuilder()).Delete("otherlib-track")
+				lr := NewLibraryRepository(adminCtx, GetDBXBuilder()).(*libraryRepository)
+				_ = lr.delete(squirrel.Eq{"id": otherLib.ID})
+				_ = NewUserRepository(adminCtx, GetDBXBuilder()).Delete(restrictedUser.ID)
+			})
+
+			It("does not resolve paths in libraries the user cannot access", func() {
+				userMr := NewMediaFileRepository(request.WithUser(GinkgoT().Context(), restrictedUser), GetDBXBuilder())
+				qualified := fmt.Sprintf("%d:hidden/test.mp3", otherLib.ID)
+				results, err := userMr.FindByPaths([]string{qualified})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(results).To(BeEmpty(), "a track outside the user's libraries must not be resolvable")
+			})
+
+			It("still resolves the path for an admin", func() {
+				adminMr := NewMediaFileRepository(request.WithUser(GinkgoT().Context(), adminUser), GetDBXBuilder())
+				qualified := fmt.Sprintf("%d:hidden/test.mp3", otherLib.ID)
+				results, err := adminMr.FindByPaths([]string{qualified})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(results).To(HaveLen(1))
+				Expect(results[0].ID).To(Equal("otherlib-track"))
+			})
+		})
 	})
 
 	Describe("wrapMediaFileCursor", func() {

persistence/share_repository.go

@@ -100,16 +100,28 @@ func (r *shareRepository) loadMedia(share *model.Share) error {
 		share.Tracks, err = mfRepo.GetAll(model.QueryOptions{Filters: noMissing(Eq{"album_id": ids}), Sort: "album"})
 		return err
 	case "playlist":
-		// Create a context with a fake admin user, to be able to access all playlists
-		ctx := request.WithUser(r.ctx, model.User{IsAdmin: true})
+		// Load tracks as the share owner so their library access is applied.
+		owner, err := NewUserRepository(r.ctx, r.db).Get(share.UserID)
+		if err != nil {
+			return fmt.Errorf("loading share owner %q: %w", share.UserID, err)
+		}
+		if owner == nil {
+			return fmt.Errorf("share owner %q not found", share.UserID)
+		}
+		ctx := request.WithUser(r.ctx, *owner)
 		plsRepo := NewPlaylistRepository(ctx, r.db)
-		tracks, err := plsRepo.Tracks(ids[0], true).GetAll(model.QueryOptions{Sort: "id", Filters: noMissing(Eq{})})
+		// Tracks returns nil when the playlist is no longer visible to the owner
+		// (e.g. it was made private after the share was created); leave the share
+		// with no tracks rather than exposing it.
+		trackRepo := plsRepo.Tracks(ids[0], true)
+		if trackRepo == nil {
+			return nil
+		}
+		tracks, err := trackRepo.GetAll(model.QueryOptions{Sort: "id", Filters: noMissing(Eq{})})
 		if err != nil {
 			return err
 		}
-		if len(tracks) >= 0 {
-			share.Tracks = tracks.MediaFiles()
-		}
+		share.Tracks = tracks.MediaFiles()
 		return nil
 	case "media_file":
 		mfRepo := NewMediaFileRepository(r.ctx, r.db)

persistence/share_repository_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"time"
 
+	"github.com/Masterminds/squirrel"
 	"github.com/deluan/rest"
 	"github.com/navidrome/navidrome/conf/configtest"
 	"github.com/navidrome/navidrome/log"
@@ -132,6 +133,101 @@ var _ = Describe("ShareRepository", func() {
 		})
 	})
 
+	Describe("Playlist share library scoping", func() {
+		var otherLib model.Library
+		var owner model.User
+		var plsID string
+
+		BeforeEach(func() {
+			adminCtx := request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
+
+			// A second library the owner has no access to, plus a track in it
+			lr := NewLibraryRepository(adminCtx, GetDBXBuilder())
+			otherLib = model.Library{ID: 0, Name: "Share Other Library", Path: "/share/other/lib"}
+			Expect(lr.Put(&otherLib)).To(Succeed())
+			mr := NewMediaFileRepository(adminCtx, GetDBXBuilder())
+			Expect(mr.Put(&model.MediaFile{ID: "share-other", LibraryID: otherLib.ID, Path: "s/other.mp3", Title: "ShareOther"})).To(Succeed())
+			Expect(mr.Put(&model.MediaFile{ID: "share-ok", LibraryID: 1, Path: "s/ok.mp3", Title: "ShareOK"})).To(Succeed())
+
+			// Non-admin owner with access to library 1 only
+			owner = createUserWithLibraries("share-owner", []int{1})
+			ur := NewUserRepository(adminCtx, GetDBXBuilder())
+			Expect(ur.Put(&owner)).To(Succeed())
+			Expect(ur.SetUserLibraries(owner.ID, []int{1})).To(Succeed())
+
+			// Owner-owned playlist containing tracks from both libraries
+			plsID = "share-scope-pls"
+			ownerCtx := request.WithUser(log.NewContext(GinkgoT().Context()), owner)
+			pr := NewPlaylistRepository(ownerCtx, GetDBXBuilder())
+			pls := &model.Playlist{ID: plsID, Name: "Scope Test", OwnerID: owner.ID}
+			pls.AddMediaFiles(model.MediaFiles{{ID: "share-ok"}, {ID: "share-other"}})
+			Expect(pr.Put(pls)).To(Succeed())
+
+			// Share row owned by the non-admin owner
+			_, err := GetDBXBuilder().NewQuery(`
+				INSERT INTO share (id, user_id, description, resource_type, resource_ids, created_at, updated_at)
+				VALUES ({:id}, {:user}, {:desc}, {:type}, {:ids}, {:created}, {:updated})
+			`).Bind(map[string]any{
+				"id": "share-scope", "user": owner.ID, "desc": "Scope test share",
+				"type": "playlist", "ids": plsID, "created": time.Now(), "updated": time.Now(),
+			}).Execute()
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		AfterEach(func() {
+			adminCtx := request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
+			b := GetDBXBuilder()
+			_, _ = b.NewQuery(`DELETE FROM share WHERE id = 'share-scope'`).Execute()
+			pr := NewPlaylistRepository(adminCtx, b)
+			_ = pr.Delete(plsID)
+			mr := NewMediaFileRepository(adminCtx, b).(*mediaFileRepository)
+			_, _ = mr.executeSQL(squirrel.Delete("media_file").Where(squirrel.Eq{"id": []string{"share-other", "share-ok"}}))
+			lr := NewLibraryRepository(adminCtx, b).(*libraryRepository)
+			_ = lr.delete(squirrel.Eq{"id": otherLib.ID})
+			_ = NewUserRepository(adminCtx, b).Delete(owner.ID)
+		})
+
+		It("excludes tracks the owner cannot access from the shared playlist", func() {
+			// Read the share as admin (mimics the public-share render path, which uses
+			// the share repository's own context). loadMedia must scope to the owner.
+			adminRepo := NewShareRepository(request.WithUser(log.NewContext(GinkgoT().Context()), adminUser), GetDBXBuilder())
+			share, err := adminRepo.Get("share-scope")
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(share.Tracks).To(ContainElement(HaveField("ID", "share-ok")))
+			Expect(share.Tracks).ToNot(ContainElement(HaveField("ID", "share-other")),
+				"a track outside the owner's libraries must not appear in the share")
+		})
+
+		It("returns no tracks when the playlist is not visible to the owner", func() {
+			// A private playlist owned by someone else: the share owner can no longer
+			// see it, so Tracks() returns nil. The share must render with no tracks
+			// instead of panicking.
+			privatePlsID := "private-pls"
+			adminCtx := request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
+			pr := NewPlaylistRepository(adminCtx, GetDBXBuilder())
+			privatePls := &model.Playlist{ID: privatePlsID, Name: "Private", OwnerID: adminUser.ID, Public: false}
+			privatePls.AddMediaFiles(model.MediaFiles{{ID: "share-ok"}})
+			Expect(pr.Put(privatePls)).To(Succeed())
+			DeferCleanup(func() { _ = pr.Delete(privatePlsID) })
+
+			_, err := GetDBXBuilder().NewQuery(`
+				INSERT INTO share (id, user_id, description, resource_type, resource_ids, created_at, updated_at)
+				VALUES ({:id}, {:user}, {:desc}, {:type}, {:ids}, {:created}, {:updated})
+			`).Bind(map[string]any{
+				"id": "share-private", "user": owner.ID, "desc": "Private share",
+				"type": "playlist", "ids": privatePlsID, "created": time.Now(), "updated": time.Now(),
+			}).Execute()
+			Expect(err).ToNot(HaveOccurred())
+			DeferCleanup(func() { _, _ = GetDBXBuilder().NewQuery(`DELETE FROM share WHERE id = 'share-private'`).Execute() })
+
+			adminRepo := NewShareRepository(adminCtx, GetDBXBuilder())
+			share, err := adminRepo.Get("share-private")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(share.Tracks).To(BeEmpty())
+		})
+	})
+
 	Describe("Ownership Checks", func() {
 		var ownerUser = model.User{ID: "2222", UserName: "regular-user"}
 		var otherUser = model.User{ID: "3333", UserName: "third-user"}

server/public/handle_streams.go

@@ -25,6 +25,7 @@ func (pub *Router) handleStream(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	var shareOwner *model.User
 	if info.shareID != "" {
 		share, err := pub.ds.Share(ctx).Get(info.shareID)
 		if err != nil {
@@ -35,6 +36,12 @@ func (pub *Router) handleStream(w http.ResponseWriter, r *http.Request) {
 			checkShareError(ctx, w, model.ErrExpired, info.shareID)
 			return
 		}
+		shareOwner, err = pub.ds.User(ctx).Get(share.UserID)
+		if err != nil {
+			log.Error(ctx, "Error retrieving share owner for shared stream", "share", info.shareID, "owner", share.UserID, err)
+			http.Error(w, "internal error", http.StatusInternalServerError)
+			return
+		}
 	}
 
 	mf, err := pub.ds.MediaFile(ctx).Get(info.id)
@@ -48,6 +55,12 @@ func (pub *Router) handleStream(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// 404 rather than 403 so the response doesn't reveal whether the id exists.
+	if shareOwner != nil && !shareOwner.HasLibraryAccess(mf.LibraryID) {
+		http.Error(w, "not found", http.StatusNotFound)
+		return
+	}
+
 	stream, err := pub.streamer.NewStream(ctx, mf, streampkg.Request{
 		Format: info.format, BitRate: info.bitrate,
 	})

server/public/handle_streams_test.go

@@ -131,11 +131,22 @@ var _ = Describe("handleStream", func() {
 		return w
 	}
 
-	It("passes all validation and reaches the streamer for a valid token", func() {
+	shareOwnedBy := func(owner model.User, mf model.MediaFile) {
 		shareRepo.ID = "share123"
+		shareRepo.Entity = &model.Share{ID: "share123", UserID: owner.ID}
+		userRepo := tests.CreateMockUserRepo()
+		Expect(userRepo.Put(&owner)).To(Succeed())
+		ds.MockedUser = userRepo
 		mfRepo := tests.CreateMockMediaFileRepo()
-		mfRepo.SetData(model.MediaFiles{{ID: "mf-123", Title: "Test Song"}})
+		mfRepo.SetData(model.MediaFiles{mf})
 		ds.MockedMediaFile = mfRepo
+	}
+
+	It("passes all validation and reaches the streamer for a valid token", func() {
+		shareOwnedBy(
+			model.User{ID: "owner1", UserName: "owner1", IsAdmin: true},
+			model.MediaFile{ID: "mf-123", Title: "Test Song"},
+		)
 
 		claims := auth.Claims{ID: "mf-123", Format: "mp3", BitRate: 192, ShareID: "share123"}
 		token, _ := auth.CreateExpiringPublicToken(time.Now().Add(time.Hour), claims)
@@ -146,6 +157,33 @@ var _ = Describe("handleStream", func() {
 		Expect(streamer.req.BitRate).To(Equal(192))
 	})
 
+	It("returns 404 when the track is outside the share owner's libraries", func() {
+		shareOwnedBy(
+			model.User{ID: "owner1", UserName: "owner1", Libraries: model.Libraries{{ID: 1}}},
+			model.MediaFile{ID: "mf-restricted", Title: "Other Lib Track", LibraryID: 2},
+		)
+
+		claims := auth.Claims{ID: "mf-restricted", ShareID: "share123"}
+		token, _ := auth.CreateExpiringPublicToken(time.Now().Add(time.Hour), claims)
+		w := makeRequest(token)
+
+		Expect(w.Code).To(Equal(http.StatusNotFound))
+		Expect(streamer.called).To(BeFalse())
+	})
+
+	It("streams a track inside the share owner's libraries", func() {
+		shareOwnedBy(
+			model.User{ID: "owner1", UserName: "owner1", Libraries: model.Libraries{{ID: 1}}},
+			model.MediaFile{ID: "mf-ok", Title: "OK", LibraryID: 1},
+		)
+
+		claims := auth.Claims{ID: "mf-ok", Format: "mp3", ShareID: "share123"}
+		token, _ := auth.CreateExpiringPublicToken(time.Now().Add(time.Hour), claims)
+		makeRequest(token)
+
+		Expect(streamer.called).To(BeTrue())
+	})
+
 	It("returns 400 for an expired token", func() {
 		claims := auth.Claims{ID: "mf-123", ShareID: "share123"}
 		token, _ := auth.CreateExpiringPublicToken(time.Now().Add(-time.Hour), claims)