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"