diff --git a/VERSION b/VERSION index fbaaafa..84767f2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.49.0 \ No newline at end of file +0.50.0 \ No newline at end of file diff --git a/backend/app/auth.py b/backend/app/auth.py index cdd8490..cb5b83e 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -8,7 +8,7 @@ from authlib.integrations.starlette_client import OAuth, OAuthError from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request from fastapi.responses import JSONResponse, RedirectResponse from starlette.requests import HTTPConnection -from sqlalchemy import delete, func, select +from sqlalchemy import delete, func, select, update from sqlalchemy.orm import Session from app import email as email_mod @@ -31,6 +31,7 @@ _DECOY_PASSWORD_HASH = hash_password(secrets.token_urlsafe(16)) _register_limiter = RateLimiter(max_events=5, window_seconds=300) _login_limiter = RateLimiter(max_events=10, window_seconds=300) _reset_limiter = RateLimiter(max_events=5, window_seconds=300) +_request_access_limiter = RateLimiter(max_events=5, window_seconds=300) # At most one "your account is suspended" email per address per hour, so a suspended user who # keeps retrying (or a script with their valid credentials) can't be used to mailbomb them. _suspend_email_limiter = RateLimiter(max_events=1, window_seconds=3600) @@ -512,16 +513,26 @@ async def logout(request: Request): @router.post("/request-access") async def request_access( payload: dict, + request: Request, background: BackgroundTasks, db: Session = Depends(get_db), ) -> dict: """Public: ask for access. Idempotent — repeat requests for the same email don't - duplicate or re-spam admins (upsert returns None for an already-pending row).""" + duplicate or re-spam admins (upsert returns None for an already-pending row). + + Answers a uniform ``"pending"`` for EVERY outcome — already-allowed, newly-pending, + already-pending, and rate-limited alike. The old ``"approved"`` reply for an allow-listed + address made this a public enumeration oracle for the allow-list (which the rest of the module + is careful to avoid, cf. demo_login); an already-allowed user can still just sign in with Google. + Rate-limited per client IP like register/login/reset, and a block returns "pending" too so the + endpoint can't be probed for throttling either.""" email = (payload.get("email") or "").strip().lower() if not valid_email(email): raise HTTPException(status_code=400, detail="Enter a valid email address.") + if not _request_access_limiter.allow(_client_ip(request)): + return {"status": "pending"} if is_allowed(db, email): - return {"status": "approved"} # already allowed — just sign in + return {"status": "pending"} # already allowed — don't reveal it inv = upsert_pending_invite(db, email) admins = admin_notify_emails(db) if inv is not None and admins: @@ -583,18 +594,27 @@ def _issue_token(db: Session, user: User, kind: str, ttl: timedelta) -> str: def _consume_token(db: Session, raw: str | None, kind: str) -> User | None: - """Validate + burn a token. Returns its user, or None if missing/expired/used/wrong kind.""" + """Validate + burn a token atomically. Returns its user, or None if missing/expired/used/wrong + kind. The burn is a single conditional UPDATE (…WHERE used_at IS NULL…RETURNING) rather than a + read-check-write, so two concurrent requests can't both consume the same verify/reset token — + only the one whose UPDATE flips used_at gets a row back.""" if not raw: return None + now = datetime.now(timezone.utc) row = db.execute( - select(AuthToken).where( - AuthToken.token_hash == hash_token(raw), AuthToken.kind == kind + update(AuthToken) + .where( + AuthToken.token_hash == hash_token(raw), + AuthToken.kind == kind, + AuthToken.used_at.is_(None), + AuthToken.expires_at >= now, ) - ).scalar_one_or_none() - if row is None or row.used_at is not None or row.expires_at < datetime.now(timezone.utc): - return None - row.used_at = datetime.now(timezone.utc) + .values(used_at=now) + .returning(AuthToken.user_id) + ).first() db.commit() + if row is None: + return None return db.get(User, row.user_id) diff --git a/backend/app/downloads/storage.py b/backend/app/downloads/storage.py index 1aee7b8..665e107 100644 --- a/backend/app/downloads/storage.py +++ b/backend/app/downloads/storage.py @@ -58,8 +58,12 @@ def sanitize(name: str, limit: int = _MAX_TITLE) -> str: text = re.sub(r"\s+", "_", text.strip()) # Tidy separator pile-ups and trim junk from the ends. text = re.sub(r"_{2,}", "_", text).strip("_.-·—–|,;:!?ّ ") - if len(text) > limit: - text = text[:limit].rstrip("_.-") + # Cap by BYTES, not characters: a path component's hard limit is 255 BYTES, and rel_path packs + # the (byte-capped) channel + date + id + ext into that same component — so an accented/multi-byte + # title capped only by char count could still overflow to ENAMETOOLONG. Truncate on a UTF-8 + # boundary (decode(..., "ignore") drops a trailing partial char) so we never split a character. + if len(text.encode("utf-8")) > limit: + text = text.encode("utf-8")[:limit].decode("utf-8", "ignore").rstrip("_.-") return text or "untitled" @@ -74,16 +78,23 @@ def display_filename(name: str) -> str: return text or "download" -def rel_path(meta: MediaMeta, ext: str, layout: str = "plex") -> str: - """Path of the media file relative to DOWNLOAD_ROOT (forward slashes).""" +def rel_path(meta: MediaMeta, ext: str, layout: str = "plex", asset_id: int | None = None) -> str: + """Path of the media file relative to DOWNLOAD_ROOT (forward slashes). + + `asset_id` disambiguates the file. A MediaAsset is unique on (source_kind, source_ref, + format_sig), so the SAME video downloaded in two formats is two distinct assets — but without + the id they compute the identical path, so the second silently overwrites the first and deleting + either removes the shared file. The `_a{id}` suffix (which Plex ignores — the .nfo sidecar carries + the metadata) keeps them apart. Optional so callers/tests that don't need it keep the plain name.""" title = sanitize(meta.title) vid = meta.video_id + disc = f"_a{asset_id}" if asset_id is not None else "" if layout == "flat": - return f"{title}_[{vid}].{ext}" + return f"{title}_[{vid}]{disc}.{ext}" channel = sanitize(meta.uploader or "Unknown_Channel", 80) year = meta.upload_date.year if meta.upload_date else 0 d = meta.upload_date.isoformat() if meta.upload_date else "0000-00-00" - epname = f"{channel}_-_{d}_-_{title}_[{vid}].{ext}" + epname = f"{channel}_-_{d}_-_{title}_[{vid}]{disc}.{ext}" return f"{channel}/Season_{year}/{epname}" diff --git a/backend/app/notifications.py b/backend/app/notifications.py index c4ce120..1e7b6ce 100644 --- a/backend/app/notifications.py +++ b/backend/app/notifications.py @@ -5,7 +5,7 @@ survive reloads/devices and are produced server-side. First producer is the main job. Unread rows never auto-expire; read rows are trimmed past a soft per-user cap so the table can't grow without bound for a heavy user. """ -from sqlalchemy import select +from sqlalchemy import delete, select from sqlalchemy.orm import Session from app.models import Notification @@ -52,7 +52,7 @@ def trim_read(db: Session, user_id: int, cap: int = READ_SOFT_CAP) -> int: ) if not ids: return 0 - for nid in ids: - db.delete(db.get(Notification, nid)) + # One bulk DELETE rather than a get+delete per row (the ids are already in hand). + db.execute(delete(Notification).where(Notification.id.in_(ids))) db.commit() return len(ids) diff --git a/backend/app/routes/channels.py b/backend/app/routes/channels.py index dca498e..6b3c4da 100644 --- a/backend/app/routes/channels.py +++ b/backend/app/routes/channels.py @@ -180,7 +180,8 @@ def discover_channels( # (fresh subscriber counts/thumbnails carry through without a re-query). Only the title can # change, which affects the tie-break order → re-sort in Python instead of re-hitting the DB. need = [ch.id for ch, _, _ in rows if ch.details_synced_at is None][:DISCOVERY_ENRICH_CAP] - if need: + # Skip the quota-spending enrich for the demo account (it just sees the stub metadata). + if need and not user.is_demo: try: with quota.attribute(user.id, quota.QuotaAction.CHANNELS_DISCOVER), YouTubeClient(db, user) as yt: apply_channel_details(db, yt.get_channels(need)) @@ -390,7 +391,10 @@ def update_channel( sub.priority = int(payload["priority"]) if "hidden" in payload: sub.hidden = bool(payload["hidden"]) - if "deep_requested" in payload: + # The demo account is SHARED, so it must not persist deep_requested (a quota-relevant flag) onto + # the shared subscription — skip the write entirely for demo, which also keeps deep_turned_on + # False so the immediate backfill below can never fire for it. + if "deep_requested" in payload and not user.is_demo: # Opt this channel into (or out of) full-history backfill. The deep scheduler # picks the flag up on its next run; turning it off won't undo already-fetched # videos, it just stops further deep paging if nobody else still wants it. @@ -402,6 +406,7 @@ def update_channel( # Opting a channel into full history should give an immediate feed: if we haven't even # fetched its recent uploads yet, do that one channel now (cheap) instead of waiting for # the scheduler. Deep paging still follows on the scheduler's next run (recent-then-deep). + # (deep_turned_on is only ever True for a non-demo user — see the guard above.) if deep_turned_on and channel is not None and channel.recent_synced_at is None: with quota.attribute(user.id, quota.QuotaAction.VIDEOS_BACKFILL_RECENT): run_recent_backfill(db, [channel], max_channels=1) diff --git a/backend/app/routes/feed.py b/backend/app/routes/feed.py index e5d2952..443ee19 100644 --- a/backend/app/routes/feed.py +++ b/backend/app/routes/feed.py @@ -737,6 +737,11 @@ def get_video_detail( "duration_seconds": v.duration_seconds, } + # The DB path above is free; the off-catalog YouTube lookup below spends a quota unit, so the + # demo account can't reach it — it just sees "unknown" for a video we don't already store. + if user.is_demo: + raise HTTPException(status_code=404, detail="Unknown video") + try: with quota.attribute(user.id, quota.QuotaAction.VIDEOS_LOOKUP), YouTubeClient(db, user) as yt: items = yt.get_videos([video_id]) diff --git a/backend/app/worker.py b/backend/app/worker.py index b35d6f1..d808046 100644 --- a/backend/app/worker.py +++ b/backend/app/worker.py @@ -66,6 +66,22 @@ def _job_status(job_id: int) -> str | None: ).scalar() +def _requeue_if_running(job_id: int, phase: str) -> None: + """Put the job back to 'queued' for a later retry, but ONLY while it is still 'running'. The + route flips status to paused/cancelled to stop a job; a blind `status='queued'` here would + clobber that and silently revive a job the user just cancelled. The conditional WHERE makes the + requeue a no-op unless we still own a running job.""" + with SessionLocal() as db: + db.execute( + text( + "UPDATE download_jobs SET status='queued', phase=:phase, updated_at=now() " + "WHERE id=:id AND status='running'" + ), + {"id": job_id, "phase": phase}, + ) + db.commit() + + def _parse_upload_date(raw) -> date | None: if not raw: return None @@ -179,13 +195,13 @@ def _process_job(job_id: int) -> None: return if asset_status == "downloading": # Another worker owns this asset; wait and let the job be reclaimed later. - _set_job(job_id, status="queued", phase="waiting") + _requeue_if_running(job_id, "waiting") time.sleep(2) return # asset_status == "pending": try to win the download / edit. if not _claim_asset(asset_id): - _set_job(job_id, status="queued", phase="waiting") + _requeue_if_running(job_id, "waiting") time.sleep(2) return @@ -414,7 +430,7 @@ def _download(job_id: int, asset_id: int, source_kind: str, source_ref: str, spe ) layout, retention_days = _download_settings() - rel = storage.rel_path(meta, ext, layout) + rel = storage.rel_path(meta, ext, layout, asset_id=asset_id) thumb = _find_thumbnail(staging, produced) storage.place_file(produced, settings.download_root, rel) nfo_ok = storage.write_sidecars(settings.download_root, rel, meta, thumb) diff --git a/backend/tests/test_storage.py b/backend/tests/test_storage.py index ab2c8e7..fdd357e 100644 --- a/backend/tests/test_storage.py +++ b/backend/tests/test_storage.py @@ -29,6 +29,14 @@ class TestSanitize: def test_length_is_capped(self): assert len(sanitize("x" * 500, limit=10)) <= 10 + def test_cap_is_by_bytes_not_characters(self): + # Accented (2-byte UTF-8) letters must not blow past the byte limit — the FS component cap + # is 255 BYTES, so a char-only cap would overflow for multi-byte titles. + out = sanitize("á" * 500, limit=10) + assert len(out.encode("utf-8")) <= 10 + # And it never splits a character (would raise otherwise / leave a replacement char). + out.encode("utf-8").decode("utf-8") + def test_empty_or_all_junk_falls_back_to_untitled(self): assert sanitize("") == "untitled" assert sanitize("🔥🔥") == "untitled" @@ -54,6 +62,29 @@ class TestRelPath: out = rel_path(meta, "mp4") assert "Season_0/" in out and "0000-00-00" in out + def test_asset_id_disambiguates_two_formats_of_the_same_video(self): + # Same video, two assets (e.g. 1080p vs 720p) must NOT collide on one path. + a = rel_path(self.meta, "mp4", asset_id=5) + b = rel_path(self.meta, "mp4", asset_id=6) + assert a != b + assert a.endswith("_[abc123]_a5.mp4") and b.endswith("_[abc123]_a6.mp4") + # Omitting it keeps the plain name (unchanged for callers/tests that don't pass one). + assert rel_path(self.meta, "mp4", layout="flat") == "Cool_Clip_[abc123].mp4" + assert rel_path(self.meta, "mp4", layout="flat", asset_id=7) == "Cool_Clip_[abc123]_a7.mp4" + + def test_every_component_stays_within_the_255_byte_fs_limit(self): + # A long accented channel AND title, plus the asset-id suffix, must not overflow any path + # component (the byte cap in sanitize is what guarantees this). + meta = MediaMeta( + video_id="abcdefghijk", + title="Á" * 300, + uploader="Ö" * 300, + upload_date=date(2024, 5, 2), + ) + out = rel_path(meta, "webm", asset_id=999999) + for component in out.split("/"): + assert len(component.encode("utf-8")) <= 255, component + class TestDownloadFilename: def test_appends_the_container_extension(self): diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index b84dea2..4c4c06d 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -35,7 +35,10 @@ import ErrorDialog from "./components/ErrorDialog"; import VersionBanner from "./components/VersionBanner"; import DemoBanner from "./components/DemoBanner"; import { focusAccessRequestsTab } from "./lib/adminUsersTab"; -import { CURRENT_VERSION } from "./lib/releaseNotes"; +// The built app version, NOT imported from releaseNotes — importing anything from that module +// pulls its whole ~900-line changelog into the eager App chunk and defeats the lazy ReleaseNotes +// split. FRONTEND_VERSION equals RELEASE_NOTES[0].version by release convention (both bump together). +import { FRONTEND_VERSION } from "./lib/version"; // The page modules (and their side rails) are code-split per route via the module registry // (components/moduleRegistry). App keeps only the persistent-shell chunks + the channel page and the @@ -389,7 +392,7 @@ export default function App() { banners={ <> {meQuery.data!.is_demo && } - openReleaseNotes(CURRENT_VERSION)} /> + openReleaseNotes(FRONTEND_VERSION)} /> } > diff --git a/frontend/src/components/ChannelPage.tsx b/frontend/src/components/ChannelPage.tsx index 303367f..4005cc2 100644 --- a/frontend/src/components/ChannelPage.tsx +++ b/frontend/src/components/ChannelPage.tsx @@ -120,6 +120,9 @@ export default function ChannelPage({ qc.invalidateQueries({ queryKey: ["feed"] }); qc.invalidateQueries({ queryKey: ["blocked-channels"] }); }, + // No onError: block/unblock is a local DB op (not a YouTube write, so no 403 "connect" case), + // and its 400/409/422/500 are already surfaced by the global error modal (see api.ts req) — + // a caller toast here would just double it. }); const subscribe = useMutation({ @@ -144,6 +147,7 @@ export default function ChannelPage({ qc.invalidateQueries({ queryKey: ["feed"] }); qc.invalidateQueries({ queryKey: ["channels"] }); }, + onError: (err) => notifyYouTubeActionError(err, t("channels.notify.unsubscribeFailed")), }); const onUnsubscribe = async () => { const ok = await confirm({ diff --git a/frontend/src/components/DownloadCenter.tsx b/frontend/src/components/DownloadCenter.tsx index fba5411..8e3f91a 100644 --- a/frontend/src/components/DownloadCenter.tsx +++ b/frontend/src/components/DownloadCenter.tsx @@ -681,6 +681,8 @@ export default function DownloadCenter() { ? api.cancelDownload(id) : api.deleteDownload(id), onSuccess: invalidate, + // No onError: a failed job action returns 400/409/422/500, already surfaced by the global + // error modal (api.ts req) with the backend's specific message — a caller toast would double it. }); // Remove a shared-with-me item from your list (only your grant; the owner's file is untouched). diff --git a/frontend/src/components/Feed.tsx b/frontend/src/components/Feed.tsx index 210c1ae..ad46558 100644 --- a/frontend/src/components/Feed.tsx +++ b/frontend/src/components/Feed.tsx @@ -255,7 +255,7 @@ export default function Feed({ meta: { kind: "video-hidden", videoId: id, - title: v?.title ?? "this video", + title: v?.title ?? i18n.t("feed.thisVideo"), channelId: v?.channel_id ?? "", channelName: v?.channel_title ?? "", }, @@ -270,7 +270,7 @@ export default function Feed({ meta: { kind: "video-watched", videoId: id, - title: v?.title ?? "this video", + title: v?.title ?? i18n.t("feed.thisVideo"), channelId: v?.channel_id ?? "", channelName: v?.channel_title ?? "", }, diff --git a/frontend/src/components/Modal.tsx b/frontend/src/components/Modal.tsx index 85906b1..805743f 100644 --- a/frontend/src/components/Modal.tsx +++ b/frontend/src/components/Modal.tsx @@ -1,5 +1,6 @@ import { useEffect, useRef, type ReactNode } from "react"; import { createPortal } from "react-dom"; +import { useTranslation } from "react-i18next"; import { X } from "lucide-react"; import { useBackToClose } from "../lib/history"; @@ -8,6 +9,13 @@ import { useBackToClose } from "../lib/history"; let modalStack: number[] = []; let nextModalId = 1; +/** How many s are currently open. The players (which use their own window-level Escape + * listeners, not this stack) check it so their Escape defers to a Modal opened above them — + * otherwise one Escape closes the player UNDER the dialog and then the dialog too (U-C). */ +export function modalCount(): number { + return modalStack.length; +} + // Small centered modal shell (portaled to ): backdrop + ESC + scroll-lock close. export default function Modal({ title, @@ -20,6 +28,7 @@ export default function Modal({ children: ReactNode; maxWidth?: string; }) { + const { t } = useTranslation(); // Browser/mouse Back closes the topmost modal instead of navigating away. useBackToClose(onClose); @@ -69,7 +78,8 @@ export default function Modal({

{title}