fix(playlists): the push side spends its own allowance, so a scripted loop cannot drain the shared YouTube budget

push-plan reads the live list and push reads and writes it, both against the
instance-wide budget, and neither had a limit. They now share one per-user
allowance of 20 calls per 10 minutes, far above a person's plan-then-push,
refused as playlist/push_cooldown with a sentence in both languages.
This commit is contained in:
2026-11-06 04:11:36 +01:00
parent 5a93a9a8e7
commit 995a493bd3
4 changed files with 83 additions and 3 deletions
+17 -3
View File
@@ -41,11 +41,16 @@ router = APIRouter(prefix="/api/playlists", tags=["playlists"])
# It guards EVERY route that spends under QuotaAction.PLAYLISTS_PULL, not just the bulk one: a
# scripted loop over the single-playlist re-pull drains the same shared budget, and limiting one of
# two routes with the same quota action is the asymmetric fix this codebase keeps re-learning.
# Deliberately NOT on the push side (`push-plan` / `push`): those run on every push attempt,
# including the no-op "already up to date" path, so a 3-per-10-minute allowance would refuse
# legitimate work. Their exposure is tracked as SEC-18 instead.
_playlist_pull_limiter = RateLimiter(max_events=3, window_seconds=600)
# SEC-18, the push side: `push-plan` reads the live list (1 unit a page) and `push` reads and writes
# it, both against the same instance-wide budget. They get a budget of their own, counted in calls
# and far wider than the pull's: a press in the UI is one plan and, unless nothing changed, one push,
# so no person reaches it, while a scripted loop stops at it. A big push is not this limit's job —
# `push` already refuses one the day's remaining budget cannot cover.
PUSH_ALLOWANCE = 20
_playlist_push_limiter = RateLimiter(max_events=PUSH_ALLOWANCE, window_seconds=600)
def _check_pull_cooldown(user: User) -> None:
"""Shared gate so the two PLAYLISTS_PULL routes cannot drift apart again."""
@@ -58,6 +63,7 @@ def _check_pull_cooldown(user: User) -> None:
PlaylistRefusal = Literal[
"name_required",
"not_on_youtube",
"push_cooldown",
"quota_exceeded",
"sync_all_busy",
"sync_busy",
@@ -244,6 +250,12 @@ def revert_youtube(
raise youtube_refusal(exc, playlist=True) from exc
def _check_push_cooldown(user: User) -> None:
"""Shared gate of the two push routes, so they spend one allowance and cannot drift apart."""
if not _playlist_push_limiter.allow(str(user.id)):
raise _refused(429, "push_cooldown")
def _pushable(db: Session, user: User, playlist_id: int) -> "Playlist":
"""A local playlist the user may push to YouTube (create or update). The built-in
Watch later and read-only YouTube mirrors are never pushable."""
@@ -262,6 +274,7 @@ def push_plan(
"""Dry run: what a two-way 'Sync to YouTube' merge would do (what each side gains/loses) +
the quota estimate, so the UI can show a confirm. For a linked playlist this reads the live
YouTube state (cheap, 1 unit/page). Write scope is required — syncing is a YouTube write."""
_check_push_cooldown(user)
pl = _pushable(db, user, playlist_id)
try:
with quota.attribute(user.id, quota.QuotaAction.PLAYLISTS_PUSH):
@@ -282,6 +295,7 @@ def push(
"""Two-way sync a local playlist with YouTube (create it on first push, else 3-way merge so
neither side loses an item). Refuses if the estimated quota exceeds what's left today, so a
big sync can't strand half-applied. Write scope required (require_write_scope)."""
_check_push_cooldown(user)
pl = _pushable(db, user, playlist_id)
# One sync of a list at a time, across tabs, devices and the scheduler (`sync_claim`): the
# client's own in-flight flag only covers the menus of one page.