From 66da05aa927fff6f9eee48b27955220821aa1df2 Mon Sep 17 00:00:00 2001 From: Vijit Singh Date: Sat, 15 Aug 2026 16:11:52 -0500 Subject: [PATCH 1/4] =?UTF-8?q?refactor(#353):=20polish=20sweep=20?= =?UTF-8?q?=E2=80=94=20verb=20ERR=20context,=20apply-plan=20drift,=20stagi?= =?UTF-8?q?ng=20traps,=20appliance=20disable=20--runtime,=20comment=20spli?= =?UTF-8?q?ces,=20reader=20dedup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six small debts from the 2026-08-14 audit, bundled per the issue: 1. CURRENT_STEP was only ever set inside main() (setup), so an unexpected failure in any other verb reported the stale "aborted while starting up" default. Set it once at dispatch time from the verb name itself — setup's own fine-grained per-phase steps still overwrite it a moment later. 2. _apply_plan's step 3 named 3 reconciles (autotune/API/firewall) while apply() actually runs 6 install_* calls, silently omitting watchdog and control from the printed plan. Named all of them; added the same name-vs-plan drift guard _setup_plan already has for main(). 3. backup/restore/support_bundle's mktemp -d staging (config.json, tokens) leaked on a set -e abort — the explicit `rm -rf` cleanup lines were never reached. Gave all three the EXIT-trap treatment tune() got in #135. (The trap has to reference a script-global, not a `local`: a `local` staging var is out of scope by the time the trap fires on a normal return, and dies "unbound variable" under set -u — caught by the existing backup/restore/support-bundle black-box tests.) 4. Verified empirically against a real systemd (255, enable/disable round-trip in a container) that a plain `systemctl disable` only ever removes the /etc-side wants-symlink: a --runtime-enabled unit's /run symlink survives, and `is-enabled` still reports "enabled-runtime". Every disable call now passes ${ENABLE_RUNTIME:+...}, mirroring its enable counterpart. 5. Two comment splices: xmrig_already_built()'s docstring was sitting above the unrelated _sha256() instead of itself; a section-header banner (`--- Auto-tuning: search strategies & seeding ---`) had landed mid- sentence inside _hillclimb()'s docstring. 6. _read_api_summary and _xmrig_summary_json were near-identical /2/summary readers, differing only in an API_CMD hook and whether a curl failure propagates or is swallowed. Merged into one function with a propagate/swallow mode argument, preserving each call site's original behavior exactly. Tests: tests/run.sh gains one section per item above (black-box + unit), 153 -> 158 sections. Full suite: 1685 passed, 0 failed (up from 1668 baseline). make lint clean. Co-Authored-By: Claude Fable 5 --- rigforge.sh | 201 +++++++++++++++++++++++++++------------------------ tests/run.sh | 152 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 260 insertions(+), 93 deletions(-) diff --git a/rigforge.sh b/rigforge.sh index fd20087..46f17f4 100755 --- a/rigforge.sh +++ b/rigforge.sh @@ -113,7 +113,12 @@ if [ "$RIGFORGE_APPLIANCE" = 1 ]; then SYSTEMD_DIR="${SYSTEMD_DIR:-/run/systemd/system}" fi # Unit-enablement mode: appliance units live in /run, so their wants/ symlinks must too (a plain -# `enable` would write them to the volatile /etc overlay — working until reboot, then gone). +# `enable` would write them to the volatile /etc overlay — working until reboot, then gone). The +# same flag is REQUIRED on `disable` too (#353) — verified empirically (systemd 255, real enable/ +# disable round-trip): a plain `disable` only ever removes the /etc-side wants-symlink, silently +# leaving a --runtime-enabled unit's /run one in place (`is-enabled` still reports +# "enabled-runtime", rc 0) — it is NOT a superset that also cleans up /run. Every disable call +# below passes ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"}, mirroring its enable counterpart exactly. ENABLE_RUNTIME="" if [ "$RIGFORGE_APPLIANCE" = 1 ]; then ENABLE_RUNTIME="--runtime"; fi @@ -204,14 +209,14 @@ compute_build_jobs() { # echo "$jobs" } -# True if a finished XMRig build for the pinned commit already exists, so we can skip the recompile. -# Requires BOTH the built binary and a commit marker that matches XMRIG_COMMIT (a marker without a -# binary means an incomplete build → rebuild). # SHA-256 of a file, portable (Linux sha256sum / macOS shasum). _sha256() { # if command -v sha256sum >/dev/null 2>&1; then sha256sum "$1"; else shasum -a 256 "$1"; fi | awk '{print $1}' } +# True if a finished XMRig build for the pinned commit already exists, so we can skip the recompile. +# Requires BOTH the built binary and a commit marker that matches XMRIG_COMMIT (a marker without a +# binary means an incomplete build → rebuild). xmrig_already_built() { local marker="$WORKER_ROOT/xmrig/.rigforge-commit" sums="$WORKER_ROOT/xmrig/.rigforge-sha256" [ -x "$WORKER_ROOT/xmrig/build/xmrig" ] && [ -f "$marker" ] && [ "$(cat "$marker" 2>/dev/null)" = "$XMRIG_COMMIT" ] || return 1 @@ -1204,7 +1209,7 @@ install_autotune() { local svc="$SYSTEMD_DIR/rigforge-autotune.service" tmr="$SYSTEMD_DIR/rigforge-autotune.timer" if [ "${AUTOTUNE_MODE:-disabled}" = "disabled" ]; then if [ -f "$tmr" ]; then - sudo systemctl disable --now rigforge-autotune.timer 2>/dev/null || true + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} --now rigforge-autotune.timer 2>/dev/null || true sudo rm -f "$svc" "$tmr" sudo systemctl daemon-reload 2>/dev/null || true log "Periodic autotune disabled." @@ -1232,7 +1237,7 @@ install_watchdog() { local svc="$SYSTEMD_DIR/rigforge-watchdog.service" tmr="$SYSTEMD_DIR/rigforge-watchdog.timer" if [ "${WATCHDOG_MODE:-disabled}" = "disabled" ]; then if [ -f "$tmr" ]; then - sudo systemctl disable --now rigforge-watchdog.timer 2>/dev/null || true + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} --now rigforge-watchdog.timer 2>/dev/null || true sudo rm -f "$svc" "$tmr" sudo systemctl daemon-reload 2>/dev/null || true log "Miner watchdog disabled." @@ -1303,13 +1308,13 @@ install_api() { local svc="$SYSTEMD_DIR/rigforge-api.service" rsvc="$SYSTEMD_DIR/rigforge-api-refresh.service" rtmr="$SYSTEMD_DIR/rigforge-api-refresh.timer" # v1.2.x shipped a per-connection socket pair (Accept=yes) — remove it on sight so upgrades converge. if [ -f "$SYSTEMD_DIR/rigforge-api.socket" ]; then - sudo systemctl disable --now rigforge-api.socket 2>/dev/null || true + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} --now rigforge-api.socket 2>/dev/null || true sudo rm -f "$SYSTEMD_DIR/rigforge-api.socket" "$SYSTEMD_DIR/rigforge-api@.service" sudo systemctl daemon-reload 2>/dev/null || true fi if [ "${API_MODE:-disabled}" = "disabled" ]; then if [ -f "$svc" ] || [ -f "$rtmr" ]; then - sudo systemctl disable --now rigforge-api.service rigforge-api-refresh.timer 2>/dev/null || true + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} --now rigforge-api.service rigforge-api-refresh.timer 2>/dev/null || true sudo rm -f "$svc" "$rsvc" "$rtmr" sudo systemctl daemon-reload 2>/dev/null || true log "Sister API disabled." @@ -1341,7 +1346,7 @@ install_control() { if [ "${CONTROL_MODE:-disabled}" = "disabled" ]; then if [ -f "$svc" ] || [ -f "$apath" ] || [ -f "$upath" ]; then # #308: tear down the remote-upgrade units alongside the control path — they never outlive it. - sudo systemctl disable --now rigforge-control.service rigforge-control-apply.path rigforge-control-upgrade.path 2>/dev/null || true + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} --now rigforge-control.service rigforge-control-apply.path rigforge-control-upgrade.path 2>/dev/null || true sudo rm -f "$svc" "$asvc" "$apath" "$usvc" "$upath" sudo systemctl daemon-reload 2>/dev/null || true # Under load systemctl can transiently drop the stop half of `disable --now` (real @@ -1372,7 +1377,7 @@ install_control() { sudo tee "$upath" <"$SCRIPT_DIR/systemd/rigforge-control-upgrade.path.template" >/dev/null log "Remote upgrade ENABLED — the stack can trigger a RigForge self-upgrade to the latest release (default-off surface; ADR 0002)." elif [ -f "$upath" ] || [ -f "$usvc" ]; then - sudo systemctl disable --now rigforge-control-upgrade.path 2>/dev/null || true + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} --now rigforge-control-upgrade.path 2>/dev/null || true sudo rm -f "$usvc" "$upath" fi sudo systemctl daemon-reload @@ -2045,28 +2050,28 @@ uninstall() { log "Left system user '$_mu' in place — remove it yourself with: sudo userdel $_mu" fi if [ -f "$SYSTEMD_DIR/rigforge-api.socket" ] || [ -f "$SYSTEMD_DIR/rigforge-api.service" ]; then - sudo systemctl disable --now rigforge-api.socket rigforge-api.service rigforge-api-refresh.timer 2>/dev/null || true + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} --now rigforge-api.socket rigforge-api.service rigforge-api-refresh.timer 2>/dev/null || true sudo rm -f "$SYSTEMD_DIR/rigforge-api.socket" "$SYSTEMD_DIR/rigforge-api@.service" \ "$SYSTEMD_DIR/rigforge-api.service" "$SYSTEMD_DIR/rigforge-api-refresh.service" "$SYSTEMD_DIR/rigforge-api-refresh.timer" fi if [ -f "$SYSTEMD_DIR/rigforge-control.service" ] || [ -f "$SYSTEMD_DIR/rigforge-control-apply.path" ] || [ -f "$SYSTEMD_DIR/rigforge-control-upgrade.path" ]; then # #308: tear down the remote-upgrade units too, or uninstall leaves a live code-update surface. - sudo systemctl disable --now rigforge-control.service rigforge-control-apply.path rigforge-control-upgrade.path 2>/dev/null || true + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} --now rigforge-control.service rigforge-control-apply.path rigforge-control-upgrade.path 2>/dev/null || true sudo rm -f "$SYSTEMD_DIR/rigforge-control.service" "$SYSTEMD_DIR/rigforge-control-apply.service" "$SYSTEMD_DIR/rigforge-control-apply.path" \ "$SYSTEMD_DIR/rigforge-control-upgrade.service" "$SYSTEMD_DIR/rigforge-control-upgrade.path" fi command -v nft >/dev/null 2>&1 && sudo nft destroy table inet rigforge 2>/dev/null || true if [ -f "$SYSTEMD_DIR/rigforge-autotune.timer" ]; then - sudo systemctl disable --now rigforge-autotune.timer 2>/dev/null || true + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} --now rigforge-autotune.timer 2>/dev/null || true sudo rm -f "$SYSTEMD_DIR/rigforge-autotune.timer" "$SYSTEMD_DIR/rigforge-autotune.service" fi if [ -f "$SYSTEMD_DIR/rigforge-watchdog.timer" ]; then - sudo systemctl disable --now rigforge-watchdog.timer 2>/dev/null || true + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} --now rigforge-watchdog.timer 2>/dev/null || true sudo rm -f "$SYSTEMD_DIR/rigforge-watchdog.timer" "$SYSTEMD_DIR/rigforge-watchdog.service" fi if [ -f "$SYSTEMD_DIR/$SERVICE_NAME.service" ]; then sudo systemctl stop "$SERVICE_NAME" 2>/dev/null || true - sudo systemctl disable "$SERVICE_NAME" 2>/dev/null || true + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} "$SERVICE_NAME" 2>/dev/null || true sudo rm -f "$SYSTEMD_DIR/$SERVICE_NAME.service" sudo systemctl daemon-reload 2>/dev/null || true log "Removed the $SERVICE_NAME service." @@ -2667,9 +2672,9 @@ _thread_candidates() { #
echo "$list" } -# Coordinate hill-climb from the current S_* state: sweep each active knob, adopt the best value that # --- Auto-tuning: search strategies & seeding --- +# Coordinate hill-climb from the current S_* state: sweep each active knob, adopt the best value that # beats the running best by TUNE_MIN_DELTA, and repeat rounds until a pass makes no gain (plateau). # Echoes the best hashrate reached; leaves S_* at the winning combination. _hillclimb() { @@ -3357,21 +3362,38 @@ autotune() { _AUTOTUNE_DIRTY=0 # #347: a deliberate final mode is in place; the abort-restore stands down } -# Read the current total hashrate from the worker's HTTP API (empty if unreachable). Overridable for -# tests via API_CMD. This is RigForge's own local reader (loopback) used by tune/autotune; it uses the -# `/2/summary` endpoint. Pithead's dashboard separately reads `/1/summary` from the stack host — both -# are valid XMRig endpoints, the divergence is intentional. -# Raw /2/summary JSON from the worker API, or nothing when curl is missing/unreachable (#143). -_read_api_summary() { +# Raw /2/summary JSON from the worker's HTTP API (loopback), or empty when curl/API_CMD is +# missing/unreachable (#143). Pithead's dashboard separately reads `/1/summary` from the stack host — +# both are valid XMRig endpoints, the divergence is intentional. Overridable for tests via API_CMD +# (evaluated as-is — a raw hashrate for _read_api_hashrate's own check below, a full JSON body for +# every other caller). Shared by tune/autotune/status and the sister API's stats superset — was two +# near-identical readers, _read_api_summary + _xmrig_summary_json (#353). +# : "propagate" lets a curl failure raise under set -e — what tune/autotune/status relied on so +# an unreachable API surfaces upstream. Default "swallow" always returns 0 — what the sister API +# refresh loop and the pool-connection probe need, since an unreachable miner there is routine, not a +# crash. The API is open (read-only) with no token by default; only send a Bearer when ACCESS_TOKEN is +# set (XMRig 401s a token it never asked for, and curl -f's exit 22 would then hit the mode check +# below either way). Branches on the Bearer header rather than an empty-array curl arg, and on mode +# rather than appending to a built-up arg list — both trip set -u on bash 3.2 (macOS). +_read_api_summary() { # [propagate] local url="http://127.0.0.1:8080/2/summary" + if [ -n "${API_CMD:-}" ]; then + eval "$API_CMD" + return + fi command -v curl >/dev/null 2>&1 || return 0 - # The API is open (read-only) with no token by default; only send a Bearer when ACCESS_TOKEN is set. - # XMRig 401s a token it never asked for, and curl -f (exit 22) would then abort the caller under set -e. - # Branch rather than an empty-array curl arg, which also trips set -u on bash 3.2 (macOS). - if [ -n "${ACCESS_TOKEN:-}" ]; then - curl -fsS --max-time 5 -H "Authorization: Bearer $ACCESS_TOKEN" "$url" 2>/dev/null + if [ "${1:-}" = propagate ]; then + if [ -n "${ACCESS_TOKEN:-}" ]; then + curl -fsS --max-time 5 -H "Authorization: Bearer $ACCESS_TOKEN" "$url" 2>/dev/null + else + curl -fsS --max-time 5 "$url" 2>/dev/null + fi else - curl -fsS --max-time 5 "$url" 2>/dev/null + if [ -n "${ACCESS_TOKEN:-}" ]; then + curl -fsS --max-time 5 -H "Authorization: Bearer $ACCESS_TOKEN" "$url" 2>/dev/null || true + else + curl -fsS --max-time 5 "$url" 2>/dev/null || true + fi fi } @@ -3380,7 +3402,7 @@ _read_api_hashrate() { eval "$API_CMD" return fi - _read_api_summary | jq -r '.hashrate.total[0] // empty' 2>/dev/null + _read_api_summary propagate | jq -r '.hashrate.total[0] // empty' 2>/dev/null } # Median of N live API hashrate samples, seconds apart. Smooths the jittery live reading so a @@ -3404,6 +3426,13 @@ _sample_api_median() { # # machines so you tune once and roll the result out across a fleet. (Tuning is CPU-specific — only reuse # it between identical CPUs.) Mirrors Pithead's backup/restore UX. +# The secret-bearing staging dir shared by backup/restore/support_bundle below — a script global (like +# TUNE_TMP), not `local`, because each function's EXIT trap must still resolve it when it actually +# fires: on a clean run that's after the function has already returned, where a `local` would be out +# of scope and die "unbound variable" under set -u (#353). One at a time is safe: the three verbs are +# mutually exclusive per invocation, same as tune()/autotune() sharing TUNE_TMP. +STAGE_DIR="" + # backup: write config.json + tuning into a timestamped tar.gz under ./backups (owner-only). backup() { local arg @@ -3415,27 +3444,30 @@ backup() { done [ -f "$CONFIG_JSON" ] || error "No config.json to back up. Run 'setup' first." - local wr stage included="config.json" f + local wr included="config.json" f wr=$(_worker_root_from_config) - stage=$(mktemp -d) - cp "$CONFIG_JSON" "$stage/config.json" + STAGE_DIR=$(mktemp -d) + # Cleanup is armed the moment the temp dir exists — a set -e abort anywhere below (disk full, + # tar failure) must not leak this secret-bearing staging content (config.json, tokens). Same + # EXIT-trap treatment tune() got in #135. (#353) + trap 'rm -rf "$STAGE_DIR"' EXIT + cp "$CONFIG_JSON" "$STAGE_DIR/config.json" # The tuning files live under the worker root; include whichever exist (a fresh worker has none yet). for f in tune-overrides.json rigforge-tune.json rigforge-bios.json; do if [ -n "$wr" ] && [ -f "$wr/$f" ]; then - cp "$wr/$f" "$stage/$f" + cp "$wr/$f" "$STAGE_DIR/$f" included="$included $f" fi done # A small manifest for provenance — handy when rolling a tune out across a fleet. jq -n --arg v "$(cmd_version)" --arg host "$(hostname 2>/dev/null)" --arg files "$included" \ - '{rigforge: $v, source_host: $host, files: ($files | split(" "))}' >"$stage/rigforge-backup.json" 2>/dev/null || true + '{rigforge: $v, source_host: $host, files: ($files | split(" "))}' >"$STAGE_DIR/rigforge-backup.json" 2>/dev/null || true local backups_dir="$SCRIPT_DIR/backups" stamp archive mkdir -p "$backups_dir" stamp=$(date +%Y%m%d-%H%M%S) archive="$backups_dir/rigforge-backup-$stamp.tar.gz" - (umask 077 && tar -czf "$archive" -C "$stage" .) - rm -rf "$stage" + (umask 077 && tar -czf "$archive" -C "$STAGE_DIR" .) chmod 600 "$archive" 2>/dev/null || true log "Backed up: $included" @@ -3465,44 +3497,37 @@ restore() { } fi - local stage - stage=$(mktemp -d) - tar -xzf "$archive" -C "$stage" 2>/dev/null || { - rm -rf "$stage" - error "Could not extract $archive — is it a RigForge backup?" - } - [ -f "$stage/config.json" ] || { - rm -rf "$stage" - error "Archive has no config.json — not a RigForge backup." - } + STAGE_DIR=$(mktemp -d) + # Cleanup is armed the moment the temp dir exists — a set -e abort anywhere below (an error() call + # included: EXIT traps fire for those same as any other exit) must not leak this secret-bearing + # staging content. Same EXIT-trap treatment tune() got in #135. (#353) + trap 'rm -rf "$STAGE_DIR"' EXIT + tar -xzf "$archive" -C "$STAGE_DIR" 2>/dev/null || error "Could not extract $archive — is it a RigForge backup?" + [ -f "$STAGE_DIR/config.json" ] || error "Archive has no config.json — not a RigForge backup." # Validate the staged config BEFORE it ever touches the live one — same subshell idiom as # _control_commit (rigforge.sh:~3652): parse_config's error() only exits the subshell, so a bad # backup can be rejected without corrupting this shell's CONFIG_JSON. shellcheck flags this as # SC2031 (info), same as there — intentional, not suppressed. - if ! (CONFIG_JSON="$stage/config.json" && parse_config) >/dev/null 2>&1; then - rm -rf "$stage" - error "Backup's config.json failed validation — existing config left untouched." - fi - if [ -f "$stage/rigforge-backup.json" ]; then + (CONFIG_JSON="$STAGE_DIR/config.json" && parse_config) >/dev/null 2>&1 || error "Backup's config.json failed validation — existing config left untouched." + if [ -f "$STAGE_DIR/rigforge-backup.json" ]; then local src - src=$(jq -r '.source_host // empty' "$stage/rigforge-backup.json" 2>/dev/null) + src=$(jq -r '.source_host // empty' "$STAGE_DIR/rigforge-backup.json" 2>/dev/null) [ -n "$src" ] && log "Backup was made on host: $src" fi # config.json -> repo root; tuning -> the worker root resolved from the RESTORED config (so it lands # correctly even if this machine's paths differ from the source's). - cp "$stage/config.json" "$CONFIG_JSON" + cp "$STAGE_DIR/config.json" "$CONFIG_JSON" _stamp_config_meta restore # #254: attribute this config to a restore (bumps revision if it differs) local restored="config.json" wr f wr=$(_worker_root_from_config) for f in tune-overrides.json rigforge-tune.json rigforge-bios.json; do - if [ -f "$stage/$f" ]; then + if [ -f "$STAGE_DIR/$f" ]; then mkdir -p "$wr" 2>/dev/null || sudo mkdir -p "$wr" - cp "$stage/$f" "$wr/$f" 2>/dev/null || sudo cp "$stage/$f" "$wr/$f" + cp "$STAGE_DIR/$f" "$wr/$f" 2>/dev/null || sudo cp "$STAGE_DIR/$f" "$wr/$f" restored="$restored $f" fi done - rm -rf "$stage" log "Restored: $restored" case " $restored " in @@ -3533,38 +3558,41 @@ support_bundle() { done [ -f "$CONFIG_JSON" ] || error "No config.json to collect. Run 'setup' first." - local wr stage collected="" skipped="" f + local wr collected="" skipped="" f wr=$(_worker_root_from_config) - stage=$(mktemp -d) + STAGE_DIR=$(mktemp -d) + # Cleanup is armed the moment the temp dir exists — a set -e abort anywhere below must not leak + # this secret-bearing staging content. Same EXIT-trap treatment tune() got in #135. (#353) + trap 'rm -rf "$STAGE_DIR"' EXIT _take() { collected="$collected $1"; } _skip() { skipped="$skipped $1"; } - cmd_version >"$stage/version.txt" && _take version.txt + cmd_version >"$STAGE_DIR/version.txt" && _take version.txt # Subprocess, not a function call: doctor's error-exits can't kill the bundle. Strip ANSI codes. - ("$0" doctor &1 || true) | sed -e $'s/\x1b\[[0-9;]*m//g' >"$stage/doctor.txt" && _take doctor.txt + ("$0" doctor &1 || true) | sed -e $'s/\x1b\[[0-9;]*m//g' >"$STAGE_DIR/doctor.txt" && _take doctor.txt # Fail closed: if jq can't redact a file, the file stays OUT of the bundle — never the original. - if _redact_config <"$CONFIG_JSON" >"$stage/config.redacted.json" 2>/dev/null; then + if _redact_config <"$CONFIG_JSON" >"$STAGE_DIR/config.redacted.json" 2>/dev/null; then _take config.redacted.json else - rm -f "$stage/config.redacted.json" + rm -f "$STAGE_DIR/config.redacted.json" _skip "config.redacted.json(unparseable)" fi if [ -n "$wr" ] && [ -f "$wr/xmrig/build/config.json" ]; then - if _redact_config <"$wr/xmrig/build/config.json" >"$stage/xmrig-config.redacted.json" 2>/dev/null; then + if _redact_config <"$wr/xmrig/build/config.json" >"$STAGE_DIR/xmrig-config.redacted.json" 2>/dev/null; then _take xmrig-config.redacted.json else - rm -f "$stage/xmrig-config.redacted.json" + rm -f "$STAGE_DIR/xmrig-config.redacted.json" _skip "xmrig-config.redacted.json(unparseable)" fi fi if [ -n "$wr" ] && [ -f "$wr/xmrig.log" ]; then - tail -n 500 "$wr/xmrig.log" >"$stage/xmrig.log.tail" 2>/dev/null && _take xmrig.log.tail + tail -n 500 "$wr/xmrig.log" >"$STAGE_DIR/xmrig.log.tail" 2>/dev/null && _take xmrig.log.tail fi for f in tune-overrides.json rigforge-tune.json; do - [ -n "$wr" ] && [ -f "$wr/$f" ] && cp "$wr/$f" "$stage/$f" && _take "$f" + [ -n "$wr" ] && [ -f "$wr/$f" ] && cp "$wr/$f" "$STAGE_DIR/$f" && _take "$f" done for f in "$SERVICE_NAME.service" rigforge-autotune.service rigforge-autotune.timer; do - [ -f "$SYSTEMD_DIR/$f" ] && cp "$SYSTEMD_DIR/$f" "$stage/$f" 2>/dev/null && _take "$f" + [ -f "$SYSTEMD_DIR/$f" ] && cp "$SYSTEMD_DIR/$f" "$STAGE_DIR/$f" 2>/dev/null && _take "$f" done { uname -a @@ -3575,16 +3603,15 @@ support_bundle() { sysctl -n machdep.cpu.brand_string 2>/dev/null || true sysctl -n hw.memsize 2>/dev/null || true fi - } >"$stage/system.txt" 2>/dev/null && _take system.txt + } >"$STAGE_DIR/system.txt" 2>/dev/null && _take system.txt jq -n --arg v "$(cmd_version)" --arg host "$(hostname 2>/dev/null)" --arg files "${collected# }" --arg skipped "${skipped# }" \ '{rigforge: $v, source_host: $host, files: ($files | split(" ")), not_collected: (["journalctl (system-wide)", "shell history", "unredacted configs", "backups/"] + (if $skipped != "" then ($skipped | split(" ")) else [] end))}' \ - >"$stage/manifest.json" 2>/dev/null || true + >"$STAGE_DIR/manifest.json" 2>/dev/null || true local stamp archive stamp=$(date +%Y%m%d-%H%M%S) archive="$SCRIPT_DIR/rigforge-support-$(hostname 2>/dev/null)-$stamp.tar.gz" - (umask 077 && tar -czf "$archive" -C "$stage" .) - rm -rf "$stage" + (umask 077 && tar -czf "$archive" -C "$STAGE_DIR" .) chmod 600 "$archive" 2>/dev/null || true log "Collected:${collected}" @@ -3733,7 +3760,7 @@ mac_disable() { # markers (that's doctor's job); plain aligned lines stay grep-friendly. Never sudo, never prompts. _status_api_summary() { local body hs pool up acc rej hp - body=$(_read_api_summary) + body=$(_read_api_summary propagate) if [ -z "$body" ]; then echo "RigForge: worker API not reachable at 127.0.0.1:8080 (miner stopped or still starting)." return 0 @@ -3816,7 +3843,7 @@ svc_disable() { mac_disable return } - sudo systemctl disable "$SERVICE_NAME" && log "Disabled $SERVICE_NAME (won't start on boot)." + sudo systemctl disable ${ENABLE_RUNTIME:+"$ENABLE_RUNTIME"} "$SERVICE_NAME" && log "Disabled $SERVICE_NAME (won't start on boot)." } # --- Commands: version, apply & bench --- @@ -3913,7 +3940,7 @@ _apply_plan() { else echo " 2. restart the miner manually ('$0 restart') — no service on $OS_TYPE" fi - echo " 3. reconcile the autotune timer + sister API + firewall to config (autotune: $AUTOTUNE_MODE, api: $API_MODE)" + echo " 3. reconcile the autotune timer + watchdog + sister API + control path + firewall to config (autotune: $AUTOTUNE_MODE, api: $API_MODE, control: $CONTROL_MODE)" echo "Dry run — nothing was changed. Run 'sudo $0 apply' to apply." } @@ -4645,22 +4672,6 @@ EOF # --- Sister API (#99): read-only stats superset on its own port --- -# Full /2/summary body from the local worker API (empty when unreachable). Sibling of -# _read_api_hashrate with the same API_CMD test hook and Bearer branch (see the comment there). -_xmrig_summary_json() { - local url="http://127.0.0.1:8080/2/summary" - if [ -n "${API_CMD:-}" ]; then - eval "$API_CMD" - return - fi - command -v curl >/dev/null 2>&1 || return 0 - if [ -n "${ACCESS_TOKEN:-}" ]; then - curl -fsS --max-time 5 -H "Authorization: Bearer $ACCESS_TOKEN" "$url" 2>/dev/null || true - else - curl -fsS --max-time 5 "$url" 2>/dev/null || true - fi -} - # {watts, hs_per_watt} over a 1-second RAPL energy window; nulls when unmeasurable (no RAPL / # non-root). RAPL only — TUNE_POWER_CMD is an operator-session env var whose value is eval'd, and # config-derived text must never reach eval inside a network-facing handler. @@ -4822,7 +4833,7 @@ api_refresh() { parse_config >/dev/null local dir="${RIGFORGE_API_DATA:-/run/rigforge-api}" sum hr rf body mkdir -p "$dir" - sum=$(_xmrig_summary_json || true) + sum=$(_read_api_summary || true) printf '%s' "$sum" | jq -e . >/dev/null 2>&1 || sum="" hr=$(printf '%s' "$sum" | jq -r '.hashrate.total[0] // empty' 2>/dev/null || true) rf=$(_api_rigforge_block "$hr") @@ -4839,7 +4850,7 @@ api_refresh() { # --- Pool-connection probe (#343), shared by doctor and apply --- # The miner's own verdict on its pool connection, read from the local /2/summary (API_CMD test hook -# + Bearer discipline via _xmrig_summary_json). One TSV line: +# + Bearer discipline via _read_api_summary). One TSV line: # connected — a stratum connection is live # disconnected — miner answers, but no live connection # api-down — no parseable summary (API unreachable) @@ -4849,7 +4860,7 @@ api_refresh() { # disagree with the miner's (proxied, TLS) one, and the miner is the party that has to be connected. _pool_conn_status() { local body pool cup fails acc - body=$(_xmrig_summary_json) + body=$(_read_api_summary) if ! printf '%s' "$body" | jq -e '.connection' >/dev/null 2>&1; then echo api-down return 0 @@ -5665,6 +5676,10 @@ if [ "$_RIGFORGE_SOURCED" = "0" ]; then [ -z "${2:-}" ] || error "Unexpected argument for $1: '$2'. Run '$0 help'." ;; esac + # #353 (1): name the verb before dispatch so an UNEXPECTED failure in any of them reports where it + # actually happened, not the stale "starting up" default — setup's own main() overwrites this a + # moment later with its fine-grained per-phase steps, so this is a no-op there. + CURRENT_STEP="running '${1:-setup}'" case "${1:-setup}" in setup) # if-form, not `[ $# -gt 0 ] && shift`: a false && list at top level trips set -e on the diff --git a/tests/run.sh b/tests/run.sh index 236f019..20eaa56 100644 --- a/tests/run.sh +++ b/tests/run.sh @@ -1210,6 +1210,21 @@ out="$( assert_contains "err trap names the step" "$out" "compiling XMRig" assert_contains "err trap suggests bash -x" "$out" "bash -x" +# #353 (1): CURRENT_STEP used to be set only inside main() (setup) — an unexpected failure in ANY +# other verb reported the stale "starting up" default. Force a real dispatch-time failure (a +# systemctl that dies, uncaught by svc_start's `&&`) and check on_err names the actual verb. +echo "== black-box: on_err names the running verb, not the stale setup default (#353) ==" +CSV="$(mktemp -d "$SANDBOX/current-step-verb.XXXXXX")" +mkdir -p "$CSV/bin" +cat >"$CSV/bin/systemctl" <<'EOF' +#!/usr/bin/env bash +exit 7 +EOF +chmod +x "$CSV/bin/systemctl" +csv_out="$(cd "$CSV" && PATH="$CSV/bin:$STUBS:$PATH" STUB_UNAME_S=Linux RIGFORGE_HOME="$PWD" bash "$SCRIPT" start &1)" +assert_contains "on_err names the 'start' verb on an unexpected failure (#353)" "$csv_out" "aborted while running 'start'" +assert_absent "on_err no longer falls back to the stale setup default (#353)" "$csv_out" "aborted while starting up" + # prepare_workspace archives the existing build and must prune old archives so re-runs don't grow the # disk without bound (#4). KEEP_ARCHIVES caps how many are retained. echo "== unit: prepare_workspace prunes old build archives (#4) ==" @@ -2407,6 +2422,27 @@ assert_rc "apply --dry-run exits 0 (#146)" "$?" "0" assert_contains "apply plan: config regen target (#146)" "$ap_out" "regenerate" assert_contains "apply plan: reconcile line (#146)" "$ap_out" "autotune: performance" assert_absent "apply --dry-run never restarts (#146)" "$(cat "$DR/calls.log" 2>/dev/null)" "restart" +# #353 (2): drift guard — every install_* apply() actually reconciles must be named in _apply_plan's +# line 3, or the plan under-reports what apply does (found: watchdog + control were called but never +# named). Hand-maintained map from function name -> plan wording, same shape as the #207 FLAGMAP below +# (reconciles change rarely; the failure message says exactly where to add the plan line) since the +# plan's prose can't be derived from the function names automatically. +apply_reconciles="$(sed -n '/^apply() {/,/^}/p' "$SCRIPT" | grep -oE 'install_[a-z_]+' | sort -u)" +[ -n "$apply_reconciles" ] || bad "could not extract install_* calls from apply() (#353)" "sed/grep extraction was empty" +while IFS= read -r _fn; do + case "$_fn" in + install_autotune) _word="autotune" ;; + install_watchdog) _word="watchdog" ;; + install_api) _word="sister API" ;; + install_control) _word="control path" ;; + install_api_firewall) _word="firewall" ;; + *) + bad "apply --dry-run plan drift guard (#353)" "apply() now calls $_fn but the map above doesn't know its plan wording — add it" + _word="" + ;; + esac + [ -n "$_word" ] && assert_contains "apply --dry-run plan covers apply()'s $_fn (#353)" "$ap_out" "$_word" +done <<<"$apply_reconciles" # Unknown setup arg: hard error, house style. bash "$SCRIPT" setup --bogus >/dev/null 2>&1 || setup_arg_rc=$? assert_rc "unknown setup arg errors (#146)" "${setup_arg_rc:-0}" "1" @@ -2447,6 +2483,22 @@ comp_rc=0 bash "$SCRIPT" completion >/dev/null 2>&1 || comp_rc=$? assert_rc "completion without a shell errors with usage (#145)" "$comp_rc" "1" +# #353 (6): _read_api_summary + _xmrig_summary_json were near-identical readers, merged into one +# function with a mode argument. Prove the two modes actually differ: "propagate" (tune/autotune/ +# status's shape) surfaces a curl failure as a nonzero exit; the default "swallow" (the sister API's +# shape) always returns 0. Both must still yield an EMPTY body either way — only the exit code differs. +echo "== unit: _read_api_summary propagate vs swallow (#353) ==" +rap_out="$( ( + source "$SCRIPT" + unset API_CMD + curl() { return 7; } # simulate an unreachable worker API + set +e + printf 'default:[%s] rc=%s\n' "$(_read_api_summary)" "$?" + printf 'propagate:[%s] rc=%s\n' "$(_read_api_summary propagate)" "$?" +) 2>&1)" +assert_contains "default mode swallows a curl failure (rc 0, empty body)" "$rap_out" "default:[] rc=0" +assert_contains "propagate mode surfaces a curl failure (nonzero rc, empty body)" "$rap_out" "propagate:[] rc=7" + # #143: `status` prepends a one-glance live summary from ONE /2/summary fetch — facts, no ✓/! markers, # never sudo. Unreachable API (miner stopped / http off) degrades to a single explanatory line and the # untouched platform block; a bad config can't crash it (parse_config runs in a subshell). @@ -4326,6 +4378,39 @@ assert_rc "uninstall 'n' exits 0" "$?" "0" assert_contains "uninstall 'n' reports it aborted" "$out" "Aborted" assert_eq "uninstall 'n' left the service unit in place" "$([ -f "$UNN/etc/systemd/system/xmrig.service" ] && echo present || echo gone)" "present" +# #353 (4): appliance disable must mirror appliance enable — verified empirically against a real +# systemd (255): a plain `disable` only ever removes the /etc-side wants-symlink, silently leaving a +# --runtime-enabled unit's /run symlink in place (`is-enabled` still reports "enabled-runtime", rc +# 0). Same sandbox shape as the uninstall test above (reuses UN_OPT_UNITS so the seeded unit list +# can't drift from it), just under RIGFORGE_APPLIANCE=1 with SYSTEMD_DIR standing in for /run — +# mirrors the #797 "every enable is --runtime" guard, on the disable side. +echo "== black-box: appliance uninstall disables with --runtime too (#353) ==" +UNA="$(mktemp -d "$SANDBOX/uninst-appliance.XXXXXX")" +cp "$ROOT/VERSION" "$UNA/" +mkdir -p "$UNA/run-systemd" "$UNA/dev/hp1g" "$UNA/home/worker/xmrig/build" "$UNA/usr-local-bin" +: >"$UNA/run-systemd/xmrig.service" +for _u in $UN_OPT_UNITS; do + : >"$UNA/run-systemd/$_u" +done +cat >"$UNA/config.json" <&1)" +assert_rc "appliance uninstall exits 0 (#353)" "$?" "0" +disable_calls="$(grep -F '[systemctl] disable' "$UNA/calls.log" 2>/dev/null)" +# If the sandbox seeding ever drifts from what uninstall() actually disables, an empty $disable_calls +# would make the grep -c below vacuously pass (0 non---runtime lines out of 0 total) — fail loudly +# instead, same principle as the CURRENT_STEP extraction guard above. +[ -n "$disable_calls" ] || bad "appliance uninstall drift guard (#353)" "no '[systemctl] disable' calls were logged — the guard below would vacuously pass" +assert_eq "appliance uninstall: every systemctl disable is --runtime (#353)" \ + "$(printf '%s\n' "$disable_calls" | grep -cv -- --runtime)" "0" + # #54: tune is an iterative, noise-aware, multi-knob hill-climb. It sweeps prefetch_mode, cpu.yield and # the RandomX thread count (cpu.rx, around L3/2 MB), measures each candidate as the MEDIAN of N runs, # memoizes so a combo is never benchmarked twice, climbs from two seeds (auto + educated guess), and @@ -5853,6 +5938,73 @@ out="$(cd "$NOC" && PATH="$STUBS:$PATH" RIGFORGE_HOME="$PWD" bash "$SCRIPT" back assert_rc "backup without a config fails" "$?" "1" assert_contains "backup no-config message" "$out" "No config.json" +# #353 (3): backup/restore/support-bundle's mktemp -d staging (config.json, tokens) must not survive +# a set -e abort — same EXIT-trap treatment tune() got in #135. Force a real abort (a tar that always +# fails) and confirm the staged tempdir is actually GONE afterward, not just that the command failed. +# mktemp is wrapped, not replaced — it still creates a real dir; the wrapper only logs the path so the +# test can check it from outside the subprocess that owned it. +REAL_MKTEMP="$(command -v mktemp)" +_leak_test_bins() { # -> writes a logging mktemp wrapper + an always-fails tar into /bin + mkdir -p "$1/bin" + cat >"$1/bin/mktemp" <>"$1/mktemp.log" +printf '%s' "\$p" +EOF + cat >"$1/bin/tar" <<'EOF' +#!/usr/bin/env bash +exit 1 +EOF + chmod +x "$1/bin/mktemp" "$1/bin/tar" + : >"$1/mktemp.log" +} + +echo "== black-box: backup leaks no staging tempdir on a set -e abort (#353) ==" +BKT="$(mktemp -d "$SANDBOX/backup-trap.XXXXXX")" +_leak_test_bins "$BKT" +cat >"$BKT/config.json" </dev/null 2>&1) || bkt_rc=$? +bkt_stage="$(tail -1 "$BKT/mktemp.log" 2>/dev/null)" +if [ -z "$bkt_stage" ]; then + bad "backup leak-test setup (#353)" "mktemp was never logged — the wrapper stub isn't wired correctly" +else + assert_rc "backup with a failing tar exits nonzero (sanity: the abort really happened)" "$bkt_rc" "1" + assert_eq "backup's EXIT trap removed the staging tempdir after the abort (#353)" "$([ -d "$bkt_stage" ] && echo leaked || echo clean)" "clean" +fi + +echo "== black-box: restore leaks no staging tempdir on a set -e abort (#353) ==" +RST="$(mktemp -d "$SANDBOX/restore-trap.XXXXXX")" +_leak_test_bins "$RST" +rst_rc=0 +(cd "$RST" && PATH="$RST/bin:$STUBS:$PATH" RIGFORGE_HOME="$PWD" bash "$SCRIPT" restore -y "$ARCHIVE" /dev/null 2>&1) || rst_rc=$? +rst_stage="$(tail -1 "$RST/mktemp.log" 2>/dev/null)" +if [ -z "$rst_stage" ]; then + bad "restore leak-test setup (#353)" "mktemp was never logged — the wrapper stub isn't wired correctly" +else + assert_rc "restore with a failing tar -xzf exits nonzero (sanity: the abort really happened)" "$rst_rc" "1" + assert_eq "restore's EXIT trap removed the staging tempdir after the abort (#353)" "$([ -d "$rst_stage" ] && echo leaked || echo clean)" "clean" +fi + +echo "== black-box: support-bundle leaks no staging tempdir on a set -e abort (#353) ==" +SBT="$(mktemp -d "$SANDBOX/support-bundle-trap.XXXXXX")" +_leak_test_bins "$SBT" +cat >"$SBT/config.json" </dev/null 2>&1) || sbt_rc=$? +sbt_stage="$(tail -1 "$SBT/mktemp.log" 2>/dev/null)" +if [ -z "$sbt_stage" ]; then + bad "support-bundle leak-test setup (#353)" "mktemp was never logged — the wrapper stub isn't wired correctly" +else + assert_rc "support-bundle with a failing tar exits nonzero (sanity: the abort really happened)" "$sbt_rc" "1" + assert_eq "support-bundle's EXIT trap removed the staging tempdir after the abort (#353)" "$([ -d "$sbt_stage" ] && echo leaked || echo clean)" "clean" +fi + echo "== unit: VERSION is SemVer (#3) ==" ver="$(tr -d '[:space:]' <"$ROOT/VERSION" 2>/dev/null)" if [[ "$ver" =~ ^[0-9]+\.[0-9]+\.[0-9]+([-+.].*)?$ ]]; then ok "VERSION is SemVer ($ver)"; else bad "VERSION is SemVer" "got [$ver]"; fi From 1240e168d427d07e5cfc94537683549b98c01e4e Mon Sep 17 00:00:00 2001 From: Vijit Singh Date: Sat, 15 Aug 2026 16:12:32 -0500 Subject: [PATCH 2/4] test(e2e-real): make the upgrade phase's forward leg mandatory, not opt-in (#350) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tests/e2e-real.sh's upgrade phase only ever proved the two REFUSAL legs (noop, forged-tag rollback) unless an operator remembered to set E2E_UPGRADE_TARGET — so a broken fetch/rebuild path could pass the release gate silently. The forward leg is now on by default. It auto-derives the previous real release tag and the current/installed one from `git tag` (the release this gate is actually cutting has no tag yet at this point in RELEASING.md's flow, so "current" stands in for it), rewinds the checkout to the previous tag out of band (git checkout + `rigforge.sh upgrade`, the same two steps _control_upgrade_do takes — D10's anti-rollback in control_upgrade() refuses a downgrade POST, so this can't happen through the control channel itself), then drives a real forward upgrade back to current through the same wire/path-unit/oneshot chain the other legs use. _upgrade_cleanup restores the checkout to exactly where the phase started on any exit, success or a hard abort mid-leg — unlike the old opt-in leg (which permanently advanced the checkout past "current"), this one is repeatable, which is what lets it be the default. E2E_UPGRADE_TARGET=vX.Y.Z still works as an explicit override (same PERMANENT semantics as before — useful for a deliberate real deploy). E2E_UPGRADE_SKIP_REASON="..." is the new escape hatch: it skips the leg but requires (and logs) a reason, so a skip is never silent. RELEASING.md's `upgrade` line updated to describe the new default and the skip knob. Needs live-rig validation — this whole leg (the rewind, the real fetch/build through the control channel, the restore) has no automated coverage; it can only run for real on the release rig. See #362's require_traversable_checkout for the one piece of this file that IS unit-tested via extraction. Co-Authored-By: Claude Fable 5 --- RELEASING.md | 2 +- tests/e2e-real.sh | 89 +++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/RELEASING.md b/RELEASING.md index 09bdeca..5a027ed 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -29,7 +29,7 @@ promoted to `main` and tagged. The steps below build the release commit on `deve sudo reboot # HugePages (1G + GRUB cmdline) take effect on boot; reconnect sudo bash tests/e2e-real.sh verify # doctor (HugePages/MSR/governor/service) + bench (real H/s) + a short tune + a live auto-tune pass sudo bash tests/e2e-real.sh control # the writable control path (#236) against real systemd: enable, POST a change, poll to applied, revert - sudo bash tests/e2e-real.sh upgrade # the remote-upgrade chain (#308/#322) with REAL git: noop + refused-tag rollback legs, revert (opt-in forward leg: E2E_UPGRADE_TARGET=vX.Y.Z) + sudo bash tests/e2e-real.sh upgrade # the remote-upgrade chain (#308/#322) with REAL git: noop + refused-tag rollback legs, plus a mandatory forward leg that auto-derives the previous real release tag -> current and proves it, then reverts (skip with a reason: E2E_UPGRADE_SKIP_REASON="...") sudo bash tests/e2e-real.sh perf # offline bench vs the committed per-host baseline + best-ever history (the release perf gate) sudo bash tests/e2e-real.sh teardown # uninstall + assert a clean revert ``` diff --git a/tests/e2e-real.sh b/tests/e2e-real.sh index 670c37f..5e1e3ed 100755 --- a/tests/e2e-real.sh +++ b/tests/e2e-real.sh @@ -24,8 +24,10 @@ # upgrade : the remote-upgrade chain (#308/#322) against the real units and REAL git — a noop leg # (POST the installed version -> terminal `noop`), a rollback leg (a forged tag the D10 # ancestry guard must refuse -> `rolled_back`, tree + VERSION untouched, throttle -# stamped), and an opt-in forward leg (E2E_UPGRADE_TARGET). Same snapshot/revert -# guarantees as control. +# stamped), and a MANDATORY forward leg (#350: auto-derives the previous real release +# tag -> current and proves a genuine fetch/build/apply into it, then restores the +# checkout — E2E_UPGRADE_TARGET/E2E_UPGRADE_SKIP_REASON override it). Same +# snapshot/revert guarantees as control. # teardown : sudo ./rigforge.sh uninstall --yes -> assert a clean revert of every system path + idempotency # # Env knobs: @@ -37,8 +39,11 @@ # E2E_PERF_TOLERANCE_PCT allowed drop vs the committed baseline/best-ever (default 5) # E2E_PERF_RECORD 1 = record the baseline + append history instead of judging # E2E_PERF_TAG release tag stamped into the history entry (with E2E_PERF_RECORD) -# E2E_UPGRADE_TARGET vX.Y.Z = the upgrade phase also drives a REAL forward upgrade to this -# release and asserts it lands (PERMANENT: upgrades the checkout) +# E2E_UPGRADE_TARGET vX.Y.Z = override the forward leg's target explicitly instead of the +# auto-derived previous-tag -> current-tag pair (PERMANENT: upgrades +# the checkout — does not restore afterward, unlike the default leg) +# E2E_UPGRADE_SKIP_REASON set (to a reason string) to skip the now-mandatory forward leg — an +# explicit, logged escape hatch, not a silent bypass # # Linux-only and root-only (kernel tuning, modprobe, apt). Typical flow on the release rig: # sudo bash tests/e2e-real.sh provision @@ -63,6 +68,10 @@ FAIL=0 # _control_cleanup). CTL_SAVED_CFG="" CTL_CLEANUP_DONE=0 +# #350: the pre-forward-leg HEAD sha, set only by the auto-derived forward leg (never by the +# E2E_UPGRADE_TARGET override, which stays deliberately PERMANENT). Same script-global reasoning as +# above — _upgrade_cleanup must see it from a late trap fire too. +UPG_ORIG_REF="" ok() { PASS=$((PASS + 1)) printf ' \033[1;32m✓\033[0m %s\n' "$1" @@ -791,15 +800,24 @@ _control_cleanup() { # This runs the real git calls (fetch, rev-parse, merge-base, checkout) as the root # oneshot with no $HOME — the #308 dubious-ownership class dies here, not in the # stubbed suite. Cheap: the forward refusal happens before any checkout or build. -# forward : opt-in via E2E_UPGRADE_TARGET=vX.Y.Z (a real release newer than the installed one) -# -> poll to `applied`, assert VERSION landed. PERMANENTLY upgrades this checkout, so -# it is not part of the repeatable default — it's the release-flow leg that would have -# caught #318 (a legit upgrade being refused). +# forward : MANDATORY (#350) — a broken fetch/rebuild path must fail the gate, not slip through on +# an operator forgetting to opt in. Auto-derives the last two REAL, already-published +# release tags (git tag listing: previous -> current/installed) — the release this gate +# is actually cutting has no tag yet at this point in RELEASING.md's flow, so "current" +# stands in for it. Rewinds the checkout to the previous tag, proves a genuine forward +# upgrade back to current through the same wire/path-unit/oneshot chain as the other +# legs, then restores the checkout to exactly where this phase found it (_upgrade_cleanup) +# — repeatable, unlike a target past current, so it can be the default. This is the leg +# that would have caught #318 (a legit upgrade being refused) and would catch a broken +# fetch/rebuild path before it ships. E2E_UPGRADE_TARGET=vX.Y.Z overrides the target +# explicitly (PERMANENT — does not restore, the pre-#350 shape, still useful for a +# deliberate real deploy); E2E_UPGRADE_SKIP_REASON="..." skips it with a logged reason. # # Sits after control (same restart churn perf must not measure through) and reuses control's # snapshot/cleanup machinery (CTL_ globals + _control_cleanup) — config is snapshotted and BOTH # control flags are forced off again on ANY exit, plus the upgrade-phase leftovers (probe tag, -# throttle stamp) are removed. Also the producer half of pithead#597's cross-repo tier-4 gate. +# throttle stamp, and #350's checkout rewind) are removed. Also the producer half of pithead#597's +# cross-repo tier-4 gate. # POST /upgrade {"version":} and poll /status?change_id= to a terminal status (echoed). # `started` (#320) is non-terminal — keep polling through it. Echoes "post-failed:" when @@ -834,6 +852,22 @@ _upg_post_and_poll() { # -> terminal status _upgrade_cleanup() { git -C "$HERE" tag -d v99.99.99 >/dev/null 2>&1 || true rm -f /var/lib/rigforge-control/upgrade-last 2>/dev/null || true + # #350: the auto-derived forward leg rewinds the checkout to a real previous tag to prove the + # forward step for real, then must land back on the exact ref this phase started from — on ANY + # exit, success or a hard abort mid-leg. A release gate must never leave the rig pinned to an + # older release. Restore BEFORE _control_cleanup's `apply` below, so apply runs the right code. + if [ -n "$UPG_ORIG_REF" ]; then + if [ "$(git -C "$HERE" rev-parse HEAD 2>/dev/null)" != "$UPG_ORIG_REF" ]; then + if git -C "$HERE" checkout --quiet --force "$UPG_ORIG_REF" 2>/dev/null; then + echo " restored the checkout to ${UPG_ORIG_REF:0:12} (the ref this phase started from)" + "$RIGFORGE" upgrade >/tmp/e2e-upgrade-restore.log 2>&1 || + echo " WARNING: 'rigforge.sh upgrade' failed while restoring the pre-leg ref (see /tmp/e2e-upgrade-restore.log)" >&2 + else + echo " WARNING: could not restore the checkout to ${UPG_ORIG_REF:0:12} — check $HERE by hand" >&2 + fi + fi + UPG_ORIG_REF="" + fi _control_cleanup } @@ -919,7 +953,13 @@ upgrade() { ok "miner service is active after the rollback" || bad "miner service is not active after the rollback" - if [ -n "${E2E_UPGRADE_TARGET:-}" ]; then + # #350: MANDATORY by default (was opt-in) — a broken fetch/rebuild path must fail the gate. See + # the phase header above for what each branch proves and why "current" stands in for the release + # actually being cut. + if [ -n "${E2E_UPGRADE_SKIP_REASON:-}" ]; then + phase "upgrade — forward leg: SKIPPED" + ok "SKIP forward leg — ${E2E_UPGRADE_SKIP_REASON}" + elif [ -n "${E2E_UPGRADE_TARGET:-}" ]; then phase "upgrade — forward leg: POST $E2E_UPGRADE_TARGET (PERMANENT — upgrades this checkout)" rm -f "$stamp" 2>/dev/null || true # the rollback leg stamped; this leg is operator-requested st=$(_upg_post_and_poll "$tok" "$control_port" "$E2E_UPGRADE_TARGET" 600) @@ -929,6 +969,35 @@ upgrade() { [ "v$(tr -d '[:space:]' <"$HERE/VERSION" 2>/dev/null)" = "$E2E_UPGRADE_TARGET" ] && ok "VERSION reads ${E2E_UPGRADE_TARGET#v} — the target landed" || bad "VERSION did not land at $E2E_UPGRADE_TARGET" + else + phase "upgrade — forward leg: auto-derive previous -> current real tags, prove a forward upgrade" + local all_tags cur_tag="v$installed" prev_tag + git -C "$HERE" fetch --quiet --tags origin 2>/dev/null || true + all_tags="$(git -C "$HERE" tag --list --sort=-v:refname 2>/dev/null | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | grep -vxF v99.99.99 || true)" + prev_tag="$(printf '%s\n' "$all_tags" | awk -v c="$cur_tag" '$0==c{f=1;next} f{print; exit}')" + if [ -z "$prev_tag" ]; then + ok "SKIP forward leg — fewer than two real release tags reachable yet (installed $cur_tag); nothing to prove a forward upgrade from" + else + # Rewind OUT OF BAND (direct git + rigforge.sh upgrade, the same two steps + # _control_upgrade_do takes) to a real prior release, so the control channel then has a + # genuinely older rig to upgrade forward from — D10's anti-rollback in control_upgrade() + # refuses a downgrade POST, so this step can only happen this way, same as a genuinely + # older rig got here. _upgrade_cleanup restores $UPG_ORIG_REF on ANY exit from here on. + UPG_ORIG_REF="$(git -C "$HERE" rev-parse HEAD)" + if git -C "$HERE" checkout --quiet --force "$prev_tag" 2>/dev/null && "$RIGFORGE" upgrade >/tmp/e2e-upgrade-rewind.log 2>&1; then + ok "rewound the checkout to $prev_tag (a real prior release, to prove the forward leg from)" + rm -f "$stamp" 2>/dev/null || true # the rollback leg stamped; this is a fresh attempt + st=$(_upg_post_and_poll "$tok" "$control_port" "$cur_tag" 600) + [ "$st" = applied ] && + ok "forward upgrade $prev_tag -> $cur_tag reached 'applied' (real fetch + build + health gate)" || + bad "forward leg ended '$st' (expected applied, $prev_tag -> $cur_tag)" + [ "v$(tr -d '[:space:]' <"$HERE/VERSION" 2>/dev/null)" = "$cur_tag" ] && + ok "VERSION reads ${cur_tag#v} — the forward leg landed" || + bad "VERSION did not land at $cur_tag after the forward leg" + else + bad "could not rewind the checkout to $prev_tag to stage the forward leg (see /tmp/e2e-upgrade-rewind.log)" + fi + fi fi # Explicit cleanup now (not just on exit) for the same reason control() does it — later phases From c421ac20e527e91da7d2f6249a6ce24f3ad737c6 Mon Sep 17 00:00:00 2001 From: Vijit Singh Date: Sat, 15 Aug 2026 16:17:56 -0500 Subject: [PATCH 3/4] test(e2e-real): pre-flight the checkout path's traversability for DynamicUser (#362) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found live (miner-2): running tests/e2e-real.sh from a checkout under $HOME (mode 750) makes the control phase fail 6 asserts confusingly — the receiver dies in a restart loop with "can't open file .../util/control- server.py: Permission denied", because rigforge-control.service and rigforge-api.service run as systemd DynamicUser and can't traverse the home directory. The phase then reads as a product failure (receiver down, POST 000, DONATION unchanged) when it's a harness-placement problem — nothing pointed at the path. require_traversable_checkout() walks every ancestor directory from the checkout up to /, checking the "others" execute bit on each (root can always stat regardless of permissions, so this never false-fails), and dies immediately with the fix (move the checkout to /opt) if any of them block traversal. Wired into require_linux_root(), which every phase already calls first — so a lone `control` or `upgrade` re-run gets the same immediate, honest failure, not just a fresh `provision`. Split into its own function (rather than inlined in require_linux_root) so it's unit-testable in isolation via the same extraction+eval technique tests/run.sh already uses for rig_lock/_perf_judge: `stat` is faked, so the test's result never depends on the real host's tmp/HOME permissions (which turned out to vary by OS during development — macOS's own mktemp -d default and its TMPDIR parent are both mode 700). One line added to the header comment block per the issue's ask. Co-Authored-By: Claude Fable 5 --- tests/e2e-real.sh | 27 ++++++++++++++++++++++++++ tests/run.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/tests/e2e-real.sh b/tests/e2e-real.sh index 5e1e3ed..2d47bd2 100755 --- a/tests/e2e-real.sh +++ b/tests/e2e-real.sh @@ -45,6 +45,12 @@ # E2E_UPGRADE_SKIP_REASON set (to a reason string) to skip the now-mandatory forward leg — an # explicit, logged escape hatch, not a silent bypass # +# The checkout itself must be traversable by an unprivileged user too (#362): rigforge-control.service +# and rigforge-api.service run as systemd DynamicUser, so a checkout under $HOME (typically mode 750) +# makes them die in a Permission-denied restart loop that reads as a product failure. Every phase +# pre-flights this and fails immediately with the fix if not — move the checkout somewhere +# world-traversable, e.g. /opt/rigforge-e2e. +# # Linux-only and root-only (kernel tuning, modprobe, apt). Typical flow on the release rig: # sudo bash tests/e2e-real.sh provision # sudo reboot # then reconnect @@ -156,10 +162,31 @@ _set_boot() { # return 1 } +# #362: rigforge-control.service and rigforge-api.service run as systemd DynamicUser (an +# unprivileged, ephemeral UID) and must traverse every directory from / down to the checkout to open +# util/*.py. A checkout under $HOME (typically mode 750) blocks that — the service dies in a restart +# loop with "Permission denied" and the control/upgrade phases then read as a product failure +# (receiver down, POST 000, DONATION unchanged) instead of a harness-placement problem. Checks every +# ancestor directory's o+x bit; root can always stat regardless of permissions, so this never +# false-fails. Split out from require_linux_root so it's unit-testable without root/Linux/a real rig. +require_traversable_checkout() { # + local _d="$1" _mode + while true; do + _mode="$(stat -c '%a' "$_d" 2>/dev/null)" || break + case "$_mode" in + *[1357]) ;; # last digit (others) has the execute bit set — traversable + *) die "checkout path is not traversable by an unprivileged user: '$_d' is mode $_mode (others lack +x). rigforge-control.service/rigforge-api.service run as DynamicUser and would fail to open files under $1. Move the checkout to a world-traversable path, e.g. /opt/rigforge-e2e." ;; + esac + [ "$_d" = / ] && break + _d="$(dirname "$_d")" + done +} + require_linux_root() { [ "$(uname -s)" = "Linux" ] || die "Linux-only (this host is $(uname -s)) — run on the release rig." [ "$(id -u)" -eq 0 ] || die "must run as root (kernel tuning / modprobe / apt): sudo bash tests/e2e-real.sh $*" [ -x "$RIGFORGE" ] || die "$RIGFORGE not found or not executable." + require_traversable_checkout "$HERE" # #362: called by every phase, not just provision } hugepages_total() { awk '/^HugePages_Total:/ {print $2; exit}' /proc/meminfo 2>/dev/null || echo 0; } diff --git a/tests/run.sh b/tests/run.sh index 20eaa56..a946e91 100644 --- a/tests/run.sh +++ b/tests/run.sh @@ -7483,6 +7483,55 @@ rc=$? assert_rc "record: first-ever recording needs no judge (#214)" "$rc" "0" assert_eq "record: first-ever wrote the baseline (#214)" "$(jq -r .bench_1m_hs "$PJ/tests/perf-baselines/$host.json")" "9000.0" +# #362: the DynamicUser services (control/api) can't traverse a checkout under a mode-750 $HOME — +# require_traversable_checkout catches it before any phase runs. Same extraction+eval technique as +# rig_lock below. `stat` is faked (not a real directory tree) so the result never depends on the REAL +# host's tmp/HOME permissions, which this exact suite run already showed vary by OS (#362 dev note: +# mktemp -d is 700 on macOS, and even its TMPDIR parent can be 700) — a real-directory version of this +# test would be hostage to whatever the CI runner or developer's box happens to have. +echo "== unit: require_traversable_checkout — DynamicUser traversal pre-flight (#362) ==" +RTC_SRC="$(sed -n '/^require_traversable_checkout()/,/^}/p' "$ROOT/tests/e2e-real.sh")" +if [ -z "$RTC_SRC" ]; then + bad "could not extract require_traversable_checkout from e2e-real.sh (#362)" "sed extraction was empty" +else + RTCD="$(mktemp -d "$SANDBOX/travcheck.XXXXXX")" + mkdir -p "$RTCD/bin" + # The exact shape #362 found live: the checkout itself (mode 750) blocks; its ancestors don't. + cat >"$RTCD/bin/stat" <<'EOF' +#!/usr/bin/env bash +case "$3" in +"/home/vijit/rigforge") echo 750 ;; +*) echo 755 ;; +esac +EOF + chmod +x "$RTCD/bin/stat" + out="$( ( + eval "$RTC_SRC" + die() { + echo "DIE: $1" + exit 2 + } + set +e + PATH="$RTCD/bin:$PATH" require_traversable_checkout /home/vijit/rigforge/tests + echo "rc=$?" + ) 2>&1)" + assert_contains "a mode-750 ancestor is caught (#362)" "$out" "DIE:" + assert_contains "names the exact blocking path and mode (#362)" "$out" "'/home/vijit/rigforge' is mode 750" + assert_contains "names the remedy (#362)" "$out" "/opt/rigforge-e2e" + out2="$( ( + eval "$RTC_SRC" + die() { + echo "DIE: $1" + exit 2 + } + set +e + PATH="$RTCD/bin:$PATH" require_traversable_checkout /opt/rigforge-e2e + echo "rc=$?" + ) 2>&1)" + assert_absent "a fully-traversable path never dies (#362)" "$out2" "DIE:" + assert_contains "a fully-traversable path returns cleanly (#362)" "$out2" "rc=0" +fi + echo "== unit: rig_lock — the shared-rig flock (#183) ==" RL_SRC="$(sed -n '/^rig_lock()/,/^}/p' "$ROOT/tests/e2e-real.sh")" assert_eq "e2e-real.sh and e2e-pithead.sh carry the identical helper (#183)" \ From bbf3e12d0d8d44280ade1c6af029549d1f9bcf3b Mon Sep 17 00:00:00 2001 From: Vijit Singh Date: Sat, 15 Aug 2026 16:18:36 -0500 Subject: [PATCH 4/4] docs(RELEASING): promise only the perf-baseline practice the fleet actually delivers (#354) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RELEASING.md said to record each rig's benchmark after every fleet re-tag. In practice only miner-0 (the dedicated release-gate rig, per tests/README.md) has a current entry every release; miner-1..7 stop at v1.11.1 — three releases of fleet perf history the fleet was never actually re-tagged for, or the recording step was skipped. Rewrote the paragraph to describe the real policy instead of inventing one nobody follows: record whichever rig(s) you actually re-tag, which in practice means miner-0 every time (the gate always runs there) plus whatever else happens to get touched that release. tests/perf-baselines/ legitimately carries gaps between releases for untouched rigs — that's now stated as expected, not implied to be a lapse. Co-Authored-By: Claude Fable 5 --- RELEASING.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/RELEASING.md b/RELEASING.md index 5a027ed..2578b9f 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -105,14 +105,19 @@ Pushing the tag triggers the release pipeline - creates the GitHub Release as a draft. Review the generated notes and bundles, then click Publish (pre-1.0 `0.x` tags are marked pre-release; `1.0.0`+ are full releases). -After the fleet is re-tagged, record each rig's benchmark for the release +After a rig is re-tagged, record its benchmark for the release (`E2E_PERF_TAG=vX.Y.Z E2E_PERF_RECORD=1 sudo bash tests/e2e-real.sh perf` on the rig) and commit the updated `tests/perf-baselines/` files — the per-release history is what lets the perf gate -catch slow drift across releases (see `tests/perf-baselines/README.md`). The recording is also +catch slow drift across releases (see `tests/perf-baselines/README.md`). In practice that means +miner-0 every time, since the release gate itself always runs there (see +[`tests/README.md`](./tests/README.md#the-shared-rig-miner-0)); the rest of the fleet isn't re-tagged +on every release, so its baselines are only as fresh as the last time each rig was actually +touched. `tests/perf-baselines/` legitimately carries gaps between releases for rigs that went +untouched — it is not a promise that every rig has an entry for every tag. The recording is also the per-rig perf gate (#214): it judges against the committed baseline and best-ever history before writing, refuses to record a regressed number (fix it, or consciously override with -`E2E_PERF_FORCE=1`), so a failed rig means investigate before calling the fleet healthy. Once the collected -baselines are merged, reset each rig's copy (`sudo git checkout -- tests/perf-baselines/` in +`E2E_PERF_FORCE=1`), so a failed rig means investigate before calling it healthy. Once a rig's +baseline is merged, reset its copy (`sudo git checkout -- tests/perf-baselines/` in `/opt/rigforge`): the recording dirties the rig's checkout, and the *next* release's `git checkout ` aborts on exactly those files (this bit both the v1.4.0 and v1.5.0 deploys).