Commit Graph
3 Commits
Author SHA1 Message Date
b71c60fe30 [Security] Stop a video title from becoming the Download Video play command (#7847)
* Stop a video title from becoming the Download Video play command

The host parsed yt-dlp's after_move line as title plus path, so a newline in page metadata could forge the path. Clicking the toast then handed that value to mpv as options. Print only the real file, ignore anything that is not inside the download dir, and invoke mpv with --.

* Refuse downloads whose video title contains control characters

The hoodie page still offered a real hidden clip, so yt-dlp saved it even after the play-action fix. A title with newlines is not a legitimate name; abort before the download and tell the user it was refused.

* Test the forged record in the order yt-dlp emits it

The records ran forged-first and good-last, so the assertion measured recovery after
bad records rather than preservation of an already-captured path when a forged record
arrives afterwards. That is the shape a hostile title actually produces, because a
title ending in a newline closes its own record and leaves the genuine path on a line
the loop ignores. As written the assertion passed with resolve_download_file replaced
by a no-op.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Resolve the download path without dropping a trailing newline

Command substitution strips trailing newlines, so an in-directory symlink pointing at
a regular file whose name ends in one canonicalised to a different path -- which may
itself exist -- and that path then passed the containment check and reached ffmpeg and
the click command. Reading realpath's NUL-terminated output keeps the name intact, and
a resolved path carrying a control character is refused outright.

Not reachable through a yt-dlp download, since --restrict-filenames strips control
characters from the name it writes; it is the helper's contract that was wrong.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Codex XHigh <noreply@openai.com>

* Accept a download directory that resolves to /

realpath returns "/" for the root directory, which made the containment pattern "//*"
and rejected every file saved directly under it, so the host reported a failed download
after saving the file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Codex XHigh <noreply@openai.com>

* Stop refusing a download because its title has control characters

The gate cannot tell a hostile title from a legitimate one. --print emits one line per
extracted video and --no-playlist does not collapse a multi_video result, so a page
holding two clips arrives as two titles separated by a newline and is refused exactly
like a forged record would be.

It also guaranteed nothing it was read as guaranteeing. The simulate run and the
download run are separate fetches, so a site is free to answer them differently, and
the check never constrained the metadata the download actually used.

What stands between a record and the click command is resolve_download_file, which is
untouched here. Leaving a check that refuses valid pages while securing nothing invites
the path validation to be relaxed later on the strength of it. A gate that would work
is possible -- --print '%(title)j' encodes each title as JSON on its own line, which
separates a newline in the metadata from a newline between videos -- but it belongs
with a use for the title rather than as a bare refusal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Codex XHigh <noreply@openai.com>

* Disarm the legacy exec-before-download hook too

--no-exec clears the modern --exec map but leaves --exec-before-download stored
separately, and yt-dlp restores it as a before_dl postprocessor, so a hook configured
in the user's yt-dlp config still ran during the download this host drives.

The accompanying test runs download_url itself against stubbed tools. Everything else
in this file exercises the helpers in isolation, which left the invocation uncovered:
restoring the title to the record template, or dropping --no-exec or the trailing --,
passed every assertion here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Codex XHigh <noreply@openai.com>

* Toast the page title again instead of the saved filename

Deriving the toast text from the sanitised filename cost the title it was meant to
show: "My Great Clip" arrived as "My_Great_Clip [My_Great_Clip]". The title is safe as
notification text -- it is an argv element, never part of a command -- so the only
question was getting it out of yt-dlp without reopening the record forgery.

It now comes from the download run, so it describes the file that was actually saved,
and it is printed as %(title)j. JSON-encoding is what makes that safe: a newline or tab
in page metadata becomes an escape sequence inside one quoted string rather than a
record boundary, so a title can no longer split itself across lines. The decoder keeps
only what precedes the first control character, refuses anything notify-send would read
as an option, and leaves the filename-derived title as the fallback when a page offers
nothing usable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Name the saved file after the page title

The download landed as "My_Great_Clip [My_Great_Clip].mp4" when the page called it
"My Great Clip". --restrict-filenames was carrying more weight than it earns here: it
folds spaces to underscores and strips non-ASCII, which is what mangles the name, and
it is not what keeps the record stream safe. yt-dlp removes control characters from a
filename either way -- a newline becomes a space, tabs and DEL and NUL are dropped --
so a path printed after the move is still only ever one line, which is the property
resolve_download_file depends on.

Dropping the [%(id)s] suffix is the other half of matching the title, and it trades
away the uniqueness that suffix bought: two videos sharing a title now share a name,
and yt-dlp skips a download whose file already exists, so the second one toasts as a
failure. Restoring the suffix is a one-line change if that trade is the wrong way
round.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Omabot <omabot@omarchy.org>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Codex XHigh <noreply@openai.com>
2026-08-23 16:18:38 +02:00
David Heinemeier HanssonandClaude Opus 5 19572a13c7 Skip compositor tests without recording a failure
Two problems made a missing Wayland compositor look like broken tests.

The cleanup traps ended on a bare conditional, so when a test skipped
before creating its TMPDIR the trap's last command returned 1 and, under
set -e, that overrode the explicit exit 0.

Six tests that launch quickshell had no compositor guard at all, so they
ran anyway and failed on the Qt platform plugin.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-09 04:29:26 -07:00
David Heinemeier HanssonandClaude Opus 4.8 e5290b0a12 Add yt-dlp "Download Video" Chromium extension
Download the current page's video with yt-dlp via Alt+Shift+D or a click
on the toolbar icon. A native-messaging host runs the download, shows live
progress on the Quickshell OSD, and posts a clickable "Download complete"
toast that opens the file in mpv.

- Extension: pinned key for a stable id, green download-video icon,
  keyboard command + toolbar action (reads the active tab URL).
- Native host (omarchy-chromium-ytdlp-host): verifies the URL with
  yt-dlp --simulate (else "No video found"), streams progress to the OSD
  (time-throttled to ~4/s), saves to ~/Videos, opens mpv on click.
- Installer (omarchy-install-chromium-ytdlp) writes the native-messaging
  manifest into installed Chromium/Chrome/Brave/Edge profiles; wired into
  browser install and chromium refresh, with a migration for existing users.
- omarchy-osd: add -d/--duration so the OSD can persist during a download.
- Add yt-dlp to base packages, load the extension via --load-extension,
  and document the Alt+Shift+D binding.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 11:15:28 +02:00