From 66f7d5ad68a1e7036075273ac046755a97942d94 Mon Sep 17 00:00:00 2001 From: KrasimirKralev <263465593+KrasimirKralev@users.noreply.github.com> Date: Wed, 12 Aug 2026 14:59:35 +0300 Subject: [PATCH] fix: classify hermes dashboard-auth failures and serialise config writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed Hermes provision could not be told apart from a good one. Three independent defects made a box that provisioned badly look healthy: - The dashboard-auth check returned a bare exit 1 for every failure — a real password/hash mismatch, a config another writer had just rewritten, an unreadable file, or an interpreter without scrypt — and then blamed the credentials, which were correct. It now classifies the outcome: 'could not run the check' and 'the password does not match the hash' are distinct outcomes with distinct messages and exit codes, and an environment failure is never reported as a credential fault. Exposed as a reusable '--check' mode. - Root cause of the recurring failure: ~/.hermes/config.yaml has more than one writer (this script and register-mcp.sh, run seconds apart at install time), and a lost update erased the dashboard block between the write and the verify. Both scripts now take one shared flock over the config so their read-modify-write cycles serialise. - A non-fatal step_hermes_edition failure never reached the summary: install.sh printed 'Setup Complete' and exited 0 even after provisioning reported errors. It now records the failure, prints an INCOMPLETE summary, writes a provision-status marker, and exits non-zero so the flash host cannot report success over it. - step_validate_services could report every check healthy right after auth failed, because its only auth probe accepted the failure symptom. It now verifies the auth provider directly via the auth script's --check. Adds regression tests for each and keeps the existing behavioural tests green. --- install.sh | 117 +++++- scripts/register-mcp.sh | 36 ++ scripts/setup-hermes-dashboard-auth.sh | 365 +++++++++++++----- src/tests/unit/hermes-config-lock.test.ts | 118 ++++++ .../unit/hermes-dashboard-auth-yaml.test.ts | 176 +++++++++ .../unit/install-hermes-edition-step.test.ts | 87 +++++ 6 files changed, 805 insertions(+), 94 deletions(-) create mode 100644 src/tests/unit/hermes-config-lock.test.ts diff --git a/install.sh b/install.sh index b4663609..0c00c5a4 100755 --- a/install.sh +++ b/install.sh @@ -61,6 +61,37 @@ PROJECT_DIR="/home/clawbox/clawbox" CLAWBOX_USER="clawbox" CLAWBOX_HOME="/home/clawbox" +# ── Provisioning-status signal (read by the flash host) ────────────────────── +# A full install keeps some steps NON-FATAL on purpose — a half-provisioned box +# should still finish and come up reachable rather than abort mid-run. But +# "non-fatal" must never become "invisible": a step that reported errors has to +# reach the operator's summary AND the caller's exit status, or a flash host +# prints "Setup: 1/1 succeeded" over an install that told itself it was broken. +# So every non-fatal failure is recorded here, the summary lists them, install.sh +# exits non-zero, and a machine-readable marker is left for the flash host. +PROVISION_FAILURES=() +PROVISION_STATUS_FILE="${CLAWBOX_PROVISION_STATUS_FILE:-/etc/clawbox/provision-status}" + +record_provision_failure() { + PROVISION_FAILURES+=("$1") +} + +# Persist the final provisioning verdict where the flash host (or an operator, +# or the next update) can read it without re-parsing install.sh's stdout. +write_provision_status() { + local status="$1"; shift + local dir + dir="$(dirname "$PROVISION_STATUS_FILE")" + mkdir -p "$dir" 2>/dev/null || true + { + echo "# Written by install.sh at the end of a full install. Machine-readable." + echo "STATUS=$status" + echo "FAILED_STEPS=$*" + echo "TIMESTAMP=$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || true)" + } > "$PROVISION_STATUS_FILE" 2>/dev/null || true + chmod 644 "$PROVISION_STATUS_FILE" 2>/dev/null || true +} + # ── Edition (single-harness lock) ──────────────────────────────────────────── # openclaw | hermes | dual. "openclaw" is the native product (single, locked), # "hermes" is its own SKU, "dual" is premium (both harnesses + the runtime @@ -3038,18 +3069,52 @@ step_validate_services() { fi # Probe 4 (hermes only): the dashboard auth proxy actually answers. The # unit being "active" only means node started; without this, a proxy that - # crashed on its first request (or a dashboard with no auth provider) - # counted as a healthy install. Unauthenticated, so 401/403/3xx are the - # expected healthy answers — anything is fine except "no answer". + # crashed on its first request counted as a healthy install. An + # unauthenticated request (no clawbox_session) is answered by a healthy + # proxy with a 302 to /login, or a 403 from its origin/rebind guard — so + # 3xx/403 are healthy and "no answer" is not. 401 is NOT whitelisted: + # a browserless request never earns one from a healthy proxy, and the one + # place the proxy DOES emit 401 is when the SSO login desynced (see + # hermes-dashboard-proxy.js) — the exact failure this SKU must not hide. local proxy_code proxy_code=$(curl -sS --max-time 5 -o /dev/null -w '%{http_code}' \ http://127.0.0.1:"${HERMES_DASH_PROXY_PORT:-8090}"/ 2>/dev/null) || proxy_code="000" case "$proxy_code" in - 2*|3*|401|403) ;; + 2*|3*|403) ;; *) failed_probe+=("Hermes: dashboard proxy on :${HERMES_DASH_PROXY_PORT:-8090} returned HTTP $proxy_code") ;; esac fi + # Probe (hermes + dual): the dashboard auth PROVIDER is genuinely usable — + # the stored password actually verifies against the stored password_hash. + # This is the check that was missing when a Hermes provision printed + # "dashboard auth setup returned non-zero" and the validator, seconds later, + # reported every check healthy: the proxy liveness probe above returns 3xx + # whether or not the provider works, so it can see a dead node but never a + # desynced or absent auth provider. Runs the auth script's OWN classifier + # (`--check`), so there is a single source of truth for the invariant and no + # duplicated scrypt logic here. Runs as root, which can read the clawbox-owned + # 0600 config + password file. A self-healed box (the dashboard's + # ExecStartPre re-mints a coherent pair within this loop's retry window) + # passes honestly, because by then the invariant genuinely holds. + if has_hermes_harness; then + local auth_script="$PROJECT_DIR/scripts/setup-hermes-dashboard-auth.sh" + local auth_rc=0 + if [ -f "$auth_script" ]; then + HERMES_CONFIG="$CLAWBOX_HOME/.hermes/config.yaml" CLAWBOX_ROOT="$PROJECT_DIR" \ + bash "$auth_script" --check >/dev/null 2>&1 || auth_rc=$? + else + auth_rc=99 + fi + case "$auth_rc" in + 0) ;; + 3) failed_probe+=("Hermes: dashboard auth is DESYNCED — the stored password does not verify against the stored password_hash (setup-hermes-dashboard-auth.sh --check == 3); the dashboard SSO will 401. Fix: sudo bash $PROJECT_DIR/install.sh --step hermes_edition") ;; + 4) failed_probe+=("Hermes: no usable dashboard auth provider in $CLAWBOX_HOME/.hermes/config.yaml (--check == 4) — the dashboard refuses to start on its non-loopback bind without one. Fix: sudo bash $PROJECT_DIR/install.sh --step hermes_edition") ;; + 5) failed_probe+=("Hermes: the dashboard password file is missing or empty (--check == 5). Fix: sudo bash $PROJECT_DIR/install.sh --step hermes_edition") ;; + *) failed_probe+=("Hermes: could not verify the dashboard auth provider (setup-hermes-dashboard-auth.sh --check == $auth_rc, environment error — not a confirmed-healthy state)") ;; + esac + fi + # Probe: no unit belonging to ANOTHER edition is running here. # # Every check above asks whether this edition's own units are UP, so a @@ -3087,6 +3152,8 @@ step_validate_services() { if is_test_mode; then probe_count=1; fi # +3: gateway-inactive, gateway-port-silent, dashboard-proxy-answers. if is_hermes_edition; then probe_count=$(( probe_count + 3 )); fi + # +1 (hermes AND dual): the dashboard auth provider actually verifies. + if has_hermes_harness; then probe_count=$(( probe_count + 1 )); fi # One per foreign unit. Counted even when the unit is absent: "the other # harness is not here" is a check that ran and passed, and folding it into the # total is what stops the healthy line from being printable on a box that is @@ -3276,11 +3343,18 @@ if has_hermes_harness; then echo " # WARNING: Hermes provisioning FAILED." echo " # Re-run: sudo bash $PROJECT_DIR/install.sh --step hermes_edition" echo " ############################################################" + # Record it so the final summary + exit status + status marker report the + # failure even though the run continues. Without this the box could still + # print "Setup Complete", exit 0, and be shipped as healthy. + record_provision_failure hermes_edition } fi log "Validating services..." -step_validate_services +# Capture rather than let set -e abort here: we still want to print the summary +# AND fold this into the single honest exit status at the very end. +VALIDATE_RC=0 +step_validate_services || VALIDATE_RC=$? # ── Done ───────────────────────────────────────────────────────────────────── @@ -3301,3 +3375,36 @@ echo " systemctl status clawbox-ap" echo " systemctl status clawbox-setup" echo " systemctl status clawbox-gateway" echo "" + +# ── Honest final status ────────────────────────────────────────────────────── +# One exit code that reflects the WHOLE run: a non-fatal provisioning step that +# reported errors, or a failed service validation, must not be reportable as +# success by the caller (the flash host's "Setup: N/N succeeded"). The marker +# file carries the same verdict for a caller that reads a file instead of the +# exit code, and the sentinel line ([provision-status] ...) for one that greps +# stdout. Keep all three in agreement. +FINAL_RC=0 +if [ "${#PROVISION_FAILURES[@]}" -gt 0 ] || [ "${VALIDATE_RC:-0}" -ne 0 ]; then + FINAL_RC=1 +fi + +if [ "$FINAL_RC" -ne 0 ]; then + echo " ############################################################" + echo " # PROVISIONING INCOMPLETE — the box came up but the install" + echo " # reported errors. Do NOT ship this box as healthy." + if [ "${#PROVISION_FAILURES[@]}" -gt 0 ]; then + echo " # Steps that failed: ${PROVISION_FAILURES[*]}" + echo " # Re-run: sudo bash $PROJECT_DIR/install.sh --step ${PROVISION_FAILURES[0]}" + fi + if [ "${VALIDATE_RC:-0}" -ne 0 ]; then + echo " # Service validation FAILED (see the checks listed above)." + fi + echo " ############################################################" + write_provision_status incomplete "${PROVISION_FAILURES[*]:-}" + echo "[provision-status] INCOMPLETE${PROVISION_FAILURES[*]:+ (${PROVISION_FAILURES[*]})}" +else + write_provision_status ok "" + echo "[provision-status] OK" +fi + +exit "$FINAL_RC" diff --git a/scripts/register-mcp.sh b/scripts/register-mcp.sh index 17d6bbbd..28d78e6f 100755 --- a/scripts/register-mcp.sh +++ b/scripts/register-mcp.sh @@ -44,9 +44,40 @@ MCP_ENTRY="$PROJECT_DIR/mcp/clawbox-mcp.ts" MCP_TOKEN_FILE="$PROJECT_DIR/data/.mcp-token" EDITION_FILE="${CLAWBOX_EDITION_FILE:-/etc/clawbox/edition.env}" API_BASE="${CLAWBOX_API_BASE:-http://127.0.0.1:80}" +# Shared with setup-hermes-dashboard-auth.sh: BOTH scripts read-modify-write +# ~/.hermes/config.yaml, and at install time they run seconds apart +# (production-server.js fire-and-forgets this script on the clawbox-setup +# restart in step_start_services; setup-hermes-edition.sh runs the auth script +# right after). Without a shared lock, whichever one snapshotted the file first +# and wrote last silently erased the other's block — the auth script's dashboard +# block vanished and its verify failed, blaming credentials that were correct. +# Same path derivation on both sides (HERMES_CONFIG + ".lock") so they collide. +CONFIG_LOCK="${HERMES_CONFIG}.lock" log() { echo "[register-mcp] $*"; } +# Take the exclusive config lock for the rest of the run (fd 9, released on +# exit). Covers BOTH the PyYAML reconcile below AND the `hermes tools disable` +# call — the Hermes CLI does its own wide load→save_config on the same file, so +# it has to be inside the same critical section. Best-effort: proceed without +# the lock rather than skip registering the device's tools if flock is missing. +acquire_config_lock() { + command -v flock >/dev/null 2>&1 || { + log "flock unavailable — proceeding without the config lock" + return 0 + } + mkdir -p "$(dirname "$CONFIG_LOCK")" 2>/dev/null || true + # Probe writability in a scoped subshell before opening fd 9; keep the `exec` + # redirect CLEAN (a `2>/dev/null` on it would silence the whole script, + # because redirections on exec are permanent). + if ! ( : > "$CONFIG_LOCK" ) 2>/dev/null; then + log "could not create $CONFIG_LOCK — proceeding without the config lock" + return 0 + fi + exec 9>"$CONFIG_LOCK" + flock -w 120 9 || log "could not acquire $CONFIG_LOCK within 120s — proceeding without it" +} + # ── 1. Which edition is this? ─────────────────────────────────────────────── # Root-owned lock first, environment second, "openclaw" last — the same order # and the same reasons as src/lib/edition-source.ts. Reading the file rather @@ -112,6 +143,11 @@ chmod 600 "$MCP_TOKEN_FILE" 2>/dev/null || true # NOT via `hermes mcp add`: that command performs live tool discovery and # rewrites the whole config through Hermes' own save_config(), which is a much # wider blast radius for a boot-time provisioning step, and it is slow. +# +# Everything from here to the end of the script touches config.yaml, so take the +# shared lock now and hold it until exit. +acquire_config_lock + export CLAWBOX_MCP_HERMES_CONFIG="$HERMES_CONFIG" export CLAWBOX_MCP_BUN_BIN="$BUN_BIN" export CLAWBOX_MCP_ENTRY="$MCP_ENTRY" diff --git a/scripts/setup-hermes-dashboard-auth.sh b/scripts/setup-hermes-dashboard-auth.sh index ecc5f7d9..232bf6cc 100644 --- a/scripts/setup-hermes-dashboard-auth.sh +++ b/scripts/setup-hermes-dashboard-auth.sh @@ -24,98 +24,177 @@ # re-running the script never repaired it because the gate passed again. # The check below therefore VERIFIES the pair instead of merely observing that # both artefacts exist, and minting a password now always rewrites the block. +# +# TWO THINGS THIS SCRIPT LEARNED THE HARD WAY, both about ~/.hermes/config.yaml +# having MORE THAN ONE WRITER: +# +# 1. The write and the verify race other writers. register-mcp.sh (run +# fire-and-forget by production-server.js on every web-server boot), the +# Hermes CLI's own load→save_config, and the /setup-api/hermes/* routes all +# do read-modify-write on this same file. A writer that snapshotted the +# file BEFORE we wrote our block, and lands its os.replace AFTER, silently +# erases the block — a lost update. The verify then re-reads a config with +# no dashboard block and fails, and the OLD message blamed the credentials, +# which were never wrong. So this script now takes an flock over +# $CONFIG_LOCK across its whole critical section, and register-mcp.sh takes +# the SAME lock. Cooperating writers serialise; the block survives. +# +# 2. Even with the cause fixed, the CHECK must be honest. "I could not run the +# check" (python/scrypt missing, config unreadable) and "another process +# erased the block I just wrote" and "the password genuinely does not match +# the hash" are three different failures with three different fixes. The +# old check collapsed all of them — plus a truncated password file, plus a +# missing python3 — into one exit 1 and one message that asserted the one +# thing that is definitely true (both artefacts are correct). classify the +# outcome instead, so the next unknown failure describes itself. set -euo pipefail PROJECT_DIR="${CLAWBOX_ROOT:-/home/clawbox/clawbox}" HERMES_CONFIG="${HERMES_CONFIG:-$HOME/.hermes/config.yaml}" PWFILE="$PROJECT_DIR/data/.hermes-dashboard-pw" USERNAME="${HERMES_DASH_USERNAME:-clawbox}" +# One lock file, next to the config, shared by every ClawBox writer of +# ~/.hermes/config.yaml (this script + register-mcp.sh). A writer computes it +# from the SAME HERMES_CONFIG path it is about to touch, so both land on the +# same file without a hard-coded absolute path. +CONFIG_LOCK="${HERMES_CONFIG}.lock" log() { echo "[hermes-dash-auth] $*"; } -# ── Already correctly configured? ─────────────────────────────────────────── -# Not "do both artefacts exist" but "does the stored hash actually verify the -# stored password". That is the only check that can't leave a desynced pair -# behind, and it also repairs a truncated/corrupted password file. -creds_are_consistent() { - [ -s "$PWFILE" ] || return 1 - [ -f "$HERMES_CONFIG" ] || return 1 - CFG="$HERMES_CONFIG" PW_PATH="$PWFILE" python3 - <<'PY' +# ── Serialise all writers of config.yaml ──────────────────────────────────── +# Hold an exclusive flock for the LIFE of the script (fd 9, released on exit), +# so the early read, the mint, and the verify are one atomic critical section +# against the other cooperating writer (register-mcp.sh, same lock file). +# Best-effort: if flock is unavailable, or we can't get it inside the window, we +# proceed WITHOUT it rather than leave the dashboard with no auth provider — the +# honest classification below then still describes a lost write correctly if one +# happens. `flock` is util-linux and present on the Jetson image and on CI. +acquire_config_lock() { + command -v flock >/dev/null 2>&1 || { + log "flock unavailable — proceeding without the config lock" + return 0 + } + mkdir -p "$(dirname "$CONFIG_LOCK")" 2>/dev/null || true + # Probe writability in a SUBSHELL first (its redirect is scoped, so a failure + # can't abort the install and can't leak past this line). Only then open fd 9. + if ! ( : > "$CONFIG_LOCK" ) 2>/dev/null; then + log "could not create $CONFIG_LOCK — proceeding without the config lock" + return 0 + fi + # Open fd 9 for the LIFE of the script (that permanence is the point — the lock + # must outlive this function). Redirections on `exec` are permanent, so this + # line must carry NO other redirect: `exec 9>file 2>/dev/null` would silence + # the whole script's stderr, hiding every error message below. + exec 9>"$CONFIG_LOCK" + flock -w 120 9 || log "could not acquire $CONFIG_LOCK within 120s — proceeding without it" +} + +# ── Classify the stored (password, hash) pair ─────────────────────────────── +# Returns a code that names WHICH kind of not-consistent, never a bare 1: +# 0 MATCH the stored hash verifies the stored password +# 3 MISMATCH hash is well-formed and scrypt ran, but pw != hash +# 4 NOT_CONFIGURED no dashboard block, or no/!corrupt password_hash/secret +# 5 PW_MISSING the password file is absent or empty +# 6 CONFIG_UNREADABLE config.yaml exists but could not be read (permissions) +# 7 COMPUTE_ERROR scrypt itself could not run in this interpreter +# 127 python3 is not on PATH (the shell reports this for us) +CREDS_MATCH=0 +CREDS_MISMATCH=3 +CREDS_NOT_CONFIGURED=4 +CREDS_PW_MISSING=5 +CREDS_CONFIG_UNREADABLE=6 +CREDS_COMPUTE_ERROR=7 + +classify_creds() { + [ -f "$HERMES_CONFIG" ] || return "$CREDS_NOT_CONFIGURED" + [ -s "$PWFILE" ] || return "$CREDS_PW_MISSING" + local rc=0 + CFG="$HERMES_CONFIG" PW_PATH="$PWFILE" \ + NOT_CONFIGURED="$CREDS_NOT_CONFIGURED" PW_MISSING="$CREDS_PW_MISSING" \ + CONFIG_UNREADABLE="$CREDS_CONFIG_UNREADABLE" COMPUTE_ERROR="$CREDS_COMPUTE_ERROR" \ + MISMATCH="$CREDS_MISMATCH" \ + python3 - <<'PY' || rc=$? import base64, hashlib, os, re, sys +NOT_CONFIGURED = int(os.environ["NOT_CONFIGURED"]) +PW_MISSING = int(os.environ["PW_MISSING"]) +CONFIG_UNREADABLE = int(os.environ["CONFIG_UNREADABLE"]) +COMPUTE_ERROR = int(os.environ["COMPUTE_ERROR"]) +MISMATCH = int(os.environ["MISMATCH"]) + try: with open(os.environ["CFG"], "r", encoding="utf-8") as fh: cfg = fh.read() +except OSError: + sys.exit(CONFIG_UNREADABLE) +try: with open(os.environ["PW_PATH"], "r", encoding="utf-8") as fh: pw = fh.read().strip() except OSError: - sys.exit(1) - + sys.exit(PW_MISSING) if not pw: - sys.exit(1) + sys.exit(PW_MISSING) # Pull the password_hash out of the top-level `dashboard:` block only, so an # unrelated hash elsewhere in the config can't make us think we're configured. block = re.search(r"(?m)^dashboard:[ \t]*\n((?:[ \t].*\n|[ \t]*\n)*)", cfg) if not block: - sys.exit(1) + # The block is simply not here: either never written, or a concurrent + # writer erased it. Either way there is no credential to compare against. + sys.exit(NOT_CONFIGURED) found = re.search(r"(?m)^\s*password_hash:\s*[\"']?([^\"'\s]+)[\"']?\s*$", block.group(1)) secret = re.search(r"(?m)^\s*secret:\s*[\"']?([^\"'\s]+)[\"']?\s*$", block.group(1)) if not found or not secret: - sys.exit(1) + sys.exit(NOT_CONFIGURED) parts = found.group(1).split("$") if len(parts) != 6 or parts[0] != "scrypt": - sys.exit(1) + sys.exit(NOT_CONFIGURED) try: n, r, p = int(parts[1]), int(parts[2]), int(parts[3]) salt = base64.b64decode(parts[4]) expected = base64.b64decode(parts[5]) +except Exception: + # A hash is present but its fields don't parse — a corrupt block, not a + # broken environment. Treat it as "not configured" so we re-mint. + sys.exit(NOT_CONFIGURED) +try: dk = hashlib.scrypt(pw.encode(), salt=salt, n=n, r=r, p=p, dklen=len(expected), maxmem=0) except Exception: - sys.exit(1) + # scrypt genuinely could not run here (missing OpenSSL scrypt, a memory + # limit, a uv-managed interpreter without it). This is an ENVIRONMENT + # failure — it says nothing about whether the stored pair is correct. + sys.exit(COMPUTE_ERROR) -sys.exit(0 if dk == expected else 1) +sys.exit(0 if dk == expected else MISMATCH) PY + return "$rc" } -if creds_are_consistent; then - log "already configured (stored hash verifies the stored password) — skipping" - # Still re-assert the file modes: config.yaml carries the dashboard's - # session-signing secret and the scrypt hash, and the heredoc that used to - # write it inherited the caller's umask, leaving it world-readable (0664 on - # the shipping device). Anyone who can read as any user could forge a - # dashboard session cookie from it. - chmod 600 "$HERMES_CONFIG" 2>/dev/null || true - for bak in "$HERMES_CONFIG".bak*; do - [ -f "$bak" ] && chmod 600 "$bak" 2>/dev/null || true - done - chmod 600 "$PWFILE" 2>/dev/null || true - exit 0 -fi +# ── Mint a fresh (password, hash, secret) and install it ──────────────────── +# Returns 0 once both artefacts are on disk; 4 for an environment failure that +# stopped generation (scrypt/python), 1 for a write failure. Does NOT verify — +# the caller does that so a lost write can be told apart from a bad one. +mint_credentials() { + mkdir -p "$(dirname "$PWFILE")" + mkdir -p "$(dirname "$HERMES_CONFIG")" -mkdir -p "$(dirname "$PWFILE")" -mkdir -p "$(dirname "$HERMES_CONFIG")" - -# A missing config is no longer a reason to give up. It is the NORMAL state -# both on a fresh flash (install.sh clones Hermes, but config.yaml only appears -# on the first `hermes` run) and after a factory reset (which wipes ~/.hermes -# precisely because it holds the previous owner's provider keys, OAuth tokens -# and chat DB). Skipping here meant the dashboard came up on its non-loopback -# bind with no auth provider and crash-looped, with nothing to repair it. -# Creating a config that contains only our dashboard block is safe: Hermes -# merges its own defaults for everything else. -if [ ! -f "$HERMES_CONFIG" ]; then - log "$HERMES_CONFIG not found — creating one with just the dashboard block" - : > "$HERMES_CONFIG" - chmod 600 "$HERMES_CONFIG" -fi + # A missing config is the NORMAL state on a fresh flash (config.yaml only + # appears on the first `hermes` run) and after a factory reset (which wipes + # ~/.hermes). Create one holding only our dashboard block: Hermes merges its + # own defaults for everything else. + if [ ! -f "$HERMES_CONFIG" ]; then + log "$HERMES_CONFIG not found — creating one with just the dashboard block" + : > "$HERMES_CONFIG" + chmod 600 "$HERMES_CONFIG" + fi -# Generate password + scrypt password_hash + token-signing secret. The hash -# format matches Hermes's plugins.dashboard_auth.basic.hash_password -# (scrypt$n$r$p$salt_b64$dk_b64) but uses only stdlib so we don't depend on the -# Hermes venv/plugin path. -GEN="$(python3 - <<'PY' + # Generate password + scrypt password_hash + token-signing secret. The hash + # format matches Hermes's plugins.dashboard_auth.basic.hash_password + # (scrypt$n$r$p$salt_b64$dk_b64) but uses only stdlib so we don't depend on + # the Hermes venv/plugin path. + local gen + if ! gen="$(python3 - <<'PY' import secrets, base64, hashlib pw = secrets.token_urlsafe(24) salt = secrets.token_bytes(16) @@ -125,26 +204,28 @@ print(pw) print(h) print(secrets.token_urlsafe(32)) PY -)" -PW="$(printf '%s\n' "$GEN" | sed -n '1p')" -HASH="$(printf '%s\n' "$GEN" | sed -n '2p')" -SECRET="$(printf '%s\n' "$GEN" | sed -n '3p')" - -if [ -z "$PW" ] || [ -z "$HASH" ] || [ -z "$SECRET" ]; then - log "ERROR: failed to generate credentials" >&2 - exit 1 -fi + )"; then + log "ERROR: could not generate credentials — python3/hashlib.scrypt failed in this environment. This is NOT a credential problem." >&2 + return 4 + fi -# Install the dashboard block FIRST, then the plaintext. If the rewrite fails we -# exit non-zero having changed nothing the proxy depends on, rather than leaving -# a new plaintext next to an old hash — the exact desync this script guards -# against. -# -# REPLACE, never append: a second top-level `dashboard:` key is invalid YAML -# (and, depending on the loader, silently shadows the first), so an append-only -# path could only ever be run once. Written via a temp file + rename so a crash -# mid-write can't truncate the customer's config. -CFG="$HERMES_CONFIG" USERNAME="$USERNAME" HASH="$HASH" SECRET="$SECRET" PW_PATH="$PWFILE" python3 - <<'PY' || { log "ERROR: failed to write the dashboard block to $HERMES_CONFIG" >&2; exit 1; } + local pw hash secret + pw="$(printf '%s\n' "$gen" | sed -n '1p')" + hash="$(printf '%s\n' "$gen" | sed -n '2p')" + secret="$(printf '%s\n' "$gen" | sed -n '3p')" + if [ -z "$pw" ] || [ -z "$hash" ] || [ -z "$secret" ]; then + log "ERROR: the credential generator produced empty output (environment error)." >&2 + return 4 + fi + + # Install the dashboard block FIRST, then the plaintext. If the rewrite fails + # we return non-zero having changed nothing the proxy depends on, rather than + # leaving a new plaintext next to an old hash. + # + # REPLACE, never append: a second top-level `dashboard:` key is invalid YAML + # (and, depending on the loader, silently shadows the first). Written via a + # temp file + rename so a crash mid-write can't truncate the config. + if ! CFG="$HERMES_CONFIG" USERNAME="$USERNAME" HASH="$hash" SECRET="$secret" PW_PATH="$PWFILE" python3 - <<'PY'; then import json, os, re, sys cfg_path = os.environ["CFG"] @@ -195,25 +276,131 @@ except Exception as exc: print("write failed: %s" % exc, file=sys.stderr) sys.exit(1) PY + log "ERROR: failed to write the dashboard block to $HERMES_CONFIG" >&2 + return 1 + fi -# config.yaml now holds the session-signing secret and the password hash. -chmod 600 "$HERMES_CONFIG" -for bak in "$HERMES_CONFIG".bak*; do - [ -f "$bak" ] && chmod 600 "$bak" 2>/dev/null || true -done -log "wrote dashboard.basic_auth to $HERMES_CONFIG (username=$USERNAME, ttl=7d, mode 600)" - -# Store the plaintext password for the proxy ONLY (clawbox-owned, 0600). -umask 077 -printf '%s' "$PW" > "$PWFILE" -chmod 600 "$PWFILE" -log "wrote $PWFILE (0600)" - -# Prove the pair we just installed actually verifies, so a bug here surfaces -# now rather than as a permanent 401 loop at the proxy. -if ! creds_are_consistent; then - log "ERROR: freshly written password and hash do not verify — refusing to report success" >&2 - exit 1 + # config.yaml now holds the session-signing secret and the password hash. + chmod 600 "$HERMES_CONFIG" + for bak in "$HERMES_CONFIG".bak*; do + [ -f "$bak" ] && chmod 600 "$bak" 2>/dev/null || true + done + + # Store the plaintext password for the proxy ONLY (clawbox-owned, 0600). + ( umask 077; printf '%s' "$pw" > "$PWFILE" ) + chmod 600 "$PWFILE" + return 0 +} + +# ── Re-assert the file modes of an already-correct install ────────────────── +reassert_modes() { + # config.yaml carries the dashboard's session-signing secret and the scrypt + # hash; a world-readable copy lets anyone forge a session cookie. + chmod 600 "$HERMES_CONFIG" 2>/dev/null || true + for bak in "$HERMES_CONFIG".bak*; do + [ -f "$bak" ] && chmod 600 "$bak" 2>/dev/null || true + done + chmod 600 "$PWFILE" 2>/dev/null || true +} + +# ── Read-only check mode (--check) ────────────────────────────────────────── +# Report whether the dashboard auth provider is genuinely usable, without +# changing anything. This is the ONE source of truth for the invariant, reused +# by install.sh's step_validate_services so the validator and the provisioner +# agree on what "healthy" means. Exit code IS the classification (see the table +# above): 0 usable, 3 desynced, 4 not configured, 5 no password, 6/7/127 the +# check could not run. No write lock — a point-in-time read is enough, and the +# caller retries in its own loop. +if [ "${1:-}" = "--check" ] || [ "${1:-}" = "--verify" ]; then + check_rc=0 + classify_creds || check_rc=$? + case "$check_rc" in + "$CREDS_MATCH") log "check: OK — the stored password verifies against the stored password_hash" ;; + "$CREDS_MISMATCH") log "check: DESYNCED — the stored password does NOT match the stored password_hash" >&2 ;; + "$CREDS_NOT_CONFIGURED") log "check: NOT CONFIGURED — no usable dashboard.basic_auth block in $HERMES_CONFIG" >&2 ;; + "$CREDS_PW_MISSING") log "check: NO PASSWORD — $PWFILE is missing or empty" >&2 ;; + "$CREDS_CONFIG_UNREADABLE")log "check: CONFIG UNREADABLE — $HERMES_CONFIG could not be read (environment error)" >&2 ;; + "$CREDS_COMPUTE_ERROR"|127)log "check: COULD NOT RUN — python3/hashlib.scrypt unavailable (environment error, not a credential verdict)" >&2 ;; + *) log "check: unexpected status $check_rc" >&2 ;; + esac + exit "$check_rc" fi -log "done" +# ── Main ──────────────────────────────────────────────────────────────────── +acquire_config_lock + +gate_rc=0 +classify_creds || gate_rc=$? +case "$gate_rc" in + "$CREDS_MATCH") + log "already configured (stored hash verifies the stored password) — skipping" + reassert_modes + exit 0 + ;; + "$CREDS_CONFIG_UNREADABLE") + log "ERROR: $HERMES_CONFIG exists but could not be read (environment/permission error) — cannot safely (re)configure. This is NOT a credential problem." >&2 + exit 4 + ;; + "$CREDS_COMPUTE_ERROR"|127) + log "ERROR: could not run the credential check — python3/hashlib.scrypt is unavailable or failed in this environment. Refusing to guess; this is NOT a credential problem." >&2 + exit 4 + ;; + *) + # NOT_CONFIGURED / MISMATCH / PW_MISSING — the normal "(re)mint" path. + ;; +esac + +# Mint, then verify what actually landed on disk. Because config.yaml has more +# than one writer, a verify failure is classified rather than blamed on the +# credentials: the same generator wrote both artefacts moments ago, so a real +# scrypt mismatch here would be a hashing bug — while a vanished block is a lost +# write by another process. Retry the lost-write cases a few times (we hold the +# lock; a cooperating writer cannot clobber us — this only fires if flock is +# unavailable or a non-cooperating writer is racing). +attempt=0 +max_attempts=3 +while :; do + attempt=$((attempt + 1)) + + mint_rc=0 + mint_credentials || mint_rc=$? + if [ "$mint_rc" -ne 0 ]; then + # 4 = environment (already logged), 1 = write failure (already logged). + exit "$mint_rc" + fi + log "wrote dashboard.basic_auth to $HERMES_CONFIG (username=$USERNAME, ttl=7d, mode 600)" + log "wrote $PWFILE (0600)" + + verify_rc=0 + classify_creds || verify_rc=$? + case "$verify_rc" in + "$CREDS_MATCH") + log "done" + exit 0 + ;; + "$CREDS_MISMATCH") + log "ERROR: the password we just wrote does not verify against the password_hash we just wrote. This is a hashing fault in this script, NOT an environment problem and NOT a lost write." >&2 + exit 2 + ;; + "$CREDS_NOT_CONFIGURED"|"$CREDS_PW_MISSING") + if [ "$attempt" -lt "$max_attempts" ]; then + log "WARNING: the dashboard auth block / password file we just wrote to $HERMES_CONFIG is already gone — a concurrent process rewrote the file. The credentials generated were correct; the write was lost. Retrying under $CONFIG_LOCK (attempt $attempt/$max_attempts)." >&2 + continue + fi + log "ERROR: after $attempt attempts our dashboard auth block did not survive in $HERMES_CONFIG — another process keeps rewriting the file and is not honouring $CONFIG_LOCK. This is a write/serialisation fault, NOT a credential mismatch: the password and hash were correct on every attempt." >&2 + exit 3 + ;; + "$CREDS_CONFIG_UNREADABLE") + log "ERROR: could not read $HERMES_CONFIG to verify what we just wrote (environment/permission error) — NOT a credential mismatch." >&2 + exit 4 + ;; + "$CREDS_COMPUTE_ERROR"|127) + log "ERROR: could not run the verification — python3/hashlib.scrypt is unavailable or failed. The stored password and hash may well be correct; this is an environment error, NOT a credential mismatch." >&2 + exit 4 + ;; + *) + log "ERROR: the credential check returned an unexpected status ($verify_rc)." >&2 + exit 1 + ;; + esac +done diff --git a/src/tests/unit/hermes-config-lock.test.ts b/src/tests/unit/hermes-config-lock.test.ts new file mode 100644 index 00000000..3ff21e82 --- /dev/null +++ b/src/tests/unit/hermes-config-lock.test.ts @@ -0,0 +1,118 @@ +import { describe, expect, it } from "vitest"; +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +/** + * ~/.hermes/config.yaml has MORE THAN ONE writer. At install time the auth + * script (setup-hermes-dashboard-auth.sh) and the MCP registrar + * (register-mcp.sh, fire-and-forgotten by production-server.js on the + * clawbox-setup restart) both read-modify-write it seconds apart. Whichever one + * snapshotted the file first and wrote last silently erased the other's block — + * a lost update. That is what erased the dashboard block between the auth + * script's write and its verify, making it look like the credentials were wrong. + * + * The fix is a single flock both scripts take, derived from the SAME config + * path so they always collide on one lock file. These pin the contract and the + * runtime behaviour. + */ +const REPO = process.cwd(); +const AUTH = path.join(REPO, "scripts", "setup-hermes-dashboard-auth.sh"); +const REGISTER = path.join(REPO, "scripts", "register-mcp.sh"); +const AUTH_SRC = fs.readFileSync(AUTH, "utf-8"); +const REGISTER_SRC = fs.readFileSync(REGISTER, "utf-8"); + +const RUNNABLE = + process.platform !== "win32" && + spawnSync("bash", ["-c", "command -v python3"], { encoding: "utf-8" }).status === 0; +const FLOCK = + RUNNABLE && spawnSync("bash", ["-c", "command -v flock"], { encoding: "utf-8" }).status === 0; + +describe("both writers share ONE lock file", () => { + it("derive the lock from the same config path, so they collide", () => { + // Same right-hand side in both scripts => same absolute lock file for the + // same config. If one ever changes this expression, they stop excluding + // each other and the lost update comes back. + expect(AUTH_SRC).toContain('CONFIG_LOCK="${HERMES_CONFIG}.lock"'); + expect(REGISTER_SRC).toContain('CONFIG_LOCK="${HERMES_CONFIG}.lock"'); + }); + + it("both take the lock before they touch config.yaml", () => { + // The auth script acquires before its mint; the registrar acquires before + // its PyYAML reconcile AND holds it across the `hermes tools disable` CLI + // call (which does its own wide load->save_config on the same file). + expect(AUTH_SRC).toMatch(/acquire_config_lock[\s\S]*mint_credentials/); + const regTail = REGISTER_SRC.slice(REGISTER_SRC.indexOf("acquire_config_lock\n\nexport")); + expect(regTail).toContain("acquire_config_lock"); + expect(REGISTER_SRC.indexOf("acquire_config_lock\n\nexport")).toBeLessThan( + REGISTER_SRC.indexOf("tools disable browser"), + ); + expect(AUTH_SRC).toContain("flock -w 120 9"); + expect(REGISTER_SRC).toContain("flock -w 120 9"); + }); + + it("keeps the exec that opens the lock fd free of a stderr redirect", () => { + // Redirections on `exec` are permanent: `exec 9>file 2>/dev/null` would + // silence the whole script and hide every error message. Guard against that + // exact regression in both scripts. + expect(AUTH_SRC).not.toMatch(/exec 9>"\$CONFIG_LOCK"\s+2>/); + expect(REGISTER_SRC).not.toMatch(/exec 9>"\$CONFIG_LOCK"\s+2>/); + expect(AUTH_SRC).toContain('exec 9>"$CONFIG_LOCK"'); + expect(REGISTER_SRC).toContain('exec 9>"$CONFIG_LOCK"'); + }); +}); + +describe.runIf(RUNNABLE)("the lock is really taken at runtime", () => { + it("opens the lock file beside the config", () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "clawbox-lock-")); + const configPath = path.join(root, "hermes", "config.yaml"); + const proc = spawnSync("bash", [AUTH], { + encoding: "utf-8", + env: { ...process.env, CLAWBOX_ROOT: root, HERMES_CONFIG: configPath }, + }); + expect(proc.status, proc.stderr).toBe(0); + // The lock file exists next to the config (it is opened even on the happy + // path). On a box without flock the script logs and skips — tolerate that. + if (FLOCK) expect(fs.existsSync(`${configPath}.lock`)).toBe(true); + }); + + it.runIf(FLOCK)("waits for a held lock instead of racing through it", () => { + // Hold the shared lock for ~800ms in the background, then run the auth + // script. If it honours the lock it blocks until release (elapsed ≳ hold); + // if it ignored it, it would finish in well under 300ms. This is the mutual + // exclusion that stops the lost update. + const root = fs.mkdtempSync(path.join(os.tmpdir(), "clawbox-lockwait-")); + const configPath = path.join(root, "hermes", "config.yaml"); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + // Pre-seed a foreign writer's content; it must survive the auth script's + // write (the auth script preserves unrelated top-level keys). + fs.writeFileSync(configPath, "mcp_servers:\n clawbox:\n enabled: true\n"); + const lockFile = `${configPath}.lock`; + + // One bash driver so the holder and the auth script actually overlap: launch + // a background holder that takes the lock for ~0.8s, wait 0.15s so it wins + // the lock first, then run the auth script and time how long it blocks. + const script = ` + set -e + LOCK="${lockFile}" + ( exec 9>"$LOCK"; flock 9; sleep 0.8 ) & + hold=$! + sleep 0.15 # ensure the holder has the lock first + start=$(date +%s.%N) + CLAWBOX_ROOT="${root}" HERMES_CONFIG="${configPath}" bash "${AUTH}" >/dev/null 2>&1 + end=$(date +%s.%N) + wait $hold + awk -v s="$start" -v e="$end" 'BEGIN{printf "%.3f", e - s}' + `; + const proc = spawnSync("bash", ["-c", script], { encoding: "utf-8", timeout: 20000 }); + const elapsed = parseFloat(proc.stdout.trim() || "0"); + // Held ~0.8s, started ~0.15s in, so the auth script should wait ≳0.5s. + expect(elapsed).toBeGreaterThan(0.5); + + // And the foreign writer's key survived alongside the new dashboard block. + const config = fs.readFileSync(configPath, "utf-8"); + expect(config).toMatch(/^dashboard:/m); + expect(config).toMatch(/^mcp_servers:/m); + }); +}); diff --git a/src/tests/unit/hermes-dashboard-auth-yaml.test.ts b/src/tests/unit/hermes-dashboard-auth-yaml.test.ts index a7f9951c..f3704b2d 100644 --- a/src/tests/unit/hermes-dashboard-auth-yaml.test.ts +++ b/src/tests/unit/hermes-dashboard-auth-yaml.test.ts @@ -34,6 +34,82 @@ function run(root: string, configPath: string, username?: string) { return spawnSync("bash", [SCRIPT], { encoding: "utf-8", env }); } +/** Run the read-only classifier (`--check`) against a given root + config. */ +function check(root: string, configPath: string, extraEnv: NodeJS.ProcessEnv = {}) { + const env: NodeJS.ProcessEnv = { + ...process.env, + CLAWBOX_ROOT: root, + HERMES_CONFIG: configPath, + ...extraEnv, + }; + return spawnSync("bash", [SCRIPT, "--check"], { encoding: "utf-8", env }); +} + +const REAL_PYTHON = + spawnSync("bash", ["-c", "command -v python3"], { encoding: "utf-8" }).stdout.trim() || + "/usr/bin/python3"; + +/** + * A dir holding a `python3` whose `hashlib.scrypt` always raises — the "the + * check could not run in THIS interpreter" case the diagnosis worried about + * (a uv-managed python without OpenSSL scrypt). Absolute shebang so the stub + * never re-resolves to itself on PATH. Returns the dir to prepend to PATH. + */ +function scryptBrokenPythonDir(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "clawbox-nopy-")); + fs.writeFileSync( + path.join(dir, "python3"), + [ + `#!${REAL_PYTHON}`, + "import hashlib, sys", + 'def boom(*a, **k): raise ValueError("scrypt unavailable in this interpreter")', + "hashlib.scrypt = boom", + "src = sys.stdin.read()", + 'exec(compile(src, "", "exec"), {"__name__": "__main__"})', + "", + ].join("\n"), + { mode: 0o755 }, + ); + return dir; +} + +/** A valid scrypt password_hash for a chosen plaintext, via the real python. */ +function scryptHash(plaintext: string): string { + const proc = spawnSync( + "python3", + [ + "-c", + [ + "import base64,hashlib,sys", + "pw=sys.argv[1].encode(); salt=bytes(range(16))", + "dk=hashlib.scrypt(pw,salt=salt,n=2**14,r=8,p=1,dklen=32,maxmem=0)", + "print('scrypt$%d$%d$%d$%s$%s'%(2**14,8,1,base64.b64encode(salt).decode(),base64.b64encode(dk).decode()))", + ].join("\n"), + plaintext, + ], + { encoding: "utf-8" }, + ); + return proc.stdout.trim(); +} + +/** Write a self-consistent-looking dashboard block for a given hash. */ +function seedBlock(configPath: string, passwordHash: string) { + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync( + configPath, + [ + "dashboard:", + " basic_auth:", + ' username: "clawbox"', + ` password_hash: "${passwordHash}"`, + ' secret: "deadbeef"', + " session_ttl_seconds: 604800", + "", + ].join("\n"), + { mode: 0o600 }, + ); +} + /** Provision a throwaway root once and return what the script wrote. */ function provision(username: string) { const { root, configPath } = makeRoot(); @@ -146,3 +222,103 @@ describe.runIf(RUNNABLE)("hermes dashboard auth block", () => { expect(check.stdout.trim()).toBe("match"); }); }); + +/** + * Honest failure classes. Two Hermes provisions in a row printed + * "freshly written password and hash do not verify" and blamed the credentials + * — which were provably correct — because the check returned a bare exit 1 for + * every kind of failure: a genuine mismatch, a block a racing writer had erased, + * a config it couldn't read, and an interpreter where scrypt couldn't run. The + * fix is a classifier: "could not run the check" and "the password does not + * match the hash" must be DIFFERENT outcomes with different exit codes, and an + * environment failure must never be reported as a credential problem. `--check` + * exposes the classifier as its own exit code (0 ok, 3 mismatch, 4 not + * configured, 5 no password, 6/7 environment). + */ +describe.runIf(RUNNABLE)("dashboard auth: honest failure classes", () => { + it("reports a good pair as usable (exit 0)", () => { + const { root, configPath } = makeRoot(); + expect(run(root, configPath, "clawbox").status).toBe(0); + const proc = check(root, configPath); + expect(proc.status, proc.stderr).toBe(0); + expect(proc.stdout).toContain("OK"); + }); + + it("catches a GENUINE mismatch (exit 3), distinctly from an environment error", () => { + // A valid, well-formed block for password "AAA", but the stored plaintext + // is "BBB": the hash genuinely does not verify the password. + const { root, configPath } = makeRoot(); + seedBlock(configPath, scryptHash("AAA")); + fs.mkdirSync(path.join(root, "data"), { recursive: true }); + fs.writeFileSync(path.join(root, "data", ".hermes-dashboard-pw"), "BBB", { mode: 0o600 }); + + const proc = check(root, configPath); + expect(proc.status).toBe(3); + expect(proc.stderr).toContain("DESYNCED"); + // And it must NOT masquerade as one of the environment codes. + expect([6, 7]).not.toContain(proc.status); + }); + + it("does NOT report a vanished block as a credential mismatch", () => { + // The lost-update artefact: another writer's config (mcp_servers only, no + // dashboard block) with a password file still present. The block is simply + // gone — there is no credential to compare — so this is NOT_CONFIGURED (4), + // never MISMATCH (3). This is the exact state the racing writer leaves, and + // blaming the credentials for it is what hid the real bug for two provisions. + const { root, configPath } = makeRoot(); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync(configPath, "mcp_servers:\n clawbox:\n enabled: true\n"); + fs.mkdirSync(path.join(root, "data"), { recursive: true }); + fs.writeFileSync(path.join(root, "data", ".hermes-dashboard-pw"), "leftover-password", { + mode: 0o600, + }); + + const proc = check(root, configPath); + expect(proc.status).toBe(4); + expect(proc.stdout + proc.stderr).toContain("NOT CONFIGURED"); + expect(proc.status).not.toBe(3); + }); + + it("classifies a scrypt-less interpreter as an environment error, NOT a mismatch", () => { + // First provision a genuinely-correct pair with the real python. + const { root, configPath } = makeRoot(); + expect(run(root, configPath, "clawbox").status).toBe(0); + + // Now verify it with an interpreter whose scrypt raises. The pair is CORRECT + // — the check just cannot run. That must be exit 7 (COULD NOT RUN), never 3. + const stubDir = scryptBrokenPythonDir(); + const proc = check(root, configPath, { PATH: `${stubDir}${path.delimiter}${process.env.PATH}` }); + expect(proc.status).toBe(7); + expect(proc.stderr).toContain("COULD NOT RUN"); + expect(proc.stderr).not.toContain("does not match"); + }); + + it("a provision that cannot compute a hash fails as ENVIRONMENT, not as a bad credential", () => { + // Full provision under a scrypt-less interpreter: the OLD script would still + // reach its verify and print "do not verify", blaming credentials it never + // managed to write. The classifier stops at generation with an environment + // exit code and an environment message. + const { root, configPath } = makeRoot(); + const stubDir = scryptBrokenPythonDir(); + const proc = run2(root, configPath, { + PATH: `${stubDir}${path.delimiter}${process.env.PATH}`, + }); + // 4 = environment error (could not generate). Definitely not the success 0, + // and not the credential-mismatch 2. + expect(proc.status).toBe(4); + const all = `${proc.stdout}\n${proc.stderr}`; + expect(all).not.toMatch(/do not verify|does not match/); + expect(all).toMatch(/could not generate|environment/i); + }); +}); + +/** Full provision with extra env (used to inject a broken PATH). */ +function run2(root: string, configPath: string, extraEnv: NodeJS.ProcessEnv) { + const env: NodeJS.ProcessEnv = { + ...process.env, + CLAWBOX_ROOT: root, + HERMES_CONFIG: configPath, + ...extraEnv, + }; + return spawnSync("bash", [SCRIPT], { encoding: "utf-8", env }); +} diff --git a/src/tests/unit/install-hermes-edition-step.test.ts b/src/tests/unit/install-hermes-edition-step.test.ts index 2835df02..b5acd1fe 100644 --- a/src/tests/unit/install-hermes-edition-step.test.ts +++ b/src/tests/unit/install-hermes-edition-step.test.ts @@ -110,6 +110,93 @@ describe("the TS and shell harness predicates agree", () => { * through its own update. `install.sh --step gateway_setup` by hand is the same * hole. These pin both ends so the trap fails loudly instead of shipping. */ +/** + * Propagation. A full install keeps `step_hermes_edition` NON-FATAL — a + * half-provisioned box should still finish and come up reachable — but two real + * provisions proved that "non-fatal" had become "invisible": install.sh printed + * "Hermes provisioning FAILED", then "All 20 checks healthy", then the flash + * host printed "Setup: 1/1 succeeded". A failed step must reach the operator's + * summary AND the exit status, or a broken box ships as healthy. + */ +describe("a failed provisioning step is not reportable as success", () => { + it("records the hermes_edition failure instead of only warning", () => { + // The non-fatal `|| { ... }` block must record the failure, not just echo a + // banner that scrolls off screen. + const tail = INSTALL_SH.slice(INSTALL_SH.indexOf("Provisioning Hermes")); + const block = tail.slice(0, tail.indexOf("\nfi\n")); + expect(block).toContain("record_provision_failure hermes_edition"); + }); + + it("folds provisioning failures AND a failed validation into one honest exit", () => { + // Validation is captured (not left to abort via set -e) so the summary still + // prints, then a single FINAL_RC reflects BOTH signals, and the script exits + // with it. Without this, install.sh exited 0 whenever validation self-healed. + expect(INSTALL_SH).toContain("VALIDATE_RC=0"); + expect(INSTALL_SH).toContain("step_validate_services || VALIDATE_RC=$?"); + expect(INSTALL_SH).toContain("FINAL_RC=1"); + // FINAL_RC rises from EITHER a recorded provisioning failure OR a failed + // validation — both signals feed the one exit code. + expect(INSTALL_SH).toContain('"${#PROVISION_FAILURES[@]}" -gt 0'); + expect(INSTALL_SH).toContain('"${VALIDATE_RC:-0}" -ne 0'); + expect(INSTALL_SH).toContain('exit "$FINAL_RC"'); + }); + + it("prints an INCOMPLETE summary and a machine-readable status for the flash host", () => { + expect(INSTALL_SH).toContain("PROVISIONING INCOMPLETE"); + // A sentinel line for a caller that greps stdout, and a marker file for one + // that reads a file — both must agree with the exit code. + expect(INSTALL_SH).toContain("[provision-status] INCOMPLETE"); + expect(INSTALL_SH).toContain("[provision-status] OK"); + expect(INSTALL_SH).toContain("write_provision_status incomplete"); + expect(INSTALL_SH).toContain("write_provision_status ok"); + }); + + it("defines the accumulator and marker writer", () => { + expect(INSTALL_SH).toContain("record_provision_failure()"); + expect(INSTALL_SH).toContain("write_provision_status()"); + expect(INSTALL_SH).toContain("PROVISION_FAILURES=()"); + }); +}); + +/** + * Validation must include the thing that failed. Right after dashboard auth + * failed, the 26/26 step reported every check healthy — because its only Hermes + * auth probe hit the proxy and whitelisted the failure (a 401 was "healthy"), + * and the proxy answers an un-cookied request with a 302 whether or not the auth + * provider works. The validator now verifies the provider directly, reusing the + * auth script's own `--check` classifier so both agree on "healthy". + */ +describe("service validation checks the dashboard auth provider", () => { + it("runs the auth script's --check on hermes AND dual", () => { + const fn = extractShellFunction("step_validate_services"); + expect(fn).toContain('bash "$auth_script" --check'); + // Guarded by has_hermes_harness (hermes + dual), not is_hermes_edition. + const probe = fn.slice(fn.indexOf("dashboard auth PROVIDER")); + expect(probe.slice(0, probe.indexOf("case"))).toContain("has_hermes_harness"); + }); + + it("counts the new probe in the healthy total", () => { + const fn = extractShellFunction("step_validate_services"); + expect(fn).toMatch(/has_hermes_harness; then probe_count=\$\(\( probe_count \+ 1 \)\)/); + }); + + it("no longer counts a proxy 401 as healthy on hermes", () => { + // 401 is the desynced-SSO symptom (see hermes-dashboard-proxy.js); a healthy + // browserless probe gets a 302/403, so a 401 must fail the check. Assert the + // case pattern precisely — the word "401" still appears in the DESYNCED + // message, and that is correct. + const fn = extractShellFunction("step_validate_services"); + expect(fn).toContain("2*|3*|403) ;;"); + expect(fn).not.toContain("2*|3*|401|403"); + }); + + it("maps --check's classes to distinct operator messages", () => { + const fn = extractShellFunction("step_validate_services"); + expect(fn).toContain("DESYNCED"); + expect(fn).toContain("no usable dashboard auth provider"); + }); +}); + describe("install.sh keeps its own edition guards", () => { it("post_update still calls gateway_setup on every edition", () => { expect(extractShellFunction("step_post_update")).toContain("step_gateway_setup");