From 11640f2e4d2e807bce3b473474a35af96dd4e9cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Thu, 4 Jun 2026 23:07:13 -0400 Subject: [PATCH] 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. --- persistence/transcoding_repository.go | 19 ++++++- persistence/transcoding_repository_test.go | 60 ++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/persistence/transcoding_repository.go b/persistence/transcoding_repository.go index 870da61c8..96fd3efdb 100644 --- a/persistence/transcoding_repository.go +++ b/persistence/transcoding_repository.go @@ -53,14 +53,29 @@ func (r *transcodingRepository) Count(options ...rest.QueryOptions) (int64, erro } func (r *transcodingRepository) Read(id string) (any, error) { - return r.Get(id) + res, err := r.Get(id) + if err != nil { + return nil, err + } + if !loggedUser(r.ctx).IsAdmin { + res.Command = "" + } + return res, nil } func (r *transcodingRepository) ReadAll(options ...rest.QueryOptions) (any, error) { sel := r.newSelect(r.parseRestOptions(r.ctx, options...)).Columns("*") res := model.Transcodings{} err := r.queryAll(sel, &res) - return res, err + if err != nil { + return nil, err + } + if !loggedUser(r.ctx).IsAdmin { + for i := range res { + res[i].Command = "" + } + } + return res, nil } func (r *transcodingRepository) EntityName() string { diff --git a/persistence/transcoding_repository_test.go b/persistence/transcoding_repository_test.go index eddc5047a..73250163c 100644 --- a/persistence/transcoding_repository_test.go +++ b/persistence/transcoding_repository_test.go @@ -64,9 +64,69 @@ var _ = Describe("TranscodingRepository", func() { _, err = adminRepo.Get("to-delete") Expect(err).To(MatchError(model.ErrNotFound)) }) + + 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() { + 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))