yt-dlp reports a download 'finished' as soon as the media bytes have
landed, but the ffmpeg work that follows -- merging, re-encoding, chapter
splitting, sponsor removal -- routinely takes longer than the download
itself. The postprocessor hook only ever looked at MoveFiles and
SplitChapters finishing, so that whole phase said nothing: the row sat in
the Downloading table on a full, frozen progress bar, and the item counted
as neither active nor queued in the header stats.
Report a 'postprocessing' status when a postprocessor starts, and reuse
the indeterminate bar the UI already runs for 'preparing', labelled so a
long re-encode is distinguishable from a stall.
Measured on a real download re-encoded with the reporter's FFmpegCopyStream
config, the UI now receives:
[ 0.27s] downloading moving bar
[ 0.28s] finished <- yt-dlp, bytes are down
[ 0.28s] postprocessing animated "Post-processing"
[13.18s] finished done
i.e. 12.9s that used to render as a static 100% bar.
The hook is latched off once MoveFiles reports finished, since that branch
announces the finished file: a 'postprocessing' arriving afterwards would
flip the row back out of its completed state for no reason. It cannot fail
the download -- _download puts an unconditional 'finished' once download()
returns, so the terminal status is settled either way -- but the flicker
and the extra broadcast are both pointless. yt-dlp does run an 'after_move'
stage after MoveFiles, though every postprocessor MeTube configures is
'after_filter' or 'post_process', both of which precede it.
update_status drops a repeated 'postprocessing': yt-dlp's metaclass wraps
run() once per class in a postprocessor's MRO, so one whose subclass
overrides run reports started twice -- FFmpegCopyStream did exactly that in
the live run, three queued statuses collapsing to a single broadcast.
Extracted to _make_postprocessor_hook, mirroring _make_progress_hook, so
the ordering above is testable; every new regression test was confirmed to
fail with the fix backed out.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
aiohttp hands HOST straight to getaddrinfo, which has no notion of a '*'
wildcard: the lookup fails and MeTube dies at startup on an opaque DNS
error. '*' is nevertheless what people reach for when they want to serve
both IP stacks -- it is the answer given on #795 -- while the value that
actually does it, an empty string, is undiscoverable.
asyncio expands an empty host to one listening socket per address family,
so map '*' onto it. Verified against the real stack:
'' -> [('0.0.0.0', p), ('::', p, 0, 0)]
'0.0.0.0' -> [('0.0.0.0', p)]
'::' -> [('::', p, 0, 0)]
'*' -> gaierror (before this change)
The README claimed the 0.0.0.0 default was "all interfaces", which is
only true of IPv4; document the three modes instead. Note that '::' is
IPv6-only whatever the host's bindv6only says, because asyncio always
sets IPV6_V6ONLY on the sockets it binds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PersistentQueue.put/delete wrote the whole queue inline: serialize, write
a temp file, fsync it, rename, then fsync the directory. All of that ran
synchronously inside async callers, so on a slow or contended filesystem
a single queue mutation stalled every other request for as long as the
two fsyncs took. Adds and completions are exactly when it fires, which
matches the reported "hiccups happen when something is pushing into the
queue".
put/delete are now coroutines. The payload is still serialized on the
event loop -- it is pure CPU and sub-millisecond -- and only the write
goes to a thread, so the writer never walks live DownloadInfo objects
while the loop mutates them. Each queue gets its own single-worker
executor rather than sharing the default one, because extract_info can
hold default-executor threads for minutes and would leave state writes
queued behind exactly when they are needed.
Awaiting the write makes interleaving possible where it was not before,
so a lock now covers the mutate-write-rollback section: the invariant
that in-memory state never diverges from what is on disk is unchanged,
including the rollback when a write fails. On shutdown the queues are
drained rather than cancelled, so a write in flight still lands.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
yt-dlp documents 'filename' as always present in a progress hook, so the
update_status branch that stats it ran on every forwarded tick: throttled
to one every 0.5s per download, times MAX_CONCURRENT_DOWNLOADS. Those are
blocking syscalls on the event loop, and when the filesystem is slow each
one freezes every other request the server is serving -- which is what a
bare GET timing out at >10s looks like from outside.
The call was also useless while it was expensive. Until the download
finishes the bytes live in tmpfilename; 'filename' is the destination,
which does not exist yet, so os.path.exists() returned False and size
stayed None. It only yields a real value on a terminal status, and the
Downloading table has no size column, so nothing displayed it before
completion either way.
Stat only when the status is 'finished'. That covers both moments a file
genuinely exists at that path: yt-dlp's own finished status, and the
MoveFiles postprocessor reporting the final merged name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
get_custom_dirs() builds the folder dropdown by removing the base path
as a prefix from every subdirectory it finds. The base directory's own
path does not carry a trailing slash, so with DOWNLOAD_DIR=/downloads/
the base failed to match itself and fell through to the leading-slash
trim, leaking 'downloads' into the dropdown as a bogus folder option.
Selecting it would have downloaded into /downloads/downloads.
Normalised in Config alongside URL_PREFIX, after the '%%' indirection so
AUDIO_DOWNLOAD_DIR is resolved first. '/' and '///' still resolve to '/'
rather than the empty string.
Found while trying to reproduce #542, which does not reproduce on
current code -- both directory listings populate correctly with a
distinct AUDIO_DOWNLOAD_DIR.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reported in #881, where the reporter had to reverse-engineer this from
the outside over several days: setting
YTDL_OPTIONS={"cookiefile": "/cookies/cookies.txt"} appeared to do
nothing whenever a cookies file had also been uploaded through the UI.
Uploaded cookies winning is correct and stays as it is. The upload
exists so cookies can be refreshed without restarting the container, and
letting YTDL_OPTIONS win instead would leave a visible UI button that
silently does nothing.
The defect is that it happened in silence, and could not be reported
afterwards even in principle. set_runtime_override writes into
YTDL_OPTIONS directly, so the moment an uploaded file is applied the
configured path is gone from the live config: delete_cookies' existing
has_manual_cookiefile check compares against COOKIES_PATH and therefore
cannot fire once the value has been replaced. The two override points
are the only places where both paths are still visible, so that is where
the warning has to go.
The startup path previously logged only "Cookie file detected"; both it
and the upload handler now say plainly which file is being ignored and
how to get it back.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A bookmarklet could never reach an instance behind reverse-proxy auth.
on_prepare echoed Access-Control-Allow-Origin but never sent
Access-Control-Allow-Credentials, and hardcoded Allow-Headers to
Content-Type, so both approaches the reporter suggested in 2022 still
failed in a browser today: credentials:'include' was rejected for the
missing Allow-Credentials, and an explicit Authorization header was
rejected as not allowed by the preflight.
Naming an origin in CORS_ALLOWED_ORIGINS is a deliberate trust grant, so
a named origin may now send credentials and an Authorization header. The
'*' wildcard is not such a grant: it matches origins the operator never
enumerated, and since credentials require echoing the origin back rather
than sending '*', pairing the two would let any site the user visits
drive their instance with their own session. The wildcard therefore
keeps byte-for-byte the uncredentialed behaviour it has always had, and
a '*' anywhere in the list disables credentials for every origin in it,
with a startup warning so that combination is not silently confusing.
This matches the boundary socket.io already enforces: engineio defaults
cors_credentials to True, and its wildcard test is against the string
'*' while we pass a list, so it too grants credentials only to
explicitly listed origins.
Also sets Vary: Origin, appending rather than clobbering, so a shared
cache cannot hand one origin's Allow-Origin to another.
Verified in a real browser across the matrix: with the caller's origin
named, plain/credentialed/Authorization requests all succeed; under '*'
the credentialed and Authorization requests stay blocked; from an
unlisted origin everything stays blocked.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds a "Remove sponsor segments" switch to the shared options panel,
building the same SponsorBlock + ModifyChapters pair the CLI's
--sponsorblock-remove sponsor does, and carries the flag through
subscriptions so the panel's control applies to both forms.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The image ships yt-dlp's bgutil PO token provider and starts it on
loopback, where the plugin dials it at http://127.0.0.1:4416. Since
482381d scoped the connect-time allowance to the configured proxy, the
download subprocess could no longer resolve it:
Refusing to connect to non-global address for host '127.0.0.1'
which surfaces as the plugin's "Error reaching GET .../ping". Metadata
extraction runs in the main process and installs no guard, so titles kept
resolving while the download itself ran without a token — and YouTube
increasingly answers those with 403.
The allowance already had the right shape for this; it was just named for
its only user. Endpoints the operator or the image configured are now
allowed as a class: install_socket_guard takes service_urls alongside
proxy_urls, and ytdl derives them from the bundled default plus any
base_url set through the youtubepot-bgutilhttp (or the deprecated youtube
getpot_bgutil_baseurl) extractor argument. The bundled server runs either
way, so it stays allowed when a base URL is configured.
Matching stays exact host:port on the configured string, so nothing else
on loopback opens up: a hostile media URL naming the endpoint reaches a
token server with two endpoints and nothing worth reading.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Subscriptions download unattended, which is where skipping sponsor reads
is most useful, so the flag now travels the same path the other download
options take: stored on SubscriptionInfo, persisted in the record, and
passed to add_entry for every entry a check queues.
Like the clip bounds, it is set when the subscription is created; the
update endpoint's field list is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A "Remove sponsor segments" switch next to "Split by chapters" queues
the download with the same postprocessor pair the CLI's
--sponsorblock-remove sponsor builds (SponsorBlock + ModifyChapters).
The flag persists as a cookie like the other form options, survives in
the queue records, and is carried into retries.
The pair is registered above the chapter-splitting block: yt-dlp runs
same-stage postprocessors in list order, so ModifyChapters has to
rewrite the chapter list before FFmpegSplitChapters cuts the file up,
matching what the CLI builds for --sponsorblock-remove sponsor
--split-chapters. With both toggles on the other way around the chapter
files keep the sponsor segments and the removal desyncs the remaining
chapter timings.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
__is_channel_extraction keyed on id == channel_id. That holds for a channel
tab - /channel/UC..., and the videos, streams, shorts and playlists tabs of a
/@handle URL - but not for a channel addressed on its own: yt-dlp reports the
id in the form the channel was asked for, so a bare handle URL yields '@handle'
and a legacy /c/ URL yields the vanity name.
Neither matched, so both fell through to OUTPUT_TEMPLATE_PLAYLIST and the
folder came out as the feed's title. That is why e2c7778 fixed the reporter's
tab URLs while a bare channel URL - what you get copying the address bar - went
on ignoring OUTPUT_TEMPLATE.
Both forms match uploader_id, which is the handle either way, so compare
against that as well, without case: a legacy vanity name and the handle it
became need not agree on it. A real playlist carries its owner's channel_id and
uploader_id but keeps an id of its own, so it still reads as a playlist; no
playlist id can collide with a handle, since those are 'PL...', 'OLAK...' and
the like.
Verified against the live extractor for all six channel URL forms and a real
playlist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MeTube never sets ignoreerrors, so yt-dlp's trouble() raises DownloadError
instead of returning nonzero for failures like "No video formats found!".
That is the path issue #1047 reports, and it dropped the warnings that
explain the failure because only the nonzero-return branch attached them.
The exception branch now reports the retained warnings with the exception
text as the final line, so the actual error stays prominent under the
context. yt-dlp labels errors "ERROR:" but hands warnings to the logger
unlabelled, so a last warning that repeats the error text is skipped.
Retention is bounded to the last five distinct warnings: fragmented and
live downloads emit a warning per fragment, and the joined message is
persisted with the completed queue and broadcast to every client.
Co-authored-by: Matt Van Horn <mvanhorn@users.noreply.github.com>
Sites that put a description in the title produce output names past what
the filesystem accepts, and the download fails outright with
"[Errno 36] File name too long". The name is now shortened to fit inside
prepare_filename, which every output path already passes through, so the
main file, its chapter files, thumbnails and subtitles stay consistent.
The limit is read from the filesystem (PC_NAME_MAX, falling back to 255)
and counted in bytes, not characters: a title of accented or CJK
characters reaches it in a third of the characters. The extension is
kept, a cut landing inside a multi-byte character does not leave a broken
sequence, and a reserve is held back for the suffixes yt-dlp appends
afterwards -- '.part', '.ytdl', '.f<format_id>', '-Frag<n>' -- since it
is those that pushed the reported name over the limit.
Names that already fit are untouched, so nothing that downloads today
changes name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Most downloads from a given install land in the same custom directory,
which today means picking it by hand every time. DEFAULT_FOLDER seeds the
folder field once the configuration arrives; the field stays editable, so
per-download folders still work, and a folder already typed this session
is not overwritten.
The value is trimmed of surrounding slashes, and dropped with a warning
when CUSTOM_DIRS is off, since the UI hides the field in that mode and
the download path check rejects a folder anyway.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A subscription already carries every other download option and applies it to
each video it queues. Clip bounds were the one exception: 4f83174 added them
for one-off downloads and carved subscriptions out, rejecting the fields in the
subscribe route and stripping them in the UI before the request was built. So
the fields sat visible in the shared advanced-options panel while quietly doing
nothing for a subscription.
Carry them like the rest: two fields on SubscriptionInfo, threaded through the
check flow to add_entry. Stored records that predate the fields take the
defaults, since _from_stored filters by field name.
One thing does not carry over. parse_download_options reads a YouTube t=
timestamp from the URL and turns it into a clip start, which is right when you
paste a link to a moment in a video you want. A subscription URL is a channel
or a playlist, so a timestamp left on it says nothing about the videos that
feed will yield, and honouring it would silently truncate every future
download. The subscribe route therefore takes clip bounds only when the caller
sent the fields explicitly; the t= param is still stripped from the stored URL.
Worth knowing when using this: the range is a fixed offset applied to every
video, so it suits feeds with a consistent shape - a standing intro, a fixed
sponsor read - and will cut in the wrong place on a feed without one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The folder was already persisted on the subscription and already applied to
every download it queued, but it was missing from the small tuple of fields the
update route accepts, so it could be set when the subscription was created and
never afterwards. That is the same gap the subscription name had in #1044.
Add it to the accepted fields and validate it on the way in, following the
validate_* helpers already in this module. The check is deliberately narrow —
it rejects absolute paths and any '..' component, values that could never be
valid — because the authoritative resolution stays where it already lives, in
DownloadQueue at download time, along with the CUSTOM_DIRS / CREATE_CUSTOM_DIRS
rules and the directory creation. Doing it this way reports a bad edit while
the user is looking at the field instead of failing every check from then on,
without a second copy of the path logic drifting out of step with the first.
subscriptions.py cannot import ytdl.py in any case: ytdl imports _entry_id from
it.
An empty folder stays valid and means the base download directory. A change
applies to future downloads only; files already downloaded are not moved.
This covers the API side of the request. The subscriptions table does not show
the folder at all today, so exposing it in the UI is a separate change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Scoping the connect-time allowance to the configured proxy kept the exception
tied to loopback, so a proxy anywhere else internal — the common case of a
socks5 or HTTP proxy on the LAN — was refused with "Refusing to connect to
non-global address". The only workaround was ALLOW_PRIVATE_ADDRESSES, which
switches the whole guard off, a far larger concession than the setup needs.
The allowance was never really about loopback: it is about the operator having
named this host:port as a proxy. Widen it to any address at a configured proxy
endpoint and nothing is given away, because the match is on the configured host
string rather than the resolved address — a hostile media URL that resolves to
the proxy's address under another name gets no allowance, and one that names the
proxy endpoint itself only reaches the proxy. Every other internal destination
stays refused.
Also log each configured proxy endpoint, so the next report of this shape can be
diagnosed from the log rather than from the guard's source.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The SSRF guard classified addresses with ipaddress.is_global, which looks only
at the outer address. An IPv6 form that carries an IPv4 address at a fixed
offset therefore passed a check the bare address would have failed: the NAT64
well-known prefix 64:ff9b::/96 sits in the 2000::/3 global unicast range, so
64:ff9b::a9fe:a9fe was accepted while 169.254.169.254 was refused. The
deprecated IPv4-compatible form ::/96 has the same property. Both the ingress
validator and the connect-time socket guard classified through the same helper,
so both were affected.
Judge every address a verdict has to account for: the outer address plus any
IPv4 it tunnels, allowed only when all of them are global. Unwrapping this way
can only tighten the verdict, which matters for 6to4 and Teredo — Python
already rejects 2002::/16 and 2001::/32 wholesale, and replacing the outer
address with its payload would have turned 2002:0808:0808:: from blocked into
allowed. Reaching an internal service this way additionally requires NAT64
routing on the host network, which the attacker does not control.
Reported by tonghuaroot in GHSA-5mq5-qr7m-f4wx.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The connect-time guard permitted every loopback address so that a locally
configured proxy (proxy: http://127.0.0.1:9050) stayed reachable. But the
connect guard is the only check that media URLs derived from remote metadata
ever face — validate_url sees just the submitted URL — so that blanket
allowance let a remote manifest steer the download subprocess at services
bound to the server's loopback interface, with the response written to the
download directory and served back by the UI.
Permit loopback only at the host:port of a proxy the operator configured,
taken from yt-dlp's proxy option and the *_proxy environment variables.
Nothing is lost: when a proxy is in use yt-dlp hands it the media URL instead
of resolving that URL locally, so the two cases never overlap. Every other
loopback destination now falls under the same is_global policy as the rest.
Reported by m3rl1nu5 (https://github.com/hai135) in GHSA-73g4-qhhq-c32c.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
yt-dlp emits the feed-level .info.json, description and thumbnail from
__process_playlist_result without consulting `download`, so they fell out of
MeTube's classification pass with nothing steering them: they landed in
DOWNLOAD_DIR under yt-dlp's own pl_* names, ignoring OUTPUT_TEMPLATE, the
download's folder and AUDIO_DOWNLOAD_DIR. #1040 reported the names; #660 asked
for control over them. Same accident from both sides.
The classification pass now writes nothing at all, and the files are produced
once the feed has been accepted and its type is known, reusing the template its
items use — OUTPUT_TEMPLATE_CHANNEL for a channel, OUTPUT_TEMPLATE_PLAYLIST for
a playlist, falling back to OUTPUT_TEMPLATE — evaluated against the feed dict.
With the defaults that puts them in the same folder as the videos, named after
the feed, which is the Jellyfin layout #660 asked for. No new environment
variable: knowing the feed type is what makes reusing the item template
possible, and doing this after extraction is what makes the type known.
The write re-runs yt-dlp over a copy of the feed with its entries removed,
which reaches the playlist-file writing without re-extracting anything and
without touching yt-dlp's private write helpers. It runs in an executor and
never fails the add.
Nothing new appears for anyone who hasn't enabled writeinfojson /
writethumbnail, an explicit allow_playlist_files=false still turns it off, and
a failed or cancelled add no longer leaves metadata behind.
Subscription scans keep allow_playlist_files=False: a scan is a timer-driven
poll, and items it finds are queued through the download queue, which does the
writing. Previously every check interval rewrote these files.
Test doubles for __extract_info now take *args/**kwargs.
A subscription's name is captured once at subscribe time from the feed's own
title, so playlists — particularly the UULF-prefixed channel-uploads playlists
— all come back named "Videos" and stay that way. Adds an inline editor on the
Name cell, mirroring the existing title-filter edit next to it.
The update route already whitelisted `name` and update_subscription already
applied it, so this is mostly the missing UI. The backend side is validation:
the old `str(changes["name"])` accepted any type, any length and any
whitespace, for a value that is persisted and broadcast to every connected
client. validate_subscription_name now requires a string, collapses interior
whitespace to keep the label single-line, and caps it at 200 characters.
The name is display-only — it is used for the subscription list and log lines,
never for download paths — so renaming cannot move where files land.
The endpoint accepted {ids: [x]} and then rejected anything but exactly one
id, so the schema advertised a batch it never supported. Retry is genuinely
singular: unlike the /delete, /start and /cancel batches, which act on local
state and can't meaningfully fail for one id and not another, each retry
re-extracts the URL and the caller removes that item's done record only once
it is confirmed re-queued. A real batch form would need per-id results in the
response for the caller to know which records to remove; that only becomes
worth designing alongside moving done-record deletion server-side.
/retry has not shipped yet, so there is no compatibility cost.
Two review fixes on top of the retry endpoint:
- __add_entry dropped retry_entry when extraction returned an unprocessed
url/url_transparent result and it recursed back into add(). Since
__extract_info runs with extract_flat=True, that path is live, and a retried
playlist item taking it fell back to OUTPUT_TEMPLATE and landed in the root
directory instead of its playlist folder. The playlist child loop keeps
passing retry_entry=None on purpose: those entries get fresh playlist context
stamped on them from the current extraction.
- retry() called dqueue.add() directly, so it bypassed the
parse_download_options gates that /add applies. Stored ytdl_options_overrides
were re-applied even after ALLOW_YTDL_OPTIONS_OVERRIDES was turned off, and
preset names removed from the configuration were still passed through. Both
are re-checked against the current configuration at retry time.
Adds __record_add_failure so a URL that fails before a real download starts
(unsupported/unextractable URL, SSRF-rejected, extraction error) appears in the
Completed list as a red-cross entry with retry and error-detail, instead of only
a transient toast and a server log line. Keyed by info.url like any errored
download. Includes _short_title_for_failed_url for a readable hostname title.
The frontend hunk removing the 'Click for details' hint from every error row was
dropped from this merge; that affordance (added in fd3aaea, #143) is kept.
Co-authored-by: streamer1122 <streamer1122@users.noreply.github.com>
Adds MusicMetadataPreProcessor (app/music_metadata.py), a pre_process
postprocessor that enriches the info dict using only extractor-owned fields:
track-number/total resolution, album-title fallback, and square-thumbnail
preference. Track numbers and album flow into files through yt-dlp's existing
FFmpegMetadata embedding, so no per-format tag-writing code and no new
dependency. The earlier mutagen writer and its download-failing
PostProcessingError path were dropped per review.
Co-authored-by: jahruz67 <jahruz67@users.noreply.github.com>
Wraps Download.close() in try/finally and nulls self.status_queue so the
per-download manager.Queue() proxy is released once the completed Download is
retained in the done list. Previously every finished download permanently
pinned one Manager-process connection, accumulating file descriptors until
the instance hit 'too many open files' and self-terminated (#485, #980).
Co-authored-by: Matt Van Horn <mvanhorn@users.noreply.github.com>
The SSRF guard rejects any address that isn't globally routable, which breaks
proxy/VPN setups that resolve hosts into private or special-use ranges. The
reported case is Fake-IP clients (sing-box, Clash, Mihomo) that map YouTube to
the RFC 2544 benchmarking range 198.18.0.0/15 to tunnel the traffic; MeTube
rejected it with "Refusing to fetch internal address" even though yt-dlp itself
handles it fine.
Add a boolean ALLOW_PRIVATE_ADDRESSES (default false) that, when set, skips both
layers: validate_url returns after scheme validation without the internal-host
checks, and the connect-time socket guard is not installed. Threaded to the
download subprocess via Download so the guard sees it too. Scheme validation
(http/https only) still applies. Documented in the README as a trust-your-network
opt-out that disables SSRF protection.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
validate_url only inspects the submitted URL string. yt-dlp then follows HTTP
redirects and resolves media URLs from remote metadata without re-checking, so
an allowed URL that 302s to http://169.254.169.254/ (cloud metadata) or an
RFC1918 host is still fetched — the guard's own docstring scoped this out.
Install a getaddrinfo guard in the download subprocess that re-validates every
resolved address at actual connect time, covering redirects and DNS rebinding
for any backend resolving through Python's socket module (urllib, requests).
Loopback is permitted so locally-configured proxies keep working; link-local,
RFC1918 and unique-local are blocked. Native resolvers (curl_cffi/libcurl via
--impersonate) bypass this and rely on network isolation as the backstop.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
validate_url() previously returned None (allow) on socket.gaierror, so a
host that failed to resolve at check time was passed straight to yt-dlp. A
host we cannot resolve is a host we cannot verify as non-internal, and it may
resolve differently when yt-dlp fetches it. Reject it instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The per-download chapter_template was validated only against literal ".."
in the template string, but yt-dlp expands %(section_title)s (and every
other field) from attacker-controlled metadata at download time. On POSIX
hosts yt-dlp does not neutralise a ".." path component, so a chapter titled
".." turns a guard-passing template like
"%(section_title)s/%(section_title)s/x.%(ext)s" into "../../x.mp4" and writes
outside DOWNLOAD_DIR. The same class of escape applies to any multi-segment
output template (default/playlist/channel) whose fields resolve to "..".
The template string can never see the "..": it only exists after expansion.
So move the check to the one point every output path flows through —
YoutubeDL.prepare_filename — via a _ConfinedYoutubeDL subclass that refuses
any resolved path outside the download/temp roots (fail closed). This covers
the main file, split-chapter files, thumbnails and subtitles in one place.
With the chokepoint authoritative, the scattered ingress string checks
(chapter_template, custom_name_prefix) and the weaker _output_dir_escapes
literal-prefix check are removed. Tests move from the ingress layer to the
chokepoint, exercising the real metadata-resolution vector.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds _AlbumArtistPostProcessor, a yt-dlp pre_process postprocessor that
fills album_artist from the '<artist> - Topic' channel/uploader signal,
falling back to the first credited artist, when album metadata exists
but no album artist is set.
Closes#1025.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
User-submitted URLs were passed straight to yt-dlp's generic extractor,
letting the server fetch internal endpoints (cloud metadata, loopback,
RFC1918 hosts). Add a url_guard.validate_url check at every URL ingress
(add, subscribe, and nested playlist recursion) that rejects non-http(s)
schemes and hosts resolving to non-global addresses, while leaving bare
video IDs and search prefixes untouched.
Sanitize path separators and .. segments in playlist/channel titles before they are baked into yt-dlp output templates, and refuse downloads whose resolved output directory escapes DOWNLOAD_DIR.
Addresses a full-project review. Backend correctness and availability:
- ytdl: cancel() only SIGKILLs the child's process group when the child
actually became its own group leader, so a race (or failed setpgrp)
can no longer kill the whole server; kill the group on cancel and on
shutdown to avoid orphaned ffmpeg children
- ytdl: dedicated ThreadPoolExecutor for download supervision so active
downloads can't starve extract_info / live probes on the default pool
- ytdl/main/subscriptions: route fire-and-forget tasks through a
bg_tasks helper that keeps a strong ref and logs failures
- subscriptions: run flat-playlist extraction in an executor and check
feeds with bounded concurrency so one slow feed can't block the loop;
set last_checked on failure so broken feeds aren't retried every 60s
- main: validate ids on /start & /delete and numeric env vars at startup;
return 400 (not 500) on bad subscriptions/update input; serve /history
from memory; move get_custom_dirs off the event loop; restrict t=
stripping to YouTube hosts; drop double percent-decode in state guard
- dl_formats/ytdl: enforce requested caption format via
FFmpegSubtitlesConvertor and strip VTT header metadata only in the
pre-cue region so real dialogue is preserved
- ytdl: throttle progress events, dedup adds against pending, clear
filename/size on error and reject out-of-dir trashcan deletes, pin
fork start-method on Linux only
Frontend:
- retry deletes the done record only after a successful re-add
- surface HTTP errors for delete/start and reset the deleting flag
- ignore late 'updated' events for rows no longer in the queue
- track table rows by map key; FileSizePipe uses base-1024
Also: HTTPS-aware Docker healthcheck, dead-code removal, and shared
helpers for path-containment and yt-dlp option merging. Adds/updates
unit tests throughout (250 backend tests passing).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Detect YouTube channel tabs that yt-dlp reports as playlists so channel downloads use OUTPUT_TEMPLATE_CHANNEL and its empty fallback instead of OUTPUT_TEMPLATE_PLAYLIST.
Alex Shnitman