From fe56d68e905c8dc63a4c6a43727fef40833360d7 Mon Sep 17 00:00:00 2001 From: bastidotnet <233381911+bastidotnet@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:24:00 +0200 Subject: [PATCH] Validate the cached Apple-display device path before use The cached device path was trusted for merely existing, not for being a hiddev node, and fell back to a predictable /tmp path when XDG_RUNTIME_DIR was unset. Validate the cache shape (hiddev char device) and cache only under the user-private runtime dir; asdcontrol already gates non-Apple devices downstream, so this is defense-in-depth in the layer Omarchy owns. --- bin/omarchy-brightness-display-apple | 21 ++- .../brightness-display-apple-cache-test.sh | 120 ++++++++++++++++++ 2 files changed, 137 insertions(+), 4 deletions(-) create mode 100755 test/shell.d/brightness-display-apple-cache-test.sh diff --git a/bin/omarchy-brightness-display-apple b/bin/omarchy-brightness-display-apple index 81202b87..1027686d 100755 --- a/bin/omarchy-brightness-display-apple +++ b/bin/omarchy-brightness-display-apple @@ -4,7 +4,13 @@ # omarchy:args=[--no-osd] [+N%|N%-|N%] # omarchy:examples=omarchy brightness display apple | omarchy brightness display apple +5% | omarchy brightness display apple --no-osd 50% -device_cache="${XDG_RUNTIME_DIR:-/tmp}/omarchy-brightness-display-apple.device" +# Only cache under the user-private runtime dir. With no XDG_RUNTIME_DIR we skip +# caching (detect every run) rather than fall back to a predictable, world-writable +# /tmp path another user could pre-create. +device_cache="" +if [[ -n "${XDG_RUNTIME_DIR:-}" ]]; then + device_cache="$XDG_RUNTIME_DIR/omarchy-brightness-display-apple.device" +fi no_osd=0 if [[ ${1:-} == "--no-osd" ]]; then no_osd=1 @@ -28,9 +34,14 @@ find_apple_display_device() { local cached="" local device="" - if [[ -r $device_cache ]]; then + if [[ -n "$device_cache" && -r $device_cache ]]; then read -r cached <"$device_cache" || true - if [[ -n $cached && -e $cached ]]; then + # Trust a cached value only if it still names a hiddev character device. A + # stale or unexpected cache (a regular file, a non-hiddev node) is ignored and + # we re-detect instead of handing an arbitrary path to asdcontrol. The globs + # are left unquoted on purpose: [[ ]] pattern-matches an unquoted right side, + # and quoting them would turn the match into a literal string comparison. + if [[ ( $cached == /dev/hiddev* || $cached == /dev/usb/hiddev* ) && -c $cached ]]; then printf '%s\n' "$cached" return 0 fi @@ -39,7 +50,9 @@ find_apple_display_device() { device="$(detect_apple_display_device)" || return 1 [[ -n $device ]] || return 1 - printf '%s\n' "$device" >"$device_cache" + if [[ -n "$device_cache" ]]; then + printf '%s\n' "$device" >"$device_cache" + fi printf '%s\n' "$device" } diff --git a/test/shell.d/brightness-display-apple-cache-test.sh b/test/shell.d/brightness-display-apple-cache-test.sh new file mode 100755 index 00000000..2808f93f --- /dev/null +++ b/test/shell.d/brightness-display-apple-cache-test.sh @@ -0,0 +1,120 @@ +#!/bin/bash + +set -euo pipefail + +source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh" + +TMPDIR=$(mktemp -d) +trap 'rm -rf "$TMPDIR"' EXIT + +# Stubs on PATH: drop sudo so asdcontrol runs directly, record every asdcontrol +# invocation, make detection deterministic by having --detect report no device, +# and no-op the OSD. On a host without any /dev/*hiddev* node the wrapper's +# detect_apple_display_device returns before it ever runs asdcontrol, so the +# reject cases assert on the negative: a refused cache value is never handed to +# `asdcontrol -- `. Blind-trust validation would hand it over and be +# caught here. +stub_dir="$TMPDIR/stubs" +mkdir -p "$stub_dir" + +asd_log="$TMPDIR/asdcontrol.log" + +cat >"$stub_dir/sudo" <<'STUB' +#!/bin/bash +exec "$@" +STUB +chmod +x "$stub_dir/sudo" + +cat >"$stub_dir/asdcontrol" <>"$asd_log" +# --detect reports nothing, so detection never yields a device. +if [[ \$1 == "--detect" ]]; then + exit 0 +fi +# A brightness read (a lone device arg) returns a plausible value; a set +# ( -- ) just succeeds. +if [[ \$# -eq 1 ]]; then + printf '%s: BRIGHTNESS=30000\n' "\$1" +fi +exit 0 +STUB +chmod +x "$stub_dir/asdcontrol" + +cat >"$stub_dir/omarchy-osd" <<'STUB' +#!/bin/bash +exit 0 +STUB +chmod +x "$stub_dir/omarchy-osd" + +run_wrapper() { + # $1: value for XDG_RUNTIME_DIR ("" means unset); remaining args go to the wrapper. + local xdg="$1" + shift + : >"$asd_log" + if [[ -n $xdg ]]; then + XDG_RUNTIME_DIR="$xdg" PATH="$stub_dir:$ROOT/bin:$PATH" \ + omarchy-brightness-display-apple "$@" 2>&1 || true + else + env -u XDG_RUNTIME_DIR PATH="$stub_dir:$ROOT/bin:$PATH" \ + omarchy-brightness-display-apple "$@" 2>&1 || true + fi +} + +# --- A cache value that is not a hiddev character device is rejected ---------- +xdg_dir="$TMPDIR/xdg" +mkdir -p "$xdg_dir" +cache_file="$xdg_dir/omarchy-brightness-display-apple.device" + +regular_file="$TMPDIR/not-a-device" +: >"$regular_file" + +for poison in "/dev/null" "$regular_file" "/tmp/omarchy-evil"; do + printf '%s\n' "$poison" >"$cache_file" + output=$(run_wrapper "$xdg_dir" "+5%") + if grep -qF -- "$poison -- +5%" "$asd_log"; then + fail "wrapper handed a non-hiddev cache value to asdcontrol: $poison" "$output" + fi +done +pass "wrapper rejects a cached path that is not a hiddev character device" + +# NOTE: the complementary arm (a cache value that DOES match /dev/hiddev* but is +# not a character device) cannot be built without root -- only real device nodes +# live under /dev. It is covered by the -c test and exercised below only when a +# real hiddev node happens to be present. + +# --- A legitimate cached hiddev node is trusted (only where HW is present) ---- +real_hiddev="" +for candidate in /dev/usb/hiddev* /dev/hiddev*; do + if [[ -c $candidate ]]; then + real_hiddev="$candidate" + break + fi +done +if [[ -n $real_hiddev ]]; then + printf '%s\n' "$real_hiddev" >"$cache_file" + run_wrapper "$xdg_dir" "+5%" >/dev/null + grep -qF -- "$real_hiddev -- +5%" "$asd_log" || + fail "wrapper did not trust a valid cached hiddev node: $real_hiddev" + pass "wrapper trusts a cached hiddev character device without re-detecting" +else + pass "no /dev/hiddev* character device present; skipping the valid-cache case" +fi + +# --- With no XDG_RUNTIME_DIR, the predictable /tmp cache is not consulted ------ +# Guard on the real path not pre-existing so we never clobber a live cache, and +# remove what we create. Old code read /tmp and would hand /dev/null to +# asdcontrol; new code has no cache path at all when XDG_RUNTIME_DIR is unset. +tmp_cache="/tmp/omarchy-brightness-display-apple.device" +if [[ -e $tmp_cache ]]; then + pass "$tmp_cache already exists on this host; skipping the /tmp-fallback case" +else + printf '%s\n' "/dev/null" >"$tmp_cache" + output=$(run_wrapper "" "+5%") + used=1 + grep -qF -- "/dev/null -- +5%" "$asd_log" || used=0 + rm -f "$tmp_cache" + (( used == 0 )) || + fail "wrapper consulted the world-writable /tmp cache with no XDG_RUNTIME_DIR" "$output" + pass "wrapper ignores the /tmp cache path when XDG_RUNTIME_DIR is unset" +fi