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.
This commit is contained in:
@@ -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"
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user