[Security] Keep screen-recording state out of world-writable /tmp (#8374)

* Keep screen-recording state out of world-writable /tmp * Compare the /tmp name across the run instead of requiring it absent Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fall back to the state directory when there is no runtime dir * Let the /tmp snapshot come back empty Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Resolve the region file the same way in the resizer Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Protect recording fallback state and document its path --------- Co-authored-by: Omabot <omabot@omarchy.org> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-15 17:53:37 +02:00
1 parent 49418942c8
commit b44fb74780
@@ -15,8 +15,9 @@
 # start.
 #
 # Env: OMARCHY_SCREENRECORD_DEBUG=true appends gpu-screen-recorder's stderr (and
-# the picker target it was launched with) to /tmp/omarchy-screenrecord.log so
-# users can attach a log when reporting capture failures.
+# the picker target it was launched with) to
+# $XDG_RUNTIME_DIR/omarchy-screenrecord.log so users can attach a log when
+# reporting capture failures.

 [[ -f ~/.config/user-dirs.dirs ]] && source ~/.config/user-dirs.dirs
 OUTPUT_DIR="${OMARCHY_SCREENRECORD_DIR:-${XDG_VIDEOS_DIR:-$HOME/Videos}}"
@@ -34,9 +35,18 @@ WEBCAM_SIZE="medium"
 RESOLUTION=""
 FULLSCREEN="false"
 STOP_RECORDING="false"
-RECORDING_FILE="/tmp/omarchy-screenrecord-filename"
-REGION_FILE="${XDG_RUNTIME_DIR:-/tmp}/omarchy-screenrecord-region"
-LOG_FILE=$([[ ${OMARCHY_SCREENRECORD_DEBUG:-false} == "true" ]] && echo "/tmp/omarchy-screenrecord.log" || echo "/dev/null")
+# Both of these live in the per-user runtime directory, which is 0700. What
+# RECORDING_FILE holds is read back on stop and used as a path -- ffmpeg writes
+# beside it, `mv` replaces it, and `rm -f` deletes its preview -- so a name in
+# world-writable /tmp is a name any other local account can create first and
+# then point wherever it likes. The debug log carries the recorder's stderr and
+# the picked geometry, which is nobody else's business either. Fall back to
+# the state directory when there is no session runtime dir -- :-/tmp would put
+# both files back at the fixed names this comment describes.
+RUNTIME_DIR="${XDG_RUNTIME_DIR:-${XDG_STATE_HOME:-$HOME/.local/state}/omarchy}"
+RECORDING_FILE="$RUNTIME_DIR/omarchy-screenrecord-filename"
+REGION_FILE="$RUNTIME_DIR/omarchy-screenrecord-region"
+LOG_FILE=$([[ ${OMARCHY_SCREENRECORD_DEBUG:-false} == "true" ]] && echo "$RUNTIME_DIR/omarchy-screenrecord.log" || echo "/dev/null")

 for arg in "$@"; do
  case "$arg" in
@@ -144,6 +154,11 @@ select_capture_target() {
 }

 start_screenrecording() {
+  mkdir -p "$RUNTIME_DIR" || return 1
+  # XDG_STATE_HOME may be outside a private home; protect existing fallbacks too.
+  if [[ -z ${XDG_RUNTIME_DIR:-} ]]; then
+    chmod 700 "$RUNTIME_DIR" || return 1
+  fi
  local capture_args=()
  local target

@@ -8,7 +8,10 @@
 set -euo pipefail

 readonly MARGIN=40
-readonly REGION_FILE="${XDG_RUNTIME_DIR:-/tmp}/omarchy-screenrecord-region"
+# Has to resolve to whatever omarchy-capture-screenrecording writes, fallback
+# included, or a recording without a session runtime dir anchors the camera to
+# the whole monitor instead of the region it recorded.
+readonly REGION_FILE="${XDG_RUNTIME_DIR:-${XDG_STATE_HOME:-$HOME/.local/state}/omarchy}/omarchy-screenrecord-region"

 usage() {
  echo "Usage: omarchy-capture-webcam-resize <smaller|larger|reset|small|medium|large>" >&2
@@ -33,7 +33,7 @@ Recordings land in the configured Videos directory (override with
 `omarchy capture webcam resize <smaller|larger|reset|small|medium|large>`.

 If recording fails to start, rerun with `OMARCHY_SCREENRECORD_DEBUG=true` to
-collect a log at `/tmp/omarchy-screenrecord.log` worth attaching to a bug
+collect a log at `$XDG_RUNTIME_DIR/omarchy-screenrecord.log` (or `${XDG_STATE_HOME:-$HOME/.local/state}/omarchy/omarchy-screenrecord.log` without a session runtime directory) worth attaching to a bug
 report.

 ## Text Capture (OCR)
@@ -38,7 +38,7 @@ drag-and-drop in the web form, so save the capture and hand the user the file
 path to attach (`gh` cannot upload media).

 For screen-recording failures specifically, rerun with
-`OMARCHY_SCREENRECORD_DEBUG=true` and attach `/tmp/omarchy-screenrecord.log`.
+`OMARCHY_SCREENRECORD_DEBUG=true` and attach `$XDG_RUNTIME_DIR/omarchy-screenrecord.log` (or `${XDG_STATE_HOME:-$HOME/.local/state}/omarchy/omarchy-screenrecord.log` without a session runtime directory).

 File the issue with `gh` when available:

@@ -299,3 +299,114 @@ grep -F 'move = { "(monitor_w-monitor_h*2/9-40)", "(monitor_h-monitor_h/4-40)" }
 grep -F 'move = { "(monitor_w-monitor_h*3/10-40)", "(monitor_h-monitor_h*27/80-40)" }' "$webcam_rules" >/dev/null || \
  fail "large webcam starts at its final corner position"
 pass "webcam size rules place the initial window in its final corner"
+
+# The stop path reads the recording state file back and uses its contents as a
+# path -- ffmpeg writes beside it, `mv` replaces it, `rm -f` deletes its
+# preview -- so it has to live in the per-user runtime directory rather than
+# under a name in world-writable /tmp that another account can create first.
+recording_dir="$tmp_dir/recordings"
+mkdir -p "$recording_dir"
+
+cat >"$stub_bin/pgrep" <<'SH'
+#!/bin/bash
+exit 1
+SH
+
+cat >"$stub_bin/omarchy-hyprland-monitor-focused" <<'SH'
+#!/bin/bash
+printf 'DP-1\n'
+SH
+
+cat >"$stub_bin/gpu-screen-recorder" <<'SH'
+#!/bin/bash
+for i in "$@"; do
+  [[ -n ${take_next:-} ]] && { : >"$i"; break; }
+  [[ $i == "-o" ]] && take_next=1
+done
+sleep 5
+SH
+
+cat >"$stub_bin/omarchy-shell" <<'SH'
+#!/bin/bash
+exit 0
+SH
+
+chmod +x "$stub_bin"/pgrep "$stub_bin"/omarchy-hyprland-monitor-focused \
+  "$stub_bin"/gpu-screen-recorder "$stub_bin"/omarchy-shell
+
+# Compare that name across the run rather than demanding it be absent: the
+# whole point of the finding is that anyone can own it already, and a leftover
+# from a pre-fix recording would red-light the fixed script.
+tmp_state="/tmp/omarchy-screenrecord-filename"
+tmp_state_before=$(stat -c '%y %s' "$tmp_state" 2>/dev/null || true)
+
+OMARCHY_SCREENRECORD_DIR="$recording_dir" \
+  "$ROOT/bin/omarchy-capture-screenrecording" --fullscreen >/dev/null 2>&1
+
+pkill -f "$stub_bin/gpu-screen-recorder" 2>/dev/null || true
+
+[[ $(stat -c '%y %s' "$tmp_state" 2>/dev/null) == "$tmp_state_before" ]] ||
+  fail "screen recording keeps no state under a fixed /tmp name"
+pass "screen recording keeps no state under a fixed /tmp name"
+
+[[ -s $XDG_RUNTIME_DIR/omarchy-screenrecord-filename ]] ||
+  fail "the recording state file lives in the per-user runtime directory" \
+    "$(ls -a "$XDG_RUNTIME_DIR")"
+pass "the recording state file lives in the per-user runtime directory"
+
+[[ $(<"$XDG_RUNTIME_DIR/omarchy-screenrecord-filename") == "$recording_dir"/* ]] ||
+  fail "the recording state file names the recording that was started" \
+    "$(<"$XDG_RUNTIME_DIR/omarchy-screenrecord-filename")"
+pass "the recording state file names the recording that was started"
+
+# The :-/tmp fallback would reopen the hole this PR closes. A recording
+# started without a session runtime dir has to land under the state directory.
+state_home="$tmp_dir/state-home"
+mkdir -p "$state_home/omarchy" "$tmp_dir/home-fallback"
+chmod 755 "$state_home/omarchy"
+tmp_state_before=$(stat -c '%y %s' "$tmp_state" 2>/dev/null || true)
+
+env -u XDG_RUNTIME_DIR \
+  HOME="$tmp_dir/home-fallback" \
+  XDG_STATE_HOME="$state_home" \
+  OMARCHY_SCREENRECORD_DIR="$recording_dir" \
+  "$ROOT/bin/omarchy-capture-screenrecording" --fullscreen >/dev/null 2>&1
+
+pkill -f "$stub_bin/gpu-screen-recorder" 2>/dev/null || true
+
+[[ $(stat -c '%y %s' "$tmp_state" 2>/dev/null) == "$tmp_state_before" ]] ||
+  fail "without a runtime dir, screen recording still keeps no state under a fixed /tmp name"
+pass "without a runtime dir, screen recording still keeps no state under a fixed /tmp name"
+
+fallback_file="$state_home/omarchy/omarchy-screenrecord-filename"
+[[ -s $fallback_file ]] ||
+  fail "without a runtime dir the recording state file lives in the state directory" \
+    "$(ls -la "$state_home/omarchy" 2>/dev/null || true)"
+pass "without a runtime dir the recording state file lives in the state directory"
+
+[[ $(<"$fallback_file") == "$recording_dir"/* ]] ||
+  fail "the fallback state file names the recording that was started" \
+    "$(<"$fallback_file")"
+pass "the fallback state file names the recording that was started"
+
+# The overlay resizer reads the region file the recorder writes, so the two
+# have to resolve the same fallback as well as the same runtime dir.
+: >"$OMARCHY_TEST_HYPRCTL_ARGS"
+echo "800x600+100+100" >"$state_home/omarchy/omarchy-screenrecord-region"
+env -u XDG_RUNTIME_DIR \
+  HOME="$tmp_dir/home-fallback" \
+  XDG_STATE_HOME="$state_home" \
+  "$ROOT/bin/omarchy-capture-webcam-resize" reset
+
+printf '%s\n' \
+  'dispatch hl.dsp.window.resize({ window = "address:0xabc", x = 133, y = 150 })' \
+  'dispatch hl.dsp.window.move({ window = "address:0xabc", x = 727, y = 510 })' >"$expected_hyprctl_args"
+
+if ! cmp -s "$OMARCHY_TEST_HYPRCTL_ARGS" "$expected_hyprctl_args"; then
+  fail "without a runtime dir the webcam anchors to the recorded region" "$(diff -u "$expected_hyprctl_args" "$OMARCHY_TEST_HYPRCTL_ARGS")"
+fi
+pass "without a runtime dir the webcam anchors to the recorded region"
+
+mode=$(stat -c '%a' "$state_home/omarchy" 2>/dev/null || stat -f '%Lp' "$state_home/omarchy")
+[[ $mode == "700" ]] || fail "fallback directory is private even when it already existed" "mode: $mode"
+pass "fallback directory is private even when it already existed"