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.
This commit is contained in:
@@ -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 -- <path>" "$(cat "$notify_argv")"
|
||||
pass "yt-dlp native host builds the click command as mpv -- <path>"
|
||||
grep -q -- "--exec-arg mpv --exec-arg -- --exec-arg " "$notify_argv" ||
|
||||
fail "yt-dlp native host builds the click command as an mpv -- <path> argv" "$(cat "$notify_argv")"
|
||||
pass "yt-dlp native host builds the click command as an mpv -- <path> 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")"
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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),
|
||||
{
|
||||
|
||||
@@ -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" &&
|
||||
|
||||
Reference in New Issue
Block a user