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.
A downloaded video was silently demoted to a download-only "file" (no
in-app play/edit) whenever yt-dlp's info dict omitted vcodec/acodec — which
it does for Facebook and other sources. Finalize derived media_kind from
those nulls (`derive(None, None)` -> file), so a genuine H.264/AAC MP4
became non-playable in the library.
Fix: probe the finished file on disk (the ground truth) at finalize and
derive from that, falling back to the info dict only for a field the probe
couldn't read. This is the same ffprobe the upload ingest already used, now
extracted to a shared downloads/probe.py so the download worker and the
upload path share one classifier (and can't drift). Also backfills the
stored vcodec/acodec/width/height from the real file.
Tests: TestResolveMediaFacts pins probe-wins-over-info (incl. the Facebook
no-codec case and yt-dlp's literal "none"); upload's probe tests repoint to
the shared module.
Deep-review remediation for the E7 upload path, all findings adversarially verified:
- Serve every library file with a Content-Type chosen from the ffprobe-verified media_kind
+ a strict container allow-list (storage.served_media_type), never guessed from the
user-controlled file extension. Closes a stored-XSS vector where a media/HTML polyglot
uploaded as "evil.html" was served inline as text/html on the login-free /watch surface.
Applied to both the authenticated /file and the public watch_file endpoints.
- Classify uploaded still images (jpg/png/gif/webp/…) as media_kind='file' so they are
force-downloaded and don't get a broken in-app video player.
- Roll back the on-disk side of an upload when the ingest transaction fails after place_file
(unlink the placed file + its deterministic poster path), so a failed commit can't orphan
bytes in the GC-skipped .uploads tree.
- Floor download_upload_max_bytes at min=1 so a stored 0 can't silently disable the per-file cap.
- Dedupe the ffprobe-JSON invocation into edit.run_ffprobe_json (shared by probe_codecs and
upload._probe) and the upload extension into storage._clean_upload_ext; drop the now-unused
parse_probe_streams.
Tests: public + authenticated force-download and the polyglot-not-served-as-HTML guard, the
real _probe classifier (image/cover-art/dims/duration), the ingest rollback/cleanup path, the
max_jobs branch, the cap-floor invariant, and run_ffprobe_json error handling.
The download/edit child (yt-dlp + ffmpeg) shares one LAN box with the API,
Postgres, and Plex playback. A CPU re-encode pegged every core for ~30s and
starved web requests + playback until it finished. Full-scope S4 mitigations:
- Child self-de-prioritizes at startup (inherited by every ffmpeg/yt-dlp
descendant): CPU nice increment + best-effort idle ionice class + an
OMP_NUM_THREADS budget, all config-gated and independently disable-able.
- Cap the browser-safe re-encode's encoder threads (`-threads`) so a transcode
reserves a core for foreground work instead of claiming all of them.
- Throttle the retention GC's whole-tree orphan sweep (yield IO after each
batch of files) so its rglob can't monopolize disk IO in the API process.
- Idle-back-off the Download Center (2s→15s when nothing queued/running) and
UsageBar (2s→15s when running_jobs is 0) polls; both snap back to 2s on the
next fetch after an enqueue (invalidateDownloads refetches immediately).
Tests: encode_thread_cap (pin/reserve-core/edge), -threads flag presence,
_deprioritize (nice/ionice/OMP branches), GC sweep throttle batching.
- worker.py: wrap the watchdog loop so a failure in it (e.g. a transient DB error in
_job_liveness) still hard-kills the child + fails the job, instead of leaving the child
running unsupervised — the exact wedge the feature prevents (#1).
- worker.py: _touch_job heartbeat bumps updated_at at each extraction attempt, so a slow /
multi-client-set fallback extraction (which writes no progress, and re-writes identical
values so ORM onupdate does not fire) is not false-killed as "stalled" (#2).
- edit.py: extract escalate_kill (terminate->wait->kill escalation); _stop and worker's
_stop_child (process-group variant) now share it (#3).
- worker.py: document the edit path's second, tighter run_ffmpeg watchdog and that the
supervisor is POSIX/Linux-container-only (#4, #6).
- tests: a long download stays alive while progress is fresh, and a watchdog-loop error
still stops the child (#1, #5).
- edit.py: extract _run_ffmpeg_best_effort (one run+timeout+swallow policy shared by
thumbnail_to_jpg / ensure_poster / ensure_storyboard) + a build_thumbnail_jpg_cmd
builder, matching the module's other ffmpeg commands.
- thumbnail_to_jpg: copy an already-JPEG thumbnail (Facebook, and Reddit's jpeg-under-
a-.png-name) instead of re-encoding it; only webp/png-proper are decoded via ffmpeg.
- errors.py: drop the redundant "account authentication" classify marker
("authentication is required" already covers the known Reddit message).
- formats.py: document that the opt-in EmbedThumbnail is unaffected by dropping the
convertor (it converts unsupported thumbs to PNG, not the crashing mjpeg2jpeg path).
- test_titles.py: guard the ReDoS regression with a thread + timeout (deterministic
"must not hang") instead of a wall-clock assertion.
Three prod-observed download bugs, root-caused from a frozen prod queue with the
worker pinned at 100% CPU:
- ReDoS in normalize_title: _TRAILING_HASHTAGS had two overlapping unbounded
quantifiers around the required letter, so a Facebook title with a long hashtag
cluster that is NOT at the end ("... #a #b #c | Page") backtracked catastrophically.
It runs inside the yt-dlp progress hook holding the GIL, so one such download froze
the whole worker (and could stall feed enrichment too - sync/videos.py calls it).
Rewrite with a zero-width "has a letter" lookahead + a possessive tag body: one
unambiguous split, nothing to backtrack. 40-tag stress case: ~0.1ms.
- Reddit download killed by a cosmetic thumbnail convert: yt-dlp's
FFmpegThumbnailsConvertor runs "-bsf:v mjpeg2jpeg" on a JPEG-served-under-a-.png-name
(Reddit) and fails; being a postprocessor, it failed the WHOLE download even though
the media was fine. Drop it and produce the <base>.jpg poster/og:image ourselves via
editmod.thumbnail_to_jpg (plain ffmpeg decode->mjpeg, best-effort -> ensure_poster
cuts a frame on failure). The FB Messenger / MS Teams share preview keeps working
(posters verified as real JPEGs).
- classify: map "authentication is required" (Reddit's auth wall) to login_required
instead of unknown, so the UI explains it rather than offering a pointless retry.
Tests: ReDoS-timing + numeric-marker regression, the Reddit classify case, and
thumbnail_to_jpg (real JPEG out / None on bad input, never raises).
- run_ffmpeg stops the process from ONE exit path (`except BaseException`), so
an exception out of on_progress (a DB write) can no longer orphan a live
ffmpeg after a 10s join on its pipes; _stop is now a no-op on a dead process
- both run_ffmpeg callbacks are throttled to ~1s (they were a DB round-trip per
output line: ~10^5 queries on a long re-encode), mirroring the download hook
- volume arithmetic extracted to lib/playerVolume with unit tests; a corrupt
level now falls back to the LAST GOOD one instead of full blast, and the
sanitising happens where the stored value enters
- RSS is back to two except clauses (the shared tail already sits after the try)
- new tests: callback-exception cleanup, the two throttles; the fake Popen grew
poll() and its wait count is now asserted
Mutation-checked: each new group goes red when its fix is reverted.
- HLS sweep only deletes directories matching the session-name shape; the root
is admin-configurable and may be a shared path
- playlist unwrap picks the first entry that actually downloaded, not entries[0]
- concat quoting keeps backslashes literal (ffmpeg treats them so inside '')
- ArrowUp/Down defer while the focused element can still scroll that way
- the staging fallback skips yt-dlp working files (.part-Frag/.ytdl/.temp/.fNNN)
- the boot HLS sweep runs off the event loop
- volume persistence is debounced (was a setItem per wheel notch) and a spin no
longer loses notches to the iframe's lagging getVolume
- _Breaker.run owns the whole open/call/record protocol; four call sites shrink
- should_prune extracted and unit-tested, plus RSS error-vs-empty tests
- the worker exits non-zero when the schema never lands
ffprobe prints codec_name BEFORE codec_type, so the line parser attributed every
name to the previous stream: a plain H.264+AAC mp4 probed as (aac, None) and the
browser-playability check re-encoded a perfectly good file on EVERY download.
Verified on localdev — the same source now stores without a re-encode pass.
- pick the produced file safely: Path("") is Path("."), which passes .exists(),
so a missing yt-dlp filepath skipped the staging fallback and crashed in
with_name ("PosixPath('.') has an empty name") on non-YouTube sources
- unwrap a playlist/multi_video extraction to the entry that carries the media
- run_ffmpeg: drain stderr while it runs (a full 64KB pipe blocked ffmpeg
forever and hung the worker thread), 1s cancel polling, stall watchdog and a
bounded wait on exit
- escape apostrophes/backslashes in the ffmpeg concat list (pre-0.51.0 names)
- sweep orphaned HLS segment dirs at startup (nothing else ever reaped them)
- redownload: never clobber a co-held (deduped) asset. Drop the force-reset-to-pending;
_release_asset already deletes+recreates a solo asset (genuine re-fetch), while a shared
ready asset is left intact and the re-queued job re-points to it via _finish_from_cache —
so one user's re-download can't delete another's file mid-serve (finding #1).
- redownload: reset queue_pos to the tail so a re-download doesn't front-run newer queued
jobs on the worker's `ORDER BY queue_pos` claim (finding #3). Made service.next_queue_pos public.
- formats: video-only mp4 selector prefers every video-only option before any progressive
`best`, so a "Video only" download stays audio-free whenever a video-only stream exists (#2).
- titles: a title that is only the engagement prefix no longer resurrects the stripped stats
(return the stripped text / empty, not the raw) (#4).
- worker: probe the file's duration when yt-dlp gave none, so a long re-encode shows real
progress instead of a frozen 0% (#5); tighten the ensure_browser_playable docstring to its
actual mp4-profile scope (#6).
Mini-epic "Downloads correctness & UX" (S1–S3), on top of the iOS-AV1 + 720p fixes.
S1 — naming & titles:
- ASCII-fold on-disk dir/file names AND the served (Content-Disposition) filename: no
spaces (underscore-joined), no accents (á→a, ő→o, ß→ss), pure ASCII — portable and free
of the `?` mojibake accented names show in non-UTF-8 tools. (Forward-only; existing files
clean up via re-download.)
- Strip a leading social engagement prefix ("1.6M views · 66K reactions | …") from titles.
S2 — universal browser playability (generalizes the iOS re-encode):
- ensure_browser_playable guarantees the stored file is H.264 + AAC/MP3 in mp4 — the only
combo every browser decodes. Re-encode just the offending stream(s) (video for AV1/VP9/HEVC,
audio for Opus/etc), or remux when only the container is wrong. Gated on the mp4 compat
profile (an explicit vcodec / non-mp4 custom profile opts out).
S3 — force re-download (preserves share links):
- POST /downloads/{job_id}/redownload keeps the job row so its public DownloadLink survives
and resolves to the freshly-downloaded file; deletes the old file and re-fetches under the
current rules. A failed re-download leaves the job in error (the intended broken-link case).
- Library "Re-download" action (icon + confirm), api method, hu/en strings.
Tests: naming/title-strip (test_naming), browser_transcode_flags matrix; existing
download_filename expectation updated for the underscore policy.
Shared /watch links showed a "broken play button" on iPad (any iOS
browser — all forced onto WebKit) while playing fine on desktop Chrome.
Root cause: the stored mp4's video codec was AV1 (confirmed via ffprobe),
which WebKit can't decode. The existing H.264 guard was only a
format_sort *preference*, so a non-YouTube source (Facebook) that ships
AV1 as its best video-only stream slipped through.
- B1: for the mp4 compat profile, prefer H.264 at the SELECTION level
with a progressive fallback (bestvideo[vcodec^=avc1]+bestaudio /
best[vcodec^=avc1] / anything) so an avc1 progressive is chosen over
an AV1-only video-only stream.
- B2: re-encode the rare AV1/VP9-only result to H.264+AAC (+faststart)
in the worker before storing, gated on the compat profile; corrects
the asset codec metadata to match.
- Bump _SIG_VERSION 2->3 so cached AV1 assets re-derive.
- Default download preset is now 720p (migration 0058 re-seeds builtins
720p-first; ProfileEditor BLANK 1080->720).
- Tests for the selector, compat opt-outs, and the transcode predicate.
- B1 (sticky errored asset): get_or_create_asset now resets a reused status=='error'
asset back to 'pending' (+clears the error), so a fresh enqueue/edit of a once-failed
(source,format) pair actually re-downloads instead of the worker short-circuiting the
new job with the stale error. Errored assets carry no expires_at, so without this the
pair was permanently poisoned for all users until someone hit resume.
- B2 (ref_count leak): _release_asset counts 'error' as a holding state, so deleting an
errored job decrements the ref_count that enqueue always incremented (the worker never
decrements on failure). Errored rows are deliberately NOT deleted here — a concurrent
B1 reuse could otherwise be lost-updated + FK-nulled; the row is fileless and harmless.
- B3: admin storage dashboard reads sysconfig.get_int(db,'download_total_max_bytes')
(the admin-editable DB value GC enforces) instead of the raw env default.
- B4: the single-trim branch of normalize_edit_spec guards its float() coercion like the
crop/segments branches — a malformed trim now yields a 400, not an unhandled 500.
- B5: formats.normalize guards int(max_height) → falls back to "best" instead of 500.
Reviewed (race in an earlier B2 draft caught + fixed); localdev boots, B4/B5 unit-verified.
Two visual gaps for non-catalog downloads:
- Channel link: YouTube already exposes channel_url; Facebook exposes none but a
numeric uploader_id that resolves at facebook.com/<id>. `_uploader_url` derives
it so the auto-detected channel renders as a real clickable link.
- Poster: a source with no thumbnail (e.g. a direct reddit HLS URL) showed a
blank image box. The worker now cuts a representative frame with ffmpeg
(`ensure_poster`) into the `<base>.jpg` sidecar and records `poster_path`
(migration 0050). The card, the public watch page (<video poster> + og:image),
and link previews fall back to it via new authed + public poster endpoints.
Adds `app.downloads.backfill` (one-off, re-run-safe) to fill uploader_url
(re-extract YouTube/Facebook metadata) and posters for pre-existing downloads.
Editor v2 backend: an edit_spec can carry a 'segments' cut-list that concatenates the kept
ranges into ONE file (accurate=filter_complex trim+concat re-encode; fast=concat-demuxer
stream-copy with per-segment inpoint/outpoint). normalize/edit_sig/clip_duration/needs_reencode
handle segments; worker writes the concat list + runs build_concat_plan. Single-trim path (used by
'separate files' export) unchanged.
peter