From eb988b42e678772d26113a5b9e5d08279b74b179 Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 23 Aug 2026 13:35:02 -0400 Subject: [PATCH] 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"