navidrome/persistence/transcoding_repository_test.go

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

161 lines
5.8 KiB
Go
Raw Permalink Normal View History

package persistence
import (
"github.com/deluan/rest"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("TranscodingRepository", func() {
var repo model.TranscodingRepository
var adminRepo model.TranscodingRepository
BeforeEach(func() {
ctx := log.NewContext(GinkgoT().Context())
ctx = request.WithUser(ctx, regularUser)
repo = NewTranscodingRepository(ctx, GetDBXBuilder())
adminCtx := log.NewContext(GinkgoT().Context())
adminCtx = request.WithUser(adminCtx, adminUser)
adminRepo = NewTranscodingRepository(adminCtx, GetDBXBuilder())
})
AfterEach(func() {
// Clean up any transcoding created during the tests
tc, err := adminRepo.FindByFormat("test_format")
if err == nil {
err = adminRepo.(*transcodingRepository).Delete(tc.ID)
Expect(err).ToNot(HaveOccurred())
}
})
Describe("Admin User", func() {
It("creates a new transcoding", func() {
base, err := adminRepo.CountAll()
Expect(err).ToNot(HaveOccurred())
err = adminRepo.Put(&model.Transcoding{ID: "new", Name: "new", TargetFormat: "test_format", DefaultBitRate: 320, Command: "ffmpeg"})
Expect(err).ToNot(HaveOccurred())
count, err := adminRepo.CountAll()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(Equal(base + 1))
})
It("updates an existing transcoding", func() {
tr := &model.Transcoding{ID: "upd", Name: "old", TargetFormat: "test_format", DefaultBitRate: 100, Command: "ffmpeg"}
Expect(adminRepo.Put(tr)).To(Succeed())
tr.Name = "updated"
err := adminRepo.Put(tr)
Expect(err).ToNot(HaveOccurred())
res, err := adminRepo.FindByFormat("test_format")
Expect(err).ToNot(HaveOccurred())
Expect(res.Name).To(Equal("updated"))
})
It("deletes a transcoding", func() {
err := adminRepo.Put(&model.Transcoding{ID: "to-delete", Name: "temp", TargetFormat: "test_format", DefaultBitRate: 256, Command: "ffmpeg"})
Expect(err).ToNot(HaveOccurred())
err = adminRepo.(*transcodingRepository).Delete("to-delete")
Expect(err).ToNot(HaveOccurred())
_, err = adminRepo.Get("to-delete")
Expect(err).To(MatchError(model.ErrNotFound))
})
fix: restrict transcoding config reads to admins (#5564) * fix(security): restrict transcoding config reads to admins Authenticated non-admin users could read transcoding configs through the native API (GET /api/transcoding and /api/transcoding/{id}) when EnableTranscodingConfig was enabled. The responses included the full command templates, disclosing admin-configured ffmpeg invocations and local command paths. Write operations were already admin-only. The /transcoding route was registered in the general authenticated group, and only the repository's write methods checked IsAdmin. This applies the boundary at two layers: - Move the route under adminOnlyMiddleware, alongside the other admin-only resources (/library, /config, /inspect). - Add an IsAdmin guard to the repository's rest.Repository read methods (Read, ReadAll, Count) as defense-in-depth. The guard is scoped to the REST methods only. The streaming pipeline resolves profiles via Get/FindByFormat (model.TranscodingRepository), which stay open so transcoding keeps working for non-admin users. Adds regression tests covering non-admin read denial and confirming non-admin streaming lookups (Get/FindByFormat) still succeed. * fix(security): redact transcoding Command for non-admins instead of blocking reads Reworks the previous approach after review (Codex P2): moving /transcoding under adminOnlyMiddleware and denying non-admin reads broke legitimate non-admin UI flows. The web UI reads the transcoding resource as a regular user in several places that need only the profile name and target format: the player edit dropdown (ReferenceInput), the player list (ReferenceField), and the share/download format pickers (useGetList -> {targetFormat, name}). The only sensitive field is Command (the admin-owned ffmpeg template). So: - Revert the route move; /transcoding stays in the authenticated group. - Read/ReadAll now return the profiles to any authenticated user but blank the Command field for non-admins (mirrors user_repository's field-level redaction). Count is no longer denied (the UI needs list pagination). - Writes remain admin-only (Save/Update/Delete/Put). - Streaming is unaffected: it resolves profiles via Get/FindByFormat, which are not redacted, so on-the-fly transcoding keeps working for non-admins. Tests updated: non-admin reads succeed with Command blank, admin reads keep Command, non-admin Get/FindByFormat keep Command, writes still denied.
2026-06-04 23:07:13 -04:00
fix(server): return 404 instead of 500 for missing native API resources (#6131) * fix: return 404 instead of 500 for missing native API resources The deluan/rest controller only maps rest.ErrNotFound to 404, comparing with ==. Most repositories return model.ErrNotFound, which had the same message but was a different value, so requesting a missing playlist, album, artist, song, radio, player, transcoding or library returned 500. This also applied to other users' private playlists. Make model.ErrNotFound the same value as rest.ErrNotFound. This fixes every REST route at once, with no per-route wrapping. errors.Is checks against either error keep working, and nothing wraps model.ErrNotFound before it reaches the controller. Fixes #6130 * fix(radio): return not found when deleting a missing radio station radioRepository.Delete used the shared delete helper, which never reports a missing row because SQL DELETE on zero rows is not an error. Deleting an unknown id silently succeeded: DELETE /api/radio/{id} returned 200, and the Subsonic deleteInternetRadioStation endpoint returned ok. Delete now checks the affected row count and returns model.ErrNotFound when nothing was deleted. The native API returns 404, and deleteInternetRadioStation returns error 70 (data not found). This matches Subsonic 6.1.6, gonic (both verified live) and Ampache (verified in source). Airsonic-Advanced does not implement this endpoint. The shared delete helper is unchanged, as several callers rely on deletes of absent rows succeeding. * fix(ui): return 404 for missing files that are not missing or do not exist missingRepository.Read filtered media files by bare "id" and "missing" columns. The media file query joins the library table, so SQLite rejected the query as ambiguous and GET /api/missing/{id} returned 500. Read now loads the file with MediaFileRepository.Get, which qualifies the column, and returns not found when the file does not exist or is not marked missing. * fix: report missing rows on single-item deletes and adopt deluan/rest errors.Is Bump github.com/deluan/rest to the version whose controller matches errors with errors.Is and errors.As. model.ErrNotFound stays the same value as rest.ErrNotFound, so the many hand-written conversions from model.ErrNotFound to rest.ErrNotFound in repositories, core services and test mocks did nothing. Remove them, along with the duplicate rest.ErrNotFound check in the Subsonic error mapper. Mappings from model.ErrNotAuthorized stay, as those are different errors. User and transcoding deletes had the same silent success as radio: the shared delete helper never reports a missing row, so their not-found checks never fired and DELETE /api/user/{id} and /api/transcoding/{id} returned 200 for unknown ids. Add deleteByID, which returns model.ErrNotFound when no row matched, and use it for radio, user and transcoding. Also drop the dead sql.ErrNoRows branch from delete, since a DELETE never returns it. The Subsonic deleteUser endpoint is not implemented (501), so this does not change the Subsonic API. Plugin deletes keep the silent helper: there is no REST route for them, and the plugin manager only deletes rows it just read. * chore: drop ErrNotFound comment and its identity test The alias to rest.ErrNotFound is self-explanatory, and the identity test only restated the declaration. * refactor: alias model.ErrNotAuthorized to rest.ErrPermissionDenied Like ErrNotFound, make model.ErrNotAuthorized the same value as the rest library's error, so REST endpoints map it to 403 directly. This removes the ErrNotAuthorized to rest.ErrPermissionDenied mappings in the library and playlist REST adapters and the duplicate check in the Subsonic error mapper. Handlers that check model.ErrNotAuthorized now also recognize rest.ErrPermissionDenied returned by repositories, so writePlaylistError, the image upload handlers and the public share handler return 403 for it instead of their fallback status. The error message changes from "not authorized" to "permission denied".
2026-09-14 22:46:21 -04:00
It("returns not found when deleting a missing transcoding", func() {
err := adminRepo.(*transcodingRepository).Delete("does-not-exist")
Expect(err).To(MatchError(model.ErrNotFound))
})
fix: restrict transcoding config reads to admins (#5564) * fix(security): restrict transcoding config reads to admins Authenticated non-admin users could read transcoding configs through the native API (GET /api/transcoding and /api/transcoding/{id}) when EnableTranscodingConfig was enabled. The responses included the full command templates, disclosing admin-configured ffmpeg invocations and local command paths. Write operations were already admin-only. The /transcoding route was registered in the general authenticated group, and only the repository's write methods checked IsAdmin. This applies the boundary at two layers: - Move the route under adminOnlyMiddleware, alongside the other admin-only resources (/library, /config, /inspect). - Add an IsAdmin guard to the repository's rest.Repository read methods (Read, ReadAll, Count) as defense-in-depth. The guard is scoped to the REST methods only. The streaming pipeline resolves profiles via Get/FindByFormat (model.TranscodingRepository), which stay open so transcoding keeps working for non-admin users. Adds regression tests covering non-admin read denial and confirming non-admin streaming lookups (Get/FindByFormat) still succeed. * fix(security): redact transcoding Command for non-admins instead of blocking reads Reworks the previous approach after review (Codex P2): moving /transcoding under adminOnlyMiddleware and denying non-admin reads broke legitimate non-admin UI flows. The web UI reads the transcoding resource as a regular user in several places that need only the profile name and target format: the player edit dropdown (ReferenceInput), the player list (ReferenceField), and the share/download format pickers (useGetList -> {targetFormat, name}). The only sensitive field is Command (the admin-owned ffmpeg template). So: - Revert the route move; /transcoding stays in the authenticated group. - Read/ReadAll now return the profiles to any authenticated user but blank the Command field for non-admins (mirrors user_repository's field-level redaction). Count is no longer denied (the UI needs list pagination). - Writes remain admin-only (Save/Update/Delete/Put). - Streaming is unaffected: it resolves profiles via Get/FindByFormat, which are not redacted, so on-the-fly transcoding keeps working for non-admins. Tests updated: non-admin reads succeed with Command blank, admin reads keep Command, non-admin Get/FindByFormat keep Command, writes still denied.
2026-06-04 23:07:13 -04:00
It("reads the Command field via the REST Read method", func() {
tr := &model.Transcoding{ID: "adminread", Name: "temp", TargetFormat: "test_format", DefaultBitRate: 64, Command: "ffmpeg -secret"}
Expect(adminRepo.Put(tr)).To(Succeed())
res, err := adminRepo.(*transcodingRepository).Read("adminread")
Expect(err).ToNot(HaveOccurred())
Expect(res.(*model.Transcoding).Command).To(Equal("ffmpeg -secret"))
})
})
Describe("Regular User", func() {
fix: restrict transcoding config reads to admins (#5564) * fix(security): restrict transcoding config reads to admins Authenticated non-admin users could read transcoding configs through the native API (GET /api/transcoding and /api/transcoding/{id}) when EnableTranscodingConfig was enabled. The responses included the full command templates, disclosing admin-configured ffmpeg invocations and local command paths. Write operations were already admin-only. The /transcoding route was registered in the general authenticated group, and only the repository's write methods checked IsAdmin. This applies the boundary at two layers: - Move the route under adminOnlyMiddleware, alongside the other admin-only resources (/library, /config, /inspect). - Add an IsAdmin guard to the repository's rest.Repository read methods (Read, ReadAll, Count) as defense-in-depth. The guard is scoped to the REST methods only. The streaming pipeline resolves profiles via Get/FindByFormat (model.TranscodingRepository), which stay open so transcoding keeps working for non-admin users. Adds regression tests covering non-admin read denial and confirming non-admin streaming lookups (Get/FindByFormat) still succeed. * fix(security): redact transcoding Command for non-admins instead of blocking reads Reworks the previous approach after review (Codex P2): moving /transcoding under adminOnlyMiddleware and denying non-admin reads broke legitimate non-admin UI flows. The web UI reads the transcoding resource as a regular user in several places that need only the profile name and target format: the player edit dropdown (ReferenceInput), the player list (ReferenceField), and the share/download format pickers (useGetList -> {targetFormat, name}). The only sensitive field is Command (the admin-owned ffmpeg template). So: - Revert the route move; /transcoding stays in the authenticated group. - Read/ReadAll now return the profiles to any authenticated user but blank the Command field for non-admins (mirrors user_repository's field-level redaction). Count is no longer denied (the UI needs list pagination). - Writes remain admin-only (Save/Update/Delete/Put). - Streaming is unaffected: it resolves profiles via Get/FindByFormat, which are not redacted, so on-the-fly transcoding keeps working for non-admins. Tests updated: non-admin reads succeed with Command blank, admin reads keep Command, non-admin Get/FindByFormat keep Command, writes still denied.
2026-06-04 23:07:13 -04:00
It("reads a transcoding but with the Command field redacted", func() {
tr := &model.Transcoding{ID: "readreg", Name: "temp", TargetFormat: "test_format", DefaultBitRate: 64, Command: "ffmpeg -secret"}
Expect(adminRepo.Put(tr)).To(Succeed())
res, err := repo.(*transcodingRepository).Read("readreg")
Expect(err).ToNot(HaveOccurred())
t := res.(*model.Transcoding)
Expect(t.Name).To(Equal("temp"))
Expect(t.TargetFormat).To(Equal("test_format"))
Expect(t.Command).To(BeEmpty())
})
It("lists transcodings but with the Command field redacted", func() {
tr := &model.Transcoding{ID: "listreg", Name: "temp", TargetFormat: "test_format", DefaultBitRate: 64, Command: "ffmpeg -secret"}
Expect(adminRepo.Put(tr)).To(Succeed())
res, err := repo.(*transcodingRepository).ReadAll()
Expect(err).ToNot(HaveOccurred())
list := res.(model.Transcodings)
Expect(list).ToNot(BeEmpty())
for _, t := range list {
Expect(t.Command).To(BeEmpty())
}
})
It("counts transcodings", func() {
count, err := repo.(*transcodingRepository).Count()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(BeNumerically(">=", 0))
})
It("can still resolve a transcoding for streaming via Get (Command not redacted)", func() {
tr := &model.Transcoding{ID: "streamreg", Name: "temp", TargetFormat: "test_format", DefaultBitRate: 64, Command: "ffmpeg -secret"}
Expect(adminRepo.Put(tr)).To(Succeed())
res, err := repo.Get("streamreg")
Expect(err).ToNot(HaveOccurred())
Expect(res.ID).To(Equal("streamreg"))
Expect(res.Command).To(Equal("ffmpeg -secret"))
})
It("can still resolve a transcoding for streaming via FindByFormat (Command not redacted)", func() {
tr := &model.Transcoding{ID: "fmtreg", Name: "temp", TargetFormat: "test_format", DefaultBitRate: 64, Command: "ffmpeg -secret"}
Expect(adminRepo.Put(tr)).To(Succeed())
res, err := repo.FindByFormat("test_format")
Expect(err).ToNot(HaveOccurred())
Expect(res.ID).To(Equal("fmtreg"))
Expect(res.Command).To(Equal("ffmpeg -secret"))
})
It("fails to create", func() {
err := repo.Put(&model.Transcoding{ID: "bad", Name: "bad", TargetFormat: "test_format", DefaultBitRate: 64, Command: "ffmpeg"})
Expect(err).To(Equal(rest.ErrPermissionDenied))
})
It("fails to update", func() {
tr := &model.Transcoding{ID: "updreg", Name: "old", TargetFormat: "test_format", DefaultBitRate: 64, Command: "ffmpeg"}
Expect(adminRepo.Put(tr)).To(Succeed())
tr.Name = "bad"
err := repo.Put(tr)
Expect(err).To(Equal(rest.ErrPermissionDenied))
//_ = adminRepo.(*transcodingRepository).Delete("updreg")
})
It("fails to delete", func() {
tr := &model.Transcoding{ID: "delreg", Name: "temp", TargetFormat: "test_format", DefaultBitRate: 64, Command: "ffmpeg"}
Expect(adminRepo.Put(tr)).To(Succeed())
err := repo.(*transcodingRepository).Delete("delreg")
Expect(err).To(Equal(rest.ErrPermissionDenied))
//_ = adminRepo.(*transcodingRepository).Delete("delreg")
})
})
})