From f221c1962ab0ffbbdb9fc3676ebfe5e04c94e6ca Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 10 Oct 2026 03:58:27 +0200 Subject: [PATCH] fix(downloads): a YouTube clip goes with the user's own cookie and is diagnosed like a download A clip fetched its range signed out, so a video behind YouTube's per-video "not a bot" wall could never be clipped while its download went through with the user's cookie - and the failure copy told the user to add a cookie the clip would not use. The clip asset is private by construction (`clip.asset_ref` names its user), so `usable_clip_cookie` attaches the user's YouTube cookie on that identity, and a failed range fetch now carries the download catalog's diagnosis (`youtube.bot_wall` / `youtube.cookie_rejected`) instead of a bare category. --- backend/app/downloads/credentials.py | 31 +++++++++++ backend/app/downloads/pipeline.py | 79 +++++++++++++++++++++------- backend/tests/test_clip.py | 78 +++++++++++++++++++++++++++ 3 files changed, 170 insertions(+), 18 deletions(-) diff --git a/backend/app/downloads/credentials.py b/backend/app/downloads/credentials.py index f7cef6cd..648c441d 100644 --- a/backend/app/downloads/credentials.py +++ b/backend/app/downloads/credentials.py @@ -30,6 +30,7 @@ from sqlalchemy import select from sqlalchemy.orm import Session from app import logs +from app.downloads import clip as clipmod from app.downloads import formats from app.downloads.adapters import _host_is from app.models import DownloadCredential, MediaAsset @@ -259,6 +260,28 @@ def usable_cookie( return None if not is_authed_asset(asset, spec, user_id, url): return None # shared asset — never attach a cookie to a non-private identity + return _readable(cred, user_id, host_key) + + +def usable_clip_cookie( + db: Session, user_id: int, url: str | None, asset: MediaAsset, clip_spec: dict +) -> str | None: + """`usable_cookie` for an ad-hoc clip's range fetch. The second gate is the clip's own identity: + a clip asset is private by construction (`clip.asset_ref` names its user, see downloads/clip), + so a cookie goes with it exactly when that asset is THIS user's clip of `clip_spec`. Without it a + video YouTube gates signed out could never be clipped, while its download goes through.""" + host_key = host_key_for_url(url) + if host_key is None: + return None + cred = get_credential(db, user_id, host_key) + if cred is None: + return None + if asset.source_kind != "clip" or asset.source_ref != clipmod.asset_ref(user_id, clip_spec): + return None + return _readable(cred, user_id, host_key) + + +def _readable(cred: DownloadCredential, user_id: int, host_key: str) -> str | None: raw = decrypt_optional(cred.cookies_enc) if not raw: log.warning( @@ -284,6 +307,14 @@ def resolve_cookiefile( return _write_cookiefile(raw, staging) if raw else None +def resolve_clip_cookiefile( + db: Session, user_id: int, url: str | None, asset: MediaAsset, clip_spec: dict, staging: Path +) -> Path | None: + """`resolve_cookiefile` for an ad-hoc clip (`usable_clip_cookie`).""" + raw = usable_clip_cookie(db, user_id, url, asset, clip_spec) + return _write_cookiefile(raw, staging) if raw else None + + def _write_cookiefile(raw: str, staging: Path) -> Path: """Write the cookie blob to a private 0600 file under `staging/.auth/` and return its path. diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py index 478cb428..591ab6e1 100644 --- a/backend/app/downloads/pipeline.py +++ b/backend/app/downloads/pipeline.py @@ -41,6 +41,15 @@ class _Aborted(Exception): """Raised inside the progress hook when the job was paused/canceled mid-download.""" +class _ClipFetchFailed(Exception): + """A clip's YouTube range fetch failed: its message and the download catalog's diagnosis, so a + clip on YouTube's wall says what a download of the same video would.""" + + def __init__(self, msg: str, diagnosis: dlerrors.Diagnosis): + super().__init__(msg) + self.diagnosis = diagnosis + + class _OwnerRefused(Exception): """A refusal that belongs to THIS job's owner, not to the source. @@ -351,7 +360,8 @@ def _extract_with_fallback( download as itself. `cookiefile` 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` (a clip: `resolve_clip_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. @@ -747,11 +757,18 @@ def _log_download_failed(job_id: int, msg: str, code: str | None, exc: BaseExcep def _fail_edit( - job_id: int, asset_id: int, msg: str, exc: BaseException | None = None, *, what: str = "edit" + job_id: int, + asset_id: int, + msg: str, + exc: BaseException | None = None, + *, + what: str = "edit", + diagnosis: dlerrors.Diagnosis | None = None, ) -> None: - """Fail a local ffmpeg job — an edit, or an upload's conversion (`what="upload"`) — and say so - ONCE: every way one fails comes through here, so this is where its line and its event code - (`.job.failed`) live. `exc` is the exception when there was one. + """Fail a local ffmpeg job — an edit, a clip, or an upload's conversion (`what="upload"`) — and + say so ONCE: every way one fails comes through here, so this is where its line and its event code + (`.job.failed`) live. `exc` is the exception when there was one; `diagnosis` replaces the + plain category when the failure was a fetch the download catalog can name (a clip's). Its traceback goes with the line unless the message already tells it all (`_failure_stack`). Logged after the write, as on the download error path.""" @@ -766,9 +783,12 @@ def _fail_edit( job_id, status=JobStatus.ERROR, error=msg, - error_code=dlerrors.classify_conversion(msg) + error_code=diagnosis.code + if diagnosis + else dlerrors.classify_conversion(msg) if what == "upload" else dlerrors.classify(msg), + error_detail=diagnosis.detail if diagnosis else None, phase=None, ) db.commit() @@ -914,24 +934,44 @@ def _clip_source( the fetch could tell it. A YouTube clip fetches ONLY its range into `staging` (so it starts at 0 there); a Plex clip reads the library file in place.""" if spec["source"] == "youtube": + url = service.source_url("youtube", spec["ref"]) + cookiefile: Path | None = None with SessionLocal() as db: job = db.get(DownloadJob, job_id) + asset = db.get(MediaAsset, asset_id) remaining = quota.remaining_bytes(db, job.user_id) if job else None + # Signed in when the user has a YouTube cookie, as their downloads are: YouTube gates + # single videos signed out ("not a bot"), and the clip is theirs alone. + if job and asset: + cookiefile = credentials.resolve_clip_cookiefile( + db, job.user_id, url, asset, spec, staging + ) if remaining == 0: # see `_download`: zero is a refusal, not "no limit" raise _OwnerRefused(dlerrors.QUOTA_FULL_MESSAGE) _set_job(job_id, phase="downloading", progress=0) - # Signed out, like a download without a cookie: the cookie gate is written for a download's - # salted asset identity (`credentials.resolve_cookiefile`), not for a clip's. - info = _extract_with_fallback( - job_id, - asset_id, - service.source_url("youtube", spec["ref"]), - formats.normalize(clipmod.YOUTUBE_FETCH_SPEC), - staging, - adapters.resolve_adapter("youtube", spec["ref"]), - max_bytes=remaining, - section=(spec["start_s"], spec["end_s"]), - ) + try: + info = _extract_with_fallback( + job_id, + asset_id, + url, + formats.normalize(clipmod.YOUTUBE_FETCH_SPEC), + staging, + adapters.resolve_adapter("youtube", spec["ref"]), + cookiefile=str(cookiefile) if cookiefile else None, + max_bytes=remaining, + section=(spec["start_s"], spec["end_s"]), + ) + except (_Aborted, _OwnerRefused): + raise + except Exception as exc: # re-raised below with the download catalog's diagnosis + msg = str(exc)[:500] + raise _ClipFetchFailed( + msg, dlerrors.diagnose(msg, url=url, cookie_attached=cookiefile is not None) + ) from exc + finally: + # Never leave a plaintext session cookie on disk past the fetch (see `_download`). + if cookiefile is not None: + cookiefile.unlink(missing_ok=True) info = _media_info(dict(info)) title = normalize_title(adapters.resolve_adapter("youtube", spec["ref"]).title(info)) return _produced_file(info, staging), 0.0, title @@ -1065,6 +1105,9 @@ def _process_clip(job_id: int, asset_id: int) -> None: formats.log_safe(msg), extra=logs.event("clip.job.refused", job_id=job_id), ) + except _ClipFetchFailed as exc: + # silent-ok: not silent — `_fail_edit` is the report, with the fetch's diagnosis. + _fail_edit(job_id, asset_id, str(exc), exc, what="clip", diagnosis=exc.diagnosis) except Exception as exc: # noqa: BLE001 — surface the failure, keep the worker alive # silent-ok: not silent — `_fail_edit` is the report, and it takes `exc` for its stack. _fail_edit(job_id, asset_id, str(exc)[:500], exc, what="clip") diff --git a/backend/tests/test_clip.py b/backend/tests/test_clip.py index 8c7451be..8371a11f 100644 --- a/backend/tests/test_clip.py +++ b/backend/tests/test_clip.py @@ -440,3 +440,81 @@ def test_a_youtube_clip_outside_the_catalog_takes_its_title_from_the_fetch( "Found On YouTube (clip)", "Found On YouTube (clip)", ) + + +# --- a YouTube clip goes with the user's own cookie, and its failure is diagnosed ----------------- + +_YT_COOKIES = "# Netscape HTTP Cookie File\n.youtube.com\tTRUE\t/\tTRUE\t0\tSID\tabc\n" + + +@pytest.fixture +def fernet(monkeypatch): + from cryptography.fernet import Fernet + + from app import security + + monkeypatch.setattr(security, "_fernet", Fernet(Fernet.generate_key())) + + +def _capture_extract(monkeypatch, asked: dict): + def extract(job_id, asset_id, url, spec, staging, adapter, **kw): + cookiefile = kw.get("cookiefile") + asked["cookiefile"] = cookiefile + asked["cookie_text"] = Path(cookiefile).read_text() if cookiefile else None + f = Path(staging) / "abcdefghijk.mp4" + f.write_bytes(b"range") + return {"requested_downloads": [{"filepath": str(f)}]} + + monkeypatch.setattr(pipeline, "_extract_with_fallback", extract) + + +def test_a_youtube_clip_goes_signed_in_with_the_users_own_cookie_and_leaves_none_behind( + db, make_user, as_user, yt_video, worker, monkeypatch, fernet +): + from app.downloads import credentials + + user = make_user() + credentials.upsert_credential(db, user.id, "youtube", _YT_COOKIES) + db.commit() + asked: dict = {} + _capture_extract(monkeypatch, asked) + job, _asset = worker["run"](_clip(as_user(user), source="youtube", ref=yt_video).json()) + assert job.status == "done" + assert asked["cookie_text"] == _YT_COOKIES + assert not Path(asked["cookiefile"]).exists() + + +def test_a_youtube_clip_never_goes_with_another_users_cookie( + db, make_user, as_user, yt_video, worker, monkeypatch, fernet +): + from app.downloads import credentials + + other = make_user() + credentials.upsert_credential(db, other.id, "youtube", _YT_COOKIES) + db.commit() + asked: dict = {} + _capture_extract(monkeypatch, asked) + job, _asset = worker["run"](_clip(as_user(make_user()), source="youtube", ref=yt_video).json()) + assert job.status == "done" and asked["cookiefile"] is None + + +@pytest.mark.parametrize( + ("with_cookie", "detail"), [(False, "youtube.bot_wall"), (True, "youtube.cookie_rejected")] +) +def test_a_youtube_clip_on_the_bot_wall_is_diagnosed_like_a_download( + db, make_user, as_user, yt_video, worker, monkeypatch, fernet, with_cookie, detail +): + from app.downloads import credentials + + user = make_user() + if with_cookie: + credentials.upsert_credential(db, user.id, "youtube", _YT_COOKIES) + db.commit() + + def walled(*_a, **_k): + raise RuntimeError(f"ERROR: [youtube] {yt_video}: Sign in to confirm you’re not a bot.") + + monkeypatch.setattr(pipeline, "_extract_with_fallback", walled) + job, asset = worker["run"](_clip(as_user(user), source="youtube", ref=yt_video).json()) + assert (job.status, asset.status) == ("error", "error") + assert (job.error_code, job.error_detail) == ("login_required", detail)