fix: quota bypass when editing a pending request's seasons#3265
Open
kushiemoon-dev wants to merge 3 commits into
Open
fix: quota bypass when editing a pending request's seasons#3265kushiemoon-dev wants to merge 3 commits into
kushiemoon-dev wants to merge 3 commits into
Conversation
TvRequestModal, MovieRequestModal, and CollectionRequestModal all skip fetching the requester's quota whenever requestOverrides.user.id is set and the acting user lacks MANAGE_USERS. Editing an existing request always populates requestOverrides.user with the request's original owner (regardless of the acting user's permissions), so this condition was true for essentially every edit - meaning quota was silently never fetched, and the UI never disabled submission once the user's season limit was reached. A user at their quota could open a pending request and add further seasons past their limit. This also affected the new admin "Bypass User Quota" checkbox added in seerr-team#2026: its visibility depends on quota.tv.limit / quota.movie.limit being loaded, so it silently failed to appear whenever this same condition suppressed the fetch. The fix removes the permission-gated condition and always fetches quota for the effective target user. This doesn't weaken anything: User.getQuota() already returns an unrestricted quota server-side for users with MANAGE_USERS, so admins editing their own request are unaffected; the change only ensures quota is actually loaded (and enforced) for the common case of a regular user editing their own request. The corresponding server-side gap is fixed as well: PUT /request/:id recomputed which seasons were being added but never checked them against the requester's quota, so the limit could already be bypassed directly via the API regardless of the frontend. It now runs the same quota check used on request creation, including support for the explicit ignoreQuota flag from seerr-team#2026 (an admin must opt in via ignoreQuota: true - having MANAGE_REQUESTS alone no longer grants a silent bypass), and persists request.ignoreQuota so the seasons added this way are correctly excluded from future quota calculations. Added test coverage in request.test.ts for: a regular user exceeding their quota on edit, an admin edit without the explicit flag (still enforced), a non-admin attempting to set the flag themselves (rejected), and an admin using the flag to legitimately bypass quota. Fixes seerr-team#633
CodeRabbit flagged that newSeasons.length was compared against quota.tv.remaining without crediting back seasons the same edit drops, since getQuota() counts persisted rows before the save. A same-count season swap at full quota was rejected even though the net season total wouldn't change.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughTV request edits now account for removed seasons during quota checks, enforce authorized ChangesTV Request Quota Enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RequestRoute
participant QuotaCheck
participant RequestDatabase
Client->>RequestRoute: Edit TV request seasons
RequestRoute->>QuotaCheck: Evaluate added seasons minus removed seasons
QuotaCheck-->>RequestRoute: Allow, deny, or honor authorized ignoreQuota
RequestRoute->>RequestDatabase: Persist updated seasons
RequestDatabase-->>Client: Updated request or 403 response
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/routes/request.ts`:
- Around line 571-604: Update the quota validation around removedSeasonsCount
and request.ignoreQuota: only credit removed seasons when the persisted request
was quota-counted (ignoreQuota is false), then compare the resulting new-season
usage against quota.tv.used and quota.tv.limit directly rather than clamped
quota.tv.remaining. Preserve authorized bypass behavior while ensuring later
edits to previously bypassed requests cannot add seasons beyond the quota.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d34d321-0db4-48e7-9a70-3a3ed9c84afa
📒 Files selected for processing (5)
server/routes/request.test.tsserver/routes/request.tssrc/components/RequestModal/CollectionRequestModal.tsxsrc/components/RequestModal/MovieRequestModal.tsxsrc/components/RequestModal/TvRequestModal.tsx
Seasons removed from an already-exempt request (ignoreQuota: true from creation or a prior authorized edit) were never counted in quota.tv.used, so crediting them back inflated the allowed threshold and let a subsequent non-bypass edit add seasons past the real limit. Also compare against quota.tv.used/limit directly instead of the clamped quota.tv.remaining, which under-restricts once usage already exceeds the limit. Adds a regression test covering the swap-past-quota scenario.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Editing a pending TV request's seasons didn't re-check the requester's quota against the new season list.
quota.tv.remainingis computed from the currently persisted seasons, so any seasons being removed in the same edit were still counted as "used" — a user (or an admin editing on a user's behalf) could add seasons beyond the quota limit as long as the request already existed, bypassing the quota enforcement that applies to new requests.This adds a quota check on edit: removed seasons are credited back before comparing the new season count against the remaining quota, and an edit that would exceed quota is rejected (403) unless a manager explicitly passes
ignoreQuota(only usable by accounts withMANAGE_REQUESTSpermission; non-managers passing it get rejected).Also fixes the 3 request modals (
MovieRequestModal,TvRequestModal,CollectionRequestModal) fetching quota only for the modal owner or a manager — a non-manager editing their own existing request viarequestOverrideswasn't shown their own quota.Fixes #633
AI Disclosure: This PR was written primarily by Claude Code, reviewed and directed by the account holder.
How Has This Been Tested?
Added a test suite in
server/routes/request.test.tscovering: rejecting an edit beyond quota, rejecting a manager edit beyond quota withoutignoreQuota, rejecting a non-manager attempting to setignoreQuotathemselves, allowing a manager to bypass quota viaignoreQuota, and allowing a same-count season swap at full quota. Full suite passes (pnpm test, 126/126), along withpnpm lintandpnpm build.Screenshots / Logs (if applicable)
N/A — backend/logic fix, no visual change.
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit