Review round on the whole E1 stack (main..HEAD). 6 findings, all low/medium,
none blocking; fixed for a clean gate-green handoff.
- plex/sync (correctness): the un-enriched cast guard compared the stored cast
against THIS listing's length, so a trimmed Plex cast on a not-yet-enriched
row was never applied. Compare against the listing CAP (_LISTING_CAST_CAP=20)
instead: >cap = enriched (keep, survives a watermark reset), <=cap =
listing-sourced (track edits in both directions). +2 regression tests.
- main (security): add X-Frame-Options: DENY as the legacy-browser fallback for
the CSP frame-ancestors clickjacking guard.
- downloads/service (altitude): finalize_if_running now uses a SQLAlchemy 2.0
update() construct instead of a hand-built text() UPDATE string — same atomic
WHERE status='running', idiomatic, no identifier interpolation, no manual
StrEnum coercion. Drops the now-unused text import.
- auth/admin (reuse): extract the SEC-10 "drop untrusted preset password + bump
epoch" invariant into one helper (revoke_preset_password_if_unverified) so the
Google-callback and admin-activation paths can't drift.
- tests/migrations (robustness): guarantee the shared schema is restored to head
in a finally, so a downgrade/re-upgrade failure can't cascade into later tests.
- worker (simplification): align _finish_from_cache's display_name fill with the
_download path's guard (no behavior change).
Gate green: ruff clean, 334 pytest (0 skip), frontend unchanged.
SEC-10 (account pre-emption / "pre-hijack"): a password set on an account whose
email was never verified is untrusted — whoever registered the address (to
pre-empt the real owner) could have chosen it. Both the Google-adoption callback
and admin activation (_activate_user_by_email) now drop that pre-set password and
bump the session epoch when the account isn't email_verified, so a later Google
login / admin approval can't hand the pre-set password a working login. No-op for
a verified account (the legit no-SMTP flow). +2 tests.
SEC-11 (link/upgrade bound to the session default): the OAuth link/upgrade GETs
resolved to the session DEFAULT account, so a tab acting as a secondary account
would link Google / the YouTube grant to the wrong account. The frontend now
threads the active account through the navigation URL (withActiveAccount ->
?account=<active>), and _complete_link accepts any wallet account, not only the
default, so the identity lands on the account that started the flow.
Layering cleanup, no behaviour change (ruff clean backend+scripts, pytest 193):
- Invert the scheduler->routes dependency: reset_demo_state (+ its demo-seed
helpers) move from routes/admin.py to a new service module app/demo.py.
Both routes/admin.py and scheduler.py now import it from app.demo, so the
scheduler no longer reaches into the routes layer — its lazy import (the
routes<->scheduler cycle workaround) is gone, hoisted to a top-level import.
- Publish the private symbols that were imported across module boundaries (the
underscore convention was lying): feed._serialize -> serialize,
feed._to_tsquery_str -> to_tsquery_str, videos._insert_stubs -> insert_stubs,
videos._stub_from_playlist_item -> stub_from_playlist_item,
worker._uploader_url -> uploader_url, plex._shared._item_or_404 ->
item_or_404. Scoped renames — the 5 other route modules keep their own truly
private _serialize helpers.
- _release_asset moves from routes/downloads.py to app/downloads/service.py as
the public release_asset (asset bookkeeping is a service concern); the routes
call it via service.release_asset.
- admin._decide: add an idempotency guard (409 unless status was 'pending') so a
repeat/stale approve OR deny can't re-run its side effects — now that deny emails,
this stops duplicate denial mails and a rejection mail to an already-approved user.
- SavedViewsWidget: extract runViewMutation(fn, failMsg) (returns success) — dedups
the five try/catch+refresh blocks AND lets create/rename close their input only on
success, so a failed create keeps the user's typed name.
- worker._notify_download_done: take user_id/title captured BEFORE the caller's commit
(no post-commit refresh SELECT); add a so the inbox labels an edit job
'Edit ready' instead of 'Download ready' (new inbox.editDone i18n, HU+EN).
Gate green (ruff/tsc/eslint-0/prettier/knip/vitest 79/pytest 193); smoke-tested the
saved-views + notifications surfaces render clean on a fresh dev server.
Give equal-weight actions equal-weight feedback (R11).
S1 Plex error toasts: the watched-toggles (card / season mark-all / episode /
info-panel / show mark-all) used to fail silently on 403/404/429; each now surfaces
the server reason via a shared notifyActionError() + resyncs the optimistic flip.
The playback auto-mark-watched stays intentionally silent (a mid-play toast is
noise; the next sync corrects it). New generic lib/actionErrors.ts (Plex + saved
views reuse it; notifyYouTubeActionError delegates its non-403 case to it).
S2 download-done inbox notification: worker.py emits a best-effort type=download_done
notice to the job owner on real completion (regular + edit jobs, not instant
cache-hits), rendered/localized in the inbox with an Open-Downloads link.
S3 optimistic rollbacks: SavedViewsWidget create/rename/toggle-default/remove/reorder
and the inbox revert-state no longer swallow failures — they toast + refetch to
resync (the reorder's optimistic order reverts to server truth).
S4 access-request deny: deny_invite now emails the requester (send_access_denied),
mirroring approve — no more silent limbo.
- setup: SetupAdminIn (KILLS the 3rd named 500 — a non-string password reaching
argon2/len()), SetupTestEmailIn; setup_config stays a typed dict[str, Any] (a
genuinely dynamic key→value settings map, not a fixed schema).
- admin: AddInviteIn / SetRoleIn / SetSuspendedIn / AddDemoWhitelistIn. The
role/status business-rule 400s are unchanged; type errors are now 422.
All 3 ROADMAP-named 500-producers now fixed (channels priority + tag_id, setup
password). Gate: ruff clean, pytest 164 green.
Establish the R6 API-contract infra before the S1b payload:dict migration:
- iso(dt) in utils.py replaces the ~16 hand-rolled `x.isoformat() if x else None`
serializer idioms across the routes.
- get_or_404 / owned_or_404 in routes/_common.py — one 404 + existence-leak policy;
plex's _own_playlist_or_404 and saved_views' _own_view now delegate to it.
- require_write_scope dependency in auth.py collapses the inline has_write_scope→403
checks (channels subscribe/unsubscribe, playlists push/push-plan) that had drifted
into three different messages into one; the conditional delete_playlist guard stays
inline (its message was already canonical).
No response-shape change (iso == isoformat for a present value). Gate: ruff clean,
pytest 163 green (+ test_utils for iso).
set_user_role (demote) and admin_delete_user counted ALL admins incl. suspended
ones, so with e.g. 1 active + 1 suspended admin you could demote/delete the only
ACTIVE admin — leaving a single suspended admin who can never sign in → permanent
admin lockout needing DB surgery. Now both guards mirror the (already-correct)
suspend guard: block only when the target is an active admin AND it's the last one
(`not target.is_suspended and count_admins(active_only=True) <= 1`) — which also
correctly still allows removing a SUSPENDED admin while one active admin remains.
- Live-search results stay ephemeral: the discovery-cleanup job now also reclaims un-kept
via_search videos (no watch state / not playlisted / channel not subscribed) after a grace
period (search_grace_days), and POST /api/search/clear discards a given result set 'as if
never added' (drops the user's search-finds + deletes the now-orphaned, un-kept videos).
Admin POST /api/admin/purge-discovery runs it on demand (grace 0).
- Count selector: GET /api/search/youtube gains a 'limit' — the free scrape source pages
through continuations until that many results are gathered (no manual load-more); the API
source stays one ≤50 page (cost).
- Per-user channel blocklist (migration 0034 blocked_channels): block/unblock/list endpoints;
blocked channels' videos are dropped from live search before ingest and hidden from the feed
/ Library / explore / channel page; explore refuses a blocked channel.
- New-first ordering: results you already have (subscribed channel, watched/in-progress/saved/
playlisted) sink below genuinely-new discoveries, preserving relevance within each group.
- app/utils.py: shared valid_email() + now_utc() (one email regex, was duplicated
in auth.py and admin.py with a looser "@"-in check elsewhere).
- security.hash_token(): one SHA-256 token hasher (was duplicated in auth.py + state.py).
- auth.admin_user dependency + count_admins() helper, replacing the inline role
checks and the last-admin count query repeated across admin.py/me.py/tags.py.
- New Users page tabs (Users & roles / Access requests / Demo) and a tab-ified Configuration
page, both via a reusable Tabs component (persisted active tab).
- Admin can suspend/unsuspend (migration 0023 adds users.is_suspended) and delete accounts
from the Users & roles tab, with guards (demo / self / last admin).
- Email notifications to the affected user: suspended (reactive, on a blocked sign-in),
reinstated, role changed, and account deleted; the approval email now carries a clickable
app link. The scheduled/manual demo reset also seeds sample channel subscriptions.
- Hide the 'unverified email' chip for Google accounts (Google attests the email).
A security audit of every mutating endpoint found no isolation gaps — all routes
scope by current_user, admin routes are gated, and the demo account is sandboxed
in its own user_id space (can't reach others' data, escalate, or hit admin
routes). The remaining demo concern is communal pollution of its own shared state,
so add an automatic reset: a new 'demo_reset' scheduler job (admin-tunable
interval, default 12h) reuses the manual reset logic, and no-ops without ever
creating a demo account if none exists. Verified: a triggered run wiped the demo's
video states and re-seeded its sample playlists.
Add email+password auth alongside Google: argon2id hashing; users gains
password_hash/email_verified/is_active and google_sub becomes nullable (migration
0022); a single-use, hashed auth_tokens table for email verification + password
reset. Registration creates a pending (inactive, unverified) account + an access
request; sign-in needs verified email AND admin approval. Anti-enumeration: uniform
register/login/reset responses (a correct-password owner still gets a specific
pending reason). allow_registration flag (DB-tunable). Admin users-list + role
endpoint (guards: not self, not demo, not the last admin); approving access (or a
manual whitelist add) now activates the matching pending account. current_user
rejects deactivated accounts. Verified end-to-end via curl + DB.
Admin endpoints to manage the demo email whitelist (DB-backed, no env)
and a manual reset that wipes the demo account's per-user state and
re-seeds a few sample playlists from the shared catalog.
Move the access whitelist from the ALLOWED_EMAILS env var into a DB Invite table
(env kept as bootstrap fallback), and add a self-service request + admin approval
flow with fail-soft email.
- models: Invite(email, status pending|approved|denied, requested_at, decided_*)
- migration 0008: invites table; seed env ALLOWED_EMAILS u ADMIN_EMAILS as approved
- auth: is_allowed() (DB-first, env fallback); a denied Google login records a pending
request and bounces to /?access=requested instead of a raw 403; public POST
/auth/request-access; upsert is idempotent so repeats don't re-spam admins
- routes/admin.py (admin-only): list/approve/deny invites + manual add
- email.py: smtplib + Gmail App Password, fail-soft (skips if SMTP unset)
- /api/me exposes pending_invites; config + .env.example gain SMTP_*
- UI: Login 'Request access' form + access=requested/denied handling; Settings ->
Access requests (approve/deny + add); admin nudge toast on pending requests
Verified locally: request-access creates a pending invite and emails the admin;
seed approved npeter83; guinea-pig yt.trash2023 denied until approved.
peter