From 20a7012ba887df6c326848b5f8fb0f9a53535d3e Mon Sep 17 00:00:00 2001 From: Vijit Singh Date: Sat, 11 Jul 2026 20:58:33 -0500 Subject: [PATCH] test(rig-lock): portable timestamp, non-root holder default, scan hardening (#244) #244: two portability/permission bugs in the shared rig_lock breadcrumb (display-only; the flock itself was fine), mirrored from pithead: - RIG_LOCK_HOLDER defaulted to root-owned /run/rig-e2e.holder -> a non-root box errored. Default beside the lock: ${RIG_LOCK_FILE}.holder. - date -Iseconds is GNU-only -> date -u +%Y-%m-%dT%H:%M:%SZ. Plus pre-release scan hardening of the same helper: - Dropped the 'sudo chmod' fallback (a read-open (9<) only needs o+r, and sudo-chmod'ing a path in world-writable /run/lock could follow a planted symlink onto a root-owned target). - Refuse a symlinked lock/holder path outright. - Single-quote the EXIT trap (SC2064) so it re-derives the holder from the persistent env default at signal time. Both e2e helpers stay byte-identical (drift guard); 5 new tests. Closes #244 Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 8 ++++++++ tests/e2e-pithead.sh | 23 ++++++++++++++++++----- tests/e2e-real.sh | 23 ++++++++++++++++++----- tests/run.sh | 28 ++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d3eff6..e6ef89a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,14 @@ All notable changes to RigForge are documented here. The format is based on ### Fixed +- **rig-lock helper: portable timestamp + non-root holder default (#244).** The shared `rig_lock` + breadcrumb now defaults beside the lock (`.holder`) instead of root-owned + `/run/rig-e2e.holder` (a non-root box couldn't write there — noise, no breadcrumb), and stamps + `date -u +%Y-%m-%dT%H:%M:%SZ` instead of the GNU-only `date -Iseconds` (BSD/macOS rejects `-I`). + Mirrors the sibling pithead fix; both e2e helper copies stay identical. Also from a pre-release + scan: dropped the `sudo chmod` fallback and added a symlink-path guard on the lock/holder (a + planted symlink in world-writable `/run/lock` could otherwise redirect a root-side write). + Test-infra only. - **rig-lock survives a non-root reserve (#242).** The shared `rig_lock` helper now opens `/var/lock/rig-e2e.lock` read-only (`9<`) instead of write (`9>`). A lock file first created by a non-root `flock` (a manual reserve after a reboot clears the `/run/lock` tmpfs) is owned by that diff --git a/tests/e2e-pithead.sh b/tests/e2e-pithead.sh index 696e4f8..620eeb7 100644 --- a/tests/e2e-pithead.sh +++ b/tests/e2e-pithead.sh @@ -50,25 +50,38 @@ rig_lock() { # rig_lock [shared] local mode=-x [ "${3:-}" = shared ] && mode=-s local lf="${RIG_LOCK_FILE:-/var/lock/rig-e2e.lock}" + # Holder breadcrumb defaults BESIDE the lock, not under root-owned /run (a non-root box can't + # write /run/rig-e2e.holder — the lock still holds, but the write errors with stderr noise). (#244) + local hf="${RIG_LOCK_HOLDER:-$lf.holder}" + # /run/lock is world-writable + sticky; refuse a symlinked lock/holder path so a planted symlink + # can't redirect our root-side create/chmod/holder-write onto another file (defence for a + # multi-tenant box; single-tenant rigs aren't exposed, but the guard is free). + { [ -L "$lf" ] || [ -L "$hf" ]; } && { + echo "rig_lock: lock/holder path is a symlink — refusing" >&2 + exit 1 + } # Open the lock READ-only (9<). A lock file first created by a NON-root flock (a manual reserve # after a reboot clears the /run/lock tmpfs) is owned by that user, and fs.protected_regular then # blocks even root's O_CREAT-*write* of it (a 9> open) with EACCES. A read-open is never guarded, # and flock -x/-s works fine on a read fd, so this sidesteps it without rm-ing a possibly-held # lock. Create it first if absent; keep it 0666 so a shared reader can still join. (#242) [ -e "$lf" ] || : >"$lf" 2>/dev/null || true - chmod 666 "$lf" 2>/dev/null || sudo chmod 666 "$lf" 2>/dev/null || true + chmod 666 "$lf" 2>/dev/null || true # best-effort world-writable; a read-open (9<) only needs o+r exec 9<"$lf" if ! flock -n $mode 9; then if [ "${RIG_LOCK_WAIT:-0}" = 1 ]; then - echo "rig busy ($(cat "${RIG_LOCK_HOLDER:-/run/rig-e2e.holder}" 2>/dev/null || echo unknown)) — waiting..." >&2 + echo "rig busy ($(cat "$hf" 2>/dev/null || echo unknown)) — waiting..." >&2 flock $mode 9 else - echo "miner-0 busy: $(cat "${RIG_LOCK_HOLDER:-/run/rig-e2e.holder}" 2>/dev/null || echo unknown). Retry with RIG_LOCK_WAIT=1 to queue." >&2 + echo "miner-0 busy: $(cat "$hf" 2>/dev/null || echo unknown). Retry with RIG_LOCK_WAIT=1 to queue." >&2 exit 75 # EX_TEMPFAIL — callers can tell "busy, retry later" from a real failure fi fi - printf '%s %s pid=%s started=%s\n' "$1" "$2" "$$" "$(date -Iseconds)" >"${RIG_LOCK_HOLDER:-/run/rig-e2e.holder}" - trap 'rm -f "${RIG_LOCK_HOLDER:-/run/rig-e2e.holder}"' EXIT + # date -u +...Z, not the GNU-only `date -Iseconds` (BSD/macOS rejects -I). (#244) + printf '%s %s pid=%s started=%s\n' "$1" "$2" "$$" "$(date -u +%Y-%m-%dT%H:%M:%SZ)" >"$hf" + # Single-quoted so it expands at EXIT, not now ($hf is a local, out of scope by then); the + # env-var default re-derives the same path $hf resolved to above. + trap 'rm -f "${RIG_LOCK_HOLDER:-${RIG_LOCK_FILE:-/var/lock/rig-e2e.lock}.holder}"' EXIT } require_preflight() { diff --git a/tests/e2e-real.sh b/tests/e2e-real.sh index db0619c..7b1d5f3 100755 --- a/tests/e2e-real.sh +++ b/tests/e2e-real.sh @@ -70,25 +70,38 @@ rig_lock() { # rig_lock [shared] local mode=-x [ "${3:-}" = shared ] && mode=-s local lf="${RIG_LOCK_FILE:-/var/lock/rig-e2e.lock}" + # Holder breadcrumb defaults BESIDE the lock, not under root-owned /run (a non-root box can't + # write /run/rig-e2e.holder — the lock still holds, but the write errors with stderr noise). (#244) + local hf="${RIG_LOCK_HOLDER:-$lf.holder}" + # /run/lock is world-writable + sticky; refuse a symlinked lock/holder path so a planted symlink + # can't redirect our root-side create/chmod/holder-write onto another file (defence for a + # multi-tenant box; single-tenant rigs aren't exposed, but the guard is free). + { [ -L "$lf" ] || [ -L "$hf" ]; } && { + echo "rig_lock: lock/holder path is a symlink — refusing" >&2 + exit 1 + } # Open the lock READ-only (9<). A lock file first created by a NON-root flock (a manual reserve # after a reboot clears the /run/lock tmpfs) is owned by that user, and fs.protected_regular then # blocks even root's O_CREAT-*write* of it (a 9> open) with EACCES. A read-open is never guarded, # and flock -x/-s works fine on a read fd, so this sidesteps it without rm-ing a possibly-held # lock. Create it first if absent; keep it 0666 so a shared reader can still join. (#242) [ -e "$lf" ] || : >"$lf" 2>/dev/null || true - chmod 666 "$lf" 2>/dev/null || sudo chmod 666 "$lf" 2>/dev/null || true + chmod 666 "$lf" 2>/dev/null || true # best-effort world-writable; a read-open (9<) only needs o+r exec 9<"$lf" if ! flock -n $mode 9; then if [ "${RIG_LOCK_WAIT:-0}" = 1 ]; then - echo "rig busy ($(cat "${RIG_LOCK_HOLDER:-/run/rig-e2e.holder}" 2>/dev/null || echo unknown)) — waiting..." >&2 + echo "rig busy ($(cat "$hf" 2>/dev/null || echo unknown)) — waiting..." >&2 flock $mode 9 else - echo "miner-0 busy: $(cat "${RIG_LOCK_HOLDER:-/run/rig-e2e.holder}" 2>/dev/null || echo unknown). Retry with RIG_LOCK_WAIT=1 to queue." >&2 + echo "miner-0 busy: $(cat "$hf" 2>/dev/null || echo unknown). Retry with RIG_LOCK_WAIT=1 to queue." >&2 exit 75 # EX_TEMPFAIL — callers can tell "busy, retry later" from a real failure fi fi - printf '%s %s pid=%s started=%s\n' "$1" "$2" "$$" "$(date -Iseconds)" >"${RIG_LOCK_HOLDER:-/run/rig-e2e.holder}" - trap 'rm -f "${RIG_LOCK_HOLDER:-/run/rig-e2e.holder}"' EXIT + # date -u +...Z, not the GNU-only `date -Iseconds` (BSD/macOS rejects -I). (#244) + printf '%s %s pid=%s started=%s\n' "$1" "$2" "$$" "$(date -u +%Y-%m-%dT%H:%M:%SZ)" >"$hf" + # Single-quoted so it expands at EXIT, not now ($hf is a local, out of scope by then); the + # env-var default re-derives the same path $hf resolved to above. + trap 'rm -f "${RIG_LOCK_HOLDER:-${RIG_LOCK_FILE:-/var/lock/rig-e2e.lock}.holder}"' EXIT } # Drive `rigforge enable|disable` to a settled systemd boot-state. The gate hammers the service diff --git a/tests/run.sh b/tests/run.sh index c37b5d6..7f05b10 100644 --- a/tests/run.sh +++ b/tests/run.sh @@ -5475,6 +5475,34 @@ else assert_eq "normalizes + acquires a restrictive-mode lock file (#242)" "$([ -f "$RLD/ro.ok" ] && echo y || echo n)" "y" chmod 666 "$RLD/ro" 2>/dev/null || true + # #244: with RIG_LOCK_HOLDER unset, the breadcrumb defaults BESIDE the lock (not root-owned + # /run) and its timestamp is the portable Zulu form, not GNU-only `date -Iseconds` (+00:00). + RIG_LOCK_FILE="$RLD/def.lock" + (unset RIG_LOCK_HOLDER && eval "$RL_SRC" && rig_lock rigforge deftest "" && touch "$RLD/def.up" && while [ ! -f "$RLD/def.rel" ]; do sleep 0.1; done) & + DEF_PID=$! + _i=0 + while [ ! -f "$RLD/def.up" ] && [ "$_i" -lt 50 ]; do + sleep 0.1 + _i=$((_i + 1)) + done + assert_eq "holder defaults beside the lock, not /run (#244)" "$([ -f "$RLD/def.lock.holder" ] && echo y || echo n)" "y" + def_holder="$(cat "$RLD/def.lock.holder" 2>/dev/null)" + assert_contains "holder uses a portable UTC timestamp (#244)" "$def_holder" "started=$(date -u +%Y-%m-%dT)" + assert_absent "holder timestamp isn't the GNU -Iseconds offset form (#244)" "$def_holder" "+00:00" + touch "$RLD/def.rel" + wait "$DEF_PID" 2>/dev/null + assert_eq "holder cleaned up on exit (#244)" "$([ -f "$RLD/def.lock.holder" ] && echo present || echo gone)" "gone" + + # scan hardening: refuse a symlinked lock path (a planted symlink in world-writable /run/lock + # could otherwise redirect the root-side create/chmod/holder-write onto another file). + ln -s "$RLD/some-target" "$RLD/sym.lock" + ( + RIG_LOCK_FILE="$RLD/sym.lock" + eval "$RL_SRC" && rig_lock rigforge symtest "" && touch "$RLD/sym.acquired" + ) 2>/dev/null + assert_eq "refuses a symlinked lock path (scan hardening)" "$([ -f "$RLD/sym.acquired" ] && echo acquired || echo refused)" "refused" + rm -f "$RLD/sym.lock" + unset RIG_LOCK_FILE RIG_LOCK_HOLDER fi