Skip to content

fix: scope playlist and subscription group changes to their owners - #8

Merged
D3SOX merged 1 commit into
mainfrom
enforce-sync-ownership
Sep 6, 2026
Merged

fix: scope playlist and subscription group changes to their owners#8
D3SOX merged 1 commit into
mainfrom
enforce-sync-ownership

Conversation

@D3SOX

@D3SOX D3SOX commented Sep 6, 2026

Copy link
Copy Markdown
Member

The audit found that playlist deletion and subscription-group mutations could affect another account's records. Scope those database mutations to the authenticated owner, update only group titles, and use the existing foreign-key cascades to remove memberships.

Existing clients keep the same endpoints, request bodies, and successful responses. The change needs no migration or protocol bump. Updating a missing or unowned group returns the existing 404 error used by group reads.

Validation: regression tests cover two accounts sharing a playlist ID, unauthorized group updates and deletions, and legitimate rename/delete behavior. All 52 offline SQLite tests pass; PostgreSQL compilation and formatting checks pass. Four existing tests that fetch live YouTube feeds were excluded.

Detailed report issues will follow after merge and deployment, as requested.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 89e53ccc-d073-434b-ab81-bffad5cd5f9e

📥 Commits

Reviewing files that changed from the base of the PR and between cfcea1c and 9ae17a6.

📒 Files selected for processing (5)
  • src/database.rs
  • src/database/ownership_tests.rs
  • src/database/playlist.rs
  • src/database/subscription_groups.rs
  • src/handlers/subscriptions.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Playlist and subscription group database operations now enforce account ownership and use cascading membership deletes. The subscription handler maps missing owned groups to SubscriptionGroupNotFound. SQLite tests cover deletion and update behavior across accounts.

Changes

Ownership-scoped database operations

Layer / File(s) Summary
Ownership filters and cascade deletes
src/database/playlist.rs, src/database/subscription_groups.rs
Playlist deletion now filters by account. Subscription group updates require ownership and change only the title. Group membership deletion uses database cascades.
Ownership error mapping and regression coverage
src/handlers/subscriptions.rs, src/database.rs, src/database/ownership_tests.rs
The handler maps NotFound to SubscriptionGroupNotFound. SQLite tests verify playlist and subscription group ownership behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 9ae17

Playlist and subscription-group mutations now prevent cross-account changes while preserving expected rename, delete, and not-found behavior. No merge-blocking risk is evident.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: scoping playlist and subscription-group mutations to their owners.
Description check ✅ Passed The description accurately summarizes the ownership fixes, cascade behavior, unchanged interfaces, error handling, and regression-test coverage.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch enforce-sync-ownership

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant