fix(downloads,sync): bound what one user can make the server fetch, download and spend

SEC-12: the download box accepts an arbitrary http(s) URL and yt-dlp fetches
it from the WORKER, so any approved user could aim the server at the compose
network, the LAN or a cloud metadata endpoint — and read the result off
job.error, where "connection refused" vs "HTTP Error 401" vs "timed out" is a
port scanner. downloads/urlguard.py refuses a URL whose host resolves to a
loopback/private/link-local/reserved address, at enqueue AND again in the
worker (DNS can move in between, and the worker is where the socket opens).
A network-class failure on a `url` job now reports one fixed sentence instead
of the raw transport text; the error CODE is still classified from the
original, so the localized explanation is unchanged. DNS rebinding is
explicitly NOT closed — that needs the connection gated inside yt-dlp — and
the module says so rather than implying more than it does.

SEC-13: `noplaylist` only demotes a video-inside-a-playlist URL to that video,
so a URL that is purely a playlist or channel still expanded into every
entry — one enqueue, unbounded downloads. `playlist_items` pins it to one.
Size was unbounded too: `check_enqueue` compares the CURRENT footprint against
the cap, and a download's size is unknown until it lands, so one job could
overshoot by any amount; the owner's remaining budget is now handed to yt-dlp
as `max_filesize`, applied AFTER the adapter merge so no source adapter can
raise a limit the quota layer set. A join's cut-list is capped at 100: an
accurate join emits two filters per segment into one ffmpeg filter_complex.

SEC-14: the YouTube budget is instance-wide, so one user holding down a
manual sync spent the day for everyone. Per-user (not per-IP — the cost
belongs to the account) 3-per-10-minutes limiters on the two bulk pulls,
subscriptions and playlists, with separate budgets so one does not close the
other. The review also listed /rss, /backfill and /enrich; E3 deleted them,
and search stays unlimited on purpose — it is interactive, not a bulk pull.

Gate: pytest 896 -> 924 (none skipped), vitest 671.
This commit is contained in:
2026-09-05 01:03:24 +02:00
parent 688efb9ed6
commit 0cc450d1c9
15 changed files with 429 additions and 5 deletions
+36 -4
View File
@@ -15,7 +15,8 @@ from app.config import settings
from app.db import SessionLocal
from app.downloads import edit as editmod
from app.downloads import errors as dlerrors
from app.downloads import adapters, credentials, formats, mediakind, probe, service, storage
from app.downloads import adapters, credentials, formats, mediakind, probe, quota, service, storage
from app.downloads import urlguard
from app.downloads.jobdb import (
_edit_staging_dir,
_job_status,
@@ -305,13 +306,17 @@ def _media_info(info: dict) -> dict:
return {**info, **chosen}
def _extract_with_fallback(job_id, asset_id, url, spec, staging, adapter, cookiefile=None):
def _extract_with_fallback(
job_id, asset_id, url, spec, staging, adapter, cookiefile=None, max_bytes=None
):
"""Download via yt-dlp using the source `adapter`'s client sets + options. For YouTube the adapter
supplies the primary player-client set then a POT-backed fallback (retried on a bot/DRM/format
error); other sources make a single attempt. Returns the info dict of the successful attempt.
`cookiefile` (S6) is the acting user's per-host cookies.txt, already gated by
`credentials.resolve_cookiefile`; passed to every attempt so the fallback stays authenticated."""
`credentials.resolve_cookiefile`; passed to every attempt so the fallback stays authenticated.
`max_bytes` (SEC-13) is the owner's remaining storage budget, applied to every attempt."""
client_sets = adapter.client_sets()
outtmpl = str(staging / "%(id)s.%(ext)s")
last_exc: Exception | None = None
@@ -322,6 +327,7 @@ def _extract_with_fallback(job_id, asset_id, url, spec, staging, adapter, cookie
_make_progress_hook(job_id, asset_id),
extra=adapter.ydl_opts_extra(clients),
cookiefile=cookiefile,
max_bytes=max_bytes,
)
opts["postprocessor_hooks"] = [_make_pp_hook(job_id)]
_touch_job(job_id) # stall-watchdog heartbeat: extraction writes no progress, so a slow /
@@ -415,8 +421,17 @@ def _download(
_set_job(job_id, phase="downloading", progress=0)
adapter = adapters.resolve_adapter(source_kind, source_ref)
url = service.source_url(source_kind, source_ref)
# SEC-12, second half: the enqueue guard ran when the job was created, which may have been
# hours ago and against a different DNS answer. This is where the socket actually gets
# opened, so the check is repeated here and the job fails rather than fetching.
if source_kind == "url":
try:
urlguard.check_url(url)
except urlguard.BlockedUrl as e:
raise RuntimeError(f"Refusing to download from this address ({e.reason}).") from e
# S6: attach the acting user's per-host cookie ONLY when it's host-scoped to this URL AND
# this asset is that user's private, salted identity (the double gate lives in credentials).
remaining_bytes: int | None = None
with SessionLocal() as cdb:
asset_sig = cdb.execute(
select(MediaAsset.format_sig).where(MediaAsset.id == asset_id)
@@ -425,6 +440,13 @@ def _download(
cookiefile = credentials.resolve_cookiefile(
cdb, user_id, url, asset_sig, spec, staging
)
# SEC-13: what this user has left. `check_enqueue` only compares the CURRENT footprint
# against the cap — a download's size is unknown until it finishes, so a single job could
# previously blow through the cap by any amount. Handing the remainder to yt-dlp as
# `max_filesize` makes the format selector refuse an oversized file up front instead.
lim = quota.resolve(cdb, user_id)
if not lim.unlimited:
remaining_bytes = max(0, lim.max_bytes - quota.footprint(cdb, user_id))
info = _media_info(
_extract_with_fallback(
job_id,
@@ -434,6 +456,7 @@ def _download(
staging,
adapter,
cookiefile=str(cookiefile) if cookiefile else None,
max_bytes=remaining_bytes,
)
)
# Re-resolve now that yt-dlp's info reveals the true source (a bare/redirected url only
@@ -527,6 +550,15 @@ def _download(
log.info("job %s aborted (pause/cancel)", job_id)
except Exception as exc: # noqa: BLE001 — surface any failure to the user, keep worker alive
msg = str(exc)[:500]
code = dlerrors.classify(msg)
# SEC-12: for a user-supplied URL, a transport-level failure text distinguishes "connection
# refused" from "HTTP Error 401" from "timed out" per host:port — which is a port scanner
# with extra steps, since the raw text is shown to the user behind the details disclosure.
# The urlguard above stops it aiming at our own network; this stops the remaining readout.
# Only the network class, and only for `url` jobs: a YouTube job has no user-chosen host, and
# every other class (login_required, geo_blocked, drm, …) is real, useful diagnosis.
if source_kind == "url" and code == "network":
msg = "The server could not reach that address."
with SessionLocal() as db:
asset = db.get(MediaAsset, asset_id)
if asset:
@@ -539,7 +571,7 @@ def _download(
job_id,
status=JobStatus.ERROR,
error=msg,
error_code=dlerrors.classify(msg),
error_code=code,
phase=None,
)
db.commit()