diff --git a/bin/omarchy-capture-screenrecording b/bin/omarchy-capture-screenrecording index d57b26fe..0c4c5494 100755 --- a/bin/omarchy-capture-screenrecording +++ b/bin/omarchy-capture-screenrecording @@ -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 diff --git a/bin/omarchy-capture-webcam-resize b/bin/omarchy-capture-webcam-resize index cde61e08..3c9d1215 100755 --- a/bin/omarchy-capture-webcam-resize +++ b/bin/omarchy-capture-webcam-resize @@ -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 " >&2 diff --git a/default/agents/skills/omarchy/capture.md b/default/agents/skills/omarchy/capture.md index 41a52b6c..621bc298 100644 --- a/default/agents/skills/omarchy/capture.md +++ b/default/agents/skills/omarchy/capture.md @@ -33,7 +33,7 @@ Recordings land in the configured Videos directory (override with `omarchy capture webcam resize `. 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) diff --git a/default/agents/skills/omarchy/contributing.md b/default/agents/skills/omarchy/contributing.md index b5fd0533..c76c12d6 100644 --- a/default/agents/skills/omarchy/contributing.md +++ b/default/agents/skills/omarchy/contributing.md @@ -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: diff --git a/test/shell.d/screenrecording-test.sh b/test/shell.d/screenrecording-test.sh index 7859ec14..438ccc57 100644 --- a/test/shell.d/screenrecording-test.sh +++ b/test/shell.d/screenrecording-test.sh @@ -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"