From 0a1e76564da023bb480c7492589d48ecc9f57820 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 4 May 2026 09:29:37 -0400 Subject: [PATCH] feat(server/m7-352): 403 system_playlist_readonly on playlist edit endpoints Push kind='system' guard into each of the five Service edit methods (Update, Delete, AppendTracks, RemoveTrack, Reorder) immediately after the ownership check; map the typed error to 403 in writePlaylistErr; add table-driven test covering all five verbs against a seeded system playlist. --- internal/api/playlists.go | 2 ++ internal/api/playlists_test.go | 46 ++++++++++++++++++++++++++++++++++ internal/playlists/service.go | 15 +++++++++++ 3 files changed, 63 insertions(+) diff --git a/internal/api/playlists.go b/internal/api/playlists.go index f9b0dd8b..f7017453 100644 --- a/internal/api/playlists.go +++ b/internal/api/playlists.go @@ -420,6 +420,8 @@ func (h *handlers) writePlaylistErr(w http.ResponseWriter, err error, op string) writeErr(w, http.StatusNotFound, "not_found", "playlist not found") case errors.Is(err, playlists.ErrForbidden): writeErr(w, http.StatusForbidden, "not_authorized", "you don't own this playlist") + case errors.Is(err, playlists.ErrSystemPlaylistReadonly): + writeErr(w, http.StatusForbidden, "system_playlist_readonly", "system-generated playlists cannot be edited") case errors.Is(err, playlists.ErrInvalidInput): writeErr(w, http.StatusBadRequest, "bad_request", err.Error()) case errors.Is(err, playlists.ErrTrackNotFound): diff --git a/internal/api/playlists_test.go b/internal/api/playlists_test.go index 3369c244..baf59db1 100644 --- a/internal/api/playlists_test.go +++ b/internal/api/playlists_test.go @@ -569,3 +569,49 @@ func TestListPlaylists_BadKindReturns400(t *testing.T) { t.Fatalf("expected 400; got %d, body=%s", w.Code, w.Body.String()) } } + +// TestEditEndpoints_403OnSystemPlaylist verifies that all five edit verbs +// return 403 system_playlist_readonly when the target playlist has kind='system'. +func TestEditEndpoints_403OnSystemPlaylist(t *testing.T) { + h, pool := testHandlers(t) + u := seedUser(t, pool, "sysedit", "pw", false) + + // Seed a system playlist directly via SQL (BuildSystemPlaylists requires + // play_events seeding which we already exercise in playlists package tests). + var sid string + if err := pool.QueryRow(context.Background(), ` + INSERT INTO playlists (user_id, name, description, is_public, kind, system_variant) + VALUES ($1, 'For You', '', false, 'system', 'for_you') + RETURNING id::text + `, u.ID).Scan(&sid); err != nil { + t.Fatalf("seed system playlist: %v", err) + } + + cases := []struct { + name string + method string + path string + body []byte + }{ + {"PATCH", "PATCH", "/api/playlists/" + sid, []byte(`{"name":"new name"}`)}, + {"DELETE_playlist", "DELETE", "/api/playlists/" + sid, nil}, + {"POST_tracks", "POST", "/api/playlists/" + sid + "/tracks", []byte(`{"track_ids":["00000000-0000-0000-0000-000000000000"]}`)}, + {"PUT_tracks_reorder", "PUT", "/api/playlists/" + sid + "/tracks", []byte(`{"ordered_positions":[]}`)}, + {"DELETE_track_at_position", "DELETE", "/api/playlists/" + sid + "/tracks/0", nil}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + w := doPlaylistsReq(h, u, c.method, c.path, c.body) + if w.Code != http.StatusForbidden { + t.Fatalf("status: got %d want 403; body=%s", w.Code, w.Body.String()) + } + var env errorBody + if err := json.Unmarshal(w.Body.Bytes(), &env); err != nil { + t.Fatalf("decode envelope: %v", err) + } + if env.Error.Code != "system_playlist_readonly" { + t.Errorf("code: got %q want system_playlist_readonly; full body=%s", env.Error.Code, w.Body.String()) + } + }) + } +} diff --git a/internal/playlists/service.go b/internal/playlists/service.go index 3c2587f8..42347eee 100644 --- a/internal/playlists/service.go +++ b/internal/playlists/service.go @@ -233,6 +233,9 @@ func (s *Service) Update(ctx context.Context, callerID, playlistID pgtype.UUID, if !pgtypeUUIDEqual(existing.UserID, callerID) { return nil, ErrForbidden } + if existing.Kind == "system" { + return nil, ErrSystemPlaylistReadonly + } params := dbq.UpdatePlaylistParams{ ID: playlistID, @@ -281,6 +284,9 @@ func (s *Service) Delete(ctx context.Context, callerID, playlistID pgtype.UUID) if !pgtypeUUIDEqual(existing.UserID, callerID) { return ErrForbidden } + if existing.Kind == "system" { + return ErrSystemPlaylistReadonly + } deleted, err := q.DeletePlaylist(ctx, playlistID) if err != nil { @@ -341,6 +347,9 @@ func (s *Service) AppendTracks(ctx context.Context, callerID, playlistID pgtype. if !pgtypeUUIDEqual(pl.UserID, callerID) { return ErrForbidden } + if pl.Kind == "system" { + return ErrSystemPlaylistReadonly + } tx, err := s.pool.Begin(ctx) if err != nil { @@ -393,6 +402,9 @@ func (s *Service) RemoveTrack(ctx context.Context, callerID, playlistID pgtype.U if !pgtypeUUIDEqual(pl.UserID, callerID) { return ErrForbidden } + if pl.Kind == "system" { + return ErrSystemPlaylistReadonly + } tx, err := s.pool.Begin(ctx) if err != nil { @@ -442,6 +454,9 @@ func (s *Service) Reorder(ctx context.Context, callerID, playlistID pgtype.UUID, if !pgtypeUUIDEqual(pl.UserID, callerID) { return ErrForbidden } + if pl.Kind == "system" { + return ErrSystemPlaylistReadonly + } if int32(len(orderedPositions)) != pl.TrackCount { return fmt.Errorf("%w: expected %d positions, got %d", ErrInvalidInput, pl.TrackCount, len(orderedPositions))