aa87ed97c1c2bca46a0b1056549be75cf8062c61
7
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Ryan Hughes
|
bf2013e6f3 |
Make --exec take the command as rest-of-line words
Replace --exec-arg with an ergonomic --exec that consumes the rest of the line as the click command. The caller's shell tokenizes the words into discrete arguments before the tool sees them, and the shell runs them as positional parameters (never a re-parsed string), so safety is identical to the argv form while the call sites read naturally: `--exec omarchy toggle something`. Crucially the tool never splits a string itself — a single quoted whole-command argument is rejected and points at the unquoted form, because whitespace- splitting a string hands argument boundaries to whoever controls its content (the injection we are avoiding). --exec must come last; migrate every caller. |
||
|
Ryan Hughes
|
eb988b42e6 |
Remove --exec entirely; --exec-arg is the only click-command form
A free-form shell-string --exec sitting next to the safe --exec-arg is a standing invitation for the next caller to interpolate untrusted data and reintroduce the RCE. Remove it: omarchy-notification-send --exec now errors and points at --exec-arg, and the shell drops the omarchy-exec string hint and its bash -lc execution path, leaving only the argv path. Migrate the remaining string callers (the first-run invitation hooks, wifi and welcome prompts) to --exec-arg, and update their notification mocks. Trim the verbose security comments added along the way. |
||
|
Ryan Hughes
|
07443f3970 |
Run notification click actions as argv, not shell strings
The click action of a notification was a free-form shell string run through `bash -lc`, safe only when every sender shell-quoted every interpolated value perfectly. One slip is RCE: a hostile yt-dlp video title forged an output record and injected an mpv option into the click command (mehmetince.net RCE, partially addressed by #7847). Add a parameterized transport: omarchy-notification-send gains --exec-arg (repeatable), encoding a JSON argv into the omarchy-exec-argv hint. The shell runs it with Quickshell.execDetached(argv) and no shell, so data an attacker controls is only ever one argument and can never be reparsed as a command. The shell fails closed on a malformed argv hint. The legacy free-form --exec string is retained but honored only from Omarchy's own omarchy-action toasts, and deprecated. Migrate all in-repo callers (screenshot, screen recording, taildrop receive, migrate-notify, crash-watch, yt-dlp host) to --exec-arg. Update docs and tests. |
||
|
|
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> |
||
|
|
9455496990 |
Fix style inconsistencies in bin/ (#7518)
* Use (( )) for the numeric argument test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Drop the quotes on a variable inside [[ ]] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Use omarchy-pkg-drop instead of raw pacman -Rns omarchy-pkg-drop already filters to installed packages, so the 2>/dev/null || true suppression is no longer needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Drop defensive checks around default-set commands ttfx, imagemagick, and networkmanager are all in the default package set, so their commands are runtime invariants and should be invoked directly. Removing the nmcli guard also removes the degraded wifi fallthrough that only ran when nmcli was missing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
5a58f79876 |
Keep clicking a notification working after a shell restart (#6636)
* Keep clicking a notification working after a shell restart Notification actions lived only in the sending process: `-a` appended `-A default=default`, so notify-send blocked on a D-Bus ActionInvoked signal and the caller ran the command when it arrived. Nothing about that reached disk, so a restored popup had no action to run and its sender stayed blocked forever. Replace `-a` with `--exec <command>`, carried as an `omarchy-exec` hint into the snapshot's `exec` role. It travels through the popup files and history, and the shell runs it on click, so restored toasts behave exactly like live ones and the sender exits immediately. That drops the scaffolding whose only job was keeping a blocked sender alive: the first-run invitations lose their `--show` re-entry and two transient units each, omarchy-migrate-notify loses its transient service, and the screenshot, recording, download, and taildrop toasts lose their wrapper subshells. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Keep a failed toast from failing the work it announces Moving these sends out of their backgrounded subshells put a fallible command on the foreground path, where the `&` used to swallow its exit status. A notification outage — including the shell restart this branch targets — now propagates: - taildrop's receiver dies under `set -e` mid-delivery - omarchy-capture-screenshot reports failure for a screenshot it already saved - a completed download exits before scheduling its thumbnail cleanup, leaking the mktemp file Announcing is best-effort in all three: the work is already done by the time the toast goes out. Also drop the first-run sleep that spaced out the welcome and Wi-Fi toasts. It compensated for the background notify-send processes this branch removes; each send now returns only once the server has taken the toast, so sending in order is enough to stack them newest-on-top. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Stop tying the preview cleanup to the toast's expiry The shell loads a notification thumbnail into memory when the toast appears and never re-reads the file, so the preview only has to outlive that load. Deriving the cleanup delay from the expiry was false precision, and it turned -t into a variable for no reason: -t is already the helper's expiry setting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
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> |