Make --exec take the command as rest-of-line words
Replace --exec-arg with an ergonomic --exec that consumes the rest of the line as the click command. The caller's shell tokenizes the words into discrete arguments before the tool sees them, and the shell runs them as positional parameters (never a re-parsed string), so safety is identical to the argv form while the call sites read naturally: `--exec omarchy toggle something`. Crucially the tool never splits a string itself — a single quoted whole-command argument is rejected and points at the unquoted form, because whitespace- splitting a string hands argument boundaries to whoever controls its content (the injection we are avoiding). --exec must come last; migrate every caller.
This commit is contained in:
+36
-25
@@ -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 <program> [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
|
||||
|
||||
Reference in New Issue
Block a user