Harden stop path, autostart, and state file

Bug-check pass on top of the v2 rewrite. Five real issues fixed:

* autostart_enable used to return the exit code of the trailing `log`
  call, so a failing `systemctl --user enable` was silently reported
  as success. Now returns 1 with a tui_err on real failure.

* save_state wrote directly to STATE_FILE; a crash mid-write would
  leave a truncated file that load_state would partially parse.
  Switched to an atomic tmp + mv -f pattern.

* load_state used `source "$STATE_FILE"` which is arbitrary code
  execution if a video path ever contained shell metacharacters.
  Replaced with a read-based KEY=VALUE parser that only honours
  LAST_VIDEO / LAST_TARGET / LAST_DIR.

* stop_mpvpaper can be called twice in quick succession (TUI stop
  immediately followed by systemd's ExecStop). Wrapped the whole
  body in a `flock -n` on $STATE_DIR/.stop.lock so the second caller
  no-ops instead of racing against the first.

* Watcher `cleanup` trap used `[ -n VAR ] && kill`, which
  short-circuits to non-zero when VAR is unset and aborts the trap
  before `exit 0` under set -e. Restructured to a proper if/|| true.
This commit is contained in:
28allday 2026-04-23 20:52:41 +01:00
parent f31ff2b152
commit 2ce38ce9eb
2 changed files with 76 additions and 14 deletions

View file

@ -92,10 +92,19 @@ load_state() {
LAST_VIDEO="" LAST_VIDEO=""
LAST_TARGET="" LAST_TARGET=""
LAST_DIR="" LAST_DIR=""
if [ -f "$STATE_FILE" ]; then [ -f "$STATE_FILE" ] || return 0
# shellcheck source=/dev/null # Parse KEY="VALUE" lines directly instead of `source`, so a maliciously
source "$STATE_FILE" # crafted video path (e.g. containing `";rm -rf …`) can't execute.
fi local key val
while IFS='=' read -r key val; do
val="${val#\"}"
val="${val%\"}"
case "$key" in
LAST_VIDEO) LAST_VIDEO="$val" ;;
LAST_TARGET) LAST_TARGET="$val" ;;
LAST_DIR) LAST_DIR="$val" ;;
esac
done < "$STATE_FILE"
} }
save_state() { save_state() {
@ -103,11 +112,14 @@ save_state() {
# so the filesystem browser re-opens where the user last landed. # so the filesystem browser re-opens where the user last landed.
local dir="${3:-$(dirname "$1")}" local dir="${3:-$(dirname "$1")}"
umask 077 umask 077
cat > "$STATE_FILE" <<STATE # Atomic write: avoids leaving a truncated file if the process dies mid-cat.
local tmp="$STATE_FILE.tmp"
cat > "$tmp" <<STATE
LAST_VIDEO="$1" LAST_VIDEO="$1"
LAST_TARGET="$2" LAST_TARGET="$2"
LAST_DIR="$dir" LAST_DIR="$dir"
STATE STATE
mv -f "$tmp" "$STATE_FILE"
} }
is_running() { is_running() {
@ -129,8 +141,12 @@ autostart_enable() {
tui_err "motion-wallpaper.service is not installed. Re-run wallpaper.sh." tui_err "motion-wallpaper.service is not installed. Re-run wallpaper.sh."
return 1 return 1
fi fi
systemctl --user enable motion-wallpaper.service >/dev/null 2>&1 if ! systemctl --user enable motion-wallpaper.service >/dev/null 2>&1; then
tui_err "Failed to enable autostart (systemctl error)."
return 1
fi
log "autostart enabled" log "autostart enabled"
return 0
} }
autostart_disable() { autostart_disable() {
@ -281,6 +297,14 @@ start_watcher() {
restore_static_wallpaper() { restore_static_wallpaper() {
local omarchy_bg="$HOME/.config/omarchy/current/background" local omarchy_bg="$HOME/.config/omarchy/current/background"
if [ -e "$omarchy_bg" ]; then if [ -e "$omarchy_bg" ]; then
# Stop may be called twice in quick succession when the TUI stops the
# wallpaper and systemd's ExecStop fires right after. Skip if someone
# else already put swaybg back so we don't kill-and-respawn (causes a
# visible flicker).
if pgrep -x swaybg >/dev/null 2>&1 && ! pgrep -x mpvpaper >/dev/null 2>&1; then
log "swaybg already running, skipping restore"
return 0
fi
pkill -x hyprpaper 2>/dev/null || true pkill -x hyprpaper 2>/dev/null || true
pkill -x swaybg 2>/dev/null || true pkill -x swaybg 2>/dev/null || true
if command -v uwsm-app >/dev/null 2>&1; then if command -v uwsm-app >/dev/null 2>&1; then
@ -338,13 +362,18 @@ start_mpvpaper_bg() {
} }
stop_mpvpaper() { stop_mpvpaper() {
# Serialize stops so the TUI-initiated path and systemd's ExecStop (which
# fires when the TUI kills mpvpaper) can't both be mid-restore at once.
# `flock -n` → second caller skips cleanly if the first still holds it.
(
flock -n 9 || { log "stop already in progress, skipping"; exit 0; }
kill_watcher kill_watcher
pkill -x mpvpaper 2>/dev/null || true pkill -x mpvpaper 2>/dev/null || true
# mpv cleans up the IPC socket on exit, but if we killed mpvpaper hard # mpv cleans up its IPC socket on exit; a hard kill can leave it dangling.
# the dangling socket can linger and confuse the next run.
rm -f "$MPV_IPC_SOCK" 2>/dev/null || true rm -f "$MPV_IPC_SOCK" 2>/dev/null || true
restore_static_wallpaper restore_static_wallpaper
log "stopped" log "stopped"
) 9>"$STATE_DIR/.stop.lock"
} }
# ===== actions ================================================================ # ===== actions ================================================================
@ -370,6 +399,14 @@ action_toggle() {
stop_mpvpaper stop_mpvpaper
tui_ok "Stopped. Normal wallpaper restored." tui_ok "Stopped. Normal wallpaper restored."
notify "Motion wallpaper stopped." notify "Motion wallpaper stopped."
# If autostart is on, a plain stop will let the wallpaper return on
# next reboot. Offer to turn autostart off so "stop" means "stop".
if autostart_enabled; then
if gum confirm "Autostart is still enabled — also disable it so the wallpaper doesn't resume after reboot?"; then
autostart_disable
tui_ok "Autostart disabled."
fi
fi
;; ;;
"Change video") "Change video")
load_state load_state
@ -388,6 +425,15 @@ action_toggle() {
"Turn autostart OFF") "Turn autostart OFF")
autostart_disable autostart_disable
tui_ok "Autostart disabled." tui_ok "Autostart disabled."
# Motion wallpaper is still running at this point. Offer to also stop
# it now so the label "turn off" matches visible behaviour.
if is_running; then
if gum confirm "Motion wallpaper is still running — stop it now too?"; then
stop_mpvpaper
tui_ok "Stopped. Normal wallpaper restored."
notify "Motion wallpaper stopped."
fi
fi
;; ;;
Cancel) exit 0 ;; Cancel) exit 0 ;;
esac esac

View file

@ -47,9 +47,25 @@ resume_mpv() { send_mpv '{ "command": ["set_property", "pause", false] }'; }
log "watching $HYPR_SOCK" log "watching $HYPR_SOCK"
# Kill socat child on exit so it doesn't spam "Broken pipe" to the log when
# the watcher is killed by the main toggle script.
cleanup() {
# `[ ] && kill` short-circuits to non-zero when SOCAT_PID is unset, which
# under `set -e` would abort the trap before `exit 0` — wrap safely.
if [ -n "${SOCAT_PID:-}" ]; then
kill "$SOCAT_PID" 2>/dev/null || true
fi
exit 0
}
trap cleanup EXIT INT TERM
# Hyprland socket2 emits newline-separated "EVENT>>DATA" lines. We only care # Hyprland socket2 emits newline-separated "EVENT>>DATA" lines. We only care
# about the fullscreen state. `fullscreen>>1` = entered, `fullscreen>>0` = left. # about the fullscreen state. `fullscreen>>1` = entered, `fullscreen>>0` = left.
socat -u "UNIX-CONNECT:$HYPR_SOCK" - | while IFS= read -r line; do # socat stderr is dropped so EPIPE on shutdown doesn't bloat the log.
coproc SOCAT { socat -u "UNIX-CONNECT:$HYPR_SOCK" - 2>/dev/null; }
# Bash auto-exports SOCAT_PID from `coproc SOCAT`; used in cleanup trap.
while IFS= read -r line <&"${SOCAT[0]}"; do
case "$line" in case "$line" in
fullscreen\>\>1) fullscreen\>\>1)
log "fullscreen entered — pause" log "fullscreen entered — pause"