Skip to content

Conversation

@s1efhan
Copy link

@s1efhan s1efhan commented Dec 2, 2025

Description

This PR fixes an issue where users with "Advanced Request" permissions could bypass their season limits by editing a pending request.

Previously, the quota check was skipped entirely if request overrides were present and the user was not an admin.

This PR simplifies the useSWR condition in TvRequestModal.tsx to ensure quotas are always loaded for the target user, preventing the bypass.

I also added a check to ensure that admins (MANAGE_REQUESTS) are not blocked by these quotas when editing requests for other users (loading null quota for them).

AI Disclosure

I discussed the issue with an AI (Gemini 3 Pro) to verify the logic required for the fix. I wrote, reviewed, understood, and manually tested the solution myself.

How Has This Been Tested?

I tested these changes locally on macOS using Safari and Node v22.

Test Scenario 1: Verify the fix

  1. Created a test user with the "Advanced Request" permission (but NO admin rights).
  2. Set a strict quota for TV Shows (e.g., limit of 2 seasons per 7 days).
  3. Logged in as the test user and requested 2 seasons (quota full).
  4. Edited the pending request via the UI.
  5. Attempted to select a 3rd season.
  6. Result: The checkboxes for additional seasons were correctly disabled/blocked, enforcing the quota.

Test Scenario 2: Verify Admin override

  1. Logged in as an Admin user (with MANAGE_REQUESTS permission).
  2. Edited the request of the user from Scenario 1.
  3. Attempted to select additional seasons beyond the user's quota.
  4. Result: The quota check was bypassed as expected for admins, allowing me to add more seasons regardless of the user's limit.

Screenshots / Logs (if applicable)

Checklist:

  • I have read and followed the contribution guidelines.
  • Disclosed any use of AI (see our policy)
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • Successful build pnpm build
  • Translation keys pnpm i18n:extract
  • Database migration (if required)

@s1efhan s1efhan requested a review from a team as a code owner December 2, 2025 20:18
@s1efhan s1efhan force-pushed the fix/bypassing-request-limit branch from 814ab49 to 32515a9 Compare December 5, 2025 20:19
@s1efhan s1efhan force-pushed the fix/bypassing-request-limit branch from 32515a9 to 79ab2e8 Compare December 5, 2025 20:20
Copy link
Collaborator

@fallenbagel fallenbagel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only fixes it in the frontend. Vulnerability still exists at the API level.

The PUT /:requestId endpoint still doesn’t validate the quotas when adding new seasons. That means anyone with REQUEST_ADVANCED perm can skip the frontend entirely and hit the API directly to get around the limits.

Could you add the same quota validation on the backend so the API can’t be used to bypass it as well?

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.

series request limit can be bypassed by users

2 participants