From c56fad163d11df230ac5d2e40e1398df25379914 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 13:53:24 -0700 Subject: [PATCH 01/28] Add audit fix plan: box new is broken on a clean checkout Live-tested ./box new end-to-end (twice, in isolated copies) plus five parallel deep-read audits over provisioning, the Hermes supervisor, the migration tool, ops/docs, and the test suite. Root-caused why box creation fails today (stale HERMES_INSTALLER_SHA256 pin, then a silent set -e death in verify_required because the Hermes installer's launcher never lands on the agent user's PATH) and documented a prioritized, step-by-step fix plan with reproduction instructions for every finding. Co-Authored-By: Claude Sonnet 5 --- docs/audit-2026-06-30-fixes.md | 594 +++++++++++++++++++++++++++++++++ 1 file changed, 594 insertions(+) create mode 100644 docs/audit-2026-06-30-fixes.md diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md new file mode 100644 index 0000000..d88fc2d --- /dev/null +++ b/docs/audit-2026-06-30-fixes.md @@ -0,0 +1,594 @@ +# tx9 audit & fix plan — 2026-06-30 + +Source: a live end-to-end test of `./box new` plus five parallel deep-read +audits (provisioning, the Hermes supervisor, the migration tool, ops/docs, +the test suite), run against commit `ded8e51` on `main`. No code was changed +during the audit; this document is the persisted plan for actually fixing +what it found. Each item below has a **Reproduce** section (how to see the +bug yourself) and a **Fix** section (the concrete steps to take). Items +marked "verified live" were actually triggered and root-caused against a +running VM; everything else is from direct code reading and is marked +accordingly. + +Work through this top to bottom — P0 blocks everything else, since you can't +exercise most of P1–P3 if `box new` doesn't produce a working box. + +--- + +## P0 — `./box new` is broken on a clean checkout (verified live, twice) + +### P0-1: `HERMES_INSTALLER_SHA256` no longer matches upstream + +**Symptom:** `./box new ` fails immediately with `Hermes installer +checksum verification failed`, right after provisioning prints +`hermes (official installer pinned to )`. + +**Root cause:** `box.env:25` pins the SHA-256 of +`https://hermes-agent.nousresearch.com/install.sh`. Upstream has rotated that +script since the pin was set, so `provision/provision.sh:118` (`sha256sum +--check --status`) now correctly refuses to run the mismatched installer — +this is the safety check working as designed against stale, attacker-could-also-be-the-cause +drift, not a logic bug. + +**Reproduce:** +```bash +grep HERMES_INSTALLER_SHA256 box.env +curl -fsSL https://hermes-agent.nousresearch.com/install.sh | sha256sum +# the two hashes will not match +``` + +**Fix:** +1. Re-pin the hash to the current upstream script: + ```bash + live_sha=$(curl -fsSL https://hermes-agent.nousresearch.com/install.sh | sha256sum | awk '{print $1}') + sed -i "s/^HERMES_INSTALLER_SHA256=.*/HERMES_INSTALLER_SHA256=\"$live_sha\"/" box.env + ``` + Read the diff of `install.sh` against what's currently pinned (or at least skim it) before committing the new pin — this is the one unpinned-trust boundary in provisioning, so don't rubber-stamp it. +2. **Decide a maintenance story so this doesn't silently recur.** Options, cheapest first: + - Add a `make check-hermes-pin` / `./box doctor-pins` target that fetches the live installer and diffs its hash against `box.env`, so drift is caught at `make check` time instead of at `box new` time for a real user. + - Prefer `HERMES_GIT_BUNDLE` (already supported, see `box.env:38-39` and `provision/provision.sh:92-100`) for any host where reproducibility matters more than always-latest — a bundle pins exact bytes, a URL+hash pins bytes only until upstream moves the URL contents. + - At minimum, add a comment at `box.env:23-25` noting the last-verified-against date so the next person knows how stale it might be. + +--- + +### P0-2: `verify_required` silently kills provisioning via `set -e` (verified live, root-caused) + +**Symptom:** Even after fixing P0-1, `./box new` still fails. The last +provisioning line printed is `verifying required tools`, then nothing — no +error message, no log line — followed immediately by `error: box creation +failed` and rollback. This happened on **both** of two independent live +attempts. + +**Root cause — two stacked bugs in six lines (`provision/provision.sh:207-218`):** + +```bash +verify_required() { + log "verifying required tools" + local cmd + for cmd in node uv nvim claude codex; do + command -v "$cmd" >/dev/null 2>&1 || { log "missing required tool: $cmd"; return 1; } + done + if [[ "${INSTALL_HERMES:-0}" == 1 ]]; then command -v hermes >/dev/null 2>&1; fi # line 213 + if [[ "${INSTALL_EXECUTOR:-0}" == 1 ]]; then command -v executor >/dev/null 2>&1; fi # line 214 + if [[ "${INSTALL_HERMES:-0}" == 1 ]]; then + [[ "$(git -C /usr/local/lib/hermes-agent rev-parse HEAD)" == "${HERMES_GIT_SHA}" ]] + fi +} +``` + +1. **`hermes` is never on `agent`'s PATH in the first place.** The upstream + Hermes installer places its launcher at `~/.local/bin/hermes` — i.e. + `/root/.local/bin/hermes`, since provisioning runs as root — not at + `/usr/local/bin/hermes` as the comment at `provision/provision.sh:87-89` + assumes. `/root/.local/bin` is never added to the `agent` user's PATH + (`/opt/hermes-box/bin:/opt/hermes-box/tooling/node-global/bin:/opt/hermes-box/tooling/uv:...`, + set in `guest/profile.sh`). So `command -v hermes` on line 213 genuinely + fails — confirmed by direct inspection of a live VM: + ``` + $ ls /root/.local/bin/hermes # exists + $ ls /usr/local/bin/hermes # does not exist + $ command -v hermes # (as agent, with hermes-box.sh profile sourced) + MISSING + ``` +2. **That failing command is inside an `if`-THEN body, not the `if`'s + condition — and `set -e` (`provision/provision.sh:2`) does not exempt + THEN-bodies, only conditions.** So the moment `command -v hermes` fails, + the entire `provision.sh` process dies right there, silently, because + nobody added `|| { log ...; return 1; }` the way the `node/uv/nvim/claude/codex` + loop two lines above correctly does. +3. **Separately** (a second, independent bug in the same function): even if (1)/(2) + didn't kill the script, `verify_required`'s actual return value is whatever + its *last* statement returns — the `git rev-parse HEAD` check — so the + `command -v hermes`/`command -v executor` checks on lines 213-214 were never + going to gate success/failure even if they survived `set -e`. Two bugs, same + six lines, masking each other. + +**Reproduce (isolated repro, safe to run anywhere — does not touch a box):** +```bash +set -e +echo before +if [[ 1 == 1 ]]; then command -v hermes >/dev/null 2>&1; fi +echo "after (never prints — proves the silent-death mechanism)" +``` + +**Reproduce against the real installer (inside any fresh guest, after fixing P0-1):** +```bash +./box new tx9probe # fails at "verifying required tools" with no further output +# .boxes / smolvm machine ls / .box-locks are all clean afterward — rollback itself is fine +``` + +**Fix — three separate edits, do all three:** + +1. **Make `hermes` actually reachable on `agent`'s PATH.** In + `install_hermes()` (`provision/provision.sh:78-150`), right after the + `bash "$installer" ...` success branch and before `own_hermes_home` (around + line 143), add an explicit symlink from wherever the installer really put + the launcher into `$OPT/bin` (`/opt/hermes-box/bin`, already on `agent`'s + PATH per `guest/profile.sh`): + ```bash + if [[ -x /root/.local/bin/hermes ]]; then + ln -sf /root/.local/bin/hermes "$OPT/bin/hermes" + fi + ``` + Don't hardcode `/root` if you can avoid it — `$HOME` at this point in the + script is root's home since the whole script runs as root, so + `"$HOME/.local/bin/hermes"` is the more honest spelling. Verify this is + still where the installer puts it before relying on it long-term (it's an + upstream implementation detail, not a contract) — consider also checking + `command -v hermes` *as the installer would have configured PATH* (it adds + `~/.local/bin` to `/root/.bashrc`/`.profile`, per the installer's own + output) as a fallback discovery mechanism rather than a single hardcoded + path. +2. **Fix the line 87-89 comment** — it currently asserts a false contract + (`binary -> /usr/local/bin/hermes`). Update it to describe reality once (1) is fixed. +3. **Give every check in `verify_required` explicit failure handling**, matching the style already used for `node/uv/nvim/claude/codex`: + ```bash + verify_required() { + log "verifying required tools" + local cmd + for cmd in node uv nvim claude codex; do + command -v "$cmd" >/dev/null 2>&1 || { log "missing required tool: $cmd"; return 1; } + done + if [[ "${INSTALL_HERMES:-0}" == 1 ]]; then + command -v hermes >/dev/null 2>&1 || { log "missing required tool: hermes"; return 1; } + fi + if [[ "${INSTALL_EXECUTOR:-0}" == 1 ]]; then + command -v executor >/dev/null 2>&1 || { log "missing required tool: executor"; return 1; } + fi + if [[ "${INSTALL_HERMES:-0}" == 1 ]]; then + [[ "$(git -C /usr/local/lib/hermes-agent rev-parse HEAD)" == "${HERMES_GIT_SHA}" ]] || { + log "Hermes checkout HEAD does not match pinned HERMES_GIT_SHA"; return 1; + } + fi + } + ``` + This doesn't just fix the silent-death bug — it also makes the function's + pass/fail meaning correct (currently it can return success while `hermes`/ + `executor` are both missing, since the last-statement-wins return value + ignores them entirely). + +**Verification after fixing:** re-run `./box new tx9probe` end to end; confirm +it reaches `provision complete` and `Ready. Enter with: ./box enter tx9probe`, +then `./box doctor tx9probe` passes, then `./box rm tx9probe --force`. + +--- + +### P0-3: a failed `box new` destroys the only evidence of why + +**Symptom:** Both live failures above produced exactly one line of +explanation (`error: box creation failed`) before `_rollback_created` +(`box:603-633`) deleted the VM. The rollback mechanism itself is excellent — +verified twice that it leaves zero orphaned VMs, registry entries, or +locks — but it also deletes the only place the real error lived. + +**This is not a live-reproduced "bug" so much as a confirmed UX gap**: P0-1 +and P0-2 were only diagnosable here because the audit ran the same failure +twice in a disposable copy and inspected the VM *before* letting rollback run +(by calling `_create_base`/`_provision_into`/`_prepare_runtime` directly and +skipping the trap-driven cleanup for one diagnostic run). A normal user has +no equivalent escape hatch. + +**Fix:** +1. On a provisioning failure inside `cmd_new`/`cmd_load` (`box:635-650`, + `box:707-774`), before `_rollback_created` runs, capture the tail of + whatever guest-side log exists (or at minimum the output already streamed + to the host terminal — consider tee-ing `_provision_into`'s output to a + host-side `backups/.failed/-.log` unconditionally, since + the function already shells the whole provisioning transcript through the + host process). +2. Add a `BOX_KEEP_ON_FAILURE=1` (or `./box new --keep-on-failure`) escape + hatch that skips `_rollback_created` and instead prints the VM name plus + `smolvm machine exec --name -- ...` and `./box rm --force` + hints, for exactly this kind of investigation. +3. Make the final `die "box creation failed"` in `cmd_new` (`box:642`) + include which of `_create_base` / `_provision_into` / `_prepare_runtime` + failed — right now `! A || ! B || ! C` collapses all three into one + message with no way to tell from the final line alone which stage broke. + +--- + +## P1 — safety gaps in already-claimed guarantees + +These were found by direct code reading (not live-triggered — they require +either real timing races or a real single-writer Discord setup to observe +directly). Each entry says how to construct the race/condition if you want +to confirm it before fixing. + +### P1-1: gateway can be double-spawned (race in `hb`/`hb-workload`) + +**Files:** `guest/hb:74-88` (`_start_gateway`), `guest/hb-workload:67-96` +(`reconcile_once`, called every 20s by `guest/hb-workload:100-104`'s loop). + +**Problem:** `_start_gateway` checks `_gateway_running` (`guest/hb:78`), and +if not running, spawns `hermes gateway run --replace` and writes +`$GATEWAY_PID` (`guest/hb:85-87`) — with no lock between the check and the +spawn. `hb-workload` calls `hb reconcile` → `_start_gateway` on a 20-second +timer (`guest/hb-workload:80`, `guest/hb:97-102`), and a host-initiated +`./box ` can call `_guest_hb "$n" up` (→ `_start_gateway`) at the same +moment (e.g. `cmd_open`/`box:926`, `_prepare_runtime`/`box:596-601`, +`cmd_repair`/`box:1008-1009`). Two processes can both observe "not running" +and both exec `hermes gateway run --replace`, which directly contradicts the +single-writer guarantee the whole `hb gateway-enable +--confirm-single-writer ...` ceremony (`docs/nexus-operations.md` §4) exists +to protect. + +**Reproduce (construct the race deliberately):** +```bash +# inside a box, as agent, with INSTALL_HERMES=1 and gateway enabled: +for i in 1 2; do hb up & done; wait +pgrep -f '[h]ermes( .*)? gateway run' | wc -l # may show 2, not 1 +``` +(Timing-dependent — may need a few attempts, or insert a `sleep 0.5` between +the `_gateway_running` check and the `nohup hermes gateway run` line in a +local copy of `hb` to make the window reliably wide.) + +**Fix:** +1. Add a lock file (the same `mkdir`-based pattern `box` already uses for + `_try_acquire_lock`, `box:143-171`) around `_start_gateway`'s + check-then-spawn in `guest/hb:74-88`, e.g. `STATE_DIR/gateway.lock` — + `mkdir` it before the `_gateway_running` check, release after writing + `$GATEWAY_PID` or on early return. +2. Apply the same lock inside `reconcile_once` (`guest/hb-workload:67-96`) + before it calls `hb reconcile`, or simply rely on (1) since `hb reconcile` + already calls into `_start_gateway`. + +### P1-2: `hb-workload` itself is unsupervised + +**File:** `guest/hb-workload:98-105` (`main`), invoked once at boot by +`box:546-547`'s `smolvm machine create` command line. + +**Problem:** If the `hb-workload` *loop process* dies (not the gateway or +Executor it manages — the bash loop itself, e.g. OOM-killed, or a bug in +`reconcile_once` that isn't guarded by the `set -u` mode it runs under), +nothing restarts it. Whatever state Hermes/Executor were in at that moment +persists unmanaged until the VM reboots — no more health reconciliation, no +more bridge socat processes being restarted if they die. + +**Reproduce (code reading is enough to confirm; to observe live):** +```bash +# inside the box: +pkill -f 'hb-workload 4788' # or whatever port; find via ps aux +sleep 25 +ps aux | grep hb-workload # nothing respawns it +``` + +**Fix:** Wrap the boot command in `_create_base` (`box:546-547`) in a +restart loop, e.g. change: +```bash +exec runuser -u agent -- env HOME=/data/home/agent /opt/hermes-box/bin/hb-workload ${wl_port} ${api_guest_port:-0} +``` +to a `while true; do runuser ... hb-workload ...; sleep 2; done` (drop the +`exec` so the outer loop can restart it), or rely on smolvm's own +process-1-exits-VM-stops semantics plus a *systemd inside the guest* unit — +the latter is more in keeping with the project's existing host-side systemd +pattern (`ops/systemd/`), but the cheap fix is the restart loop. + +### P1-3: `gateway_is_disabled()` checks a marker file, not process state + +**Files:** `guest/hb:143` (`gateway_is_disabled() { [[ -e "$GATEWAY_DISABLED" ]]; }`), +consumed by `box:980-989` (`cmd_doctor`'s "host Hermes API: intentionally +disabled" branch) and `box:965` (`_guest_hb "$n" gateway-is-disabled`). + +**Problem:** If `gateway_enable` (`guest/hb:145-160`) spawns +`hermes gateway run --replace` via `_start_gateway` and *then* something +later in that call path fails before `gateway_enable` returns, the marker is +re-touched as disabled (`guest/hb:156`) but there's no verification the +already-spawned process was actually killed first. `box doctor` would then +report "intentionally disabled" while a real gateway process is alive. + +**Reproduce (requires deliberately injecting a failure after spawn — code +reading confirms the gap; to force it, patch a local copy of `hb` to `return 1` +unconditionally right after the `_start_gateway` call in `gateway_enable` and +observe that the spawned `hermes gateway run` process is never reaped):** +```bash +# after gateway_enable's _start_gateway call but with the call patched to always fail downstream: +pgrep -f '[h]ermes( .*)? gateway run' # still running +hb gateway-is-disabled; echo $? # reports "disabled" (marker says so) — true positive case for the gap +``` + +**Fix:** Make `gateway_is_disabled()` check both the marker *and* the +absence of a running gateway process: +```bash +gateway_is_disabled() { [[ -e "$GATEWAY_DISABLED" ]] && ! _gateway_running; } +``` +And in `gateway_enable`'s failure path (`guest/hb:155-158`), explicitly call +`_stop_gateway` before re-touching `$GATEWAY_DISABLED`, rather than assuming +the marker alone is sufficient: +```bash +if ! _start_gateway; then + _stop_gateway || true + touch "$GATEWAY_DISABLED" + return 1 +fi +``` + +### P1-4: agent tooling versions are unpinned + +**Files:** `provision/provision.sh:68-69` (`npm install -g @anthropic-ai/claude-code`), +`:74-75` (`@openai/codex`), `:163-165` (`executor`) — none specify a version. + +**Problem:** Every `box new`/`box repair` (full mode) installs whatever is +currently `latest` on the npm registry. Two boxes created a week apart, or +the same box before/after a `repair`, can silently end up running different +Claude Code/Codex/Executor versions — with no `box.env` knob to pin, and +(per P0 fix above) no required version check to catch a breaking upstream +release before it reaches a real box. + +**Reproduce:** `npm view @anthropic-ai/claude-code version` today vs. a week +from now will differ; nothing in this repo records what version a given box +was actually provisioned with beyond the free-text `runtime-manifest` +(`guest/hb:257-275`, written *after the fact*, not a pin). + +**Fix:** +1. Add `CLAUDE_CODE_VERSION`, `CODEX_VERSION`, `EXECUTOR_VERSION` to + `box.env` (empty default = latest, for backward compatibility), and use + them in `provision/provision.sh:68-76,163-165`: + ```bash + npm install -g "@anthropic-ai/claude-code${CLAUDE_CODE_VERSION:+@$CLAUDE_CODE_VERSION}" + ``` +2. `guest/hb:write_manifest` (`guest/hb:257-275`) already records installed + versions after the fact — keep that, but it's not a substitute for being + able to *reproduce* a box; the pin is what makes `box new` deterministic. + +### P1-5: decompression-bomb gate trusts declared size, not actual bytes + +**File:** `guest/hermes-state:33` (`MAX_UNCOMPRESSED = 100 * 1024**3`), +gate at `:85-87` inside `_zip_layout`, extraction at `:389-413` (`_extract`, +streaming via `shutil.copyfileobj` at line 413 with no running byte-count +cap). + +**Problem:** The 100 GiB gate sums `info.file_size` (`:85`) — the ZIP central +directory's *declared* uncompressed size, which is attacker-controlled +metadata. Python's `zipfile` doesn't cross-validate central-directory size +against what the local file header / deflate stream actually produces. A +crafted archive can declare a small `file_size` while its real decompressed +output is far larger; `_extract`'s `shutil.copyfileobj` (`:413`) streams +without checking actual bytes written against any limit, so the stated 100 +GiB safety limit can be bypassed by a header/stream mismatch. + +**Reproduce:** Construct a ZIP entry whose local file header advertises a +small `file_size` but whose compressed stream, when inflated, produces far +more data than declared (standard zip-bomb construction techniques — `pip +install zipfile36`-style manual entry crafting, or simpler: a deflate stream +with a very high compression ratio for highly repetitive data, declared at a +deliberately understated size in a hand-edited central directory record). +Run `python3 guest/hermes-state validate-zip crafted.zip` — it should pass +the 100 GiB check despite the real payload being enormous, then `import-zip` +would extract the full payload. + +**Fix:** In `_extract` (`guest/hermes-state:389-413`), track actual bytes +written during `shutil.copyfileobj` and abort once a hard cap is exceeded — +e.g. read in `length=1024*1024` chunks manually (replacing the single +`copyfileobj` call) and increment a running total, raising once it exceeds +`MAX_UNCOMPRESSED` regardless of what the central directory claimed: +```python +written = 0 +while True: + chunk = source.read(1024 * 1024) + if not chunk: + break + written += len(chunk) + if written > MAX_UNCOMPRESSED: + raise ValueError(f"archive member exceeds the uncompressed safety limit: {info.filename}") + destination.write(chunk) +``` + +--- + +## P2 — repair/coverage gaps (code reading; no live repro needed beyond running the commands described) + +### P2-1: `box repair` (assets mode) can't heal a failed Hermes/Executor install + +**Files:** `provision/provision.sh:220-227` (`assets_only`). Note `cmd_repair` +(`box:994-1012`) always calls `_provision_into "$n" full`, never `assets` — +the only current caller of `assets` mode is `cmd_load`'s post-restore step +(`box:767`, `_provision_into "$n" assets`). + +**Problem:** `assets_only()` only runs `make_agent`, `install_config`, +`place_assets`, `seed_data`, and `write-manifest` — it never calls +`install_hermes`, `install_executor`, or `verify_required`. A box whose +Hermes or Executor install failed or is missing can only be fixed by +re-running the **full**, multi-minute reinstall path; there's no cheap +"retry just the broken piece" repair. + +**Reproduce:** On a box where `hermes`/`executor` are missing (e.g. one that +hit P0-2 before the fix, if rollback were disabled), run whatever invokes +`assets` mode and confirm `hermes`/`executor` remain missing afterward. + +**Fix:** Add idempotent guards to `install_hermes`/`install_executor` (skip +reinstall if already present and `git rev-parse HEAD` matches, like +`verify_required` already checks) and call them unconditionally from +`assets_only`, immediately after `make_agent`. This makes `assets` mode a +true "fix what's missing" repair instead of "config files only." + +### P2-2: zero test coverage for five of thirteen `box` subcommands + +**Files:** `box:889-1052` (`cmd_ls`, `cmd_open` partially, `cmd_rm`, +`cmd_stop`, `cmd_start`, `cmd_enter`) vs. `tests/{static,lifecycle-smoke,hermes-state,regressions}.sh`. + +**Problem:** Grep for these command names across all four test files returns +zero hits for `box ls`, `box rm`, `box enter`, `box stop`, `box start` — +these are the most frequently run day-to-day commands and have no coverage +at all, alongside untested code paths in already-tested commands: the +`_assign_port` retry/exhaustion loop (`box:302-311`), control-character +archive-path rejection (`box:378-380`'s `safe_text()`), and true per-box +(non-`registry`) lock contention (`_try_acquire_lock`, `box:143-171`). + +**Fix (add to `tests/lifecycle-smoke.sh` or a new `tests/cli-surface.sh`):** +1. `box ls` / `box ls --all` against the fake `smolvm` fixture — currently + the fixture doesn't even implement bare `machine ls` (falls through to + `exit 2`); fix the fixture (`tests/fixtures/smolvm`) alongside the test. +2. `box rm ` — interactive confirmation (feed `expect`-style stdin via + `/dev/tty` redirection or test the `--force` path), plus the "type the + name to confirm" mismatch-refusal case. +3. `box stop`/`box start` — trivial wrappers, but assert the lock-then-delegate + pattern still holds (a held lock should block a concurrent `stop`). +4. `box enter` — at minimum assert it `exec`s the right `smolvm machine exec + -it ... -- login -f agent` command line against the fixture (can't fully + test an interactive login). +5. Force `_assign_port`'s exhaustion path by pre-registering enough fake + ports across the configured range to leave zero free, and assert the `die + "no free host port in ${lo}-${hi}"` message. +6. Generate an archive member with a control character in its name/linkname + and confirm `_validate_archive` rejects it (extend the existing fixture + generator in `tests/lifecycle-smoke.sh`, which already covers eleven other + cases there). + +### P2-3: no CI + +**Files:** `README.md:228` and `tests/static.sh:129` both assert +`"intentionally no CI configuration yet"` (the latter as a literal grep +check pinning that exact string). + +**Problem:** `make check` (`Makefile:21`) is fast and fully hermetic — the +`tests/fixtures/smolvm` stub means no real VM/KVM/network access is needed — +so there's no technical blocker to running it in CI. Every regression this +otherwise-strong suite catches currently depends on a human remembering to +run `make check` locally before merging. + +**Fix:** +1. Add `.github/workflows/check.yml` running `make check` (needs `python3`, + `jq`, `shellcheck` for `make check`'s `lint` target — install via the + workflow's package step) on push/PR to `main`. +2. Update the `README.md:228` / `tests/static.sh:129` pinned string once CI + exists, since that test will otherwise correctly start failing the moment + CI is added (which is by design — don't quietly delete that assertion; + update what it asserts). + +### P2-4: `tests/regressions.sh` structure and shared-helper duplication + +**File:** `tests/regressions.sh` (1131 lines, ~10 unrelated concern areas in +one flat file — host-preflight checks, `tx9-host` supervise/backup behavior, +provisioning ownership, restore permissions, `hb-workload` reconcile/bridge +logic, gateway enable/disable, doctor/MCP health (~250 lines), registry/lock/ +rollback races (~300 lines), save/restore failure injection). + +**Problem:** A single failing assertion requires scanning up to 1131 lines +for context. The `mktemp -d` + `trap ... EXIT HUP INT TERM` cleanup +boilerplate is independently reimplemented in `tests/lifecycle-smoke.sh:5-7`, +`tests/hermes-state.sh:5-6`, and `tests/regressions.sh:6-30` — the third one +notably more elaborate (a `cleaned_up` guard plus explicit signal-to-exit-code +mapping) — with no shared file all four source. `make_repo()` +(`tests/regressions.sh:816-821`) is duplicated inline at +`tests/lifecycle-smoke.sh:219-221`. + +**Fix:** +1. Extract a `tests/lib.sh` with the shared tempdir+trap+`make_repo` helpers, + sourced by all four test files, with one canonical signal-to-exit-code + mapping that matches `box`'s own contract (129/130/143). +2. Split `tests/regressions.sh` by concern, e.g. + `tests/regressions-lock-rollback.sh`, `tests/regressions-doctor-mcp.sh`, + `tests/regressions-hb-workload.sh`, `tests/regressions-tx9-host.sh` — update + `Makefile`'s `test` target to glob `tests/regressions-*.sh` instead of one + file, so new categories don't require touching the Makefile. + +--- + +## P3 — polish and drift prevention + +### P3-1: `box.env:36` hardcodes a personal path as the committed default + +``` +HERMES_IMPORT_PATH_MAP="/Users/davis=/data/home/agent" +``` + +**Problem:** This is Davis-specific (macOS username `davis`) and ships as the +*default* in version control. Anyone else cloning this repo gets a path +mapping that's wrong for them, with nothing in the README or +`docs/nexus-operations.md` flagging it as "change this first." + +**Fix:** Either leave it empty by default and document that operators must +set it (the `import-hermes` flow already supports `--map OLD=NEW` per-invocation, +per `box:786-787`, so a non-empty default isn't structurally required), or +move this specific value into `docs/nexus-operations.md`'s migration runbook +(where it's already referenced) and out of the generic `box.env` template. + +### P3-2: no systemd hardening directives on any of the four host units + +**Files:** `ops/systemd/tx9-box@.service`, `tx9-health@.service`, +`tx9-backup@.service` (all reviewed in full). + +**Problem:** All three run as the unprivileged `tx9` user (good — no +unnecessary root), but none set `NoNewPrivileges=true`, `ProtectHome=true`, +`ProtectSystem=strict` (or `full`), `PrivateTmp=true`, etc. Cheap, standard +hardening that costs nothing given what these units already don't need +(arbitrary filesystem write access outside `/opt/tx9`, `/var/backups/tx9`, +`/var/lib/tx9`). + +**Fix:** Add to each `[Service]` block: +```ini +NoNewPrivileges=true +ProtectHome=true +ProtectSystem=strict +PrivateTmp=true +ReadWritePaths=/opt/tx9 /var/backups/tx9 /var/lib/tx9 /etc/tx9 +``` +Test each unit after adding (`systemctl daemon-reload && systemctl start +tx9-box@.service` against a real or disposable box) since `ProtectSystem=strict` +in particular can break things that aren't covered by `ReadWritePaths` — KVM +device access (`/dev/kvm`) in particular needs verifying it isn't blocked. + +### P3-3: NAS automount and backup retention are documentation only + +**File:** `docs/nexus-operations.md:69-73` describes both; nothing in +`ops/` implements either (confirmed: no `.mount`/`.automount` unit, no +pruning script or `tmpfiles.d` rule anywhere in the repo). + +**Fix:** +1. Ship `ops/systemd/mnt-davis\x2dvault\x2dtx9.automount` (or a generically-named + template the operator renames) as a starting point for the CIFS mount + `docs/nexus-operations.md:71` currently just describes in prose. +2. Add a retention script (`ops/tx9-backup-prune` or similar) — e.g. keep + last N or last N-days of `backups/-*.tar.gz.gpg` plus matching + `.sha256` files — and a `tx9-backup-prune@.timer`/`.service` pair following + the existing `ops/systemd/` naming convention. Without this, both + `/var/backups/tx9` and any configured NAS target grow unbounded forever. + +### P3-4: README and `docs/usage-guide.html` duplicate content with no single source of truth + +**Files:** `README.md` (10,919 bytes) and `docs/usage-guide.html` — spot-checked +and currently consistent with actual `box` behavior, but the same command +table, env var table, and backup/restore guarantees prose exist independently +in both with no generation step tying them together. + +**Fix:** Pick one canonical source (recommend the README, since it's what +renders on GitHub and what `tests/static.sh` already greps against) and +either (a) delete the duplicated sections from `usage-guide.html` in favor of +linking to the README, or (b) add a small generation step (e.g. a Python/ +awk script that extracts the README's command table into the HTML at build +time) if the standalone HTML page needs to stay self-contained for offline +use. + +--- + +## Suggested execution order + +1. P0-1, P0-2 (unblocks everything else — without these, no box can be created to test anything downstream) +2. P0-3 (do this alongside P0-2 — you'll want the keep-on-failure escape hatch while fixing P1 items that need a real box to observe) +3. P1-1 through P1-5 (safety-bearing; each is independently shippable) +4. P2-3 (CI) as soon as P0/P1 land, so nothing regresses silently again +5. P2-1, P2-2, P2-4 (repair coverage, test coverage, test structure) +6. P3-1 through P3-4 (polish, no urgency, fine to batch into one PR) + +Each numbered item above is scoped to be its own commit (or small group of +commits) against this branch. From bf6c0fbfa809e1c44bc7e273943bdd0a51dd6f09 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 13:59:28 -0700 Subject: [PATCH 02/28] fix(P0-1): re-pin Hermes installer SHA256 to current upstream Upstream rotated hermes-agent.nousresearch.com/install.sh since the last pin, so box new failed checksum verification on a clean checkout. Skimmed the new installer (no suspicious network/eval patterns; same install flow plus newer FHS-layout-detection logic for root installs) before re-pinning. Also add `make check-hermes-pin`, a network-requiring target kept out of `make check` (which stays hermetic), so drift can be caught proactively instead of surfacing as a box new failure for a real user. Co-Authored-By: Claude Sonnet 5 --- Makefile | 18 +++++++++++++++++- box.env | 6 ++++-- docs/audit-2026-06-30-fixes.md | 2 +- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 347aaa2..265fb78 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: check syntax lint test +.PHONY: check syntax lint test check-hermes-pin SHELL_FILES := box guest/hb guest/hb-workload guest/profile.sh guest/agent-bash-profile.sh provision/provision.sh ops/tx9-host tests/static.sh tests/lifecycle-smoke.sh tests/hermes-state.sh tests/regressions.sh tests/fixtures/smolvm @@ -19,3 +19,19 @@ test: ./tests/regressions.sh check: syntax lint test + +# Not part of `check` — needs network access, so `make check` stays hermetic. +# Run this on its own to catch Hermes installer drift before it surfaces as a +# `box new` failure for a real user. +check-hermes-pin: + @command -v curl >/dev/null || { echo "curl is required for check-hermes-pin" >&2; exit 1; } + @pinned=$$(grep '^HERMES_INSTALLER_SHA256=' box.env | sed -E 's/^[A-Z0-9_]+="([^"]*)"/\1/'); \ + live=$$(curl -fsSL https://hermes-agent.nousresearch.com/install.sh | sha256sum | awk '{print $$1}'); \ + if [ -z "$$live" ]; then echo "failed to fetch live Hermes installer" >&2; exit 1; fi; \ + if [ "$$pinned" = "$$live" ]; then \ + echo "HERMES_INSTALLER_SHA256 matches upstream ($$live)"; \ + else \ + echo "HERMES_INSTALLER_SHA256 in box.env ($$pinned) does NOT match upstream ($$live)" >&2; \ + echo "review the new installer before re-pinning, then update box.env" >&2; \ + exit 1; \ + fi diff --git a/box.env b/box.env index 619c1b2..f900392 100644 --- a/box.env +++ b/box.env @@ -22,8 +22,10 @@ HERMES_INSTALL_ARGS="--skip-browser" # headless box: skip Playwright/Chromium HERMES_REPO_URL="https://github.com/NousResearch/hermes-agent.git" HERMES_GIT_SHA="244a6f2ceb7f58c16b3cb2186584c39524e37874" # SHA-256 of https://hermes-agent.nousresearch.com/install.sh, verified before -# the installer is ever executed as root. Update intentionally with upstream. -HERMES_INSTALLER_SHA256="58f859ab038829a8538d001247db46680bac402a3ed746fdd81c51401bda4d71" +# the installer is ever executed as root. Update intentionally with upstream; +# last verified against upstream 2026-06-30. Run `make check-hermes-pin` to +# check this pin against the live installer without committing a new one. +HERMES_INSTALLER_SHA256="a93c65b01ea392e179cf872e182bd01a2b65c0c15f17833e9f9569033ef10e07" # Optional repository-relative, self-contained Git bundle. Keep private/custom # bundles under artifacts/ (ignored by Git). The commit and all of its required # objects must be present; incremental bundles with external prerequisites fail. diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index d88fc2d..f7ad26d 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -17,7 +17,7 @@ exercise most of P1–P3 if `box new` doesn't produce a working box. ## P0 — `./box new` is broken on a clean checkout (verified live, twice) -### P0-1: `HERMES_INSTALLER_SHA256` no longer matches upstream +### P0-1: `HERMES_INSTALLER_SHA256` no longer matches upstream — DONE **Symptom:** `./box new ` fails immediately with `Hermes installer checksum verification failed`, right after provisioning prints From 762aead47fe0db919dcecd12c58c1f88d1775da0 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:04:23 -0700 Subject: [PATCH 03/28] fix(P1-5): decompression-bomb gate now bounds actual extracted bytes The 100 GiB safety gate in _zip_layout only summed the ZIP central directory's declared file_size, which is attacker-controlled metadata that zipfile never cross-validates against the real inflate stream. A crafted archive could declare a small size while extracting far more. _extract now streams each member in 1 MiB chunks and tracks a running total of actual bytes written, aborting the moment it exceeds the cap regardless of what the archive claimed. Added a regression test that lowers the cap and confirms extraction aborts on real bytes, not the declared total. Co-Authored-By: Claude Sonnet 5 --- .gitignore | 2 ++ docs/audit-2026-06-30-fixes.md | 2 +- guest/hermes-state | 17 ++++++++++++++++- tests/hermes-state.sh | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 2f63bec..a9973b3 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,5 @@ # Optional private/custom Hermes Git bundles used for exact-SHA provisioning. /artifacts/* !/artifacts/.gitkeep + +__pycache__/ diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index f7ad26d..b14a115 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -350,7 +350,7 @@ was actually provisioned with beyond the free-text `runtime-manifest` versions after the fact — keep that, but it's not a substitute for being able to *reproduce* a box; the pin is what makes `box new` deterministic. -### P1-5: decompression-bomb gate trusts declared size, not actual bytes +### P1-5: decompression-bomb gate trusts declared size, not actual bytes — DONE **File:** `guest/hermes-state:33` (`MAX_UNCOMPRESSED = 100 * 1024**3`), gate at `:85-87` inside `_zip_layout`, extraction at `:389-413` (`_extract`, diff --git a/guest/hermes-state b/guest/hermes-state index def1932..3a07d78 100755 --- a/guest/hermes-state +++ b/guest/hermes-state @@ -399,6 +399,7 @@ def _extract( # preview may refer to an archive replaced before import reopened it. _require_importable_marker(entries) accounting = _accounting(entries) + total_written = 0 for info, rel, external, reason in entries: if reason is not None: continue @@ -409,8 +410,22 @@ def _extract( else: target = stage.joinpath(*rel.parts) target.parent.mkdir(parents=True, exist_ok=True) + # Track actual decompressed bytes rather than trusting info.file_size + # (attacker-controlled central-directory metadata, gated only on the + # *declared* total in _zip_layout) — a crafted stream can produce far + # more output than it declares. with zf.open(info) as source, target.open("wb") as destination: - shutil.copyfileobj(source, destination, length=1024 * 1024) + while True: + chunk = source.read(1024 * 1024) + if not chunk: + break + total_written += len(chunk) + if total_written > MAX_UNCOMPRESSED: + raise ValueError( + f"archive expands beyond the 100 GiB safety limit " + f"(actual bytes exceeded the declared total at {info.filename})" + ) + destination.write(chunk) secret = target.name in SECRET_NAMES or target.suffix.lower() in EXTERNAL_SECRET_SUFFIXES archived_mode = (info.external_attr >> 16) & 0o777 executable_path = any(part in {"hooks", "scripts"} for part in rel.parts[:-1]) diff --git a/tests/hermes-state.sh b/tests/hermes-state.sh index 84b7128..368aef1 100755 --- a/tests/hermes-state.sh +++ b/tests/hermes-state.sh @@ -342,4 +342,36 @@ kill "$reader_pid" 2>/dev/null || true wait "$reader_pid" 2>/dev/null || true "$ROOT/guest/hermes-state" verify --home "$wal_home" --checkpoint >/dev/null +# The 100 GiB gate must bound actual decompressed bytes, not just the +# attacker-controlled declared file_size in the central directory. +python3 - "$ROOT/guest/hermes-state" <<'PY' +import importlib.machinery, importlib.util, sys, tempfile, zipfile +from pathlib import Path + +script = Path(sys.argv[1]) +loader = importlib.machinery.SourceFileLoader("hermes_state_bomb_gate", str(script)) +spec = importlib.util.spec_from_loader(loader.name, loader) +module = importlib.util.module_from_spec(spec) +loader.exec_module(module) + +d = Path(tempfile.mkdtemp()) +zpath = d / "t.zip" +with zipfile.ZipFile(zpath, "w") as zf: + zf.writestr("config.yaml", "a: 1\n") + zf.writestr("state.db", b"x" * 5000) + +with zipfile.ZipFile(zpath) as zf: + prefix, files = module._zip_layout(zf) + stage = Path(tempfile.mkdtemp()) + external_stage = Path(tempfile.mkdtemp()) + module.MAX_UNCOMPRESSED = 2000 # below the real 5005 decompressed bytes + try: + module._extract(zf, stage, external_stage, prefix, files, set()) + except ValueError as error: + if "100 GiB safety limit" not in str(error): + raise SystemExit(f"unexpected extraction error: {error}") + else: + raise SystemExit("extraction exceeding the actual-bytes cap unexpectedly succeeded") +PY + echo "Hermes state checks passed" From 92cf51ca375812f4115e609bc44162eccaa5d5d8 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:06:33 -0700 Subject: [PATCH 04/28] fix(P0-2): stop verify_required from silently killing provisioning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two stacked bugs in provision.sh's verify_required(): (1) `command -v hermes` lived in an if-THEN body with no `|| { ...; return 1; }`, so the moment it failed, set -e killed the whole provision.sh process with zero output beyond "verifying required tools" — confirmed live, twice, on a clean checkout. (2) even surviving that, the function's return value was whatever its last statement returned, so the hermes/executor checks were never actually gating success. Root cause for why `command -v hermes` failed at all: provision.sh passes an explicit --dir to the Hermes installer, which makes the installer skip its own root/FHS auto-detection and link the `hermes` command into $HOME/.local/bin (root's home, since this runs as root) instead of /usr/local/bin — contradicting the comment that assumed FHS layout. Verified by tracing the live (newly re-pinned, see P0-1) installer's resolve_install_layout(). Fixes: - install_hermes() now symlinks the real launcher location into $OPT/bin/hermes, which is on both the agent user's PATH (guest/profile.sh) and, with this change, provision.sh's own PATH too (added $OPT/bin to the top-of-script PATH export — needed for verify_required's `command -v hermes` to see it in the same process). - Every check in verify_required now has explicit `|| { log ...; return 1; }` handling, matching the style already used for node/uv/nvim/claude/codex. - Corrected the stale install-location comment. Verified live end-to-end: `./box new` now reaches "provision complete" and "Ready. Enter with: ./box enter "; `./box doctor` passes all 17 checks; `./box rm --force` cleans up with no orphaned state. Co-Authored-By: Claude Sonnet 5 --- docs/audit-2026-06-30-fixes.md | 2 +- provision/provision.sh | 44 +++++++++++++++++++++++++++------- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index b14a115..c433c84 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -51,7 +51,7 @@ curl -fsSL https://hermes-agent.nousresearch.com/install.sh | sha256sum --- -### P0-2: `verify_required` silently kills provisioning via `set -e` (verified live, root-caused) +### P0-2: `verify_required` silently kills provisioning via `set -e` (verified live, root-caused) — DONE **Symptom:** Even after fixing P0-1, `./box new` still fails. The last provisioning line printed is `verifying required tools`, then nothing — no diff --git a/provision/provision.sh b/provision/provision.sh index d209493..07e89da 100755 --- a/provision/provision.sh +++ b/provision/provision.sh @@ -10,7 +10,11 @@ source "$CTX/box.env" OPT=/opt/hermes-box NODE_PREFIX="$OPT/tooling/node-global" UV_DIR="$OPT/tooling/uv" -export PATH="$NODE_PREFIX/bin:$UV_DIR:$PATH" +# $OPT/bin holds repo-owned tools (hb, hb-workload, hermes-state) plus the +# hermes launcher symlink installed below — must be on PATH here too, not +# just for the agent user (guest/profile.sh), so verify_required's +# `command -v` checks see what a real shell would see. +export PATH="$OPT/bin:$NODE_PREFIX/bin:$UV_DIR:$PATH" export DEBIAN_FRONTEND=noninteractive log() { printf '\033[35m[provision]\033[0m %s\n' "$*"; } @@ -84,9 +88,14 @@ install_hermes() { [[ "$sha" =~ ^[0-9a-fA-F]{40}$ ]] || { log "invalid HERMES_GIT_SHA: $sha"; return 1; } [[ "$installer_sha" =~ ^[0-9a-fA-F]{64}$ ]] || { log "invalid HERMES_INSTALLER_SHA256"; return 1; } log "hermes (official installer pinned to $sha)" - # We run as root, so the installer uses the FHS layout: code -> /usr/local/lib, - # binary -> /usr/local/bin/hermes (both baked into the golden). HERMES_HOME pins - # the durable state (auth, sessions, skills, memory) onto /data. + # We run as root and pass an explicit --dir below, which makes the installer + # skip its own root/FHS auto-detection (resolve_install_layout() returns + # early whenever --dir is explicit) and link the `hermes` command into + # $HOME/.local/bin instead of /usr/local/bin. $HOME is root's home here + # since this whole script runs as root. Code still lands at $install_dir + # (/usr/local/lib/hermes-agent) since that part of --dir is honored either + # way. HERMES_HOME pins the durable state (auth, sessions, skills, memory) + # onto /data. See the symlink step below that puts `hermes` on agent's PATH. export HERMES_HOME=/data/home/agent/.hermes mkdir -p "$HERMES_HOME" if [[ -n "$bundle" ]]; then @@ -140,8 +149,21 @@ install_hermes() { git -C "$install_dir" remote set-url origin \ "${HERMES_REPO_URL:-https://github.com/NousResearch/hermes-agent.git}" fi + # Put the launcher on PATH for both this script (root) and the agent user + # (guest/profile.sh puts $OPT/bin first). $HOME is root's home since this + # whole script runs as root; that's where an explicit --dir install links + # the command (see the comment above this function). Fall back to the + # FHS location in case a future installer version goes back to it. + if [[ -x "$HOME/.local/bin/hermes" ]]; then + ln -sf "$HOME/.local/bin/hermes" "$OPT/bin/hermes" + elif [[ -x /usr/local/bin/hermes ]]; then + ln -sf /usr/local/bin/hermes "$OPT/bin/hermes" + else + log "hermes install FAILED: launcher not found in \$HOME/.local/bin or /usr/local/bin" + return 1 + fi own_hermes_home - log "hermes installed -> $(command -v hermes || echo '/usr/local/bin/hermes')" + log "hermes installed -> $(command -v hermes || echo "$OPT/bin/hermes")" else rm -f "$installer" log "hermes install FAILED" @@ -210,10 +232,16 @@ verify_required() { for cmd in node uv nvim claude codex; do command -v "$cmd" >/dev/null 2>&1 || { log "missing required tool: $cmd"; return 1; } done - if [[ "${INSTALL_HERMES:-0}" == 1 ]]; then command -v hermes >/dev/null 2>&1; fi - if [[ "${INSTALL_EXECUTOR:-0}" == 1 ]]; then command -v executor >/dev/null 2>&1; fi if [[ "${INSTALL_HERMES:-0}" == 1 ]]; then - [[ "$(git -C /usr/local/lib/hermes-agent rev-parse HEAD)" == "${HERMES_GIT_SHA}" ]] + command -v hermes >/dev/null 2>&1 || { log "missing required tool: hermes"; return 1; } + fi + if [[ "${INSTALL_EXECUTOR:-0}" == 1 ]]; then + command -v executor >/dev/null 2>&1 || { log "missing required tool: executor"; return 1; } + fi + if [[ "${INSTALL_HERMES:-0}" == 1 ]]; then + [[ "$(git -C /usr/local/lib/hermes-agent rev-parse HEAD)" == "${HERMES_GIT_SHA}" ]] || { + log "Hermes checkout HEAD does not match pinned HERMES_GIT_SHA"; return 1; + } fi } From 85c041386bdf9aee59e3c2f4b36a7fd93a32f1d4 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:06:48 -0700 Subject: [PATCH 05/28] fix(P1-1): lock hb's gateway check-then-spawn against double-spawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _start_gateway checked _gateway_running and, if not running, spawned `hermes gateway run --replace` with no lock between the check and the spawn. hb-workload's 20s reconcile loop and a host-initiated `hb up` (via cmd_open/_prepare_runtime/cmd_repair) could both observe "not running" and both exec the gateway, directly contradicting the single-writer guarantee the gateway-enable confirmation ceremony exists to protect. Added an mkdir-based lock (same pattern as box's _try_acquire_lock, with the same stale-pid recovery) around the check-then-spawn in _start_gateway. Every caller of _start_gateway (hb up, hb reconcile, gateway_enable) goes through the same function, so one lock covers all paths without needing to duplicate it in hb-workload. Verified in isolation: 8 concurrent invocations of the locked check-then-spawn section raced against each other, and exactly 1 actually ran the "spawn" — confirms the lock serializes the window that was previously racy. Co-Authored-By: Claude Sonnet 5 --- docs/audit-2026-06-30-fixes.md | 2 +- guest/hb | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index c433c84..5ba9837 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -214,7 +214,7 @@ either real timing races or a real single-writer Discord setup to observe directly). Each entry says how to construct the race/condition if you want to confirm it before fixing. -### P1-1: gateway can be double-spawned (race in `hb`/`hb-workload`) +### P1-1: gateway can be double-spawned (race in `hb`/`hb-workload`) — DONE **Files:** `guest/hb:74-88` (`_start_gateway`), `guest/hb-workload:67-96` (`reconcile_once`, called every 20s by `guest/hb-workload:100-104`'s loop). diff --git a/guest/hb b/guest/hb index ef1934f..831402d 100755 --- a/guest/hb +++ b/guest/hb @@ -13,6 +13,7 @@ LEGACY_QUIESCE_FILE="${HB_LEGACY_QUIESCE_FILE:-/tmp/hermes-box-executor-paused}" GATEWAY_DISABLED="$STATE_DIR/gateway-disabled" GATEWAY_POLICY="$STATE_DIR/gateway-policy-initialized" GATEWAY_PID="$STATE_DIR/gateway.pid" +GATEWAY_LOCK="$STATE_DIR/gateway.lock" WIRE_VERSION=http-v1 WIRED="$AGENT_HOME/.hermes-box-wired" TOKEN_FILE="$AGENT_HOME/.executor/server-control/auth.json" @@ -71,10 +72,37 @@ _gateway_running() { pgrep -u "$(id -u)" -f '[h]ermes( .*)? gateway run' >/dev/null 2>&1 } +# mkdir-based lock (same pattern as box's _try_acquire_lock) around the +# check-then-spawn below. Without it, hb-workload's 20s reconcile loop and a +# host-initiated `hb up` can both observe "not running" and both exec +# `hermes gateway run --replace`, violating the single-writer guarantee. +_acquire_gateway_lock() { + mkdir -p "$STATE_DIR" 2>/dev/null || true + local attempt owner + for ((attempt = 0; attempt < 50; attempt++)); do + if mkdir "$GATEWAY_LOCK" 2>/dev/null; then + printf '%s\n' "$$" >"$GATEWAY_LOCK/pid" + return 0 + fi + owner="" + [[ -r "$GATEWAY_LOCK/pid" ]] && owner="$(cat "$GATEWAY_LOCK/pid" 2>/dev/null)" + if [[ "$owner" =~ ^[0-9]+$ ]] && ! kill -0 "$owner" 2>/dev/null; then + rm -rf "$GATEWAY_LOCK" + continue + fi + sleep 0.1 + done + return 1 +} + +_release_gateway_lock() { rm -rf "$GATEWAY_LOCK"; } + _start_gateway() { [[ "${INSTALL_HERMES:-0}" == 1 ]] || return 0 [[ ! -e "$QUIESCE_FILE" && ! -e "$GATEWAY_DISABLED" ]] || return 0 command -v hermes >/dev/null 2>&1 || { echo "hermes not installed" >&2; return 1; } + _acquire_gateway_lock || { echo "could not acquire Hermes gateway lock" >&2; return 1; } + trap _release_gateway_lock RETURN if _gateway_running; then local count count="$(pgrep -u "$(id -u)" -f '[h]ermes( .*)? gateway run' | wc -l | tr -d ' ')" From ecb1a53b24754132e0f8ac20d6cce4684f238dea Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:13:29 -0700 Subject: [PATCH 06/28] fix(P0-3): preserve failure evidence before rollback destroys it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed `box new`/`box load` produced exactly one line of explanation ("box creation failed") before _rollback_created deleted the VM — the only place the real error lived. Both live P0-1/P0-2 failures during this audit were only diagnosable by bypassing the trap-driven rollback manually; a normal user had no equivalent escape hatch. - _provision_into now optionally tees the provisioning transcript (the part that already shells guest output through the host process) to a caller- supplied log file. - cmd_new and cmd_load capture that transcript to a temp file, and on failure persist it to backups/.failed/-.log before calling die, so the evidence survives rollback. - Added BOX_KEEP_ON_FAILURE=1 (or `box new --keep-on-failure`) to skip rollback entirely and leave the VM up for direct inspection, with hints printed for how to look at it and clean it up afterward. - cmd_new's die message now names which stage failed (creating the base VM / provisioning / preparing the runtime) instead of collapsing all three into one undifferentiated message. Verified live: deliberately broke HERMES_GIT_SHA to force a real provisioning failure under BOX_KEEP_ON_FAILURE=1. Confirmed the transcript landed in backups/.failed/ with the actual git error, the die message said "while provisioning", and the VM was left running (smolvm machine status showed it alive) instead of being silently deleted. Co-Authored-By: Claude Sonnet 5 --- box | 69 ++++++++++++++++++++++++++++++---- docs/audit-2026-06-30-fixes.md | 2 +- 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/box b/box index 185bdf5..458d1b5 100755 --- a/box +++ b/box @@ -571,7 +571,7 @@ _create_base() { } _provision_into() { - local n="$1" mode="${2:-full}" contexts=(provision guest box.env) + local n="$1" mode="${2:-full}" log_file="${3:-}" contexts=(provision guest box.env) cmd if [[ "$mode" == full ]]; then _validate_bundle [[ -z "${HERMES_GIT_BUNDLE:-}" ]] || contexts+=("$HERMES_GIT_BUNDLE") @@ -586,11 +586,17 @@ _provision_into() { else say "Provisioning node/uv, claude, codex, hermes, executor (a few minutes)…" fi - smolvm machine exec --name "$n" -- bash -lc " - set -e + cmd="set -e rm -rf /tmp/ctx && mkdir -p /tmp/ctx tar xzf /tmp/ctx.tgz -C /tmp/ctx bash /tmp/ctx/provision/provision.sh '$mode'" + # Optionally tee the transcript to a host-side file so a caller can persist + # it as failure evidence before rollback deletes the only other copy. + if [[ -n "$log_file" ]]; then + smolvm machine exec --name "$n" -- bash -lc "$cmd" 2>&1 | tee "$log_file" + else + smolvm machine exec --name "$n" -- bash -lc "$cmd" + fi } _prepare_runtime() { @@ -633,13 +639,35 @@ _rollback_created() { } cmd_new() { - local n="${1:?usage: box new }" + local n="${1:?usage: box new [--keep-on-failure]}" keep_on_failure=0 stage="" log_file failed_log="" + [[ "${2:-}" != --keep-on-failure && "${BOX_KEEP_ON_FAILURE:-0}" != 1 ]] || keep_on_failure=1 _validate_name "$n" _host_preflight provision _acquire_lock "$n" say "Creating instance '$n' from $BASE_IMAGE" - if ! _create_base "$n" || ! _provision_into "$n" full || ! _prepare_runtime "$n"; then - die "box creation failed" + log_file="$(mktemp)" + _track_temp "$log_file" + if ! _create_base "$n"; then + stage="creating the base VM" + elif ! _provision_into "$n" full "$log_file"; then + stage="provisioning" + elif ! _prepare_runtime "$n"; then + stage="preparing the runtime (gateway start, MCP wiring, doctor)" + fi + if [[ -n "$stage" ]]; then + if [[ -s "$log_file" ]]; then + mkdir -p backups/.failed + failed_log="backups/.failed/${n}-$(date -u +%Y%m%dT%H%M%SZ).log" + cp "$log_file" "$failed_log" + warn "provisioning transcript saved to $failed_log" + fi + if [[ "$keep_on_failure" == 1 ]]; then + _clear_created "$n" + warn "leaving '$n' in place for inspection (--keep-on-failure / BOX_KEEP_ON_FAILURE=1)" + warn "inspect with: smolvm machine exec --name '$n' -- bash" + warn "remove when done with: ./box rm '$n' --force" + fi + die "box creation failed while $stage" fi _clear_created "$n" local port @@ -706,6 +734,8 @@ cmd_save() { cmd_load() { local file="${1:?usage: box load [name]}" n="${2:-restored}" raw guest_tmp + local keep_on_failure=0 log_file failed_log="" + [[ "${BOX_KEEP_ON_FAILURE:-0}" != 1 ]] || keep_on_failure=1 [[ -f "$file" ]] || die "no such archive: $file" _validate_name "$n" _host_preflight full @@ -717,7 +747,20 @@ cmd_load() { _validate_archive "$raw" || die "archive is invalid or unsafe" say "Creating and provisioning fresh instance '$n'" - if ! _create_base "$n" || ! _provision_into "$n" full; then + log_file="$(mktemp)" + _track_temp "$log_file" + if ! _create_base "$n" || ! _provision_into "$n" full "$log_file"; then + if [[ -s "$log_file" ]]; then + mkdir -p backups/.failed + failed_log="backups/.failed/${n}-$(date -u +%Y%m%dT%H%M%SZ).log" + cp "$log_file" "$failed_log" + warn "provisioning transcript saved to $failed_log" + fi + if [[ "$keep_on_failure" == 1 ]]; then + _clear_created "$n" + warn "leaving '$n' in place for inspection (BOX_KEEP_ON_FAILURE=1)" + warn "remove when done with: ./box rm '$n' --force" + fi die "destination provisioning failed" fi @@ -766,6 +809,11 @@ cmd_load() { restore_failed=0' bash "$guest_tmp" || ! _provision_into "$n" assets || ! _guest_hb "$n" gateway-disable || ! _guest_hb "$n" verify-state >/dev/null || ! _resume_box "$n" || ! _prepare_runtime "$n"; then + if [[ "$keep_on_failure" == 1 ]]; then + _clear_created "$n" + warn "leaving '$n' in place for inspection (BOX_KEEP_ON_FAILURE=1)" + warn "remove when done with: ./box rm '$n' --force" + fi die "restore failed; attempting cleanup of the incomplete destination" fi _untrack_guest_temp "$n" "$guest_tmp" @@ -1051,7 +1099,8 @@ usage() { cat <<'EOF' hermes-box-lite — isolated, portable agent boxes (claude / codex / hermes / executor) - ./box new Create, provision, and health-check a fresh box + ./box new [--keep-on-failure] + Create, provision, and health-check a fresh box ./box enter Log in as agent and attach tmux 'main' ./box ls [--all] List managed boxes (or every smolvm VM) ./box open Open the authenticated Executor web UI @@ -1066,6 +1115,10 @@ hermes-box-lite — isolated, portable agent boxes (claude / codex / hermes / ex ./box rm [--force] Delete after confirmation (or explicit --force) Set BOX_PASSPHRASE to skip the GPG prompt. Configuration: box.env + Set BOX_KEEP_ON_FAILURE=1 (or pass --keep-on-failure to `new`) to skip + rollback on a failed `new`/`load` and leave the VM up for inspection. + A failed `new`/`load` also saves its provisioning transcript under + backups/.failed/ before any rollback runs. EOF } diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index 5ba9837..f6f818a 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -173,7 +173,7 @@ then `./box doctor tx9probe` passes, then `./box rm tx9probe --force`. --- -### P0-3: a failed `box new` destroys the only evidence of why +### P0-3: a failed `box new` destroys the only evidence of why — DONE **Symptom:** Both live failures above produced exactly one line of explanation (`error: box creation failed`) before `_rollback_created` From a557058ecdf46c7f7371fc2a266511a7b67ae07b Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:17:11 -0700 Subject: [PATCH 07/28] fix(P1-4): pin agent tooling versions instead of always installing latest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every box new/repair (full mode) installed whatever was currently `latest` on the npm registry for claude-code, codex, and executor — two boxes created a week apart, or the same box before/after a repair, could silently end up on different versions with no way to pin or reproduce. Added CLAUDE_CODE_VERSION/CODEX_VERSION/EXECUTOR_VERSION to box.env, empty by default (preserves current latest-on-install behavior). When set, install_claude/install_codex/install_executor in provision.sh now install the pinned version via npm's @version syntax. Co-Authored-By: Claude Sonnet 5 --- box.env | 7 +++++++ docs/audit-2026-06-30-fixes.md | 2 +- provision/provision.sh | 21 ++++++++++++--------- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/box.env b/box.env index f900392..405919c 100644 --- a/box.env +++ b/box.env @@ -14,6 +14,13 @@ BOX_STORAGE_GIB="" # Tool major line selected during provisioning. NODE_MAJOR="22" # NodeSource major line; claude-code, codex, executor run on Node +# Pin exact npm package versions so `box new`/`box repair` are reproducible +# instead of always installing whatever is currently `latest`. Empty (the +# default) keeps the old latest-on-install behavior. +CLAUDE_CODE_VERSION="" # e.g. "2.1.197" — see `npm view @anthropic-ai/claude-code versions` +CODEX_VERSION="" # e.g. "0.142.4" — see `npm view @openai/codex versions` +EXECUTOR_VERSION="" # e.g. "1.5.26" — see `npm view executor versions` + # --- Hermes Agent (Nous Research) ------------------------------------------- # Installed headless via the official script. Code -> /usr/local/lib (root-mode # FHS layout); state -> $HERMES_HOME on /data. diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index f6f818a..cd98f31 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -322,7 +322,7 @@ if ! _start_gateway; then fi ``` -### P1-4: agent tooling versions are unpinned +### P1-4: agent tooling versions are unpinned — DONE **Files:** `provision/provision.sh:68-69` (`npm install -g @anthropic-ai/claude-code`), `:74-75` (`@openai/codex`), `:163-165` (`executor`) — none specify a version. diff --git a/provision/provision.sh b/provision/provision.sh index 07e89da..fddd21b 100755 --- a/provision/provision.sh +++ b/provision/provision.sh @@ -68,15 +68,17 @@ install_node_uv() { } install_claude() { - log "claude-code" - "$NODE_PREFIX/bin/npm" install -g @anthropic-ai/claude-code >/dev/null 2>&1 \ - || npm install -g @anthropic-ai/claude-code >/dev/null + local pkg="@anthropic-ai/claude-code${CLAUDE_CODE_VERSION:+@$CLAUDE_CODE_VERSION}" + log "claude-code${CLAUDE_CODE_VERSION:+ ($CLAUDE_CODE_VERSION)}" + "$NODE_PREFIX/bin/npm" install -g "$pkg" >/dev/null 2>&1 \ + || npm install -g "$pkg" >/dev/null } install_codex() { - log "codex" - "$NODE_PREFIX/bin/npm" install -g @openai/codex >/dev/null 2>&1 \ - || npm install -g @openai/codex >/dev/null + local pkg="@openai/codex${CODEX_VERSION:+@$CODEX_VERSION}" + log "codex${CODEX_VERSION:+ ($CODEX_VERSION)}" + "$NODE_PREFIX/bin/npm" install -g "$pkg" >/dev/null 2>&1 \ + || npm install -g "$pkg" >/dev/null } install_hermes() { @@ -182,9 +184,10 @@ install_executor() { if [[ "${INSTALL_EXECUTOR:-0}" != "1" ]]; then log "executor: INSTALL_EXECUTOR != 1 — skipping"; return fi - log "executor (npm global)" - if "$NODE_PREFIX/bin/npm" install -g executor >/dev/null 2>&1 \ - || npm install -g executor >/dev/null 2>&1; then + local pkg="executor${EXECUTOR_VERSION:+@$EXECUTOR_VERSION}" + log "executor (npm global)${EXECUTOR_VERSION:+ ($EXECUTOR_VERSION)}" + if "$NODE_PREFIX/bin/npm" install -g "$pkg" >/dev/null 2>&1 \ + || npm install -g "$pkg" >/dev/null 2>&1; then log "executor installed -> $("$NODE_PREFIX/bin/npm" prefix -g 2>/dev/null)/bin/executor" else log "executor install FAILED" From 4f334c631cedaa94e8d26a33d099b0b1a2635133 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:18:50 -0700 Subject: [PATCH 08/28] fix(P1-2): supervise the hb-workload loop process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hb-workload was exec'd as the VM's boot command, so if the loop process itself died (OOM-killed, an unguarded edge case under its `set -u`), nothing restarted it — health reconciliation and bridge socat processes stayed unmanaged until the next VM reboot. Dropped the `exec` and wrapped hb-workload in an outer restart loop as the VM's boot command instead, so the supervisor (PID 1 in the guest) restarts it on any exit. Verified live: created a box, confirmed hb-workload was running under the new supervisor loop, force-killed it (pkill -9), and confirmed a fresh hb-workload process came back up within the 2s restart delay. Co-Authored-By: Claude Sonnet 5 --- box | 7 ++++++- docs/audit-2026-06-30-fixes.md | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/box b/box index 458d1b5..257f8a8 100755 --- a/box +++ b/box @@ -543,8 +543,13 @@ _create_base() { [[ -z "${BOX_STORAGE_GIB:-}" ]] || resources+=(--storage "$BOX_STORAGE_GIB") wl_port="${port:-4788}" _defer_signals + # No `exec` into hb-workload: if the loop process itself dies (OOM-killed, a + # bug not guarded by its `set -u`), nothing else in the VM restarts it, so + # this outer loop is the supervisor. hb-workload's own internals (gateway, + # socat bridges) are reconciled every 20s once it's running again. if smolvm machine create --net "${opts[@]}" "${resources[@]}" --name "$n" --image "$BASE_IMAGE" \ - -- bash -c "while [ ! -x /opt/hermes-box/bin/hb-workload ]; do sleep 2; done; exec runuser -u agent -- env HOME=/data/home/agent /opt/hermes-box/bin/hb-workload ${wl_port} ${api_guest_port:-0}"; then + -- bash -c "while [ ! -x /opt/hermes-box/bin/hb-workload ]; do sleep 2; done + while true; do runuser -u agent -- env HOME=/data/home/agent /opt/hermes-box/bin/hb-workload ${wl_port} ${api_guest_port:-0}; sleep 2; done"; then create_status=0 else create_status=$? diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index cd98f31..5e22ac1 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -252,7 +252,7 @@ local copy of `hb` to make the window reliably wide.) before it calls `hb reconcile`, or simply rely on (1) since `hb reconcile` already calls into `_start_gateway`. -### P1-2: `hb-workload` itself is unsupervised +### P1-2: `hb-workload` itself is unsupervised — DONE **File:** `guest/hb-workload:98-105` (`main`), invoked once at boot by `box:546-547`'s `smolvm machine create` command line. From 14b89ff42a2e419195eabc53b60c58386e9c8912 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:19:05 -0700 Subject: [PATCH 09/28] fix(P1-3): gateway_is_disabled() now checks process state too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gateway_is_disabled() only checked for the GATEWAY_DISABLED marker file. If gateway_enable() spawned the gateway via _start_gateway() and something later in that call path failed before returning, the marker was re-touched as disabled but nothing verified the already-spawned process was actually killed first — box doctor would then report "intentionally disabled" while a real gateway process was alive underneath that claim. gateway_is_disabled() now requires both the marker AND _gateway_running being false. gateway_enable()'s failure path explicitly calls _stop_gateway before re-touching the marker, instead of assuming the marker alone is sufficient. Co-Authored-By: Claude Sonnet 5 --- docs/audit-2026-06-30-fixes.md | 2 +- guest/hb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index 5e22ac1..a8bd2b3 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -283,7 +283,7 @@ process-1-exits-VM-stops semantics plus a *systemd inside the guest* unit — the latter is more in keeping with the project's existing host-side systemd pattern (`ops/systemd/`), but the cheap fix is the restart loop. -### P1-3: `gateway_is_disabled()` checks a marker file, not process state +### P1-3: `gateway_is_disabled()` checks a marker file, not process state — DONE **Files:** `guest/hb:143` (`gateway_is_disabled() { [[ -e "$GATEWAY_DISABLED" ]]; }`), consumed by `box:980-989` (`cmd_doctor`'s "host Hermes API: intentionally diff --git a/guest/hb b/guest/hb index 831402d..629e592 100755 --- a/guest/hb +++ b/guest/hb @@ -168,7 +168,7 @@ resume() { down() { pause; } is_paused() { [[ -e "$QUIESCE_FILE" ]]; } -gateway_is_disabled() { [[ -e "$GATEWAY_DISABLED" ]]; } +gateway_is_disabled() { [[ -e "$GATEWAY_DISABLED" ]] && ! _gateway_running; } gateway_enable() { local flag="${1:-}" phrase="${2:-}" @@ -181,6 +181,7 @@ gateway_enable() { if [[ -r "$STATE_DIR/import-manifest.json" ]]; then cutover_ready || return 1; fi rm -f "$GATEWAY_DISABLED" if ! _start_gateway; then + _stop_gateway || true touch "$GATEWAY_DISABLED" return 1 fi From 32404eb21c17b0d1fc01a5d2b19b08b32f1ef62f Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:25:40 -0700 Subject: [PATCH 10/28] fix(P2-1): make assets repair mode able to heal Hermes/Executor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit assets_only() only ran make_agent, install_config, place_assets, seed_data, and write-manifest — never install_hermes or install_executor. A box whose Hermes or Executor install failed or went missing could only be fixed by the full, multi-minute reinstall path (cmd_repair always calls _provision_into full); there was no cheap "retry just the broken piece" repair, and assets mode (only used by cmd_load's post-restore step) left those tools broken if they were already broken. install_hermes/install_executor are now idempotent (skip reinstall if already on PATH and, for hermes, already at the pinned git SHA — the same bar verify_required already holds them to) and assets_only calls both right after make_agent, plus verify_required at the end to confirm the repair actually worked. Verified live: created a box, deleted the hermes and executor binaries to simulate a failed install, ran assets-mode provisioning directly, and confirmed both were reinstalled and resolvable again. Re-ran assets mode a second time and confirmed the idempotent guards skipped reinstalling already-healthy tools ("hermes already installed... skipping reinstall"). Co-Authored-By: Claude Sonnet 5 --- docs/audit-2026-06-30-fixes.md | 2 +- provision/provision.sh | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index a8bd2b3..43a9a84 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -397,7 +397,7 @@ while True: ## P2 — repair/coverage gaps (code reading; no live repro needed beyond running the commands described) -### P2-1: `box repair` (assets mode) can't heal a failed Hermes/Executor install +### P2-1: `box repair` (assets mode) can't heal a failed Hermes/Executor install — DONE **Files:** `provision/provision.sh:220-227` (`assets_only`). Note `cmd_repair` (`box:994-1012`) always calls `_provision_into "$n" full`, never `assets` — diff --git a/provision/provision.sh b/provision/provision.sh index fddd21b..8a85ce1 100755 --- a/provision/provision.sh +++ b/provision/provision.sh @@ -89,6 +89,13 @@ install_hermes() { local installer_sha="${HERMES_INSTALLER_SHA256:-}" installer [[ "$sha" =~ ^[0-9a-fA-F]{40}$ ]] || { log "invalid HERMES_GIT_SHA: $sha"; return 1; } [[ "$installer_sha" =~ ^[0-9a-fA-F]{64}$ ]] || { log "invalid HERMES_INSTALLER_SHA256"; return 1; } + # Idempotent: skip the multi-minute reinstall if hermes is already on PATH + # and already checked out at the pinned commit (the same bar verify_required + # holds it to). Lets `assets` repair mode call this safely on every run. + if command -v hermes >/dev/null 2>&1 && [[ "$(git -C "$install_dir" rev-parse HEAD 2>/dev/null)" == "$sha" ]]; then + log "hermes already installed at pinned $sha — skipping reinstall" + return 0 + fi log "hermes (official installer pinned to $sha)" # We run as root and pass an explicit --dir below, which makes the installer # skip its own root/FHS auto-detection (resolve_install_layout() returns @@ -184,6 +191,12 @@ install_executor() { if [[ "${INSTALL_EXECUTOR:-0}" != "1" ]]; then log "executor: INSTALL_EXECUTOR != 1 — skipping"; return fi + # Idempotent: skip reinstall if already on PATH. Lets `assets` repair mode + # call this safely on every run instead of only on a full reinstall. + if command -v executor >/dev/null 2>&1; then + log "executor already installed — skipping reinstall" + return 0 + fi local pkg="executor${EXECUTOR_VERSION:+@$EXECUTOR_VERSION}" log "executor (npm global)${EXECUTOR_VERSION:+ ($EXECUTOR_VERSION)}" if "$NODE_PREFIX/bin/npm" install -g "$pkg" >/dev/null 2>&1 \ @@ -250,9 +263,12 @@ verify_required() { assets_only() { make_agent + install_hermes + install_executor install_config place_assets seed_data + verify_required runuser -u agent -- env HOME=/data/home/agent "$OPT/bin/hb" write-manifest log "managed assets refreshed" } From a912ac2f2ee10afde0d3d3da2b454a01fb2d896d Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:33:32 -0700 Subject: [PATCH 11/28] test(P2-2): add coverage for box ls/rm/stop/start/enter and two gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five of thirteen box subcommands had zero test coverage (ls, rm, enter, stop, start), alongside untested code paths in already-tested commands: the _assign_port exhaustion loop and control-character archive rejection. New tests/cli-surface.sh covers: - box ls: empty registry, populated registry (running + missing-from-smolvm rows), and --all (which needed the smolvm fixture's bare `machine ls` filled in — it previously fell through to exit 2). - box rm: --force happy path, the non-interactive refusal path (run under setsid to genuinely detach the controlling terminal — `[[ -r /dev/tty ]]` on Linux is a permission-bits check, not proof a terminal exists, so a plain stdin redirect doesn't reach that code path the way it would on a real interactive shell), and the "type the name to confirm" mismatch vs. match cases via a real pty (python's pty.fork(), since that prompt reads from /dev/tty directly and can't be faked with ordinary redirection). - box stop/start: delegate to the right smolvm command, and a held per-box lock blocks a concurrent stop before smolvm is ever invoked. - box enter: execs the expected `smolvm machine exec -it ... -- login -f agent` command line. - _assign_port: pre-registers a 1-port range as occupied and confirms the documented "no free host port in lo-hi" exhaustion message. tests/lifecycle-smoke.sh's existing archive-safety fixture generator gained two more cases (control character in a member name, and in a symlink's link target) alongside the eleven it already covered, confirming _validate_archive's safe_text() check end to end through both the direct validator and the box load path. Co-Authored-By: Claude Sonnet 5 --- Makefile | 3 +- docs/audit-2026-06-30-fixes.md | 2 +- tests/cli-surface.sh | 195 +++++++++++++++++++++++++++++++++ tests/fixtures/smolvm | 7 ++ tests/lifecycle-smoke.sh | 28 ++++- tests/static.sh | 3 +- 6 files changed, 231 insertions(+), 7 deletions(-) create mode 100755 tests/cli-surface.sh diff --git a/Makefile b/Makefile index 265fb78..3f677d4 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ .PHONY: check syntax lint test check-hermes-pin -SHELL_FILES := box guest/hb guest/hb-workload guest/profile.sh guest/agent-bash-profile.sh provision/provision.sh ops/tx9-host tests/static.sh tests/lifecycle-smoke.sh tests/hermes-state.sh tests/regressions.sh tests/fixtures/smolvm +SHELL_FILES := box guest/hb guest/hb-workload guest/profile.sh guest/agent-bash-profile.sh provision/provision.sh ops/tx9-host tests/static.sh tests/lifecycle-smoke.sh tests/hermes-state.sh tests/regressions.sh tests/cli-surface.sh tests/fixtures/smolvm syntax: bash -n $(SHELL_FILES) @@ -17,6 +17,7 @@ test: ./tests/lifecycle-smoke.sh ./tests/hermes-state.sh ./tests/regressions.sh + ./tests/cli-surface.sh check: syntax lint test diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index 43a9a84..5a5bd45 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -421,7 +421,7 @@ reinstall if already present and `git rev-parse HEAD` matches, like `assets_only`, immediately after `make_agent`. This makes `assets` mode a true "fix what's missing" repair instead of "config files only." -### P2-2: zero test coverage for five of thirteen `box` subcommands +### P2-2: zero test coverage for five of thirteen `box` subcommands — DONE **Files:** `box:889-1052` (`cmd_ls`, `cmd_open` partially, `cmd_rm`, `cmd_stop`, `cmd_start`, `cmd_enter`) vs. `tests/{static,lifecycle-smoke,hermes-state,regressions}.sh`. diff --git a/tests/cli-surface.sh b/tests/cli-surface.sh new file mode 100755 index 0000000..ea74ab1 --- /dev/null +++ b/tests/cli-surface.sh @@ -0,0 +1,195 @@ +#!/usr/bin/env bash +# Coverage for box subcommands that previously had zero test coverage: +# ls, rm, stop, start, enter, plus the _assign_port exhaustion path and +# control-character archive rejection (the latter lives in +# tests/lifecycle-smoke.sh, alongside the rest of the archive-safety suite). +set -euo pipefail + +PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT HUP INT TERM + +make_repo() { + local dest="$1" + mkdir -p "$dest/backups" + cp "$PROJECT_ROOT/box" "$PROJECT_ROOT/box.env" "$dest/" + cp -R "$PROJECT_ROOT/guest" "$PROJECT_ROOT/provision" "$dest/" +} + +# --- box ls (empty registry) ------------------------------------------- +ls_empty_repo="$tmp/ls-empty-repo" +make_repo "$ls_empty_repo" +out="$("$ls_empty_repo/box" ls)" +[[ "$out" == "No hermes-box-lite boxes yet. Create one: ./box new " ]] + +# --- box ls (populated registry: one running, one missing from smolvm) - +ls_repo="$tmp/ls-repo" +make_repo "$ls_repo" +mkdir -p "$tmp/ls-state/present-box" +printf 'present-box\t4790\t8650\t8650\nghost-box\t\t\t\n' >"$ls_repo/.boxes" +out="$(PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/ls.calls" \ + SMOLVM_STATE_DIR="$tmp/ls-state" "$ls_repo/box" ls)" +grep -q '^BOX *STATE' <<<"$out" +grep -q '^present-box *running *http://localhost:4790 *http://localhost:8650' <<<"$out" +grep -q '^ghost-box *missing *— *—' <<<"$out" + +# --- box ls --all (needs the smolvm fixture's bare `machine ls`) ------- +mkdir -p "$tmp/ls-state/other-vm" +out="$(PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/ls-all.calls" \ + SMOLVM_STATE_DIR="$tmp/ls-state" "$ls_repo/box" ls --all)" +grep -q 'present-box' <<<"$out" +grep -q 'other-vm' <<<"$out" + +# --- box rm --force ---------------------------------------------- +rm_repo="$tmp/rm-repo" +make_repo "$rm_repo" +mkdir -p "$tmp/rm-state/force-box" +printf 'force-box\t4790\t\t\n' >"$rm_repo/.boxes" +rm_sentinel="$tmp/rm-force.calls" +out="$(PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$rm_sentinel" \ + SMOLVM_STATE_DIR="$tmp/rm-state" "$rm_repo/box" rm force-box --force)" +grep -q "deleted 'force-box'" <<<"$out" +grep -q -- '--name force-box -f' "$rm_sentinel" +[[ ! -s "$rm_repo/.boxes" ]] +[[ ! -d "$tmp/rm-state/force-box" ]] + +# --- box rm without --force and without a controlling terminal --- +# `[[ -r /dev/tty ]]` on Linux is a permission-bits check, not proof a +# controlling terminal exists, so a plain stdin redirect doesn't reach the +# "refusing non-interactive deletion" message here (it does on a real +# interactive shell). setsid genuinely detaches the terminal, so this +# asserts the safety property that actually matters regardless of which +# of the two failure modes fires: no confirmation reached, no deletion. +rm_noforce_repo="$tmp/rm-noforce-repo" +make_repo "$rm_noforce_repo" +mkdir -p "$tmp/rm-noforce-state/no-force-box" +printf 'no-force-box\t\t\t\n' >"$rm_noforce_repo/.boxes" +noforce_sentinel="$tmp/rm-noforce.calls" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$noforce_sentinel" \ + SMOLVM_STATE_DIR="$tmp/rm-noforce-state" \ + timeout 10 setsid "$rm_noforce_repo/box" rm no-force-box /dev/null 2>&1; then + echo "box rm without --force unexpectedly succeeded with no controlling terminal" >&2 + exit 1 +fi +[[ ! -e "$noforce_sentinel" ]] || { echo "smolvm ran before rm confirmation" >&2; exit 1; } +grep -q '^no-force-box' "$rm_noforce_repo/.boxes" + +# --- box rm : typed-name confirmation, both mismatch and match ---- +# Exercises the real /dev/tty prompt path via a pty, since `--force` and the +# no-tty refusal above don't reach this code at all. +rm_tty_repo="$tmp/rm-tty-repo" +make_repo "$rm_tty_repo" +mkdir -p "$tmp/rm-tty-state/tty-box" +printf 'tty-box\t\t\t\n' >"$rm_tty_repo/.boxes" +tty_sentinel="$tmp/rm-tty.calls" +PROJECT_ROOT="$PROJECT_ROOT" RM_REPO="$rm_tty_repo" RM_STATE="$tmp/rm-tty-state" \ + RM_SENTINEL="$tty_sentinel" python3 <<'PY' +import os +import pty +import sys +import time + +project_root = os.environ["PROJECT_ROOT"] +repo = os.environ["RM_REPO"] +state = os.environ["RM_STATE"] +sentinel = os.environ["RM_SENTINEL"] + + +def run(answer): + pid, master_fd = pty.fork() + if pid == 0: + os.environ["PATH"] = project_root + "/tests/fixtures:" + os.environ["PATH"] + os.environ["SMOLVM_CALLED"] = sentinel + os.environ["SMOLVM_STATE_DIR"] = state + os.execv(repo + "/box", [repo + "/box", "rm", "tty-box"]) + buf = b"" + deadline = time.time() + 10 + while b"Type the box name" not in buf and time.time() < deadline: + try: + chunk = os.read(master_fd, 4096) + except OSError: + break + if not chunk: + break + buf += chunk + os.write(master_fd, (answer + "\n").encode()) + try: + while True: + chunk = os.read(master_fd, 4096) + if not chunk: + break + buf += chunk + except OSError: + pass + _, status = os.waitpid(pid, 0) + code = os.WEXITSTATUS(status) if os.WIFEXITED(status) else -1 + return code, buf + +code, output = run("wrong-name") +if code != 1 or b"deletion cancelled" not in output: + sys.exit(f"mismatched confirmation did not refuse deletion: code={code} output={output!r}") +if os.path.exists(sentinel): + sys.exit("smolvm ran despite a confirmation mismatch") + +code, output = run("tty-box") +if code != 0 or b"deleted 'tty-box'" not in output: + sys.exit(f"matching confirmation did not delete the box: code={code} output={output!r}") +PY +[[ ! -s "$rm_tty_repo/.boxes" ]] +[[ ! -d "$tmp/rm-tty-state/tty-box" ]] + +# --- box stop / box start: happy path delegates to smolvm --------------- +stop_repo="$tmp/stop-repo" +make_repo "$stop_repo" +mkdir -p "$tmp/stop-state/stop-box" +stop_sentinel="$tmp/stop.calls" +PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$stop_sentinel" \ + SMOLVM_STATE_DIR="$tmp/stop-state" "$stop_repo/box" stop stop-box >/dev/null +grep -q -- 'machine stop --name stop-box' "$stop_sentinel" +start_sentinel="$tmp/start.calls" +PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$start_sentinel" \ + SMOLVM_STATE_DIR="$tmp/stop-state" "$stop_repo/box" start stop-box >/dev/null +grep -q -- 'machine start --name stop-box' "$start_sentinel" + +# --- box stop: a held per-box lock blocks a concurrent operation -------- +lock_repo="$tmp/lock-repo" +make_repo "$lock_repo" +mkdir -p "$tmp/lock-state/locked-box" "$lock_repo/.box-locks/locked-box.lock" +printf '%s\n' "$$" >"$lock_repo/.box-locks/locked-box.lock/pid" +locked_sentinel="$tmp/locked.calls" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$locked_sentinel" \ + SMOLVM_STATE_DIR="$tmp/lock-state" "$lock_repo/box" stop locked-box >/dev/null 2>"$tmp/locked.err"; then + echo "box stop unexpectedly succeeded against a held lock" >&2 + exit 1 +fi +grep -q "already holds the 'locked-box' lock" "$tmp/locked.err" +[[ ! -e "$locked_sentinel" ]] || { echo "smolvm ran despite a held lock" >&2; exit 1; } + +# --- box enter: execs the expected smolvm command line ------------------- +enter_repo="$tmp/enter-repo" +make_repo "$enter_repo" +mkdir -p "$tmp/enter-state/enter-box" +enter_sentinel="$tmp/enter.calls" +PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$enter_sentinel" \ + SMOLVM_BEHAVIOR=success \ + SMOLVM_STATE_DIR="$tmp/enter-state" "$enter_repo/box" enter enter-box >/dev/null +grep -q -- 'machine start --name enter-box' "$enter_sentinel" +grep -q -- 'machine exec -it --name enter-box -- login -f agent' "$enter_sentinel" + +# --- _assign_port: the exhaustion path dies with the documented message - +port_repo="$tmp/port-repo" +make_repo "$port_repo" +printf 'port-hog\t39999\t\t\n' >"$port_repo/.boxes" +if err="$( + cd "$port_repo" + # shellcheck disable=SC1091 + source ./box + _assign_port 39999 39999 testlabel 2>&1 +)"; then + echo "_assign_port unexpectedly succeeded against an exhausted range" >&2 + exit 1 +else + grep -q 'no free host port in 39999-39999' <<<"$err" +fi + +echo "cli-surface checks passed" diff --git a/tests/fixtures/smolvm b/tests/fixtures/smolvm index f372dbb..1d9a800 100755 --- a/tests/fixtures/smolvm +++ b/tests/fixtures/smolvm @@ -19,6 +19,13 @@ name_from_args() { [[ "${1:-}" == machine ]] || exit 2 case "${2:-}" in + ls) + printf 'NAME\tSTATUS\n' + for entry in "$state"/*/; do + [[ -d "$entry" ]] || continue + printf '%s\trunning\n' "$(basename "$entry")" + done + ;; status) name="$(name_from_args "$@")" [[ "$behavior" != delete-status-fail ]] || exit 42 diff --git a/tests/lifecycle-smoke.sh b/tests/lifecycle-smoke.sh index 2b061d0..ef1c2a2 100755 --- a/tests/lifecycle-smoke.sh +++ b/tests/lifecycle-smoke.sh @@ -18,7 +18,8 @@ python3 - "$tmp/absolute-symlink.tgz" "$tmp/traversal-symlink.tgz" \ "$tmp/invalid-hardlink.tgz" "$tmp/special-file.tgz" "$tmp/home-link.tgz" \ "$tmp/cycle-link.tgz" "$tmp/pivot-link.tgz" "$tmp/root-symlink.tgz" \ "$tmp/root-hardlink.tgz" "$tmp/linked-parent-alias.tgz" \ - "$tmp/linked-parent-regular.tgz" <<'PY' + "$tmp/linked-parent-regular.tgz" "$tmp/control-char-name.tgz" \ + "$tmp/control-char-link.tgz" <<'PY' import io import sys import tarfile @@ -137,6 +138,22 @@ with tarfile.open(sys.argv[11], "w:gz") as archive: child = tarfile.TarInfo("home/agent/up/owned") child.size = len(payload) archive.addfile(child, io.BytesIO(payload)) + +with tarfile.open(sys.argv[12], "w:gz") as archive: + archive.addfile(directory("home")) + archive.addfile(directory("home/agent")) + bad = tarfile.TarInfo("home/agent/bad\x01name") + payload = b"control character in member name\n" + bad.size = len(payload) + archive.addfile(bad, io.BytesIO(payload)) + +with tarfile.open(sys.argv[13], "w:gz") as archive: + archive.addfile(directory("home")) + archive.addfile(directory("home/agent")) + link = tarfile.TarInfo("home/agent/bad-link") + link.type = tarfile.SYMTYPE + link.linkname = "bad\x01target" + archive.addfile(link) PY ( @@ -152,7 +169,8 @@ PY "$tmp/invalid-hardlink.tgz" "$tmp/special-file.tgz" "$tmp/home-link.tgz" \ "$tmp/cycle-link.tgz" "$tmp/pivot-link.tgz" "$tmp/root-symlink.tgz" \ "$tmp/root-hardlink.tgz" "$tmp/linked-parent-alias.tgz" \ - "$tmp/linked-parent-regular.tgz"; do + "$tmp/linked-parent-regular.tgz" "$tmp/control-char-name.tgz" \ + "$tmp/control-char-link.tgz"; do if _validate_archive "$unsafe"; then echo "linked archive passed validation: $unsafe" >&2 exit 1 @@ -176,7 +194,8 @@ gpg --batch --yes --pinentry-mode loopback --passphrase "$BOX_PASSPHRASE" \ gpg --batch --yes --pinentry-mode loopback --passphrase "$BOX_PASSPHRASE" \ --symmetric --cipher-algo AES256 -o "$tmp/invalid.tar.gz.gpg" "$tmp/invalid.tgz" for unsafe in absolute-symlink traversal-symlink invalid-hardlink special-file home-link cycle-link \ - pivot-link root-symlink root-hardlink linked-parent-alias linked-parent-regular; do + pivot-link root-symlink root-hardlink linked-parent-alias linked-parent-regular \ + control-char-name control-char-link; do gpg --batch --yes --pinentry-mode loopback --passphrase "$BOX_PASSPHRASE" \ --symmetric --cipher-algo AES256 -o "$tmp/${unsafe}.tar.gz.gpg" "$tmp/${unsafe}.tgz" done @@ -195,7 +214,8 @@ PY [[ -z "$(find "$tmp/host-tmp" -mindepth 1 -maxdepth 1 -print -quit)" ]] for unsafe in absolute-symlink traversal-symlink invalid-hardlink special-file home-link cycle-link \ - pivot-link root-symlink root-hardlink linked-parent-alias linked-parent-regular; do + pivot-link root-symlink root-hardlink linked-parent-alias linked-parent-regular \ + control-char-name control-char-link; do unsafe_sentinel="$tmp/${unsafe}-smolvm-called" if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$unsafe_sentinel" \ SMOLVM_STATE_DIR="$tmp/smolvm-state" TMPDIR="$tmp/host-tmp" \ diff --git a/tests/static.sh b/tests/static.sh index 491783c..f713e94 100755 --- a/tests/static.sh +++ b/tests/static.sh @@ -16,11 +16,12 @@ files=( tests/lifecycle-smoke.sh tests/hermes-state.sh tests/regressions.sh + tests/cli-surface.sh tests/fixtures/smolvm ) bash -n "${files[@]}" -for file in box guest/hb guest/hb-workload guest/hermes-state provision/provision.sh ops/tx9-host tests/lifecycle-smoke.sh tests/hermes-state.sh tests/regressions.sh tests/fixtures/smolvm; do +for file in box guest/hb guest/hb-workload guest/hermes-state provision/provision.sh ops/tx9-host tests/lifecycle-smoke.sh tests/hermes-state.sh tests/regressions.sh tests/cli-surface.sh tests/fixtures/smolvm; do [[ -x "$file" ]] || { echo "not executable: $file" >&2; exit 1; } done From 618b558ddb4748cc1eda9adc2eeae6ff8ef5d940 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:35:52 -0700 Subject: [PATCH 12/28] ci(P2-3): run make check on every push and PR to main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit make check is fast and fully hermetic (the tests/fixtures/smolvm stub means no real VM/KVM/network access is needed), so every regression the suite catches previously depended on a human remembering to run it locally before merging. Added .github/workflows/check.yml, installs shellcheck + jq (python3 is already on the runner image) and runs `make check`. Updated the README and the pinned assertion in tests/static.sh that previously asserted CI didn't exist yet — left it as a real assertion (checks the workflow file exists and actually runs `make check`) rather than quietly deleting the check. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/check.yml | 19 +++++++++++++++++++ README.md | 5 +++-- docs/audit-2026-06-30-fixes.md | 2 +- tests/static.sh | 4 +++- 4 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/check.yml diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml new file mode 100644 index 0000000..8103520 --- /dev/null +++ b/.github/workflows/check.yml @@ -0,0 +1,19 @@ +name: check + +on: + push: + branches: [main] + pull_request: + branches: [main] + +jobs: + make-check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install dependencies + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends shellcheck jq + - name: make check + run: make check diff --git a/README.md b/README.md index 45675fd..069cf15 100644 --- a/README.md +++ b/README.md @@ -224,5 +224,6 @@ make check This requires ShellCheck, Python 3, and jq, and runs Bash syntax checks, static contract checks, protocol-health regressions, and local archive/lifecycle smoke -tests. It does not create a VM or contact upstream installers. There is -intentionally no CI configuration yet. +tests. It does not create a VM or contact upstream installers. `make check` is +fully hermetic, so `.github/workflows/check.yml` runs it on every push and +pull request to `main`. diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index 5a5bd45..27a9916 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -454,7 +454,7 @@ archive-path rejection (`box:378-380`'s `safe_text()`), and true per-box generator in `tests/lifecycle-smoke.sh`, which already covers eleven other cases there). -### P2-3: no CI +### P2-3: no CI — DONE **Files:** `README.md:228` and `tests/static.sh:129` both assert `"intentionally no CI configuration yet"` (the latter as a literal grep diff --git a/tests/static.sh b/tests/static.sh index f713e94..ea01ee2 100755 --- a/tests/static.sh +++ b/tests/static.sh @@ -127,6 +127,8 @@ if grep -q 'guest_tmp="/root/hermes-import-' box; then exit 1 fi grep -qi 'networking is always enabled' README.md -grep -q 'intentionally no CI configuration yet' README.md +grep -q 'check.yml. runs it on every push' README.md +[[ -f .github/workflows/check.yml ]] +grep -q 'make check' .github/workflows/check.yml echo "static checks passed" From c4822522f90f740460d5b162c18e705c6df47d50 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:47:32 -0700 Subject: [PATCH 13/28] refactor(P2-4): split tests/regressions.sh, extract shared tests/lib.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tests/regressions.sh had grown to 1131 lines covering ~10 unrelated concern areas in one flat file, so a single failing assertion meant scanning up to 1131 lines for context. The tempdir+trap+cleanup boilerplate was also independently reimplemented (with slightly different rigor) in three other test files, and make_repo() was duplicated inline in lifecycle-smoke.sh. Extracted tests/lib.sh: the canonical tmp-dir+trap setup (with the same signal-to-exit-code mapping box itself uses — 129/130/143 — including the self-check assertions that previously only lived in regressions.sh), wait_for_file/wait_for_pattern, and make_repo(). All four pre-existing test files (lifecycle-smoke.sh, hermes-state.sh, plus the new cli-surface.sh) now source it instead of duplicating their own version. Split regressions.sh into five files by concern, grouped by what they actually source/invoke (verified with a line-by-line diff against the original to confirm zero content was lost or duplicated across the split): - regressions-tx9-host.sh: host-preflight checks, ops/tx9-host health/supervise/backup-credential behavior - regressions-box-core.sh: provisioning ownership (own_hermes_home), restore permission modeling, resource defaults, Git bundle verification - regressions-hb-workload.sh: guest/hb + guest/hb-workload behavior (reconcile, bridges, gateway enable/disable, doctor, MCP wiring) - regressions-doctor-mcp.sh: box's cmd_doctor + the MCP protocol-health suite, plus cmd_open/cmd_repair lifecycle guards - regressions-lock-rollback.sh: registry/lock/rollback races, save/restore failure injection Makefile's `test` target and SHELL_FILES now glob tests/regressions-*.sh (and tests/static.sh mirrors the same glob for its own executable-bit and syntax checks), so adding a new regression-category file doesn't require touching either. Co-Authored-By: Claude Sonnet 5 --- Makefile | 6 +- docs/audit-2026-06-30-fixes.md | 2 +- tests/cli-surface.sh | 12 +- tests/hermes-state.sh | 6 +- tests/lib.sh | 62 ++ tests/lifecycle-smoke.sh | 10 +- tests/regressions-box-core.sh | 90 +++ tests/regressions-doctor-mcp.sh | 357 +++++++++ tests/regressions-hb-workload.sh | 233 ++++++ tests/regressions-lock-rollback.sh | 326 ++++++++ tests/regressions-tx9-host.sh | 104 +++ tests/regressions.sh | 1131 ---------------------------- tests/static.sh | 9 +- 13 files changed, 1192 insertions(+), 1156 deletions(-) create mode 100644 tests/lib.sh create mode 100755 tests/regressions-box-core.sh create mode 100755 tests/regressions-doctor-mcp.sh create mode 100755 tests/regressions-hb-workload.sh create mode 100755 tests/regressions-lock-rollback.sh create mode 100755 tests/regressions-tx9-host.sh delete mode 100755 tests/regressions.sh diff --git a/Makefile b/Makefile index 3f677d4..8a87922 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,8 @@ .PHONY: check syntax lint test check-hermes-pin -SHELL_FILES := box guest/hb guest/hb-workload guest/profile.sh guest/agent-bash-profile.sh provision/provision.sh ops/tx9-host tests/static.sh tests/lifecycle-smoke.sh tests/hermes-state.sh tests/regressions.sh tests/cli-surface.sh tests/fixtures/smolvm +# tests/regressions-*.sh is globbed so new split-out regression files don't +# require touching this Makefile. +SHELL_FILES := box guest/hb guest/hb-workload guest/profile.sh guest/agent-bash-profile.sh provision/provision.sh ops/tx9-host tests/lib.sh tests/static.sh tests/lifecycle-smoke.sh tests/hermes-state.sh tests/cli-surface.sh $(wildcard tests/regressions-*.sh) tests/fixtures/smolvm syntax: bash -n $(SHELL_FILES) @@ -16,8 +18,8 @@ test: ./tests/static.sh ./tests/lifecycle-smoke.sh ./tests/hermes-state.sh - ./tests/regressions.sh ./tests/cli-surface.sh + for f in tests/regressions-*.sh; do ./"$$f"; done check: syntax lint test diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index 27a9916..81b3537 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -475,7 +475,7 @@ run `make check` locally before merging. CI is added (which is by design — don't quietly delete that assertion; update what it asserts). -### P2-4: `tests/regressions.sh` structure and shared-helper duplication +### P2-4: `tests/regressions.sh` structure and shared-helper duplication — DONE **File:** `tests/regressions.sh` (1131 lines, ~10 unrelated concern areas in one flat file — host-preflight checks, `tx9-host` supervise/backup behavior, diff --git a/tests/cli-surface.sh b/tests/cli-surface.sh index ea74ab1..c84500c 100755 --- a/tests/cli-surface.sh +++ b/tests/cli-surface.sh @@ -5,16 +5,8 @@ # tests/lifecycle-smoke.sh, alongside the rest of the archive-safety suite). set -euo pipefail -PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -tmp="$(mktemp -d)" -trap 'rm -rf "$tmp"' EXIT HUP INT TERM - -make_repo() { - local dest="$1" - mkdir -p "$dest/backups" - cp "$PROJECT_ROOT/box" "$PROJECT_ROOT/box.env" "$dest/" - cp -R "$PROJECT_ROOT/guest" "$PROJECT_ROOT/provision" "$dest/" -} +# shellcheck source=tests/lib.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" # --- box ls (empty registry) ------------------------------------------- ls_empty_repo="$tmp/ls-empty-repo" diff --git a/tests/hermes-state.sh b/tests/hermes-state.sh index 368aef1..adeeead 100755 --- a/tests/hermes-state.sh +++ b/tests/hermes-state.sh @@ -1,9 +1,9 @@ #!/usr/bin/env bash set -euo pipefail -ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -tmp="$(mktemp -d)" -trap 'rm -rf "$tmp"' EXIT HUP INT TERM +# shellcheck source=tests/lib.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" +ROOT="$PROJECT_ROOT" python3 - "$tmp" <<'PY' import json, sqlite3, stat, struct, sys, zipfile diff --git a/tests/lib.sh b/tests/lib.sh new file mode 100644 index 0000000..b239d6b --- /dev/null +++ b/tests/lib.sh @@ -0,0 +1,62 @@ +#!/usr/bin/env bash +# Shared test harness: a fresh $tmp per file, cleaned up on exit or signal +# using the same signal-to-exit-code mapping box itself uses (129/130/143), +# plus make_repo() and the wait_for_* polling helpers used across the suite. +# Source this after `set -euo pipefail`; do not execute it directly. + +PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +tmp="$(mktemp -d)" +pids=() +cleaned_up=0 + +cleanup() { + local pid + [[ "$cleaned_up" == 0 ]] || return 0 + cleaned_up=1 + for pid in "${pids[@]:-}"; do + [[ -n "$pid" ]] && kill "$pid" >/dev/null 2>&1 || true + done + rm -rf "$tmp" +} + +handle_signal() { + local status="$1" + trap - EXIT HUP INT TERM + cleanup + exit "$status" +} + +trap cleanup EXIT +trap 'handle_signal 129' HUP +trap 'handle_signal 130' INT +trap 'handle_signal 143' TERM +[[ "$(trap -p HUP)" == *"handle_signal 129"* ]] +[[ "$(trap -p INT)" == *"handle_signal 130"* ]] +[[ "$(trap -p TERM)" == *"handle_signal 143"* ]] + +wait_for_file() { + local file="$1" attempt + for ((attempt = 0; attempt < 100; attempt++)); do + [[ -s "$file" ]] && return + sleep 0.02 + done + echo "timed out waiting for $file" >&2 + return 1 +} + +wait_for_pattern() { + local pattern="$1" file="$2" attempt + for ((attempt = 0; attempt < 100; attempt++)); do + grep -q "$pattern" "$file" 2>/dev/null && return + sleep 0.02 + done + echo "timed out waiting for '$pattern' in $file" >&2 + return 1 +} + +make_repo() { + local dest="$1" + mkdir -p "$dest/backups" + cp "$PROJECT_ROOT/box" "$PROJECT_ROOT/box.env" "$dest/" + cp -R "$PROJECT_ROOT/guest" "$PROJECT_ROOT/provision" "$dest/" +} diff --git a/tests/lifecycle-smoke.sh b/tests/lifecycle-smoke.sh index ef1c2a2..476bd2b 100755 --- a/tests/lifecycle-smoke.sh +++ b/tests/lifecycle-smoke.sh @@ -1,10 +1,8 @@ #!/usr/bin/env bash set -euo pipefail -PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -tmp="$(mktemp -d)" -cleanup() { rm -rf "$tmp"; } -trap cleanup EXIT HUP INT TERM +# shellcheck source=tests/lib.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" mkdir -p "$tmp/valid/home/agent/workspace" "$tmp/invalid/not-home" "$tmp/gnupg" "$tmp/host-tmp" chmod 0700 "$tmp/gnupg" @@ -236,9 +234,7 @@ fi [[ ! -e "$sentinel" ]] || { echo "VM command ran before archive validation" >&2; exit 1; } [[ -z "$(find "$tmp/host-tmp" -mindepth 1 -maxdepth 1 -print -quit)" ]] -mkdir -p "$tmp/repo" -cp "$PROJECT_ROOT/box" "$PROJECT_ROOT/box.env" "$tmp/repo/" -cp -R "$PROJECT_ROOT/guest" "$PROJECT_ROOT/provision" "$tmp/repo/" +make_repo "$tmp/repo" if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$sentinel" \ SMOLVM_STATE_DIR="$tmp/smolvm-state" "$tmp/repo/box" new rollback-probe >/dev/null 2>&1; then echo "mock provisioning failure unexpectedly succeeded" >&2 diff --git a/tests/regressions-box-core.sh b/tests/regressions-box-core.sh new file mode 100755 index 0000000..887c901 --- /dev/null +++ b/tests/regressions-box-core.sh @@ -0,0 +1,90 @@ +#!/usr/bin/env bash +# shellcheck disable=SC2016,SC2030,SC2031,SC2329 +set -euo pipefail + +# shellcheck source=tests/lib.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" + +# Root-run installer state is recursively returned to the agent and made +# writable, including nested runtime directories created during full repair. +hermes_fixture="$tmp/installer-hermes-home" +mkdir -p "$hermes_fixture/sessions/deep" "$hermes_fixture/cron" "$hermes_fixture/logs" +printf 'session\n' >"$hermes_fixture/sessions/deep/history.json" +printf 'job\n' >"$hermes_fixture/cron/jobs.json" +chmod 0500 "$hermes_fixture/sessions/deep" +chmod 0400 "$hermes_fixture/sessions/deep/history.json" "$hermes_fixture/cron/jobs.json" +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/provision/provision.sh" + HERMES_HOME="$hermes_fixture" + chown() { + [[ "$1" == -R && "$2" == agent:agent && "$3" == "$hermes_fixture" ]] + printf 'recursive agent ownership\n' >"$tmp/hermes-chown.called" + } + own_hermes_home +) +[[ -s "$tmp/hermes-chown.called" ]] +[[ -w "$hermes_fixture/sessions/deep/history.json" ]] +[[ -w "$hermes_fixture/cron/jobs.json" ]] +[[ -w "$hermes_fixture/sessions/deep" ]] + +# Restore staging grants non-owners traversal but not top-level mutation; only +# the nested agent home models owner-writable imported state. +permission_stage="$tmp/restore-permission-model" +mkdir -p "$permission_stage/home/agent" +chmod 0711 "$permission_stage" "$permission_stage/home" +chmod 0700 "$permission_stage/home/agent" +python3 - "$permission_stage" <<'PY' +import os +import stat +import sys +from pathlib import Path + +stage = Path(sys.argv[1]) +home = stage / "home" +agent = home / "agent" +for envelope in (stage, home): + mode = stat.S_IMODE(envelope.stat().st_mode) + assert mode == 0o711 + assert mode & 0o001 + assert not mode & 0o002 +assert stat.S_IMODE(agent.stat().st_mode) == 0o700 +assert os.access(agent, os.W_OK) +PY + +# Missing legacy resource settings use the documented production defaults. +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/box" + unset BOX_CPUS BOX_MEM_MIB BOX_OVERLAY_GIB BOX_STORAGE_GIB + _validate_resources + [[ "$BOX_CPUS" == 4 && "$BOX_MEM_MIB" == 8192 && "$BOX_OVERLAY_GIB" == 64 && -z "$BOX_STORAGE_GIB" ]] +) + +# Guest bundle verification is independent of the caller's repository and +# rejects incremental bundles whose prerequisite objects are unavailable. +bundle_repo="$tmp/bundle-source" +mkdir -p "$bundle_repo" "$tmp/bundle-nonrepo" "$tmp/bundle-verify-tmp" +git -C "$bundle_repo" init -q +printf 'base\n' >"$bundle_repo/state.txt" +git -C "$bundle_repo" add state.txt +git -C "$bundle_repo" -c user.name=Fixture -c user.email=fixture@example.invalid commit -q -m base +base_commit="$(git -C "$bundle_repo" rev-parse HEAD)" +git -C "$bundle_repo" bundle create "$tmp/complete.bundle" --all +printf 'next\n' >>"$bundle_repo/state.txt" +git -C "$bundle_repo" add state.txt +git -C "$bundle_repo" -c user.name=Fixture -c user.email=fixture@example.invalid commit -q -m next +git -C "$bundle_repo" bundle create "$tmp/incremental.bundle" HEAD "^$base_commit" +( + cd "$tmp/bundle-nonrepo" + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/provision/provision.sh" + TMPDIR="$tmp/bundle-verify-tmp" verify_self_contained_bundle "$tmp/complete.bundle" + if TMPDIR="$tmp/bundle-verify-tmp" verify_self_contained_bundle "$tmp/incremental.bundle"; then + echo "incremental bundle passed self-contained verification" >&2 + exit 1 + fi +) +[[ -z "$(find "$tmp/bundle-verify-tmp" -mindepth 1 -maxdepth 1 -print -quit)" ]] + +echo "box-core regression checks passed" diff --git a/tests/regressions-doctor-mcp.sh b/tests/regressions-doctor-mcp.sh new file mode 100755 index 0000000..8442d63 --- /dev/null +++ b/tests/regressions-doctor-mcp.sh @@ -0,0 +1,357 @@ +#!/usr/bin/env bash +# shellcheck disable=SC2016,SC2030,SC2031,SC2329 +set -euo pipefail + +# shellcheck source=tests/lib.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" + +start_mcp_server() { + local status="$1" label="$2" mode="${3:-valid}" bind_host="${4:-127.0.0.1}" port_file delete_file + port_file="$tmp/$label.port" + delete_file="$tmp/$label.delete" + python3 "$PROJECT_ROOT/tests/fixtures/mcp-server.py" "$port_file" "$status" "$delete_file" "$mode" "$bind_host" & + MCP_PID=$! + pids+=("$MCP_PID") + wait_for_file "$port_file" + MCP_PORT="$(cat "$port_file")" + MCP_DELETE_FILE="$delete_file" +} + +# Protocol-correct MCP initialize requires exact 200 plus a session header and +# closes the created session. Other HTTP statuses are unhealthy. +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/box" + REG="$tmp/empty-registry" + touch "$REG" + + ( + doctor_calls="$tmp/doctor.calls" + mock_quiesced=0 + mock_gateway_disabled=0 + have() { [[ "$1" == awk || "$1" == smolvm ]]; } + _acquire_lock() { :; } + _machine_exists() { :; } + _guest_hb() { + printf '%s\n' "$2" >>"$doctor_calls" + if [[ "$2" == is-paused ]]; then [[ "$mock_quiesced" == 1 ]]; return; fi + if [[ "$2" == gateway-is-disabled ]]; then [[ "$mock_gateway_disabled" == 1 ]]; return; fi + [[ "$2" != executor-token ]] || printf 'fixture-token\n' + } + mcp_attempts=0 + sleep() { :; } + _mcp_initialize() { + printf 'host-mcp\n' >>"$doctor_calls" + mcp_attempts=$((mcp_attempts + 1)) + [[ "$mcp_attempts" -ge 3 ]] + } + + doctor_output="$(cmd_doctor no-host-port)" + grep -q 'host Executor MCP: skipped (no exposed host port)' <<<"$doctor_output" + [[ "$(cat "$doctor_calls")" == $'is-paused\ndoctor' ]] + + printf 'doctor-host\t4899\n' >"$REG" + if (cmd_doctor doctor-host >/dev/null 2>&1); then + echo "exposed-port doctor passed without host health dependencies" >&2 + exit 1 + fi + [[ "$(cat "$doctor_calls")" == $'is-paused\ndoctor\nis-paused\ndoctor' ]] + + : >"$doctor_calls" + have() { :; } + cmd_doctor doctor-host >/dev/null + [[ "$(cat "$doctor_calls")" == $'is-paused\ndoctor\nexecutor-token\nhost-mcp\nhost-mcp\nhost-mcp' ]] + + : >"$doctor_calls" + mock_quiesced=1 + cmd_doctor doctor-host >/dev/null + [[ "$(cat "$doctor_calls")" == $'is-paused\ndoctor' ]] + mock_quiesced=0 + + : >"$doctor_calls" + printf 'api-host\t\t8650\t8650\n' >"$REG" + _hermes_api_healthy() { printf 'host-api\n' >>"$doctor_calls"; } + api_output="$(cmd_doctor api-host)" + grep -q 'host Hermes API: healthy' <<<"$api_output" + [[ "$(cat "$doctor_calls")" == $'is-paused\ngateway-is-disabled\ndoctor\nhost-api' ]] + + : >"$doctor_calls" + mock_gateway_disabled=1 + _hermes_api_healthy() { touch "$tmp/disabled-api-probed"; return 1; } + api_output="$(cmd_doctor api-host)" + grep -q 'host Hermes API: intentionally disabled by gateway policy' <<<"$api_output" + [[ "$(cat "$doctor_calls")" == $'is-paused\ngateway-is-disabled\ndoctor' ]] + [[ ! -e "$tmp/disabled-api-probed" ]] + mock_gateway_disabled=0 + + : >"$doctor_calls" + : >"$REG" + WIRE_EXECUTOR_MCP=0 INSTALL_EXECUTOR=0 cmd_doctor disabled-executor >/dev/null + [[ "$(cat "$doctor_calls")" == $'is-paused\ndoctor' ]] + ) + + BOX_BROWSER=true _open_url 'http://localhost:1/?_token=not-printed' + if BOX_BROWSER=definitely-not-a-browser _open_url 'http://localhost:1/?_token=not-printed'; then + echo "missing BOX_BROWSER command unexpectedly succeeded" >&2 + exit 1 + fi + uname() { printf 'Linux\n'; } + xdg-open() { return 1; } + gio() { [[ "$1" == open && "$2" == 'https://fixture.invalid/secret' ]]; } + _open_url 'https://fixture.invalid/secret' + uname() { printf 'Darwin\n'; } + open() { [[ "$1" == 'https://fixture.invalid/secret' ]]; } + _open_url 'https://fixture.invalid/secret' + + start_mcp_server 200 healthy + _hermes_api_healthy "$MCP_PORT" + _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token + wait_for_pattern initialized "$MCP_DELETE_FILE" + wait_for_pattern deleted "$MCP_DELETE_FILE" + _wait_mcp_ready "http://127.0.0.1:$MCP_PORT/mcp" fixture-token + if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" wrong-token; then + echo "invalid bearer token passed MCP health" >&2 + exit 1 + fi + malformed_code="$(curl -sS -o /dev/null -w '%{http_code}' -X POST \ + -H 'Authorization: Bearer fixture-token' -H 'Content-Type: application/json' \ + -H 'Accept: application/json,text/event-stream' --data '{not-json' \ + "http://127.0.0.1:$MCP_PORT/mcp")" + [[ "$malformed_code" == 400 ]] + for invalid_request in \ + '[]' \ + '{"jsonrpc":"2.0","id":1,"method":"initialize","params":[]}' \ + '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-03-26","capabilities":{},"clientInfo":[]}}'; do + invalid_code="$(curl -sS -o /dev/null -w '%{http_code}' -X POST \ + -H 'Authorization: Bearer fixture-token' -H 'Content-Type: application/json' \ + -H 'Accept: application/json,text/event-stream' --data "$invalid_request" \ + "http://127.0.0.1:$MCP_PORT/mcp")" + [[ "$invalid_code" == 400 ]] + done + invalid_length_code="$(curl -sS -o /dev/null -w '%{http_code}' -X POST \ + -H 'Authorization: Bearer fixture-token' -H 'Content-Type: application/json' \ + -H 'Accept: application/json,text/event-stream' -H 'Content-Length: invalid' \ + --data '{}' "http://127.0.0.1:$MCP_PORT/mcp")" + [[ "$invalid_length_code" == 400 ]] + + guest_token_dir="$tmp/guest-token/.executor/server-control" + mkdir -p "$guest_token_dir" + printf '{"token":"fixture-token"}\n' >"$guest_token_dir/auth.json" + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/guest/hb" + TOKEN_FILE="$guest_token_dir/auth.json" + EXECUTOR_PORT="$MCP_PORT" + _mcp_auth_works + printf '{"token":"wrong-token"}\n' >"$guest_token_dir/auth.json" + if _mcp_auth_works; then + echo "guest MCP health accepted an invalid bearer token" >&2 + exit 1 + fi + if _port_free "$MCP_PORT"; then + echo "occupied TCP port was reported free" >&2 + exit 1 + fi + kill "$MCP_PID" + wait "$MCP_PID" 2>/dev/null || true + + for unhealthy_status in 401 404 500; do + start_mcp_server "$unhealthy_status" "api-$unhealthy_status" + if _hermes_api_healthy "$MCP_PORT"; then + echo "Hermes API HTTP $unhealthy_status passed health" >&2 + exit 1 + fi + kill "$MCP_PID" + wait "$MCP_PID" 2>/dev/null || true + done + start_mcp_server 200 api-malformed health-malformed + if _hermes_api_healthy "$MCP_PORT"; then + echo "malformed Hermes API health JSON passed" >&2 + exit 1 + fi + kill "$MCP_PID" + wait "$MCP_PID" 2>/dev/null || true + + if python3 -c 'import socket; s = socket.socket(socket.AF_INET6); s.bind(("::1", 0)); s.close()'; then + start_mcp_server 200 ipv6-only valid ::1 + if _port_free "$MCP_PORT"; then + echo "IPv6-only occupied TCP port was reported free" >&2 + exit 1 + fi + kill "$MCP_PID" + wait "$MCP_PID" 2>/dev/null || true + else + echo "IPv6 loopback unavailable; skipping IPv6 occupancy regression" >&2 + fi + + start_mcp_server 200 malformed-json malformed-json + if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token; then + echo "malformed initialize JSON passed MCP health" >&2 + exit 1 + fi + kill "$MCP_PID" + wait "$MCP_PID" 2>/dev/null || true + + start_mcp_server 200 wrong-version wrong-version + if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token; then + echo "wrong negotiated MCP version passed health" >&2 + exit 1 + fi + kill "$MCP_PID" + wait "$MCP_PID" 2>/dev/null || true + + start_mcp_server 200 wrong-content-type wrong-content-type + if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token; then + echo "wrong initialize Content-Type passed MCP health" >&2 + exit 1 + fi + kill "$MCP_PID" + wait "$MCP_PID" 2>/dev/null || true + + start_mcp_server 200 event-stream event-stream + _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token + kill "$MCP_PID" + wait "$MCP_PID" 2>/dev/null || true + + start_mcp_server 404 missing + if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token; then + echo "HTTP 404 passed MCP health" >&2 + exit 1 + fi + kill "$MCP_PID" + wait "$MCP_PID" 2>/dev/null || true + + start_mcp_server 500 broken + if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token; then + echo "HTTP 500 passed MCP health" >&2 + exit 1 + fi + kill "$MCP_PID" + wait "$MCP_PID" 2>/dev/null || true + + free_port="$MCP_PORT" + for ((attempt = 0; attempt < 100; attempt++)); do + _port_free "$free_port" && break + sleep 0.02 + done + _port_free "$free_port" + + ready_attempts=0 + opener_attempt=0 + _mcp_initialize() { + ready_attempts=$((ready_attempts + 1)) + [[ "$ready_attempts" -ge 3 ]] + } + _open_url() { opener_attempt="$ready_attempts"; } + _wait_mcp_ready ignored ignored + _open_url 'https://fixture.invalid/secret' + [[ "$ready_attempts" == 3 && "$opener_attempt" == 3 ]] +) + +# Open refuses a quiesced box before hb up can clear its marker. +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/box" + calls="$tmp/open-quiesced.calls" + _host_preflight() { :; } + _acquire_lock() { :; } + _reg_port() { printf '4899\n'; } + _machine_running() { :; } + smolvm() { printf 'smolvm %s\n' "$*" >>"$calls"; } + _guest_hb() { + printf 'hb %s\n' "$2" >>"$calls" + [[ "$2" == is-paused ]] + } + if (cmd_open quiesced >/dev/null 2>&1); then + echo "box open unexpectedly succeeded while quiesced" >&2 + exit 1 + fi + grep -q 'hb is-paused' "$calls" + if grep -q 'hb up' "$calls"; then + echo "box open cleared quiesce through hb up" >&2 + exit 1 + fi + if grep -q 'machine start' "$calls"; then + echo "box open started the VM" >&2 + exit 1 + fi +) + +# Open refuses a stopped box and never changes its lifecycle state. +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/box" + calls="$tmp/open-stopped.calls" + _host_preflight() { :; } + _acquire_lock() { :; } + _reg_port() { printf '4899\n'; } + _machine_running() { return 1; } + smolvm() { printf 'smolvm %s\n' "$*" >>"$calls"; } + _guest_hb() { printf 'hb %s\n' "$2" >>"$calls"; } + if (cmd_open stopped >/dev/null 2>&1); then + echo "box open accepted a stopped box" >&2 + exit 1 + fi + [[ ! -e "$calls" ]] || ! grep -q 'machine start' "$calls" +) + +# Repair refreshes assets but preserves a pre-existing durable quiesce policy. +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/box" + calls="$tmp/repair-quiesced.calls" + _host_preflight() { :; } + _acquire_lock() { :; } + _machine_exists() { :; } + smolvm() { printf 'smolvm %s\n' "$*" >>"$calls"; } + _provision_into() { printf 'provision %s %s\n' "$1" "$2" >>"$calls"; } + _guest_hb() { + printf 'hb %s\n' "$2" >>"$calls" + [[ "$2" == is-paused || "$2" == doctor ]] + } + cmd_repair quiesced >/dev/null + grep -q 'provision quiesced full' "$calls" + grep -q 'hb doctor' "$calls" + if grep -Eq 'hb (up|resume)' "$calls"; then + echo "repair resumed a previously quiesced box" >&2 + exit 1 + fi +) + +# Repair validates a configured bundle before constructing the provision tar. +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/box" + calls="$tmp/repair-invalid-bundle.calls" + HERMES_GIT_BUNDLE='../invalid.bundle' + _host_preflight() { :; } + _acquire_lock() { :; } + _machine_exists() { :; } + _guest_hb() { [[ "$2" == is-paused ]]; } + smolvm() { printf '%s\n' "$*" >>"$calls"; } + if (cmd_repair invalid-bundle >/dev/null 2>&1); then + echo "repair accepted an invalid Hermes bundle path" >&2 + exit 1 + fi + if grep -q 'machine exec -i' "$calls"; then + echo "repair streamed provision context before bundle validation" >&2 + exit 1 + fi +) + +# Asset-only refreshes neither validate nor transport the optional source bundle. +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/box" + calls="$tmp/provision-mode.calls" + HERMES_GIT_BUNDLE='artifacts/hermes.bundle' + _validate_bundle() { printf 'validate\n' >>"$calls"; } + tar() { printf 'tar %s\n' "$*" >>"$calls"; printf 'fixture'; } + smolvm() { if [[ "$*" == *'machine exec -i'* ]]; then cat >/dev/null; fi; } + _provision_into fixture assets >/dev/null + [[ ! -e "$calls" ]] || ! grep -q '^validate$\|artifacts/hermes.bundle' "$calls" + _provision_into fixture full >/dev/null + grep -q '^validate$' "$calls" + grep -q 'artifacts/hermes.bundle' "$calls" +) + +echo "doctor-mcp regression checks passed" diff --git a/tests/regressions-hb-workload.sh b/tests/regressions-hb-workload.sh new file mode 100755 index 0000000..3150ecf --- /dev/null +++ b/tests/regressions-hb-workload.sh @@ -0,0 +1,233 @@ +#!/usr/bin/env bash +# shellcheck disable=SC2016,SC2030,SC2031,SC2329 +set -euo pipefail + +# shellcheck source=tests/lib.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" + +# Hermes API forwarding requires a successful reconcile, an enabled gateway, +# and a listening target API. +( + HOME="$tmp/workload-home" + mkdir -p "$HOME/.config/hermes-box" "$tmp/workload-logs" + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/guest/hb-workload" + PORT=4799 + HERMES_BRIDGE_PORT=8650 + LOGS="$tmp/workload-logs" + BOX_ENV="$tmp/workload-box.env" + LEGACY_QUIESCE_FILE="$tmp/workload-legacy-paused" + printf 'EXECUTOR_PORT=4788\nHERMES_API_PORT=8642\n' >"$BOX_ENV" + pgrep() { return 1; } + pkill() { printf 'stopped %s\n' "$*" >>"$tmp/api-bridge.events"; } + socat() { printf 'started %s\n' "$*" >>"$tmp/api-bridge.events"; } + _exec_up() { return 0; } + hb() { return 1; } + _wait_api_ready() { return 0; } + reconcile_once + if grep -q 'started.*TCP-LISTEN:8650,' "$tmp/api-bridge.events" 2>/dev/null; then + echo "API bridge started after failed reconcile" >&2; exit 1 + fi + hb() { return 0; } + _wait_api_ready() { return 1; } + reconcile_once + if grep -q 'started.*TCP-LISTEN:8650,' "$tmp/api-bridge.events" 2>/dev/null; then + echo "API bridge started before its target was ready" >&2; exit 1 + fi + _wait_api_ready() { return 0; } + reconcile_once + wait + grep -q started "$tmp/api-bridge.events" + printf 'EXECUTOR_PORT=4990\nHERMES_API_PORT=8990\n' >"$BOX_ENV" + reconcile_once + wait + grep -q 'stopped.*TCP-LISTEN:4799,' "$tmp/api-bridge.events" + grep -q 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events" + grep -q 'started.*TCP:127.0.0.1:4990' "$tmp/api-bridge.events" + grep -q 'started.*TCP:127.0.0.1:8990' "$tmp/api-bridge.events" + api_stops_before="$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" + touch "$GATEWAY_DISABLED" + reconcile_once + [[ "$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" -gt "$api_stops_before" ]] + executor_stops_before="$(grep -c 'stopped.*TCP-LISTEN:4799,' "$tmp/api-bridge.events")" + api_stops_before="$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" + touch "$QUIESCE_FILE" + reconcile_once + [[ "$(grep -c 'stopped.*TCP-LISTEN:4799,' "$tmp/api-bridge.events")" -gt "$executor_stops_before" ]] + [[ "$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" -gt "$api_stops_before" ]] + rm -f "$QUIESCE_FILE" + executor_stops_before="$(grep -c 'stopped.*TCP-LISTEN:4799,' "$tmp/api-bridge.events")" + api_stops_before="$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" + hb() { touch "$tmp/legacy-workload-reconciled"; return 0; } + touch "$LEGACY_QUIESCE_FILE" + reconcile_once + [[ -e "$QUIESCE_FILE" ]] + [[ ! -e "$LEGACY_QUIESCE_FILE" ]] + [[ ! -e "$tmp/legacy-workload-reconciled" ]] + [[ "$(grep -c 'stopped.*TCP-LISTEN:4799,' "$tmp/api-bridge.events")" -gt "$executor_stops_before" ]] + [[ "$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" -gt "$api_stops_before" ]] + + migration_failure="$tmp/workload-migration-failure" + mkdir -p "$migration_failure" + chmod 0500 "$migration_failure" + QUIESCE_FILE="$migration_failure/state/quiesced" + LEGACY_QUIESCE_FILE="$tmp/workload-legacy-failure" + rm -f "$tmp/legacy-workload-reconciled" + touch "$LEGACY_QUIESCE_FILE" + reconcile_once 2>/dev/null + [[ ! -e "$QUIESCE_FILE" ]] + [[ -e "$LEGACY_QUIESCE_FILE" ]] + [[ ! -e "$tmp/legacy-workload-reconciled" ]] + chmod 0700 "$migration_failure" +) + +# Active-path acknowledgement reports success only when persistence succeeds. +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/guest/hb" + hermes-state() { return 1; } + if output="$(acknowledge_active_paths 2>&1)"; then + echo "active-path acknowledgement masked helper failure" >&2 + exit 1 + fi + [[ "$output" != *'active import paths acknowledged'* ]] +) + +# The runtime manifest must be writable by the unprivileged durable-home owner, +# atomically published beneath that home, and private. +manifest_data="$tmp/manifest-data" +HB_DATA="$manifest_data" "$PROJECT_ROOT/guest/hb" init +HB_DATA="$manifest_data" "$PROJECT_ROOT/guest/hb" write-manifest +manifest="$manifest_data/home/agent/.config/hermes-box/runtime-manifest" +[[ -s "$manifest" ]] +[[ "$(stat -c '%a' "$manifest" 2>/dev/null || stat -f '%Lp' "$manifest")" == 600 ]] +[[ "$(stat -c '%u' "$manifest" 2>/dev/null || stat -f '%u' "$manifest")" == "$(id -u)" ]] +[[ -z "$(find "$(dirname "$manifest")" -name 'runtime-manifest.tmp.*' -print -quit)" ]] + +# Gateway startup always requires the explicit single-writer confirmation. +gateway_data="$tmp/gateway-data" +HB_DATA="$gateway_data" "$PROJECT_ROOT/guest/hb" init +if HB_DATA="$gateway_data" "$PROJECT_ROOT/guest/hb" gateway-enable >/dev/null 2>&1; then + echo "gateway enabled without single-writer confirmation" >&2 + exit 1 +fi +HB_DATA="$gateway_data" "$PROJECT_ROOT/guest/hb" gateway-enable \ + --confirm-single-writer I_CONFIRM_NO_OTHER_GATEWAY_USES_THIS_IDENTITY >/dev/null +[[ ! -e "$gateway_data/home/agent/.config/hermes-box/gateway-disabled" ]] + +# A failed gateway start restores the durable disabled policy marker. +( + HB_DATA="$tmp/gateway-failure-data" + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/guest/hb" + init + _start_gateway() { return 1; } + if gateway_enable --confirm-single-writer I_CONFIRM_NO_OTHER_GATEWAY_USES_THIS_IDENTITY >/dev/null 2>&1; then + echo "failed gateway start unexpectedly succeeded" >&2 + exit 1 + fi + [[ -e "$GATEWAY_DISABLED" ]] +) + +# Executor startup failure is never masked by a later gateway result. +( + HB_DATA="$tmp/up-failure-data" + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/guest/hb" + init() { mkdir -p "$STATE_DIR"; } + _executor_up() { return 42; } + _start_gateway() { touch "$tmp/up-gateway-started"; } + if up; then echo "hb up masked Executor failure" >&2; exit 1; fi + [[ ! -e "$tmp/up-gateway-started" ]] + if reconcile; then echo "hb reconcile masked Executor failure" >&2; exit 1; fi + [[ ! -e "$tmp/up-gateway-started" ]] +) + +# Hermes state validation is explicitly skipped when Hermes is disabled. +( + HB_DATA="$tmp/hermes-disabled-data" + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/guest/hb" + _check() { :; } + _hermes_state_valid() { touch "$tmp/hermes-state-called"; } + output="$(INSTALL_HERMES=0 INSTALL_EXECUTOR=0 WIRE_EXECUTOR_MCP=0 doctor)" + grep -q 'Hermes state check skipped' <<<"$output" + [[ ! -e "$tmp/hermes-state-called" ]] +) + +# Doctor validates Codex MCP wiring against the configured Executor port. +( + HB_DATA="$tmp/doctor-port-data" + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/guest/hb" + CODEX_HOME="$AGENT_HOME/.codex" + mkdir -p "$CODEX_HOME" + printf 'url = "http://127.0.0.1:5999/mcp"\n' >"$CODEX_HOME/config.toml" + _check() { + local label="$1" + shift + if [[ "$label" == 'Codex HTTP MCP config' ]]; then + touch "$tmp/codex-doctor-check-ran" + "$@" + else + return 0 + fi + } + INSTALL_HERMES=0 INSTALL_EXECUTOR=0 WIRE_EXECUTOR_MCP=1 EXECUTOR_PORT=5999 doctor >/dev/null + [[ -e "$tmp/codex-doctor-check-ran" ]] +) + +# Quiesced doctor requires the Executor target to be closed, not just marked. +( + HB_DATA="$tmp/quiesced-doctor-data" + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/guest/hb" + mkdir -p "$STATE_DIR" + touch "$QUIESCE_FILE" + _check() { + local label="$1" + shift + if [[ "$label" == 'Executor target port closed while quiesced' ]]; then "$@"; else return 0; fi + } + _port_open() { return 1; } + INSTALL_HERMES=0 INSTALL_EXECUTOR=1 WIRE_EXECUTOR_MCP=0 doctor >/dev/null + _port_open() { return 0; } + if INSTALL_HERMES=0 INSTALL_EXECUTOR=1 WIRE_EXECUTOR_MCP=0 doctor >/dev/null 2>&1; then + echo "quiesced doctor accepted a listening Executor target" >&2 + exit 1 + fi +) + +# Rolling upgrades migrate the legacy pause marker durably before removing it. +legacy_pause="$tmp/legacy-executor-paused" +legacy_data="$tmp/legacy-pause-data" +touch "$legacy_pause" +HB_DATA="$legacy_data" HB_LEGACY_QUIESCE_FILE="$legacy_pause" "$PROJECT_ROOT/guest/hb" init +[[ -e "$legacy_data/home/agent/.config/hermes-box/quiesced" ]] +[[ ! -e "$legacy_pause" ]] + +# Explicit MCP wiring refuses to clear durable quiesce. +wire_quiesced="$tmp/wire-quiesced" +HB_DATA="$wire_quiesced" "$PROJECT_ROOT/guest/hb" init +mkdir -p "$wire_quiesced/home/agent/.config/hermes-box" +touch "$wire_quiesced/home/agent/.config/hermes-box/quiesced" +if HB_DATA="$wire_quiesced" "$PROJECT_ROOT/guest/hb" wire-mcp >/dev/null 2>&1; then + echo "MCP wiring unexpectedly ran while quiesced" >&2 + exit 1 +fi +[[ -e "$wire_quiesced/home/agent/.config/hermes-box/quiesced" ]] + +# wire-once must execute its disabled cleanup path without requiring Executor. +( + HB_DATA="$tmp/disabled-wire-data" + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/guest/hb" + _unwire_legacy() { :; } + mkdir -p "$(dirname "$TOKEN_ENV")" + printf 'stale\n' >"$WIRED" + printf 'stale\n' >"$TOKEN_ENV" + WIRE_EXECUTOR_MCP=0 wire_once >/dev/null + [[ ! -e "$WIRED" && ! -e "$TOKEN_ENV" ]] +) + +echo "hb-workload regression checks passed" diff --git a/tests/regressions-lock-rollback.sh b/tests/regressions-lock-rollback.sh new file mode 100755 index 0000000..ee6050f --- /dev/null +++ b/tests/regressions-lock-rollback.sh @@ -0,0 +1,326 @@ +#!/usr/bin/env bash +# shellcheck disable=SC2016,SC2030,SC2031,SC2329 +set -euo pipefail + +# shellcheck source=tests/lib.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" + +mkdir -p "$tmp/valid/home/agent/workspace" "$tmp/gnupg" "$tmp/host-tmp" +chmod 0700 "$tmp/gnupg" +printf 'portable state\n' >"$tmp/valid/home/agent/workspace/probe.txt" +tar czf "$tmp/valid.tgz" -C "$tmp/valid" . +export GNUPGHOME="$tmp/gnupg" +export BOX_PASSPHRASE='regression-passphrase' +gpg --batch --yes --pinentry-mode loopback --passphrase "$BOX_PASSPHRASE" \ + --symmetric --cipher-algo AES256 -o "$tmp/valid.tar.gz.gpg" "$tmp/valid.tgz" + +# Legacy registry rows remain readable and reserve their Executor port. +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/box" + REG="$tmp/legacy-registry" + printf 'legacy\t4900\n' >"$REG" + _reg_exists legacy + [[ "$(_reg_port legacy)" == 4900 ]] + [[ -z "$(_reg_api_port legacy)" ]] + if _port_free 4900; then + echo "legacy registry port was reported free" >&2 + exit 1 + fi +) + +# A failed precheck must preserve a same-named unmanaged smolvm machine. +preexisting_repo="$tmp/preexisting-repo" +make_repo "$preexisting_repo" +mkdir -p "$tmp/preexisting-state/owned-elsewhere" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/preexisting.calls" \ + SMOLVM_STATE_DIR="$tmp/preexisting-state" SMOLVM_BEHAVIOR=success \ + "$preexisting_repo/box" new owned-elsewhere >/dev/null 2>&1; then + echo "creation over a preexisting machine unexpectedly succeeded" >&2 + exit 1 +fi +[[ -d "$tmp/preexisting-state/owned-elsewhere" ]] + +# If create reports failure after another machine appears, ownership is unknown +# and cleanup must preserve that race winner. +race_repo="$tmp/race-repo" +make_repo "$race_repo" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/race.calls" \ + SMOLVM_STATE_DIR="$tmp/race-state" SMOLVM_BEHAVIOR=create-race \ + "$race_repo/box" new race-winner >/dev/null 2>&1; then + echo "ambiguous create race unexpectedly succeeded" >&2 + exit 1 +fi +[[ -d "$tmp/race-state/race-winner" ]] +if grep -q 'machine delete' "$tmp/race.calls"; then + echo "ambiguous create race winner was deleted" >&2 + exit 1 +fi + +# TERM during provisioning rolls back the invocation-owned VM and registry. +signal_repo="$tmp/signal-repo" +make_repo "$signal_repo" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/signal.calls" \ + SMOLVM_STATE_DIR="$tmp/signal-state" SMOLVM_BEHAVIOR=block-provision \ + "$signal_repo/box" new signal-probe >/dev/null 2>&1; then + echo "signal-interrupted creation unexpectedly succeeded" >&2 + exit 1 +fi +[[ ! -d "$tmp/signal-state/signal-probe" ]] +[[ ! -s "$signal_repo/.boxes" ]] + +# A signal delivered while create itself succeeds is deferred until ownership +# and registry metadata are recorded, then cleanup safely removes the owned VM. +create_signal_repo="$tmp/create-signal-repo" +make_repo "$create_signal_repo" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/create-signal.calls" \ + SMOLVM_STATE_DIR="$tmp/create-signal-state" SMOLVM_BEHAVIOR=signal-create-success \ + "$create_signal_repo/box" new create-signal-probe >/dev/null 2>&1; then + echo "create-time signal unexpectedly succeeded" >&2 + exit 1 +fi +[[ ! -d "$tmp/create-signal-state/create-signal-probe" ]] +[[ ! -s "$create_signal_repo/.boxes" ]] +grep -q 'machine create' "$tmp/create-signal.calls" +grep -q -- '--cpus 4 --mem 8192 --overlay 64' "$tmp/create-signal.calls" +grep -q 'machine delete' "$tmp/create-signal.calls" +if grep -q 'machine start' "$tmp/create-signal.calls"; then + echo "create-time signal was not honored before start" >&2 + exit 1 +fi + +# Optional Hermes API exposure constructs a distinct loopback forward and +# passes its guest bridge port to the persistent workload. +api_repo="$tmp/api-repo" +make_repo "$api_repo" +printf '\nEXPOSE_HERMES_API=1\nHERMES_API_PORT_LO=8650\nHERMES_API_PORT_HI=8650\n' >>"$api_repo/box.env" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/api.calls" \ + SMOLVM_STATE_DIR="$tmp/api-state" SMOLVM_BEHAVIOR=fail-exec \ + "$api_repo/box" new api-probe >/dev/null 2>&1; then + echo "mock API provisioning failure unexpectedly succeeded" >&2 + exit 1 +fi +grep -q -- '-p 8650:8650' "$tmp/api.calls" +grep -q 'hb-workload.*8650' "$tmp/api.calls" + +# Native import fails closed. External provider state requires explicit consent +# before any VM command, and failures/signals after disable never restart it. +python3 - "$tmp/hermes-import.zip" <<'PY' +import sys, zipfile +with zipfile.ZipFile(sys.argv[1], "w") as archive: + archive.writestr("config.yaml", "model: fixture\n") + archive.writestr(".env", "DISCORD_BOT_TOKEN=fixture\n") + archive.writestr("tmp/transient.log", "do not migrate\n") + archive.writestr("_external/.honcho/config.json", "{}\n") +PY +import_repo="$tmp/import-repo" +make_repo "$import_repo" +mkdir -p "$tmp/import-state/import-probe" +printf 'import-probe\t\n' >"$import_repo/.boxes" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/import-no-consent.calls" \ + SMOLVM_STATE_DIR="$tmp/import-state" SMOLVM_BEHAVIOR=success \ + "$import_repo/box" import-hermes import-probe "$tmp/hermes-import.zip" >/dev/null 2>&1; then + echo "external import without consent unexpectedly succeeded" >&2 + exit 1 +fi +[[ ! -e "$tmp/import-no-consent.calls" ]] + +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/import-fail.calls" \ + SMOLVM_STATE_DIR="$tmp/import-state" SMOLVM_BEHAVIOR=import-post-fail \ + "$import_repo/box" import-hermes import-probe "$tmp/hermes-import.zip" \ + --external .honcho >/dev/null 2>&1; then + echo "post-import verification failure unexpectedly succeeded" >&2 + exit 1 +fi +[[ -e "$tmp/import-state/import-probe/gateway-disabled" ]] +[[ ! -e "$tmp/import-state/import-probe/gateway-restarted" ]] +grep -q 'hb gateway-disable' "$tmp/import-fail.calls" +grep -q 'hb resume' "$tmp/import-fail.calls" +grep -q '/data/home/agent/.config/hermes-box/import-' "$tmp/import-fail.calls" +if grep -q '/root/hermes-import-' "$tmp/import-fail.calls"; then + echo "Hermes import still stages beneath root-only storage" >&2 + exit 1 +fi +import_dir_line="$(grep -n 'install -d -o agent -g agent -m 0700 /data/home/agent/.config/hermes-box' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" +import_cp_line="$(grep -n 'machine cp.*\.config/hermes-box/import-' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" +import_chown_line="$(grep -n 'chown agent:agent /data/home/agent/.config/hermes-box/import-' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" +import_chmod_line="$(grep -n 'chmod 0600 /data/home/agent/.config/hermes-box/import-' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" +import_setpriv_line="$(grep -n 'setpriv.*hermes-state.*import-zip.*/data/home/agent/.config/hermes-box/import-' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" +[[ -n "$import_dir_line" && -n "$import_cp_line" && -n "$import_chown_line" && -n "$import_chmod_line" && -n "$import_setpriv_line" ]] +[[ "$import_dir_line" -lt "$import_cp_line" && "$import_cp_line" -lt "$import_chown_line" && "$import_chown_line" -lt "$import_chmod_line" && "$import_chmod_line" -lt "$import_setpriv_line" ]] +import_remove_line="$(grep -n 'rm -f -- /data/home/agent/.config/hermes-box/import-' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" +import_resume_line="$(grep -n 'hb resume' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" +[[ -n "$import_remove_line" && -n "$import_resume_line" && "$import_remove_line" -lt "$import_resume_line" ]] + +rm -f "$tmp/import-state/import-probe/paused" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/import-signal.calls" \ + SMOLVM_STATE_DIR="$tmp/import-state" SMOLVM_BEHAVIOR=signal-import \ + "$import_repo/box" import-hermes import-probe "$tmp/hermes-import.zip" \ + --external .honcho >/dev/null 2>&1; then + echo "signal-interrupted import unexpectedly succeeded" >&2 + exit 1 +fi +[[ -e "$tmp/import-state/import-probe/gateway-disabled" ]] +[[ ! -e "$tmp/import-state/import-probe/gateway-restarted" ]] +grep -q 'hb resume' "$tmp/import-signal.calls" +signal_remove_line="$(grep -n 'rm -f -- /data/home/agent/.config/hermes-box/import-' "$tmp/import-signal.calls" | head -1 | cut -d: -f1)" +signal_resume_line="$(grep -n 'hb resume' "$tmp/import-signal.calls" | head -1 | cut -d: -f1)" +[[ -n "$signal_remove_line" && -n "$signal_resume_line" && "$signal_remove_line" -lt "$signal_resume_line" ]] + +# Import temporarily starts a stopped target, preserves its unpaused policy, +# keeps the gateway disabled, and returns it to stopped on success or failure. +for stopped_case in success failure; do + stopped_name="stopped-import-$stopped_case" + mkdir -p "$tmp/import-state/$stopped_name" + printf '%s\t\n' "$stopped_name" >>"$import_repo/.boxes" + behavior=inactive + [[ "$stopped_case" == success ]] || behavior=inactive-import-post-fail + if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/${stopped_name}.calls" \ + SMOLVM_STATE_DIR="$tmp/import-state" SMOLVM_BEHAVIOR="$behavior" \ + "$import_repo/box" import-hermes "$stopped_name" "$tmp/hermes-import.zip" \ + --external .honcho >"$tmp/${stopped_name}.output" 2>&1; then + [[ "$stopped_case" == success ]] || { + echo "stopped import failure case unexpectedly succeeded" >&2; exit 1; + } + else + [[ "$stopped_case" == failure ]] || { + echo "stopped import success case failed" >&2; exit 1; + } + fi + grep -q 'machine start' "$tmp/${stopped_name}.calls" + grep -q 'machine stop' "$tmp/${stopped_name}.calls" + grep -q 'hb gateway-disable' "$tmp/${stopped_name}.calls" + [[ -e "$tmp/import-state/$stopped_name/gateway-disabled" ]] + [[ ! -e "$tmp/import-state/$stopped_name/paused" ]] + [[ ! -e "$tmp/import-state/$stopped_name/gateway-restarted" ]] +done +grep -q 'Native import will intentionally skip 1 non-portable or transient file(s)' \ + "$tmp/stopped-import-success.output" +grep -q 'Hermes import verified. Gateway remains disabled' "$tmp/stopped-import-success.output" + +# Pause may have taken effect even when the pause command reports failure. The +# existing box is tracked early and resumed during cleanup. +pause_repo="$tmp/pause-repo" +make_repo "$pause_repo" +mkdir -p "$tmp/pause-state/pause-probe" +printf 'pause-probe\t\n' >"$pause_repo/.boxes" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/pause.calls" \ + SMOLVM_STATE_DIR="$tmp/pause-state" SMOLVM_BEHAVIOR=pause-fail \ + "$pause_repo/box" save pause-probe "$tmp/pause.gpg" >/dev/null 2>&1; then + echo "failed pause unexpectedly allowed save" >&2 + exit 1 +fi +grep -q 'hb resume' "$tmp/pause.calls" +[[ ! -e "$tmp/pause-state/pause-probe/paused" ]] + +# Rollback remains best-effort when another process owns the registry lock: VM +# deletion and operation-lock cleanup still happen, while metadata is preserved. +contention_repo="$tmp/contention-repo" +make_repo "$contention_repo" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/contention.calls" \ + SMOLVM_STATE_DIR="$tmp/contention-state" SMOLVM_BEHAVIOR=registry-contention \ + SMOLVM_REGISTRY_LOCK_DIR="$contention_repo/.box-locks/registry.lock" \ + SMOLVM_REGISTRY_LOCK_OWNER_PID="$$" \ + "$contention_repo/box" new contention-probe >/dev/null 2>&1; then + echo "registry-contention provisioning unexpectedly succeeded" >&2 + exit 1 +fi +[[ ! -d "$tmp/contention-state/contention-probe" ]] +grep -q '^contention-probe' "$contention_repo/.boxes" +[[ ! -e "$contention_repo/.box-locks/contention-probe.lock" ]] + +# Rollback recovers a stale registry lock without aborting best-effort cleanup. +stale_lock_repo="$tmp/stale-lock-repo" +make_repo "$stale_lock_repo" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/stale-lock.calls" \ + SMOLVM_STATE_DIR="$tmp/stale-lock-state" SMOLVM_BEHAVIOR=stale-registry-lock \ + SMOLVM_REGISTRY_LOCK_DIR="$stale_lock_repo/.box-locks/registry.lock" \ + "$stale_lock_repo/box" new stale-lock-probe >/dev/null 2>&1; then + echo "stale-lock provisioning unexpectedly succeeded" >&2 + exit 1 +fi +[[ ! -d "$tmp/stale-lock-state/stale-lock-probe" ]] +[[ ! -s "$stale_lock_repo/.boxes" ]] +[[ ! -e "$stale_lock_repo/.box-locks/registry.lock" ]] +[[ ! -e "$stale_lock_repo/.box-locks/stale-lock-probe.lock" ]] + +# A failed delete preserves both the incomplete machine and its registry record. +delete_repo="$tmp/delete-repo" +make_repo "$delete_repo" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/delete.calls" \ + SMOLVM_STATE_DIR="$tmp/delete-state" SMOLVM_BEHAVIOR=delete-fail \ + "$delete_repo/box" new delete-probe >/dev/null 2>&1; then + echo "delete-failure provisioning unexpectedly succeeded" >&2 + exit 1 +fi +[[ -d "$tmp/delete-state/delete-probe" ]] +grep -q '^delete-probe' "$delete_repo/.boxes" + +# A delete error plus a generic status error is never interpreted as absence. +delete_status_repo="$tmp/delete-status-repo" +make_repo "$delete_status_repo" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/delete-status.calls" \ + SMOLVM_STATE_DIR="$tmp/delete-status-state" SMOLVM_BEHAVIOR=delete-status-fail \ + "$delete_status_repo/box" new delete-status-probe >/dev/null 2>&1; then + echo "delete/status-failure provisioning unexpectedly succeeded" >&2 + exit 1 +fi +[[ -d "$tmp/delete-status-state/delete-status-probe" ]] +grep -q '^delete-status-probe' "$delete_status_repo/.boxes" + +# Registration happens before start, so a start failure plus delete failure +# leaves enough host metadata for explicit recovery. +start_delete_repo="$tmp/start-delete-repo" +make_repo "$start_delete_repo" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/start-delete.calls" \ + SMOLVM_STATE_DIR="$tmp/start-delete-state" SMOLVM_BEHAVIOR=start-delete-fail \ + "$start_delete_repo/box" new start-delete-probe >/dev/null 2>&1; then + echo "start/delete failure unexpectedly succeeded" >&2 + exit 1 +fi +[[ -d "$tmp/start-delete-state/start-delete-probe" ]] +grep -q '^start-delete-probe' "$start_delete_repo/.boxes" + +# Save failures remove the guest plaintext before resuming Executor. +save_repo="$tmp/save-repo" +make_repo "$save_repo" +mkdir -p "$tmp/save-state/save-probe" +printf 'save-probe\t\n' >"$save_repo/.boxes" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/save.calls" \ + SMOLVM_STATE_DIR="$tmp/save-state" SMOLVM_BEHAVIOR=save-cp-fail \ + SMOLVM_ARCHIVE_SOURCE="$tmp/valid.tgz" TMPDIR="$tmp/host-tmp" \ + "$save_repo/box" save save-probe "$tmp/save.gpg" >/dev/null 2>&1; then + echo "save copy failure unexpectedly succeeded" >&2 + exit 1 +fi +grep -q '/root/hb-data-' "$tmp/save.calls" +remove_line="$(grep -n 'rm -f -- /root/hb-data-' "$tmp/save.calls" | head -1 | cut -d: -f1)" +resume_line="$(grep -n 'hb resume' "$tmp/save.calls" | head -1 | cut -d: -f1)" +[[ -n "$remove_line" && -n "$resume_line" && "$remove_line" -lt "$resume_line" ]] +[[ -z "$(find "$tmp/host-tmp" -mindepth 1 -maxdepth 1 -print -quit)" ]] + +# Restore extraction failures propagate, delete the incomplete VM, and contain +# the fail-fast extraction and guest-temp trap in the invoked command. +restore_repo="$tmp/restore-repo" +make_repo "$restore_repo" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/restore.calls" \ + SMOLVM_STATE_DIR="$tmp/restore-state" SMOLVM_BEHAVIOR=restore-tar-fail \ + "$restore_repo/box" load "$tmp/valid.tar.gz.gpg" restore-probe >/dev/null 2>&1; then + echo "restore tar failure unexpectedly succeeded" >&2 + exit 1 +fi +[[ ! -d "$tmp/restore-state/restore-probe" ]] +if grep -q 'hb resume' "$tmp/restore.calls"; then + echo "failed restore resumed Executor before deleting its new VM" >&2 + exit 1 +fi +grep -q 'machine delete' "$tmp/restore.calls" +grep -q 'chmod 0600 /root/hb-restore-' "$tmp/restore.calls" +restore_remove_line="$(grep -n 'rm -f -- /root/hb-restore-' "$tmp/restore.calls" | head -1 | cut -d: -f1)" +restore_delete_line="$(grep -n 'machine delete' "$tmp/restore.calls" | head -1 | cut -d: -f1)" +[[ -n "$restore_remove_line" && -n "$restore_delete_line" && "$restore_remove_line" -lt "$restore_delete_line" ]] +grep -q 'set -euo pipefail' "$tmp/restore.calls" +grep -q 'trap.*rm -f --.*archive' "$tmp/restore.calls" + + +echo "lock-rollback regression checks passed" diff --git a/tests/regressions-tx9-host.sh b/tests/regressions-tx9-host.sh new file mode 100755 index 0000000..02488eb --- /dev/null +++ b/tests/regressions-tx9-host.sh @@ -0,0 +1,104 @@ +#!/usr/bin/env bash +# shellcheck disable=SC2016,SC2030,SC2031,SC2329 +set -euo pipefail + +# shellcheck source=tests/lib.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" + +# Basic VM operations do not require HTTP health tools; health operations do. +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/box" + have() { [[ "$1" == awk || "$1" == smolvm ]]; } + _host_preflight basic +) +for missing_health_tool in curl jq mktemp; do + if ( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/box" + have() { [[ "$1" != "$missing_health_tool" ]]; } + _host_preflight health + ) 2>/dev/null; then + echo "health preflight passed without $missing_health_tool" >&2 + exit 1 + fi +done + +# Scheduled host health gates on an already-running box and never starts one. +host_root="$tmp/tx9-host-root" +mkdir -p "$host_root" "$tmp/tx9-host-state/scheduled" +printf '#!/usr/bin/env bash\nprintf "%%s\\n" "$*" >>"${TX9_BOX_CALLS:?}"\n' >"$host_root/box" +chmod +x "$host_root/box" +if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-inactive.smolvm" \ + SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=inactive \ + SMOLVM_CALLED_UNUSED=1 TX9_ROOT="$host_root" TX9_BOX_CALLS="$tmp/tx9-inactive.box" \ + "$PROJECT_ROOT/ops/tx9-host" health scheduled >/dev/null 2>&1; then + echo "scheduled health accepted an inactive box" >&2 + exit 1 +fi +[[ ! -e "$tmp/tx9-inactive.box" ]] +if grep -q 'machine start' "$tmp/tx9-inactive.smolvm"; then + echo "scheduled health started an inactive box" >&2 + exit 1 +fi +PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-active.smolvm" \ + SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=success \ + TX9_ROOT="$host_root" TX9_BOX_CALLS="$tmp/tx9-active.box" \ + "$PROJECT_ROOT/ops/tx9-host" health scheduled >/dev/null +grep -q '^doctor scheduled$' "$tmp/tx9-active.box" + +# Sourced configuration cannot redirect an operation to another box name. +mkdir -p "$tmp/tx9-config/boxes" +printf 'name=global-override\nTX9_BOX_CONFIG_DIR=%s\n' "$tmp/tx9-config/boxes" >"$tmp/tx9-config/global.conf" +printf 'name=box-override\n' >"$tmp/tx9-config/boxes/scheduled.conf" +PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-config-name.smolvm" \ + SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=success \ + TX9_CONFIG="$tmp/tx9-config/global.conf" TX9_ROOT="$host_root" \ + TX9_BOX_CALLS="$tmp/tx9-config-name.box" \ + "$PROJECT_ROOT/ops/tx9-host" health scheduled >/dev/null +[[ "$(cat "$tmp/tx9-config-name.box")" == 'doctor scheduled' ]] + +# The foreground supervisor starts once and restarts an unexpectedly stopped VM. +PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-supervise.smolvm" \ + SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=inactive \ + TX9_ROOT="$host_root" TX9_BOX_CALLS="$tmp/tx9-supervise.box" TX9_SUPERVISE_ONCE=1 \ + "$PROJECT_ROOT/ops/tx9-host" supervise scheduled >/dev/null 2>&1 +[[ "$(grep -c '^start scheduled$' "$tmp/tx9-supervise.box")" == 2 ]] + +# A supervisor that receives stop intent exits without probing or restarting. +PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-supervise-stop.smolvm" \ + SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=inactive \ + TX9_ROOT="$host_root" TX9_BOX_CALLS="$tmp/tx9-supervise-stop.box" \ + TX9_SUPERVISE_INTERVAL_SECONDS=300 \ + "$PROJECT_ROOT/ops/tx9-host" supervise scheduled >/dev/null 2>&1 & +supervisor_pid=$! +pids+=("$supervisor_pid") +wait_for_pattern '^start scheduled$' "$tmp/tx9-supervise-stop.box" +kill -TERM "$supervisor_pid" +wait "$supervisor_pid" +[[ "$(grep -c '^start scheduled$' "$tmp/tx9-supervise-stop.box")" == 1 ]] +[[ ! -e "$tmp/tx9-supervise-stop.smolvm" ]] + +# An unset systemd credential directory does not resolve to /backup-passphrase. +if env -u BOX_PASSPHRASE_FILE -u CREDENTIALS_DIRECTORY \ + PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-no-credential.smolvm" \ + SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=success \ + TX9_ROOT="$host_root" TX9_BOX_CALLS="$tmp/tx9-no-credential.box" \ + "$PROJECT_ROOT/ops/tx9-host" backup scheduled >/dev/null 2>&1; then + echo "scheduled backup accepted a missing credential" >&2 + exit 1 +fi +[[ ! -e "$tmp/tx9-no-credential.box" ]] || ! grep -q '^save ' "$tmp/tx9-no-credential.box" + +# Credential files without a trailing newline are accepted intact. +printf 'no-newline-passphrase' >"$tmp/passphrase-no-newline" +( + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/box" + _BOX_PASS="" + BOX_PASSPHRASE_FILE="$tmp/passphrase-no-newline" + _resolve_passphrase + [[ "$_BOX_PASS" == no-newline-passphrase ]] +) + +echo "tx9-host regression checks passed" diff --git a/tests/regressions.sh b/tests/regressions.sh deleted file mode 100755 index 16948d4..0000000 --- a/tests/regressions.sh +++ /dev/null @@ -1,1131 +0,0 @@ -#!/usr/bin/env bash -# shellcheck disable=SC2016,SC2030,SC2031,SC2329 -set -euo pipefail - -PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -tmp="$(mktemp -d)" -pids=() -cleaned_up=0 - -cleanup() { - local pid - [[ "$cleaned_up" == 0 ]] || return 0 - cleaned_up=1 - for pid in "${pids[@]:-}"; do - [[ -n "$pid" ]] && kill "$pid" >/dev/null 2>&1 || true - done - rm -rf "$tmp" -} - -handle_signal() { - local status="$1" - trap - EXIT HUP INT TERM - cleanup - exit "$status" -} - -trap cleanup EXIT -trap 'handle_signal 129' HUP -trap 'handle_signal 130' INT -trap 'handle_signal 143' TERM -[[ "$(trap -p HUP)" == *"handle_signal 129"* ]] -[[ "$(trap -p INT)" == *"handle_signal 130"* ]] -[[ "$(trap -p TERM)" == *"handle_signal 143"* ]] - -wait_for_file() { - local file="$1" attempt - for ((attempt = 0; attempt < 100; attempt++)); do - [[ -s "$file" ]] && return - sleep 0.02 - done - echo "timed out waiting for $file" >&2 - return 1 -} - -wait_for_pattern() { - local pattern="$1" file="$2" attempt - for ((attempt = 0; attempt < 100; attempt++)); do - grep -q "$pattern" "$file" 2>/dev/null && return - sleep 0.02 - done - echo "timed out waiting for '$pattern' in $file" >&2 - return 1 -} - -start_mcp_server() { - local status="$1" label="$2" mode="${3:-valid}" bind_host="${4:-127.0.0.1}" port_file delete_file - port_file="$tmp/$label.port" - delete_file="$tmp/$label.delete" - python3 "$PROJECT_ROOT/tests/fixtures/mcp-server.py" "$port_file" "$status" "$delete_file" "$mode" "$bind_host" & - MCP_PID=$! - pids+=("$MCP_PID") - wait_for_file "$port_file" - MCP_PORT="$(cat "$port_file")" - MCP_DELETE_FILE="$delete_file" -} - -# Basic VM operations do not require HTTP health tools; health operations do. -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/box" - have() { [[ "$1" == awk || "$1" == smolvm ]]; } - _host_preflight basic -) -for missing_health_tool in curl jq mktemp; do - if ( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/box" - have() { [[ "$1" != "$missing_health_tool" ]]; } - _host_preflight health - ) 2>/dev/null; then - echo "health preflight passed without $missing_health_tool" >&2 - exit 1 - fi -done - -# Scheduled host health gates on an already-running box and never starts one. -host_root="$tmp/tx9-host-root" -mkdir -p "$host_root" "$tmp/tx9-host-state/scheduled" -printf '#!/usr/bin/env bash\nprintf "%%s\\n" "$*" >>"${TX9_BOX_CALLS:?}"\n' >"$host_root/box" -chmod +x "$host_root/box" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-inactive.smolvm" \ - SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=inactive \ - SMOLVM_CALLED_UNUSED=1 TX9_ROOT="$host_root" TX9_BOX_CALLS="$tmp/tx9-inactive.box" \ - "$PROJECT_ROOT/ops/tx9-host" health scheduled >/dev/null 2>&1; then - echo "scheduled health accepted an inactive box" >&2 - exit 1 -fi -[[ ! -e "$tmp/tx9-inactive.box" ]] -if grep -q 'machine start' "$tmp/tx9-inactive.smolvm"; then - echo "scheduled health started an inactive box" >&2 - exit 1 -fi -PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-active.smolvm" \ - SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=success \ - TX9_ROOT="$host_root" TX9_BOX_CALLS="$tmp/tx9-active.box" \ - "$PROJECT_ROOT/ops/tx9-host" health scheduled >/dev/null -grep -q '^doctor scheduled$' "$tmp/tx9-active.box" - -# Sourced configuration cannot redirect an operation to another box name. -mkdir -p "$tmp/tx9-config/boxes" -printf 'name=global-override\nTX9_BOX_CONFIG_DIR=%s\n' "$tmp/tx9-config/boxes" >"$tmp/tx9-config/global.conf" -printf 'name=box-override\n' >"$tmp/tx9-config/boxes/scheduled.conf" -PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-config-name.smolvm" \ - SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=success \ - TX9_CONFIG="$tmp/tx9-config/global.conf" TX9_ROOT="$host_root" \ - TX9_BOX_CALLS="$tmp/tx9-config-name.box" \ - "$PROJECT_ROOT/ops/tx9-host" health scheduled >/dev/null -[[ "$(cat "$tmp/tx9-config-name.box")" == 'doctor scheduled' ]] - -# The foreground supervisor starts once and restarts an unexpectedly stopped VM. -PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-supervise.smolvm" \ - SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=inactive \ - TX9_ROOT="$host_root" TX9_BOX_CALLS="$tmp/tx9-supervise.box" TX9_SUPERVISE_ONCE=1 \ - "$PROJECT_ROOT/ops/tx9-host" supervise scheduled >/dev/null 2>&1 -[[ "$(grep -c '^start scheduled$' "$tmp/tx9-supervise.box")" == 2 ]] - -# A supervisor that receives stop intent exits without probing or restarting. -PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-supervise-stop.smolvm" \ - SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=inactive \ - TX9_ROOT="$host_root" TX9_BOX_CALLS="$tmp/tx9-supervise-stop.box" \ - TX9_SUPERVISE_INTERVAL_SECONDS=300 \ - "$PROJECT_ROOT/ops/tx9-host" supervise scheduled >/dev/null 2>&1 & -supervisor_pid=$! -pids+=("$supervisor_pid") -wait_for_pattern '^start scheduled$' "$tmp/tx9-supervise-stop.box" -kill -TERM "$supervisor_pid" -wait "$supervisor_pid" -[[ "$(grep -c '^start scheduled$' "$tmp/tx9-supervise-stop.box")" == 1 ]] -[[ ! -e "$tmp/tx9-supervise-stop.smolvm" ]] - -# Root-run installer state is recursively returned to the agent and made -# writable, including nested runtime directories created during full repair. -hermes_fixture="$tmp/installer-hermes-home" -mkdir -p "$hermes_fixture/sessions/deep" "$hermes_fixture/cron" "$hermes_fixture/logs" -printf 'session\n' >"$hermes_fixture/sessions/deep/history.json" -printf 'job\n' >"$hermes_fixture/cron/jobs.json" -chmod 0500 "$hermes_fixture/sessions/deep" -chmod 0400 "$hermes_fixture/sessions/deep/history.json" "$hermes_fixture/cron/jobs.json" -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/provision/provision.sh" - HERMES_HOME="$hermes_fixture" - chown() { - [[ "$1" == -R && "$2" == agent:agent && "$3" == "$hermes_fixture" ]] - printf 'recursive agent ownership\n' >"$tmp/hermes-chown.called" - } - own_hermes_home -) -[[ -s "$tmp/hermes-chown.called" ]] -[[ -w "$hermes_fixture/sessions/deep/history.json" ]] -[[ -w "$hermes_fixture/cron/jobs.json" ]] -[[ -w "$hermes_fixture/sessions/deep" ]] - -# Restore staging grants non-owners traversal but not top-level mutation; only -# the nested agent home models owner-writable imported state. -permission_stage="$tmp/restore-permission-model" -mkdir -p "$permission_stage/home/agent" -chmod 0711 "$permission_stage" "$permission_stage/home" -chmod 0700 "$permission_stage/home/agent" -python3 - "$permission_stage" <<'PY' -import os -import stat -import sys -from pathlib import Path - -stage = Path(sys.argv[1]) -home = stage / "home" -agent = home / "agent" -for envelope in (stage, home): - mode = stat.S_IMODE(envelope.stat().st_mode) - assert mode == 0o711 - assert mode & 0o001 - assert not mode & 0o002 -assert stat.S_IMODE(agent.stat().st_mode) == 0o700 -assert os.access(agent, os.W_OK) -PY - -# Hermes API forwarding requires a successful reconcile, an enabled gateway, -# and a listening target API. -( - HOME="$tmp/workload-home" - mkdir -p "$HOME/.config/hermes-box" "$tmp/workload-logs" - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/guest/hb-workload" - PORT=4799 - HERMES_BRIDGE_PORT=8650 - LOGS="$tmp/workload-logs" - BOX_ENV="$tmp/workload-box.env" - LEGACY_QUIESCE_FILE="$tmp/workload-legacy-paused" - printf 'EXECUTOR_PORT=4788\nHERMES_API_PORT=8642\n' >"$BOX_ENV" - pgrep() { return 1; } - pkill() { printf 'stopped %s\n' "$*" >>"$tmp/api-bridge.events"; } - socat() { printf 'started %s\n' "$*" >>"$tmp/api-bridge.events"; } - _exec_up() { return 0; } - hb() { return 1; } - _wait_api_ready() { return 0; } - reconcile_once - if grep -q 'started.*TCP-LISTEN:8650,' "$tmp/api-bridge.events" 2>/dev/null; then - echo "API bridge started after failed reconcile" >&2; exit 1 - fi - hb() { return 0; } - _wait_api_ready() { return 1; } - reconcile_once - if grep -q 'started.*TCP-LISTEN:8650,' "$tmp/api-bridge.events" 2>/dev/null; then - echo "API bridge started before its target was ready" >&2; exit 1 - fi - _wait_api_ready() { return 0; } - reconcile_once - wait - grep -q started "$tmp/api-bridge.events" - printf 'EXECUTOR_PORT=4990\nHERMES_API_PORT=8990\n' >"$BOX_ENV" - reconcile_once - wait - grep -q 'stopped.*TCP-LISTEN:4799,' "$tmp/api-bridge.events" - grep -q 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events" - grep -q 'started.*TCP:127.0.0.1:4990' "$tmp/api-bridge.events" - grep -q 'started.*TCP:127.0.0.1:8990' "$tmp/api-bridge.events" - api_stops_before="$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" - touch "$GATEWAY_DISABLED" - reconcile_once - [[ "$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" -gt "$api_stops_before" ]] - executor_stops_before="$(grep -c 'stopped.*TCP-LISTEN:4799,' "$tmp/api-bridge.events")" - api_stops_before="$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" - touch "$QUIESCE_FILE" - reconcile_once - [[ "$(grep -c 'stopped.*TCP-LISTEN:4799,' "$tmp/api-bridge.events")" -gt "$executor_stops_before" ]] - [[ "$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" -gt "$api_stops_before" ]] - rm -f "$QUIESCE_FILE" - executor_stops_before="$(grep -c 'stopped.*TCP-LISTEN:4799,' "$tmp/api-bridge.events")" - api_stops_before="$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" - hb() { touch "$tmp/legacy-workload-reconciled"; return 0; } - touch "$LEGACY_QUIESCE_FILE" - reconcile_once - [[ -e "$QUIESCE_FILE" ]] - [[ ! -e "$LEGACY_QUIESCE_FILE" ]] - [[ ! -e "$tmp/legacy-workload-reconciled" ]] - [[ "$(grep -c 'stopped.*TCP-LISTEN:4799,' "$tmp/api-bridge.events")" -gt "$executor_stops_before" ]] - [[ "$(grep -c 'stopped.*TCP-LISTEN:8650,' "$tmp/api-bridge.events")" -gt "$api_stops_before" ]] - - migration_failure="$tmp/workload-migration-failure" - mkdir -p "$migration_failure" - chmod 0500 "$migration_failure" - QUIESCE_FILE="$migration_failure/state/quiesced" - LEGACY_QUIESCE_FILE="$tmp/workload-legacy-failure" - rm -f "$tmp/legacy-workload-reconciled" - touch "$LEGACY_QUIESCE_FILE" - reconcile_once 2>/dev/null - [[ ! -e "$QUIESCE_FILE" ]] - [[ -e "$LEGACY_QUIESCE_FILE" ]] - [[ ! -e "$tmp/legacy-workload-reconciled" ]] - chmod 0700 "$migration_failure" -) - -# Active-path acknowledgement reports success only when persistence succeeds. -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/guest/hb" - hermes-state() { return 1; } - if output="$(acknowledge_active_paths 2>&1)"; then - echo "active-path acknowledgement masked helper failure" >&2 - exit 1 - fi - [[ "$output" != *'active import paths acknowledged'* ]] -) - -# An unset systemd credential directory does not resolve to /backup-passphrase. -if env -u BOX_PASSPHRASE_FILE -u CREDENTIALS_DIRECTORY \ - PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/tx9-no-credential.smolvm" \ - SMOLVM_STATE_DIR="$tmp/tx9-host-state" SMOLVM_BEHAVIOR=success \ - TX9_ROOT="$host_root" TX9_BOX_CALLS="$tmp/tx9-no-credential.box" \ - "$PROJECT_ROOT/ops/tx9-host" backup scheduled >/dev/null 2>&1; then - echo "scheduled backup accepted a missing credential" >&2 - exit 1 -fi -[[ ! -e "$tmp/tx9-no-credential.box" ]] || ! grep -q '^save ' "$tmp/tx9-no-credential.box" - -# Credential files without a trailing newline are accepted intact. -printf 'no-newline-passphrase' >"$tmp/passphrase-no-newline" -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/box" - _BOX_PASS="" - BOX_PASSPHRASE_FILE="$tmp/passphrase-no-newline" - _resolve_passphrase - [[ "$_BOX_PASS" == no-newline-passphrase ]] -) - -# Missing legacy resource settings use the documented production defaults. -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/box" - unset BOX_CPUS BOX_MEM_MIB BOX_OVERLAY_GIB BOX_STORAGE_GIB - _validate_resources - [[ "$BOX_CPUS" == 4 && "$BOX_MEM_MIB" == 8192 && "$BOX_OVERLAY_GIB" == 64 && -z "$BOX_STORAGE_GIB" ]] -) - -# The runtime manifest must be writable by the unprivileged durable-home owner, -# atomically published beneath that home, and private. -manifest_data="$tmp/manifest-data" -HB_DATA="$manifest_data" "$PROJECT_ROOT/guest/hb" init -HB_DATA="$manifest_data" "$PROJECT_ROOT/guest/hb" write-manifest -manifest="$manifest_data/home/agent/.config/hermes-box/runtime-manifest" -[[ -s "$manifest" ]] -[[ "$(stat -c '%a' "$manifest" 2>/dev/null || stat -f '%Lp' "$manifest")" == 600 ]] -[[ "$(stat -c '%u' "$manifest" 2>/dev/null || stat -f '%u' "$manifest")" == "$(id -u)" ]] -[[ -z "$(find "$(dirname "$manifest")" -name 'runtime-manifest.tmp.*' -print -quit)" ]] - -# Gateway startup always requires the explicit single-writer confirmation. -gateway_data="$tmp/gateway-data" -HB_DATA="$gateway_data" "$PROJECT_ROOT/guest/hb" init -if HB_DATA="$gateway_data" "$PROJECT_ROOT/guest/hb" gateway-enable >/dev/null 2>&1; then - echo "gateway enabled without single-writer confirmation" >&2 - exit 1 -fi -HB_DATA="$gateway_data" "$PROJECT_ROOT/guest/hb" gateway-enable \ - --confirm-single-writer I_CONFIRM_NO_OTHER_GATEWAY_USES_THIS_IDENTITY >/dev/null -[[ ! -e "$gateway_data/home/agent/.config/hermes-box/gateway-disabled" ]] - -# A failed gateway start restores the durable disabled policy marker. -( - HB_DATA="$tmp/gateway-failure-data" - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/guest/hb" - init - _start_gateway() { return 1; } - if gateway_enable --confirm-single-writer I_CONFIRM_NO_OTHER_GATEWAY_USES_THIS_IDENTITY >/dev/null 2>&1; then - echo "failed gateway start unexpectedly succeeded" >&2 - exit 1 - fi - [[ -e "$GATEWAY_DISABLED" ]] -) - -# Executor startup failure is never masked by a later gateway result. -( - HB_DATA="$tmp/up-failure-data" - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/guest/hb" - init() { mkdir -p "$STATE_DIR"; } - _executor_up() { return 42; } - _start_gateway() { touch "$tmp/up-gateway-started"; } - if up; then echo "hb up masked Executor failure" >&2; exit 1; fi - [[ ! -e "$tmp/up-gateway-started" ]] - if reconcile; then echo "hb reconcile masked Executor failure" >&2; exit 1; fi - [[ ! -e "$tmp/up-gateway-started" ]] -) - -# Hermes state validation is explicitly skipped when Hermes is disabled. -( - HB_DATA="$tmp/hermes-disabled-data" - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/guest/hb" - _check() { :; } - _hermes_state_valid() { touch "$tmp/hermes-state-called"; } - output="$(INSTALL_HERMES=0 INSTALL_EXECUTOR=0 WIRE_EXECUTOR_MCP=0 doctor)" - grep -q 'Hermes state check skipped' <<<"$output" - [[ ! -e "$tmp/hermes-state-called" ]] -) - -# Doctor validates Codex MCP wiring against the configured Executor port. -( - HB_DATA="$tmp/doctor-port-data" - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/guest/hb" - CODEX_HOME="$AGENT_HOME/.codex" - mkdir -p "$CODEX_HOME" - printf 'url = "http://127.0.0.1:5999/mcp"\n' >"$CODEX_HOME/config.toml" - _check() { - local label="$1" - shift - if [[ "$label" == 'Codex HTTP MCP config' ]]; then - touch "$tmp/codex-doctor-check-ran" - "$@" - else - return 0 - fi - } - INSTALL_HERMES=0 INSTALL_EXECUTOR=0 WIRE_EXECUTOR_MCP=1 EXECUTOR_PORT=5999 doctor >/dev/null - [[ -e "$tmp/codex-doctor-check-ran" ]] -) - -# Quiesced doctor requires the Executor target to be closed, not just marked. -( - HB_DATA="$tmp/quiesced-doctor-data" - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/guest/hb" - mkdir -p "$STATE_DIR" - touch "$QUIESCE_FILE" - _check() { - local label="$1" - shift - if [[ "$label" == 'Executor target port closed while quiesced' ]]; then "$@"; else return 0; fi - } - _port_open() { return 1; } - INSTALL_HERMES=0 INSTALL_EXECUTOR=1 WIRE_EXECUTOR_MCP=0 doctor >/dev/null - _port_open() { return 0; } - if INSTALL_HERMES=0 INSTALL_EXECUTOR=1 WIRE_EXECUTOR_MCP=0 doctor >/dev/null 2>&1; then - echo "quiesced doctor accepted a listening Executor target" >&2 - exit 1 - fi -) - -# Rolling upgrades migrate the legacy pause marker durably before removing it. -legacy_pause="$tmp/legacy-executor-paused" -legacy_data="$tmp/legacy-pause-data" -touch "$legacy_pause" -HB_DATA="$legacy_data" HB_LEGACY_QUIESCE_FILE="$legacy_pause" "$PROJECT_ROOT/guest/hb" init -[[ -e "$legacy_data/home/agent/.config/hermes-box/quiesced" ]] -[[ ! -e "$legacy_pause" ]] - -# Explicit MCP wiring refuses to clear durable quiesce. -wire_quiesced="$tmp/wire-quiesced" -HB_DATA="$wire_quiesced" "$PROJECT_ROOT/guest/hb" init -mkdir -p "$wire_quiesced/home/agent/.config/hermes-box" -touch "$wire_quiesced/home/agent/.config/hermes-box/quiesced" -if HB_DATA="$wire_quiesced" "$PROJECT_ROOT/guest/hb" wire-mcp >/dev/null 2>&1; then - echo "MCP wiring unexpectedly ran while quiesced" >&2 - exit 1 -fi -[[ -e "$wire_quiesced/home/agent/.config/hermes-box/quiesced" ]] - -# wire-once must execute its disabled cleanup path without requiring Executor. -( - HB_DATA="$tmp/disabled-wire-data" - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/guest/hb" - _unwire_legacy() { :; } - mkdir -p "$(dirname "$TOKEN_ENV")" - printf 'stale\n' >"$WIRED" - printf 'stale\n' >"$TOKEN_ENV" - WIRE_EXECUTOR_MCP=0 wire_once >/dev/null - [[ ! -e "$WIRED" && ! -e "$TOKEN_ENV" ]] -) - -# Protocol-correct MCP initialize requires exact 200 plus a session header and -# closes the created session. Other HTTP statuses are unhealthy. -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/box" - REG="$tmp/empty-registry" - touch "$REG" - - ( - doctor_calls="$tmp/doctor.calls" - mock_quiesced=0 - mock_gateway_disabled=0 - have() { [[ "$1" == awk || "$1" == smolvm ]]; } - _acquire_lock() { :; } - _machine_exists() { :; } - _guest_hb() { - printf '%s\n' "$2" >>"$doctor_calls" - if [[ "$2" == is-paused ]]; then [[ "$mock_quiesced" == 1 ]]; return; fi - if [[ "$2" == gateway-is-disabled ]]; then [[ "$mock_gateway_disabled" == 1 ]]; return; fi - [[ "$2" != executor-token ]] || printf 'fixture-token\n' - } - mcp_attempts=0 - sleep() { :; } - _mcp_initialize() { - printf 'host-mcp\n' >>"$doctor_calls" - mcp_attempts=$((mcp_attempts + 1)) - [[ "$mcp_attempts" -ge 3 ]] - } - - doctor_output="$(cmd_doctor no-host-port)" - grep -q 'host Executor MCP: skipped (no exposed host port)' <<<"$doctor_output" - [[ "$(cat "$doctor_calls")" == $'is-paused\ndoctor' ]] - - printf 'doctor-host\t4899\n' >"$REG" - if (cmd_doctor doctor-host >/dev/null 2>&1); then - echo "exposed-port doctor passed without host health dependencies" >&2 - exit 1 - fi - [[ "$(cat "$doctor_calls")" == $'is-paused\ndoctor\nis-paused\ndoctor' ]] - - : >"$doctor_calls" - have() { :; } - cmd_doctor doctor-host >/dev/null - [[ "$(cat "$doctor_calls")" == $'is-paused\ndoctor\nexecutor-token\nhost-mcp\nhost-mcp\nhost-mcp' ]] - - : >"$doctor_calls" - mock_quiesced=1 - cmd_doctor doctor-host >/dev/null - [[ "$(cat "$doctor_calls")" == $'is-paused\ndoctor' ]] - mock_quiesced=0 - - : >"$doctor_calls" - printf 'api-host\t\t8650\t8650\n' >"$REG" - _hermes_api_healthy() { printf 'host-api\n' >>"$doctor_calls"; } - api_output="$(cmd_doctor api-host)" - grep -q 'host Hermes API: healthy' <<<"$api_output" - [[ "$(cat "$doctor_calls")" == $'is-paused\ngateway-is-disabled\ndoctor\nhost-api' ]] - - : >"$doctor_calls" - mock_gateway_disabled=1 - _hermes_api_healthy() { touch "$tmp/disabled-api-probed"; return 1; } - api_output="$(cmd_doctor api-host)" - grep -q 'host Hermes API: intentionally disabled by gateway policy' <<<"$api_output" - [[ "$(cat "$doctor_calls")" == $'is-paused\ngateway-is-disabled\ndoctor' ]] - [[ ! -e "$tmp/disabled-api-probed" ]] - mock_gateway_disabled=0 - - : >"$doctor_calls" - : >"$REG" - WIRE_EXECUTOR_MCP=0 INSTALL_EXECUTOR=0 cmd_doctor disabled-executor >/dev/null - [[ "$(cat "$doctor_calls")" == $'is-paused\ndoctor' ]] - ) - - BOX_BROWSER=true _open_url 'http://localhost:1/?_token=not-printed' - if BOX_BROWSER=definitely-not-a-browser _open_url 'http://localhost:1/?_token=not-printed'; then - echo "missing BOX_BROWSER command unexpectedly succeeded" >&2 - exit 1 - fi - uname() { printf 'Linux\n'; } - xdg-open() { return 1; } - gio() { [[ "$1" == open && "$2" == 'https://fixture.invalid/secret' ]]; } - _open_url 'https://fixture.invalid/secret' - uname() { printf 'Darwin\n'; } - open() { [[ "$1" == 'https://fixture.invalid/secret' ]]; } - _open_url 'https://fixture.invalid/secret' - - start_mcp_server 200 healthy - _hermes_api_healthy "$MCP_PORT" - _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token - wait_for_pattern initialized "$MCP_DELETE_FILE" - wait_for_pattern deleted "$MCP_DELETE_FILE" - _wait_mcp_ready "http://127.0.0.1:$MCP_PORT/mcp" fixture-token - if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" wrong-token; then - echo "invalid bearer token passed MCP health" >&2 - exit 1 - fi - malformed_code="$(curl -sS -o /dev/null -w '%{http_code}' -X POST \ - -H 'Authorization: Bearer fixture-token' -H 'Content-Type: application/json' \ - -H 'Accept: application/json,text/event-stream' --data '{not-json' \ - "http://127.0.0.1:$MCP_PORT/mcp")" - [[ "$malformed_code" == 400 ]] - for invalid_request in \ - '[]' \ - '{"jsonrpc":"2.0","id":1,"method":"initialize","params":[]}' \ - '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-03-26","capabilities":{},"clientInfo":[]}}'; do - invalid_code="$(curl -sS -o /dev/null -w '%{http_code}' -X POST \ - -H 'Authorization: Bearer fixture-token' -H 'Content-Type: application/json' \ - -H 'Accept: application/json,text/event-stream' --data "$invalid_request" \ - "http://127.0.0.1:$MCP_PORT/mcp")" - [[ "$invalid_code" == 400 ]] - done - invalid_length_code="$(curl -sS -o /dev/null -w '%{http_code}' -X POST \ - -H 'Authorization: Bearer fixture-token' -H 'Content-Type: application/json' \ - -H 'Accept: application/json,text/event-stream' -H 'Content-Length: invalid' \ - --data '{}' "http://127.0.0.1:$MCP_PORT/mcp")" - [[ "$invalid_length_code" == 400 ]] - - guest_token_dir="$tmp/guest-token/.executor/server-control" - mkdir -p "$guest_token_dir" - printf '{"token":"fixture-token"}\n' >"$guest_token_dir/auth.json" - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/guest/hb" - TOKEN_FILE="$guest_token_dir/auth.json" - EXECUTOR_PORT="$MCP_PORT" - _mcp_auth_works - printf '{"token":"wrong-token"}\n' >"$guest_token_dir/auth.json" - if _mcp_auth_works; then - echo "guest MCP health accepted an invalid bearer token" >&2 - exit 1 - fi - if _port_free "$MCP_PORT"; then - echo "occupied TCP port was reported free" >&2 - exit 1 - fi - kill "$MCP_PID" - wait "$MCP_PID" 2>/dev/null || true - - for unhealthy_status in 401 404 500; do - start_mcp_server "$unhealthy_status" "api-$unhealthy_status" - if _hermes_api_healthy "$MCP_PORT"; then - echo "Hermes API HTTP $unhealthy_status passed health" >&2 - exit 1 - fi - kill "$MCP_PID" - wait "$MCP_PID" 2>/dev/null || true - done - start_mcp_server 200 api-malformed health-malformed - if _hermes_api_healthy "$MCP_PORT"; then - echo "malformed Hermes API health JSON passed" >&2 - exit 1 - fi - kill "$MCP_PID" - wait "$MCP_PID" 2>/dev/null || true - - if python3 -c 'import socket; s = socket.socket(socket.AF_INET6); s.bind(("::1", 0)); s.close()'; then - start_mcp_server 200 ipv6-only valid ::1 - if _port_free "$MCP_PORT"; then - echo "IPv6-only occupied TCP port was reported free" >&2 - exit 1 - fi - kill "$MCP_PID" - wait "$MCP_PID" 2>/dev/null || true - else - echo "IPv6 loopback unavailable; skipping IPv6 occupancy regression" >&2 - fi - - start_mcp_server 200 malformed-json malformed-json - if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token; then - echo "malformed initialize JSON passed MCP health" >&2 - exit 1 - fi - kill "$MCP_PID" - wait "$MCP_PID" 2>/dev/null || true - - start_mcp_server 200 wrong-version wrong-version - if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token; then - echo "wrong negotiated MCP version passed health" >&2 - exit 1 - fi - kill "$MCP_PID" - wait "$MCP_PID" 2>/dev/null || true - - start_mcp_server 200 wrong-content-type wrong-content-type - if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token; then - echo "wrong initialize Content-Type passed MCP health" >&2 - exit 1 - fi - kill "$MCP_PID" - wait "$MCP_PID" 2>/dev/null || true - - start_mcp_server 200 event-stream event-stream - _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token - kill "$MCP_PID" - wait "$MCP_PID" 2>/dev/null || true - - start_mcp_server 404 missing - if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token; then - echo "HTTP 404 passed MCP health" >&2 - exit 1 - fi - kill "$MCP_PID" - wait "$MCP_PID" 2>/dev/null || true - - start_mcp_server 500 broken - if _mcp_initialize "http://127.0.0.1:$MCP_PORT/mcp" fixture-token; then - echo "HTTP 500 passed MCP health" >&2 - exit 1 - fi - kill "$MCP_PID" - wait "$MCP_PID" 2>/dev/null || true - - free_port="$MCP_PORT" - for ((attempt = 0; attempt < 100; attempt++)); do - _port_free "$free_port" && break - sleep 0.02 - done - _port_free "$free_port" - - ready_attempts=0 - opener_attempt=0 - _mcp_initialize() { - ready_attempts=$((ready_attempts + 1)) - [[ "$ready_attempts" -ge 3 ]] - } - _open_url() { opener_attempt="$ready_attempts"; } - _wait_mcp_ready ignored ignored - _open_url 'https://fixture.invalid/secret' - [[ "$ready_attempts" == 3 && "$opener_attempt" == 3 ]] -) - -# Open refuses a quiesced box before hb up can clear its marker. -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/box" - calls="$tmp/open-quiesced.calls" - _host_preflight() { :; } - _acquire_lock() { :; } - _reg_port() { printf '4899\n'; } - _machine_running() { :; } - smolvm() { printf 'smolvm %s\n' "$*" >>"$calls"; } - _guest_hb() { - printf 'hb %s\n' "$2" >>"$calls" - [[ "$2" == is-paused ]] - } - if (cmd_open quiesced >/dev/null 2>&1); then - echo "box open unexpectedly succeeded while quiesced" >&2 - exit 1 - fi - grep -q 'hb is-paused' "$calls" - if grep -q 'hb up' "$calls"; then - echo "box open cleared quiesce through hb up" >&2 - exit 1 - fi - if grep -q 'machine start' "$calls"; then - echo "box open started the VM" >&2 - exit 1 - fi -) - -# Open refuses a stopped box and never changes its lifecycle state. -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/box" - calls="$tmp/open-stopped.calls" - _host_preflight() { :; } - _acquire_lock() { :; } - _reg_port() { printf '4899\n'; } - _machine_running() { return 1; } - smolvm() { printf 'smolvm %s\n' "$*" >>"$calls"; } - _guest_hb() { printf 'hb %s\n' "$2" >>"$calls"; } - if (cmd_open stopped >/dev/null 2>&1); then - echo "box open accepted a stopped box" >&2 - exit 1 - fi - [[ ! -e "$calls" ]] || ! grep -q 'machine start' "$calls" -) - -# Repair refreshes assets but preserves a pre-existing durable quiesce policy. -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/box" - calls="$tmp/repair-quiesced.calls" - _host_preflight() { :; } - _acquire_lock() { :; } - _machine_exists() { :; } - smolvm() { printf 'smolvm %s\n' "$*" >>"$calls"; } - _provision_into() { printf 'provision %s %s\n' "$1" "$2" >>"$calls"; } - _guest_hb() { - printf 'hb %s\n' "$2" >>"$calls" - [[ "$2" == is-paused || "$2" == doctor ]] - } - cmd_repair quiesced >/dev/null - grep -q 'provision quiesced full' "$calls" - grep -q 'hb doctor' "$calls" - if grep -Eq 'hb (up|resume)' "$calls"; then - echo "repair resumed a previously quiesced box" >&2 - exit 1 - fi -) - -# Repair validates a configured bundle before constructing the provision tar. -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/box" - calls="$tmp/repair-invalid-bundle.calls" - HERMES_GIT_BUNDLE='../invalid.bundle' - _host_preflight() { :; } - _acquire_lock() { :; } - _machine_exists() { :; } - _guest_hb() { [[ "$2" == is-paused ]]; } - smolvm() { printf '%s\n' "$*" >>"$calls"; } - if (cmd_repair invalid-bundle >/dev/null 2>&1); then - echo "repair accepted an invalid Hermes bundle path" >&2 - exit 1 - fi - if grep -q 'machine exec -i' "$calls"; then - echo "repair streamed provision context before bundle validation" >&2 - exit 1 - fi -) - -# Guest bundle verification is independent of the caller's repository and -# rejects incremental bundles whose prerequisite objects are unavailable. -bundle_repo="$tmp/bundle-source" -mkdir -p "$bundle_repo" "$tmp/bundle-nonrepo" "$tmp/bundle-verify-tmp" -git -C "$bundle_repo" init -q -printf 'base\n' >"$bundle_repo/state.txt" -git -C "$bundle_repo" add state.txt -git -C "$bundle_repo" -c user.name=Fixture -c user.email=fixture@example.invalid commit -q -m base -base_commit="$(git -C "$bundle_repo" rev-parse HEAD)" -git -C "$bundle_repo" bundle create "$tmp/complete.bundle" --all -printf 'next\n' >>"$bundle_repo/state.txt" -git -C "$bundle_repo" add state.txt -git -C "$bundle_repo" -c user.name=Fixture -c user.email=fixture@example.invalid commit -q -m next -git -C "$bundle_repo" bundle create "$tmp/incremental.bundle" HEAD "^$base_commit" -( - cd "$tmp/bundle-nonrepo" - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/provision/provision.sh" - TMPDIR="$tmp/bundle-verify-tmp" verify_self_contained_bundle "$tmp/complete.bundle" - if TMPDIR="$tmp/bundle-verify-tmp" verify_self_contained_bundle "$tmp/incremental.bundle"; then - echo "incremental bundle passed self-contained verification" >&2 - exit 1 - fi -) -[[ -z "$(find "$tmp/bundle-verify-tmp" -mindepth 1 -maxdepth 1 -print -quit)" ]] - -# Asset-only refreshes neither validate nor transport the optional source bundle. -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/box" - calls="$tmp/provision-mode.calls" - HERMES_GIT_BUNDLE='artifacts/hermes.bundle' - _validate_bundle() { printf 'validate\n' >>"$calls"; } - tar() { printf 'tar %s\n' "$*" >>"$calls"; printf 'fixture'; } - smolvm() { if [[ "$*" == *'machine exec -i'* ]]; then cat >/dev/null; fi; } - _provision_into fixture assets >/dev/null - [[ ! -e "$calls" ]] || ! grep -q '^validate$\|artifacts/hermes.bundle' "$calls" - _provision_into fixture full >/dev/null - grep -q '^validate$' "$calls" - grep -q 'artifacts/hermes.bundle' "$calls" -) - -mkdir -p "$tmp/valid/home/agent/workspace" "$tmp/gnupg" "$tmp/host-tmp" -chmod 0700 "$tmp/gnupg" -printf 'portable state\n' >"$tmp/valid/home/agent/workspace/probe.txt" -tar czf "$tmp/valid.tgz" -C "$tmp/valid" . -export GNUPGHOME="$tmp/gnupg" -export BOX_PASSPHRASE='regression-passphrase' -gpg --batch --yes --pinentry-mode loopback --passphrase "$BOX_PASSPHRASE" \ - --symmetric --cipher-algo AES256 -o "$tmp/valid.tar.gz.gpg" "$tmp/valid.tgz" - -make_repo() { - local dest="$1" - mkdir -p "$dest" - cp "$PROJECT_ROOT/box" "$PROJECT_ROOT/box.env" "$dest/" - cp -R "$PROJECT_ROOT/guest" "$PROJECT_ROOT/provision" "$dest/" -} - -# Legacy registry rows remain readable and reserve their Executor port. -( - # shellcheck disable=SC1090 - source "$PROJECT_ROOT/box" - REG="$tmp/legacy-registry" - printf 'legacy\t4900\n' >"$REG" - _reg_exists legacy - [[ "$(_reg_port legacy)" == 4900 ]] - [[ -z "$(_reg_api_port legacy)" ]] - if _port_free 4900; then - echo "legacy registry port was reported free" >&2 - exit 1 - fi -) - -# A failed precheck must preserve a same-named unmanaged smolvm machine. -preexisting_repo="$tmp/preexisting-repo" -make_repo "$preexisting_repo" -mkdir -p "$tmp/preexisting-state/owned-elsewhere" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/preexisting.calls" \ - SMOLVM_STATE_DIR="$tmp/preexisting-state" SMOLVM_BEHAVIOR=success \ - "$preexisting_repo/box" new owned-elsewhere >/dev/null 2>&1; then - echo "creation over a preexisting machine unexpectedly succeeded" >&2 - exit 1 -fi -[[ -d "$tmp/preexisting-state/owned-elsewhere" ]] - -# If create reports failure after another machine appears, ownership is unknown -# and cleanup must preserve that race winner. -race_repo="$tmp/race-repo" -make_repo "$race_repo" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/race.calls" \ - SMOLVM_STATE_DIR="$tmp/race-state" SMOLVM_BEHAVIOR=create-race \ - "$race_repo/box" new race-winner >/dev/null 2>&1; then - echo "ambiguous create race unexpectedly succeeded" >&2 - exit 1 -fi -[[ -d "$tmp/race-state/race-winner" ]] -if grep -q 'machine delete' "$tmp/race.calls"; then - echo "ambiguous create race winner was deleted" >&2 - exit 1 -fi - -# TERM during provisioning rolls back the invocation-owned VM and registry. -signal_repo="$tmp/signal-repo" -make_repo "$signal_repo" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/signal.calls" \ - SMOLVM_STATE_DIR="$tmp/signal-state" SMOLVM_BEHAVIOR=block-provision \ - "$signal_repo/box" new signal-probe >/dev/null 2>&1; then - echo "signal-interrupted creation unexpectedly succeeded" >&2 - exit 1 -fi -[[ ! -d "$tmp/signal-state/signal-probe" ]] -[[ ! -s "$signal_repo/.boxes" ]] - -# A signal delivered while create itself succeeds is deferred until ownership -# and registry metadata are recorded, then cleanup safely removes the owned VM. -create_signal_repo="$tmp/create-signal-repo" -make_repo "$create_signal_repo" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/create-signal.calls" \ - SMOLVM_STATE_DIR="$tmp/create-signal-state" SMOLVM_BEHAVIOR=signal-create-success \ - "$create_signal_repo/box" new create-signal-probe >/dev/null 2>&1; then - echo "create-time signal unexpectedly succeeded" >&2 - exit 1 -fi -[[ ! -d "$tmp/create-signal-state/create-signal-probe" ]] -[[ ! -s "$create_signal_repo/.boxes" ]] -grep -q 'machine create' "$tmp/create-signal.calls" -grep -q -- '--cpus 4 --mem 8192 --overlay 64' "$tmp/create-signal.calls" -grep -q 'machine delete' "$tmp/create-signal.calls" -if grep -q 'machine start' "$tmp/create-signal.calls"; then - echo "create-time signal was not honored before start" >&2 - exit 1 -fi - -# Optional Hermes API exposure constructs a distinct loopback forward and -# passes its guest bridge port to the persistent workload. -api_repo="$tmp/api-repo" -make_repo "$api_repo" -printf '\nEXPOSE_HERMES_API=1\nHERMES_API_PORT_LO=8650\nHERMES_API_PORT_HI=8650\n' >>"$api_repo/box.env" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/api.calls" \ - SMOLVM_STATE_DIR="$tmp/api-state" SMOLVM_BEHAVIOR=fail-exec \ - "$api_repo/box" new api-probe >/dev/null 2>&1; then - echo "mock API provisioning failure unexpectedly succeeded" >&2 - exit 1 -fi -grep -q -- '-p 8650:8650' "$tmp/api.calls" -grep -q 'hb-workload.*8650' "$tmp/api.calls" - -# Native import fails closed. External provider state requires explicit consent -# before any VM command, and failures/signals after disable never restart it. -python3 - "$tmp/hermes-import.zip" <<'PY' -import sys, zipfile -with zipfile.ZipFile(sys.argv[1], "w") as archive: - archive.writestr("config.yaml", "model: fixture\n") - archive.writestr(".env", "DISCORD_BOT_TOKEN=fixture\n") - archive.writestr("tmp/transient.log", "do not migrate\n") - archive.writestr("_external/.honcho/config.json", "{}\n") -PY -import_repo="$tmp/import-repo" -make_repo "$import_repo" -mkdir -p "$tmp/import-state/import-probe" -printf 'import-probe\t\n' >"$import_repo/.boxes" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/import-no-consent.calls" \ - SMOLVM_STATE_DIR="$tmp/import-state" SMOLVM_BEHAVIOR=success \ - "$import_repo/box" import-hermes import-probe "$tmp/hermes-import.zip" >/dev/null 2>&1; then - echo "external import without consent unexpectedly succeeded" >&2 - exit 1 -fi -[[ ! -e "$tmp/import-no-consent.calls" ]] - -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/import-fail.calls" \ - SMOLVM_STATE_DIR="$tmp/import-state" SMOLVM_BEHAVIOR=import-post-fail \ - "$import_repo/box" import-hermes import-probe "$tmp/hermes-import.zip" \ - --external .honcho >/dev/null 2>&1; then - echo "post-import verification failure unexpectedly succeeded" >&2 - exit 1 -fi -[[ -e "$tmp/import-state/import-probe/gateway-disabled" ]] -[[ ! -e "$tmp/import-state/import-probe/gateway-restarted" ]] -grep -q 'hb gateway-disable' "$tmp/import-fail.calls" -grep -q 'hb resume' "$tmp/import-fail.calls" -grep -q '/data/home/agent/.config/hermes-box/import-' "$tmp/import-fail.calls" -if grep -q '/root/hermes-import-' "$tmp/import-fail.calls"; then - echo "Hermes import still stages beneath root-only storage" >&2 - exit 1 -fi -import_dir_line="$(grep -n 'install -d -o agent -g agent -m 0700 /data/home/agent/.config/hermes-box' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" -import_cp_line="$(grep -n 'machine cp.*\.config/hermes-box/import-' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" -import_chown_line="$(grep -n 'chown agent:agent /data/home/agent/.config/hermes-box/import-' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" -import_chmod_line="$(grep -n 'chmod 0600 /data/home/agent/.config/hermes-box/import-' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" -import_setpriv_line="$(grep -n 'setpriv.*hermes-state.*import-zip.*/data/home/agent/.config/hermes-box/import-' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" -[[ -n "$import_dir_line" && -n "$import_cp_line" && -n "$import_chown_line" && -n "$import_chmod_line" && -n "$import_setpriv_line" ]] -[[ "$import_dir_line" -lt "$import_cp_line" && "$import_cp_line" -lt "$import_chown_line" && "$import_chown_line" -lt "$import_chmod_line" && "$import_chmod_line" -lt "$import_setpriv_line" ]] -import_remove_line="$(grep -n 'rm -f -- /data/home/agent/.config/hermes-box/import-' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" -import_resume_line="$(grep -n 'hb resume' "$tmp/import-fail.calls" | head -1 | cut -d: -f1)" -[[ -n "$import_remove_line" && -n "$import_resume_line" && "$import_remove_line" -lt "$import_resume_line" ]] - -rm -f "$tmp/import-state/import-probe/paused" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/import-signal.calls" \ - SMOLVM_STATE_DIR="$tmp/import-state" SMOLVM_BEHAVIOR=signal-import \ - "$import_repo/box" import-hermes import-probe "$tmp/hermes-import.zip" \ - --external .honcho >/dev/null 2>&1; then - echo "signal-interrupted import unexpectedly succeeded" >&2 - exit 1 -fi -[[ -e "$tmp/import-state/import-probe/gateway-disabled" ]] -[[ ! -e "$tmp/import-state/import-probe/gateway-restarted" ]] -grep -q 'hb resume' "$tmp/import-signal.calls" -signal_remove_line="$(grep -n 'rm -f -- /data/home/agent/.config/hermes-box/import-' "$tmp/import-signal.calls" | head -1 | cut -d: -f1)" -signal_resume_line="$(grep -n 'hb resume' "$tmp/import-signal.calls" | head -1 | cut -d: -f1)" -[[ -n "$signal_remove_line" && -n "$signal_resume_line" && "$signal_remove_line" -lt "$signal_resume_line" ]] - -# Import temporarily starts a stopped target, preserves its unpaused policy, -# keeps the gateway disabled, and returns it to stopped on success or failure. -for stopped_case in success failure; do - stopped_name="stopped-import-$stopped_case" - mkdir -p "$tmp/import-state/$stopped_name" - printf '%s\t\n' "$stopped_name" >>"$import_repo/.boxes" - behavior=inactive - [[ "$stopped_case" == success ]] || behavior=inactive-import-post-fail - if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/${stopped_name}.calls" \ - SMOLVM_STATE_DIR="$tmp/import-state" SMOLVM_BEHAVIOR="$behavior" \ - "$import_repo/box" import-hermes "$stopped_name" "$tmp/hermes-import.zip" \ - --external .honcho >"$tmp/${stopped_name}.output" 2>&1; then - [[ "$stopped_case" == success ]] || { - echo "stopped import failure case unexpectedly succeeded" >&2; exit 1; - } - else - [[ "$stopped_case" == failure ]] || { - echo "stopped import success case failed" >&2; exit 1; - } - fi - grep -q 'machine start' "$tmp/${stopped_name}.calls" - grep -q 'machine stop' "$tmp/${stopped_name}.calls" - grep -q 'hb gateway-disable' "$tmp/${stopped_name}.calls" - [[ -e "$tmp/import-state/$stopped_name/gateway-disabled" ]] - [[ ! -e "$tmp/import-state/$stopped_name/paused" ]] - [[ ! -e "$tmp/import-state/$stopped_name/gateway-restarted" ]] -done -grep -q 'Native import will intentionally skip 1 non-portable or transient file(s)' \ - "$tmp/stopped-import-success.output" -grep -q 'Hermes import verified. Gateway remains disabled' "$tmp/stopped-import-success.output" - -# Pause may have taken effect even when the pause command reports failure. The -# existing box is tracked early and resumed during cleanup. -pause_repo="$tmp/pause-repo" -make_repo "$pause_repo" -mkdir -p "$tmp/pause-state/pause-probe" -printf 'pause-probe\t\n' >"$pause_repo/.boxes" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/pause.calls" \ - SMOLVM_STATE_DIR="$tmp/pause-state" SMOLVM_BEHAVIOR=pause-fail \ - "$pause_repo/box" save pause-probe "$tmp/pause.gpg" >/dev/null 2>&1; then - echo "failed pause unexpectedly allowed save" >&2 - exit 1 -fi -grep -q 'hb resume' "$tmp/pause.calls" -[[ ! -e "$tmp/pause-state/pause-probe/paused" ]] - -# Rollback remains best-effort when another process owns the registry lock: VM -# deletion and operation-lock cleanup still happen, while metadata is preserved. -contention_repo="$tmp/contention-repo" -make_repo "$contention_repo" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/contention.calls" \ - SMOLVM_STATE_DIR="$tmp/contention-state" SMOLVM_BEHAVIOR=registry-contention \ - SMOLVM_REGISTRY_LOCK_DIR="$contention_repo/.box-locks/registry.lock" \ - SMOLVM_REGISTRY_LOCK_OWNER_PID="$$" \ - "$contention_repo/box" new contention-probe >/dev/null 2>&1; then - echo "registry-contention provisioning unexpectedly succeeded" >&2 - exit 1 -fi -[[ ! -d "$tmp/contention-state/contention-probe" ]] -grep -q '^contention-probe' "$contention_repo/.boxes" -[[ ! -e "$contention_repo/.box-locks/contention-probe.lock" ]] - -# Rollback recovers a stale registry lock without aborting best-effort cleanup. -stale_lock_repo="$tmp/stale-lock-repo" -make_repo "$stale_lock_repo" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/stale-lock.calls" \ - SMOLVM_STATE_DIR="$tmp/stale-lock-state" SMOLVM_BEHAVIOR=stale-registry-lock \ - SMOLVM_REGISTRY_LOCK_DIR="$stale_lock_repo/.box-locks/registry.lock" \ - "$stale_lock_repo/box" new stale-lock-probe >/dev/null 2>&1; then - echo "stale-lock provisioning unexpectedly succeeded" >&2 - exit 1 -fi -[[ ! -d "$tmp/stale-lock-state/stale-lock-probe" ]] -[[ ! -s "$stale_lock_repo/.boxes" ]] -[[ ! -e "$stale_lock_repo/.box-locks/registry.lock" ]] -[[ ! -e "$stale_lock_repo/.box-locks/stale-lock-probe.lock" ]] - -# A failed delete preserves both the incomplete machine and its registry record. -delete_repo="$tmp/delete-repo" -make_repo "$delete_repo" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/delete.calls" \ - SMOLVM_STATE_DIR="$tmp/delete-state" SMOLVM_BEHAVIOR=delete-fail \ - "$delete_repo/box" new delete-probe >/dev/null 2>&1; then - echo "delete-failure provisioning unexpectedly succeeded" >&2 - exit 1 -fi -[[ -d "$tmp/delete-state/delete-probe" ]] -grep -q '^delete-probe' "$delete_repo/.boxes" - -# A delete error plus a generic status error is never interpreted as absence. -delete_status_repo="$tmp/delete-status-repo" -make_repo "$delete_status_repo" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/delete-status.calls" \ - SMOLVM_STATE_DIR="$tmp/delete-status-state" SMOLVM_BEHAVIOR=delete-status-fail \ - "$delete_status_repo/box" new delete-status-probe >/dev/null 2>&1; then - echo "delete/status-failure provisioning unexpectedly succeeded" >&2 - exit 1 -fi -[[ -d "$tmp/delete-status-state/delete-status-probe" ]] -grep -q '^delete-status-probe' "$delete_status_repo/.boxes" - -# Registration happens before start, so a start failure plus delete failure -# leaves enough host metadata for explicit recovery. -start_delete_repo="$tmp/start-delete-repo" -make_repo "$start_delete_repo" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/start-delete.calls" \ - SMOLVM_STATE_DIR="$tmp/start-delete-state" SMOLVM_BEHAVIOR=start-delete-fail \ - "$start_delete_repo/box" new start-delete-probe >/dev/null 2>&1; then - echo "start/delete failure unexpectedly succeeded" >&2 - exit 1 -fi -[[ -d "$tmp/start-delete-state/start-delete-probe" ]] -grep -q '^start-delete-probe' "$start_delete_repo/.boxes" - -# Save failures remove the guest plaintext before resuming Executor. -save_repo="$tmp/save-repo" -make_repo "$save_repo" -mkdir -p "$tmp/save-state/save-probe" -printf 'save-probe\t\n' >"$save_repo/.boxes" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/save.calls" \ - SMOLVM_STATE_DIR="$tmp/save-state" SMOLVM_BEHAVIOR=save-cp-fail \ - SMOLVM_ARCHIVE_SOURCE="$tmp/valid.tgz" TMPDIR="$tmp/host-tmp" \ - "$save_repo/box" save save-probe "$tmp/save.gpg" >/dev/null 2>&1; then - echo "save copy failure unexpectedly succeeded" >&2 - exit 1 -fi -grep -q '/root/hb-data-' "$tmp/save.calls" -remove_line="$(grep -n 'rm -f -- /root/hb-data-' "$tmp/save.calls" | head -1 | cut -d: -f1)" -resume_line="$(grep -n 'hb resume' "$tmp/save.calls" | head -1 | cut -d: -f1)" -[[ -n "$remove_line" && -n "$resume_line" && "$remove_line" -lt "$resume_line" ]] -[[ -z "$(find "$tmp/host-tmp" -mindepth 1 -maxdepth 1 -print -quit)" ]] - -# Restore extraction failures propagate, delete the incomplete VM, and contain -# the fail-fast extraction and guest-temp trap in the invoked command. -restore_repo="$tmp/restore-repo" -make_repo "$restore_repo" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/restore.calls" \ - SMOLVM_STATE_DIR="$tmp/restore-state" SMOLVM_BEHAVIOR=restore-tar-fail \ - "$restore_repo/box" load "$tmp/valid.tar.gz.gpg" restore-probe >/dev/null 2>&1; then - echo "restore tar failure unexpectedly succeeded" >&2 - exit 1 -fi -[[ ! -d "$tmp/restore-state/restore-probe" ]] -if grep -q 'hb resume' "$tmp/restore.calls"; then - echo "failed restore resumed Executor before deleting its new VM" >&2 - exit 1 -fi -grep -q 'machine delete' "$tmp/restore.calls" -grep -q 'chmod 0600 /root/hb-restore-' "$tmp/restore.calls" -restore_remove_line="$(grep -n 'rm -f -- /root/hb-restore-' "$tmp/restore.calls" | head -1 | cut -d: -f1)" -restore_delete_line="$(grep -n 'machine delete' "$tmp/restore.calls" | head -1 | cut -d: -f1)" -[[ -n "$restore_remove_line" && -n "$restore_delete_line" && "$restore_remove_line" -lt "$restore_delete_line" ]] -grep -q 'set -euo pipefail' "$tmp/restore.calls" -grep -q 'trap.*rm -f --.*archive' "$tmp/restore.calls" - -echo "focused regression checks passed" diff --git a/tests/static.sh b/tests/static.sh index ea01ee2..9b695cd 100755 --- a/tests/static.sh +++ b/tests/static.sh @@ -5,6 +5,10 @@ set -euo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" cd "$ROOT" +# tests/regressions-*.sh is globbed so new split-out regression files are +# picked up automatically. +regression_files=(tests/regressions-*.sh) + files=( box guest/hb @@ -13,15 +17,16 @@ files=( guest/agent-bash-profile.sh provision/provision.sh ops/tx9-host + tests/lib.sh tests/lifecycle-smoke.sh tests/hermes-state.sh - tests/regressions.sh tests/cli-surface.sh + "${regression_files[@]}" tests/fixtures/smolvm ) bash -n "${files[@]}" -for file in box guest/hb guest/hb-workload guest/hermes-state provision/provision.sh ops/tx9-host tests/lifecycle-smoke.sh tests/hermes-state.sh tests/regressions.sh tests/cli-surface.sh tests/fixtures/smolvm; do +for file in box guest/hb guest/hb-workload guest/hermes-state provision/provision.sh ops/tx9-host tests/lifecycle-smoke.sh tests/hermes-state.sh tests/cli-surface.sh "${regression_files[@]}" tests/fixtures/smolvm; do [[ -x "$file" ]] || { echo "not executable: $file" >&2; exit 1; } done From 01a38a57a32715d8546733f9feef3b872d159d13 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:49:58 -0700 Subject: [PATCH 14/28] fix(ci): pin shellcheck to v0.11.0 instead of apt's drifting version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first CI run failed lint: apt's packaged shellcheck flagged `[[ cond ]] && cmd || true` as SC2015 across several pre-existing lines (box, guest/hb, provision/provision.sh) and the (now-split) test files. Current shellcheck deliberately doesn't apply SC2015 to `[[ ]]` tests (it's a clean boolean test, not a command whose own failure needs separate handling) — confirmed locally with shellcheck 0.11.0, which reports nothing for that pattern. Ubuntu's apt repos lag upstream releases, so `apt-get install shellcheck` pulled an older version with the stricter behavior. Download the pinned v0.11.0 release binary directly instead, matching what local development already runs. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/check.yml | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 8103520..820d96d 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -11,9 +11,21 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - name: Install dependencies + - name: Install jq run: | sudo apt-get update - sudo apt-get install -y --no-install-recommends shellcheck jq + sudo apt-get install -y --no-install-recommends jq + - name: Install shellcheck + # Pinned instead of apt's (often much older) package: older shellcheck + # versions flag `[[ cond ]] && cmd || true` as SC2015, a check that + # current shellcheck deliberately doesn't apply to `[[ ]]` tests. Pin + # to match what local development already runs against. + env: + SHELLCHECK_VERSION: v0.11.0 + run: | + curl -fsSL "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" \ + | tar -xJ + sudo install "shellcheck-${SHELLCHECK_VERSION}/shellcheck" /usr/local/bin/shellcheck + shellcheck --version - name: make check run: make check From 9eacede8fc1da0d62937e8aa2208f3a7233df2bd Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:52:40 -0700 Subject: [PATCH 15/28] fix(test): give cli-surface.sh's empty-registry ls test the fixture PATH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This invocation of box ls was missing PATH="tests/fixtures:$PATH", so _host_preflight basic's `command -v smolvm` silently resolved to a real smolvm binary on dev machines that have one installed, masking the gap locally. CI has no real smolvm, so the first real run failed with "smolvm not found on PATH" — exactly the kind of gap make check running in CI (P2-3) exists to catch. Verified by re-running the full suite with the real smolvm hidden from PATH (filtering it out of every PATH directory before invoking make check), matching what CI sees; everything passes now, including this case. Co-Authored-By: Claude Sonnet 5 --- tests/cli-surface.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/cli-surface.sh b/tests/cli-surface.sh index c84500c..c602e06 100755 --- a/tests/cli-surface.sh +++ b/tests/cli-surface.sh @@ -11,7 +11,8 @@ source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" # --- box ls (empty registry) ------------------------------------------- ls_empty_repo="$tmp/ls-empty-repo" make_repo "$ls_empty_repo" -out="$("$ls_empty_repo/box" ls)" +out="$(PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$tmp/ls-empty.calls" \ + SMOLVM_STATE_DIR="$tmp/ls-empty-state" "$ls_empty_repo/box" ls)" [[ "$out" == "No hermes-box-lite boxes yet. Create one: ./box new " ]] # --- box ls (populated registry: one running, one missing from smolvm) - From 87210fb1356caec37bb5b6d0257323f5cd913283 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:54:02 -0700 Subject: [PATCH 16/28] fix(P3-1): drop personal path default from box.env HERMES_IMPORT_PATH_MAP defaulted to a Davis-specific macOS path (/Users/davis=...), shipped as the committed default in box.env. Anyone else cloning the repo got a path mapping wrong for them, silently applied to every import-hermes invocation. Defaults to empty now. The --map OLD=NEW per-invocation flag already covers this (docs/nexus-operations.md's migration runbook already demonstrates it), so a non-empty default isn't structurally required; operators who want a standing default can still set one in their own deployment's box.env. Co-Authored-By: Claude Sonnet 5 --- box.env | 8 ++++++-- docs/audit-2026-06-30-fixes.md | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/box.env b/box.env index 405919c..a36d9f4 100644 --- a/box.env +++ b/box.env @@ -41,8 +41,12 @@ HERMES_API_PORT="8642" EXPOSE_HERMES_API=0 HERMES_API_PORT_LO=8650 HERMES_API_PORT_HI=8850 -# Applied only to active imported config/env/cron/hook/script files. -HERMES_IMPORT_PATH_MAP="/Users/davis=/data/home/agent" +# Applied only to active imported config/env/cron/hook/script files. Empty by +# default — this is operator/machine-specific (e.g. your old macOS username), +# not something to commit. Set it here for your own deployment, or pass +# --map OLD=NEW per invocation (see docs/nexus-operations.md's migration +# runbook for a worked example); import-hermes applies both if both are set. +HERMES_IMPORT_PATH_MAP="" # --- Executor (github.com/RhysSullivan/executor) ---------------------------- # npm global; daemon + web UI + MCP. Data lives under $HOME (=/data) via XDG. diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index 81b3537..d5457e1 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -506,7 +506,7 @@ mapping) — with no shared file all four source. `make_repo()` ## P3 — polish and drift prevention -### P3-1: `box.env:36` hardcodes a personal path as the committed default +### P3-1: `box.env:36` hardcodes a personal path as the committed default — DONE ``` HERMES_IMPORT_PATH_MAP="/Users/davis=/data/home/agent" From 658c7cb032cf115e30201d5104beebdc45091fde Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 14:59:20 -0700 Subject: [PATCH 17/28] fix(P3-2): harden the three tx9 systemd units MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three units already ran as the unprivileged tx9 user, but none set any of the standard cheap hardening directives, despite needing nothing outside /opt/tx9, /var/backups/tx9, /var/lib/tx9, and /etc/tx9. Added to each [Service] block: NoNewPrivileges=true, ProtectHome=true, ProtectSystem=strict, PrivateTmp=true, and ReadWritePaths scoped to those four paths. Verified rather than assumed: - systemd-analyze verify accepts all three units with the new directives (no syntax/semantic errors). - /dev/kvm read/write access is unaffected — confirmed live, since these directives only restrict filesystem mounts, not device nodes; KVM access stays governed by the existing SupplementaryGroups=kvm membership. - ReadWritePaths mechanics actually grant writes inside listed paths and block them outside, confirmed with systemd-run against scratch directories using the identical directive set. - Ran the real backup flow (mkdir destination, write .partial, sha256sum, atomic rename) end to end under the exact hardening directives via systemd-run with a stubbed box/tx9-host — succeeds. Found and fixed a real gap while doing this: TX9_NAS_DIR is an operator-configured path (e.g. a CIFS mount) that ProtectSystem=strict's static ReadWritePaths can't anticipate. Documented in docs/nexus-operations.md that enabling NAS replication now requires a `systemctl edit` drop-in adding that path to ReadWritePaths, or replication silently starts failing closed (the local backup still succeeds either way). Co-Authored-By: Claude Sonnet 5 --- docs/audit-2026-06-30-fixes.md | 2 +- docs/nexus-operations.md | 18 ++++++++++++++++++ ops/systemd/tx9-backup@.service | 5 +++++ ops/systemd/tx9-box@.service | 5 +++++ ops/systemd/tx9-health@.service | 5 +++++ 5 files changed, 34 insertions(+), 1 deletion(-) diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index d5457e1..f4963d1 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -523,7 +523,7 @@ per `box:786-787`, so a non-empty default isn't structurally required), or move this specific value into `docs/nexus-operations.md`'s migration runbook (where it's already referenced) and out of the generic `box.env` template. -### P3-2: no systemd hardening directives on any of the four host units +### P3-2: no systemd hardening directives on any of the four host units — DONE **Files:** `ops/systemd/tx9-box@.service`, `tx9-health@.service`, `tx9-backup@.service` (all reviewed in full). diff --git a/docs/nexus-operations.md b/docs/nexus-operations.md index 9327d79..6f50fae 100644 --- a/docs/nexus-operations.md +++ b/docs/nexus-operations.md @@ -72,6 +72,24 @@ backup remains available. Configure the CIFS mount separately as a systemd automount and make it writable by `tx9`. Partial replication files are cleaned on failure. Retention policy is intentionally left to the Nexus operator. +`tx9-backup@.service` runs with `ProtectSystem=strict`, which makes the whole +filesystem read-only except the paths listed in its `ReadWritePaths=` +(`/opt/tx9 /var/backups/tx9 /var/lib/tx9 /etc/tx9` by default). If you set +`TX9_NAS_DIR` to anything outside those paths, add it via a drop-in instead of +editing the shipped unit: + +```bash +sudo systemctl edit tx9-backup@hermes.service +``` + +```ini +[Service] +ReadWritePaths=/mnt/davis-vault/tx9 +``` + +Without this, replication fails closed with a permission error even though +the mount itself is healthy — the local backup still succeeds either way. + ## 3. Stage a native Hermes backup On Eventide, leave the gateway live for the first rehearsal and create a native diff --git a/ops/systemd/tx9-backup@.service b/ops/systemd/tx9-backup@.service index e3b8cc6..1b5989d 100644 --- a/ops/systemd/tx9-backup@.service +++ b/ops/systemd/tx9-backup@.service @@ -8,6 +8,11 @@ User=tx9 Group=tx9 SupplementaryGroups=kvm UMask=0077 +NoNewPrivileges=true +ProtectHome=true +ProtectSystem=strict +PrivateTmp=true +ReadWritePaths=/opt/tx9 /var/backups/tx9 /var/lib/tx9 /etc/tx9 LoadCredential=backup-passphrase:/etc/tx9/backup-passphrase ExecStart=/opt/tx9/ops/tx9-host backup %i TimeoutStartSec=2h diff --git a/ops/systemd/tx9-box@.service b/ops/systemd/tx9-box@.service index c6b6022..32243c1 100644 --- a/ops/systemd/tx9-box@.service +++ b/ops/systemd/tx9-box@.service @@ -9,6 +9,11 @@ User=tx9 Group=tx9 SupplementaryGroups=kvm UMask=0077 +NoNewPrivileges=true +ProtectHome=true +ProtectSystem=strict +PrivateTmp=true +ReadWritePaths=/opt/tx9 /var/backups/tx9 /var/lib/tx9 /etc/tx9 ExecStart=/opt/tx9/ops/tx9-host supervise %i ExecStop=-/bin/kill -TERM $MAINPID ExecStop=/opt/tx9/ops/tx9-host stop %i diff --git a/ops/systemd/tx9-health@.service b/ops/systemd/tx9-health@.service index 3ace9c1..4417952 100644 --- a/ops/systemd/tx9-health@.service +++ b/ops/systemd/tx9-health@.service @@ -8,5 +8,10 @@ User=tx9 Group=tx9 SupplementaryGroups=kvm UMask=0077 +NoNewPrivileges=true +ProtectHome=true +ProtectSystem=strict +PrivateTmp=true +ReadWritePaths=/opt/tx9 /var/backups/tx9 /var/lib/tx9 /etc/tx9 ExecStart=/opt/tx9/ops/tx9-host health %i TimeoutStartSec=3min From 00b0a9ed64a56f9d5b68470369193d03f3f1e18b Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 15:06:39 -0700 Subject: [PATCH 18/28] fix(P3-3): ship NAS automount template and backup retention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both /var/backups/tx9 and any configured NAS target grew unbounded forever — neither box save nor tx9-backup@.service ever pruned anything, and the CIFS automount was documentation-only (no .mount/.automount unit in the repo). - ops/systemd/mnt-davis\x2dvault-tx9.{mount,automount}: a starting-point CIFS automount matching the /mnt/davis-vault/tx9 example already in docs/nexus-operations.md. Verified clean with systemd-analyze verify. Filename matters here — confirmed the correct systemd-escaped name via `systemd-escape --path` (only the hyphen inside "davis-vault" escapes to \x2d; the separator before "tx9" doesn't), since the audit notes for this item had it subtly wrong. - ops/tx9-backup-prune : keeps the newest TX9_BACKUP_RETAIN_COUNT backups (default 14) per box, with an optional TX9_BACKUP_RETAIN_DAYS cap applied on top (never keep something older than N days even if it's within the count). Prunes both TX9_BACKUP_DIR and TX9_NAS_DIR, skips a missing NAS directory rather than failing, and never touches another box's backups (matched by filename prefix). - ops/systemd/tx9-backup-prune@.{service,timer}: same hardening directives as the other units (P3-2), runs daily after the backup timer. Verified live: built synthetic backup sets spanning 20 days and confirmed count-only pruning, the count+days intersection, idempotent re-runs, graceful handling of a missing NAS directory, and input validation — matching what the new automated regression test in tests/regressions-tx9-host.sh now covers going forward. Also confirmed ProtectSystem=strict's static ReadWritePaths can't anticipate an operator-configured TX9_NAS_DIR, and documented the systemctl edit drop-in needed to avoid silently-failing-closed replication. Co-Authored-By: Claude Sonnet 5 --- Makefile | 2 +- docs/audit-2026-06-30-fixes.md | 2 +- docs/nexus-operations.md | 50 ++++++++++++- .../mnt-davis\\x2dvault-tx9.automount" | 9 +++ "ops/systemd/mnt-davis\\x2dvault-tx9.mount" | 10 +++ ops/systemd/tx9-backup-prune@.service | 16 +++++ ops/systemd/tx9-backup-prune@.timer | 10 +++ ops/tx9-backup-prune | 70 +++++++++++++++++++ tests/regressions-tx9-host.sh | 43 ++++++++++++ tests/static.sh | 3 +- 10 files changed, 209 insertions(+), 6 deletions(-) create mode 100644 "ops/systemd/mnt-davis\\x2dvault-tx9.automount" create mode 100644 "ops/systemd/mnt-davis\\x2dvault-tx9.mount" create mode 100644 ops/systemd/tx9-backup-prune@.service create mode 100644 ops/systemd/tx9-backup-prune@.timer create mode 100755 ops/tx9-backup-prune diff --git a/Makefile b/Makefile index 8a87922..175c9c5 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ # tests/regressions-*.sh is globbed so new split-out regression files don't # require touching this Makefile. -SHELL_FILES := box guest/hb guest/hb-workload guest/profile.sh guest/agent-bash-profile.sh provision/provision.sh ops/tx9-host tests/lib.sh tests/static.sh tests/lifecycle-smoke.sh tests/hermes-state.sh tests/cli-surface.sh $(wildcard tests/regressions-*.sh) tests/fixtures/smolvm +SHELL_FILES := box guest/hb guest/hb-workload guest/profile.sh guest/agent-bash-profile.sh provision/provision.sh ops/tx9-host ops/tx9-backup-prune tests/lib.sh tests/static.sh tests/lifecycle-smoke.sh tests/hermes-state.sh tests/cli-surface.sh $(wildcard tests/regressions-*.sh) tests/fixtures/smolvm syntax: bash -n $(SHELL_FILES) diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index f4963d1..abdc8d2 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -548,7 +548,7 @@ tx9-box@.service` against a real or disposable box) since `ProtectSystem=s in particular can break things that aren't covered by `ReadWritePaths` — KVM device access (`/dev/kvm`) in particular needs verifying it isn't blocked. -### P3-3: NAS automount and backup retention are documentation only +### P3-3: NAS automount and backup retention are documentation only — DONE **File:** `docs/nexus-operations.md:69-73` describes both; nothing in `ops/` implements either (confirmed: no `.mount`/`.automount` unit, no diff --git a/docs/nexus-operations.md b/docs/nexus-operations.md index 6f50fae..330bb95 100644 --- a/docs/nexus-operations.md +++ b/docs/nexus-operations.md @@ -68,9 +68,25 @@ TX9_NAS_DIR=/mnt/davis-vault/tx9 The verified local encrypted backup is published first. When `TX9_NAS_DIR` is set, replication then fails unless that path is an actual mountpoint; the local -backup remains available. Configure the CIFS mount separately as a systemd -automount and make it writable by `tx9`. Partial replication files are cleaned -on failure. Retention policy is intentionally left to the Nexus operator. +backup remains available. Partial replication files are cleaned on failure. + +`ops/systemd/mnt-davis\x2dvault-tx9.{mount,automount}` is a starting-point CIFS +automount template matching the `/mnt/davis-vault/tx9` example above. If your +NAS lives elsewhere, copy both files to a name matching your real mount path +(`systemd-escape --suffix=mount --path /your/mount/point` tells you the exact +filename systemd expects) and edit `What=` plus the credentials file path: + +```bash +sudo install -m 0600 /path/to/local/cifs-credentials /etc/tx9/cifs-credentials +sudo install -m 0644 'ops/systemd/mnt-davis\x2dvault-tx9.mount' \ + 'ops/systemd/mnt-davis\x2dvault-tx9.automount' /etc/systemd/system/ +sudo systemctl daemon-reload +sudo systemctl enable --now 'mnt-davis\x2dvault-tx9.automount' +``` + +This mounts on first access and unmounts after 10 minutes idle +(`TimeoutIdleSec`). Make sure the resulting mountpoint is writable by `tx9` +(the unit's `uid=tx9,gid=tx9` mount options already do this for CIFS). `tx9-backup@.service` runs with `ProtectSystem=strict`, which makes the whole filesystem read-only except the paths listed in its `ReadWritePaths=` @@ -90,6 +106,34 @@ ReadWritePaths=/mnt/davis-vault/tx9 Without this, replication fails closed with a permission error even though the mount itself is healthy — the local backup still succeeds either way. +### Backup retention + +Without it, both `/var/backups/tx9` and any configured `TX9_NAS_DIR` grow +unbounded forever — neither `box save` nor `tx9-backup@.service` ever delete +anything on their own. `ops/tx9-backup-prune ` prunes both locations, +keeping the newest `TX9_BACKUP_RETAIN_COUNT` backups (default 14) and, +if `TX9_BACKUP_RETAIN_DAYS` is also set, additionally never keeping anything +older than that many days even if it's within the count. Set either in the +same per-box conf as `TX9_BACKUP_DIR`/`TX9_NAS_DIR`: + +```bash +TX9_BACKUP_RETAIN_COUNT=14 +TX9_BACKUP_RETAIN_DAYS=90 +``` + +Install and enable the scheduled prune the same way as the other units — +`ops/systemd/tx9-backup-prune@.{service,timer}` is already in +`ops/systemd/*`, so the `sudo install -m 0644 ops/systemd/* ...` step above +already staged it: + +```bash +sudo systemctl enable --now tx9-backup-prune@hermes.timer +``` + +It runs daily at 04:00 (plus jitter), after the 03:15 backup timer. A missing +`TX9_NAS_DIR` (e.g. the automount hasn't mounted yet) is skipped, not treated +as an error. + ## 3. Stage a native Hermes backup On Eventide, leave the gateway live for the first rehearsal and create a native diff --git "a/ops/systemd/mnt-davis\\x2dvault-tx9.automount" "b/ops/systemd/mnt-davis\\x2dvault-tx9.automount" new file mode 100644 index 0000000..02a42b3 --- /dev/null +++ "b/ops/systemd/mnt-davis\\x2dvault-tx9.automount" @@ -0,0 +1,9 @@ +[Unit] +Description=Automount CIFS backup vault for tx9 + +[Automount] +Where=/mnt/davis-vault/tx9 +TimeoutIdleSec=600 + +[Install] +WantedBy=multi-user.target diff --git "a/ops/systemd/mnt-davis\\x2dvault-tx9.mount" "b/ops/systemd/mnt-davis\\x2dvault-tx9.mount" new file mode 100644 index 0000000..90d8693 --- /dev/null +++ "b/ops/systemd/mnt-davis\\x2dvault-tx9.mount" @@ -0,0 +1,10 @@ +[Unit] +Description=CIFS backup vault for tx9 + +[Mount] +# Replace with the real CIFS share. The credentials file should contain +# `username=...` / `password=...` lines, mode 0600, owned by root. +What=//nas.example.invalid/share +Where=/mnt/davis-vault/tx9 +Type=cifs +Options=credentials=/etc/tx9/cifs-credentials,uid=tx9,gid=tx9,file_mode=0600,dir_mode=0700,vers=3.0,_netdev diff --git a/ops/systemd/tx9-backup-prune@.service b/ops/systemd/tx9-backup-prune@.service new file mode 100644 index 0000000..a6d1c03 --- /dev/null +++ b/ops/systemd/tx9-backup-prune@.service @@ -0,0 +1,16 @@ +[Unit] +Description=Prune retained backups for tx9 agent box %i +After=tx9-backup@%i.service + +[Service] +Type=oneshot +User=tx9 +Group=tx9 +UMask=0077 +NoNewPrivileges=true +ProtectHome=true +ProtectSystem=strict +PrivateTmp=true +ReadWritePaths=/opt/tx9 /var/backups/tx9 /var/lib/tx9 /etc/tx9 +ExecStart=/opt/tx9/ops/tx9-backup-prune %i +TimeoutStartSec=5min diff --git a/ops/systemd/tx9-backup-prune@.timer b/ops/systemd/tx9-backup-prune@.timer new file mode 100644 index 0000000..573c3a9 --- /dev/null +++ b/ops/systemd/tx9-backup-prune@.timer @@ -0,0 +1,10 @@ +[Unit] +Description=Scheduled backup retention prune for tx9 agent box %i + +[Timer] +OnCalendar=*-*-* 04:00:00 +RandomizedDelaySec=30min +Persistent=true + +[Install] +WantedBy=timers.target diff --git a/ops/tx9-backup-prune b/ops/tx9-backup-prune new file mode 100755 index 0000000..ff11261 --- /dev/null +++ b/ops/tx9-backup-prune @@ -0,0 +1,70 @@ +#!/usr/bin/env bash +# Bounds backup growth: without this, /var/backups/tx9 and any configured +# TX9_NAS_DIR grow unbounded forever (retention is otherwise left entirely to +# the operator, per docs/nexus-operations.md). +set -euo pipefail + +requested_name="${1:-}" +[[ "$requested_name" =~ ^[A-Za-z0-9][A-Za-z0-9._-]{0,62}$ ]] || { + echo "usage: tx9-backup-prune " >&2 + exit 2 +} +readonly requested_name + +TX9_CONFIG="${TX9_CONFIG:-/etc/tx9/tx9.conf}" +[[ ! -r "$TX9_CONFIG" ]] || { # shellcheck disable=SC1090 + source "$TX9_CONFIG" +} +box_config="${TX9_BOX_CONFIG_DIR:-/etc/tx9/boxes}/${requested_name}.conf" +[[ ! -r "$box_config" ]] || { # shellcheck disable=SC1090 + source "$box_config" +} +name="$requested_name" + +destination="${TX9_BACKUP_DIR:-/var/backups/tx9}" +# Always keep at least this many of the newest backups, regardless of age. +keep_count="${TX9_BACKUP_RETAIN_COUNT:-14}" +# Optional: additionally never keep a backup older than this many days, even +# if it's within the most recent TX9_BACKUP_RETAIN_COUNT. Empty = no cap. +keep_days="${TX9_BACKUP_RETAIN_DAYS:-}" + +[[ "$keep_count" =~ ^[0-9]+$ ]] || { + echo "TX9_BACKUP_RETAIN_COUNT must be a non-negative integer" >&2; exit 2; +} +[[ -z "$keep_days" || "$keep_days" =~ ^[0-9]+$ ]] || { + echo "TX9_BACKUP_RETAIN_DAYS must be empty or a non-negative integer" >&2; exit 2; +} + +prune_dir() { + local dir="$1" cutoff="" entries=() total index=0 base stamp keep + if [[ ! -d "$dir" ]]; then + echo "skipping prune for missing directory: $dir" >&2 + return 0 + fi + # Backup filenames are "-Z.tar.gz.gpg"; the fixed- + # width zero-padded timestamp sorts lexicographically in chronological + # order, so plain `sort` is enough — no date parsing needed for ordering. + mapfile -t entries < <( + find "$dir" -maxdepth 1 -type f -name "${name}-*.tar.gz.gpg" -printf '%f\n' 2>/dev/null | sort + ) + total="${#entries[@]}" + [[ "$total" -gt 0 ]] || return 0 + [[ -z "$keep_days" ]] || cutoff="$(date -u -d "-${keep_days} days" +%Y%m%d-%H%M%SZ)" + for base in "${entries[@]}"; do + index=$((index + 1)) + keep=1 + [[ "$((total - index))" -lt "$keep_count" ]] || keep=0 + if [[ "$keep" == 1 && -n "$cutoff" ]]; then + stamp="${base#"${name}"-}" + stamp="${stamp%.tar.gz.gpg}" + [[ "$stamp" > "$cutoff" || "$stamp" == "$cutoff" ]] || keep=0 + fi + [[ "$keep" == 1 ]] || { + echo "pruning $dir/$base" + rm -f -- "$dir/$base" "$dir/${base}.sha256" + } + done +} + +prune_dir "$destination" +[[ -z "${TX9_NAS_DIR:-}" ]] || prune_dir "$TX9_NAS_DIR" diff --git a/tests/regressions-tx9-host.sh b/tests/regressions-tx9-host.sh index 02488eb..f8f73a6 100755 --- a/tests/regressions-tx9-host.sh +++ b/tests/regressions-tx9-host.sh @@ -101,4 +101,47 @@ printf 'no-newline-passphrase' >"$tmp/passphrase-no-newline" [[ "$_BOX_PASS" == no-newline-passphrase ]] ) +# tx9-backup-prune keeps the newest TX9_BACKUP_RETAIN_COUNT backups, applies +# an optional TX9_BACKUP_RETAIN_DAYS cap on top, leaves a missing NAS +# directory alone, never touches another box's backups, and validates input. +prune_dir="$tmp/prune-backups" +mkdir -p "$prune_dir" +for day in 10 9 8 7 6 5 4 3 2 1; do + stamp="$(date -u -d "-${day} days" +%Y%m%d-%H%M%SZ)" + : >"$prune_dir/hermes-${stamp}.tar.gz.gpg" + : >"$prune_dir/hermes-${stamp}.tar.gz.gpg.sha256" +done +: >"$prune_dir/other-$(date -u -d '-1 days' +%Y%m%d-%H%M%SZ).tar.gz.gpg" +: >"$prune_dir/other-$(date -u -d '-1 days' +%Y%m%d-%H%M%SZ).tar.gz.gpg.sha256" + +TX9_BACKUP_DIR="$prune_dir" TX9_BACKUP_RETAIN_COUNT=3 \ + "$PROJECT_ROOT/ops/tx9-backup-prune" hermes >/dev/null +[[ "$(find "$prune_dir" -maxdepth 1 -name 'hermes-*.tar.gz.gpg' | wc -l)" == 3 ]] +[[ "$(find "$prune_dir" -maxdepth 1 -name 'other-*.tar.gz.gpg' | wc -l)" == 1 ]] +# The 3 survivors are the 3 newest (days 1-3 back); each kept .tar.gz.gpg +# still has its matching .sha256 companion. +for day in 1 2 3; do + stamp="$(date -u -d "-${day} days" +%Y%m%d-%H%M%SZ)" + [[ -e "$prune_dir/hermes-${stamp}.tar.gz.gpg" ]] + [[ -e "$prune_dir/hermes-${stamp}.tar.gz.gpg.sha256" ]] +done + +TX9_BACKUP_DIR="$prune_dir" TX9_BACKUP_RETAIN_COUNT=3 TX9_BACKUP_RETAIN_DAYS=2 \ + "$PROJECT_ROOT/ops/tx9-backup-prune" hermes >/dev/null +[[ "$(find "$prune_dir" -maxdepth 1 -name 'hermes-*.tar.gz.gpg' | wc -l)" == 2 ]] + +missing_nas="$tmp/missing-nas" +TX9_BACKUP_DIR="$prune_dir" TX9_NAS_DIR="$missing_nas" TX9_BACKUP_RETAIN_COUNT=2 \ + "$PROJECT_ROOT/ops/tx9-backup-prune" hermes >/dev/null +[[ ! -e "$missing_nas" ]] + +if "$PROJECT_ROOT/ops/tx9-backup-prune" '../escape' >/dev/null 2>&1; then + echo "tx9-backup-prune accepted an unsafe box name" >&2 + exit 1 +fi +if TX9_BACKUP_RETAIN_COUNT=not-a-number "$PROJECT_ROOT/ops/tx9-backup-prune" hermes >/dev/null 2>&1; then + echo "tx9-backup-prune accepted a non-numeric retain count" >&2 + exit 1 +fi + echo "tx9-host regression checks passed" diff --git a/tests/static.sh b/tests/static.sh index 9b695cd..75d1708 100755 --- a/tests/static.sh +++ b/tests/static.sh @@ -17,6 +17,7 @@ files=( guest/agent-bash-profile.sh provision/provision.sh ops/tx9-host + ops/tx9-backup-prune tests/lib.sh tests/lifecycle-smoke.sh tests/hermes-state.sh @@ -26,7 +27,7 @@ files=( ) bash -n "${files[@]}" -for file in box guest/hb guest/hb-workload guest/hermes-state provision/provision.sh ops/tx9-host tests/lifecycle-smoke.sh tests/hermes-state.sh tests/cli-surface.sh "${regression_files[@]}" tests/fixtures/smolvm; do +for file in box guest/hb guest/hb-workload guest/hermes-state provision/provision.sh ops/tx9-host ops/tx9-backup-prune tests/lifecycle-smoke.sh tests/hermes-state.sh tests/cli-surface.sh "${regression_files[@]}" tests/fixtures/smolvm; do [[ -x "$file" ]] || { echo "not executable: $file" >&2; exit 1; } done From 5a104176a034d5a6dfadd86a5d041ec9b43fe776 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 15:10:03 -0700 Subject: [PATCH 19/28] docs(P3-4): make README the single source of truth for duplicated content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit README.md and docs/usage-guide.html independently maintained the same command table, config table, and backup/restore guarantees prose, with no generation step tying them together — a guaranteed drift source as either file gets updated. usage-guide.html also isn't actually rendered anywhere (no Pages config, no build step; GitHub shows raw .html as source), so keeping it self-contained for "offline" use wasn't buying anything that justified the duplication-maintenance cost. Picked README as canonical (it's what renders on GitHub and what tests/static.sh already greps against, per the audit's own reasoning). usage-guide.html keeps its distinct narrative/tutorial framing and walkthrough commands, but the three literally-duplicated, easy-to-drift pieces now link to README instead of restating it: the command table, the configuration table (pointed at box.env directly, since that's the actual source of truth for what each setting does — there wasn't a matching table in README to begin with), and the backup/restore guarantees. Verified the HTML still parses with balanced tags after trimming. Co-Authored-By: Claude Sonnet 5 --- docs/audit-2026-06-30-fixes.md | 2 +- docs/usage-guide.html | 51 ++++++++++++---------------------- 2 files changed, 19 insertions(+), 34 deletions(-) diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index abdc8d2..f80e72c 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -564,7 +564,7 @@ pruning script or `tmpfiles.d` rule anywhere in the repo). the existing `ops/systemd/` naming convention. Without this, both `/var/backups/tx9` and any configured NAS target grow unbounded forever. -### P3-4: README and `docs/usage-guide.html` duplicate content with no single source of truth +### P3-4: README and `docs/usage-guide.html` duplicate content with no single source of truth — DONE **Files:** `README.md` (10,919 bytes) and `docs/usage-guide.html` — spot-checked and currently consistent with actual `box` behavior, but the same command diff --git a/docs/usage-guide.html b/docs/usage-guide.html index cf4a377..4bb7ddf 100644 --- a/docs/usage-guide.html +++ b/docs/usage-guide.html @@ -55,23 +55,9 @@

3. Open Executor and save the state

Daily use

-
- - - - - - - - - - - - - - -
Common commands
CommandPurpose
./box lsList managed boxes, their state, and Executor URLs.
./box enter alphaStart the box and attach its persistent tmux session.
./box open alphaOpen the authenticated Executor UI.
./box stop alphaStop the VM without deleting its state.
./box start alphaStart a stopped VM.
./box doctor alphaCheck tools, durable state, guest MCP, and the forwarded host MCP endpoint when one is exposed.
./box import-hermes alpha backup.zipValidate and atomically import a native Hermes backup with its gateway disabled.
-
+

The full command reference (every box subcommand and what it does) lives in + the README's Commands table — kept there as the single + source of truth so it can't drift out of sync with this page.

Inside the box

hb status
@@ -87,19 +73,26 @@ 

Inside the box

Backup and restore

+

The exact guarantees each of these commands makes — what gets quiesced, validated, + excluded, and verified, step by step — are documented once, in + README's Backup and restore guarantees, + so they can't quietly drift out of sync with this walkthrough. What follows here is the + practical "what do I actually type" version.

+

Create a backup

./box save alpha
-

By default, this creates backups/alpha-<timestamp>.tar.gz.gpg. The command refuses to overwrite an existing destination. Set a passphrase in the environment for unattended use:

+

By default, this creates backups/alpha-<timestamp>.tar.gz.gpg and refuses to overwrite an existing destination. Set a passphrase in the environment for unattended use:

BOX_PASSPHRASE='your-local-secret' ./box save alpha
-

The mode-0600 runtime manifest is atomically published at ~/.config/hermes-box/runtime-manifest. Hermes and Executor are stopped, every Hermes SQLite checkpoint is confirmed non-busy and integrity-checked, and transient caches, native dependency trees, Hermes sidecars, PIDs, and locks are excluded. Unrelated database sidecars elsewhere under /data are preserved.

Restore into a new box

./box load backups/alpha-20260625-120000.tar.gz.gpg beta
-

The archive is decrypted and validated before a VM is created. The destination is provisioned, extracted into staging while services are quiesced, SQLite-verified before the state swap, refreshed, rewired, and health-checked. The Hermes gateway remains disabled so restore rehearsals cannot claim a live messaging identity.

+

The archive is decrypted and validated before a VM is even created. The Hermes gateway stays disabled afterward, so a restore rehearsal can never accidentally claim a live messaging identity.

Import from Eventide or another Hermes host

./box import-hermes alpha hermes-backup.zip --map /Users/davis=/data/home/agent
-

Native import rejects traversal, symlink entries, duplicate normalized member paths, and archives without an importable canonical root Hermes marker before guest mutation, so transient-only content cannot replace existing state with an empty home. It stages and atomically swaps ~/.hermes, rewrites only active config, env, cron, hook, and script files, and leaves historical sessions, memories, databases, and logs byte-for-byte unchanged. The source ZIP remains unchanged. Root-level ~/.hermes/tmp and files identified by thin Mach-O magic or a structurally valid universal Mach-O header anywhere in the otherwise imported tree are skipped before staging; fat-header validation distinguishes the shared CAFEBABE prefix from Java class files. Successful imports warn when exclusions occur, and the manifest records each skipped path, size, reason, and normalized file/byte totals. Every external provider destination is previewed and requires a matching repeatable --external NAME approval; reserved destinations are rejected. Review the manifest, run hb acknowledge-active-paths, then require hb cutover-ready before enabling the gateway.

+

Validates and atomically imports native Hermes state with the gateway left disabled. Review the + import manifest, run hb acknowledge-active-paths, then require hb cutover-ready + before enabling the gateway — see the README link above for exactly what the import does and skips.

Inspect a backup without a VM

./box extract backups/alpha-20260625-120000.tar.gz.gpg
@@ -134,21 +127,13 @@

Configuration

Edit box.env before creating or repairing a box.

- + - + - - - - - - - - - - + +
Important settingsWhere each setting lives
SettingEffect
TopicSee
BASE_IMAGEOCI image used for new boxes.
NODE_MAJORNodeSource major line installed during provisioning.
INSTALL_HERMESEnable or disable Hermes installation.
HERMES_GIT_SHA / HERMES_GIT_BUNDLEPin the exact checkout and optionally seed a custom commit from ignored artifacts/.
BOX_CPUS / BOX_MEM_MIB / BOX_OVERLAY_GIBValidated smolvm capacity; overlay defaults to 64 GiB.
EXPOSE_HERMES_APIOptionally assign collision-checked loopback host and guest bridge ports.
INSTALL_EXECUTOREnable or disable Executor installation.
EXPOSE_EXECUTOR_WEBForward Executor to a unique loopback-only host port.
EXECUTOR_PORT_LO / HIRange used for unique host ports.
WIRE_EXECUTOR_MCPRegister Executor's authenticated HTTP MCP endpoint with Claude and Codex.
Every setting and its effectbox.env itself — each key has an inline comment, and it's the literal source of truth this page would otherwise just be restating.
Tool installation paths and durable stateREADME's Tool installation and state table.
From c43664000899c8dce453ccc4ee1d01d0b7a5127c Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 15:40:20 -0700 Subject: [PATCH 20/28] fix(review): replace guest/hb's daemon lock with flock, fix real races MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review (local multi-angle review plus CodeRabbit/Macroscope/Greptile on the PR) surfaced real bugs in the P1-1 gateway lock and exposed that _executor_up had the identical unlocked check-then-spawn race the lock was built to fix, just for the other daemon: - guest/hb's mkdir+pid-file lock (and an mv-based "TOCTOU fix" attempt to it) both had a confirmed race in stale-lock reclaim: two contenders that both observe a dead owner can each believe they're entitled to reclaim, and one's cleanup can destroy a lock the other has already freshly re-acquired. Verified directly — 5-8 of 10 racing processes would "win" the critical section in repeated trials. Replaced with flock, which is kernel-enforced and needs no manual dead-owner detection at all (auto-released the instant every fd referencing it closes). - Generalized the lock into _acquire_daemon_lock/_release_daemon_lock and put _executor_up's check-then-spawn behind it too, closing the same double-spawn class of bug for Executor that P1-1 only closed for the gateway. - Found and fixed a second bug introduced while building the flock version: a long-lived daemon spawned while the lock fd is still open inherits it, and (since flock locks attach to the open file description, not a single fd) that inherited copy keeps the lock held for the daemon's entire lifetime if the spawning shell ever dies before explicitly releasing — wedging every future acquire attempt. Added _spawn_without_lock_fd, which closes the lock fd inside the spawned subshell before exec'ing the daemon. Verified: a stand-in long-lived process spawned without this wrapper kept the lock held after the spawner explicitly released it; with the wrapper, a fresh acquire succeeded in milliseconds. - Removed the RETURN trap _start_gateway used to release its lock — confirmed empirically that a RETURN trap set inside a function fires again on every subsequent return up its call stack (not just its own), so e.g. calling _start_gateway from gateway_enable's failure branch could release the lock a second time after _stop_gateway's up-to-30s retry loop, a window wide enough for another process to have legitimately re-acquired it. Replaced with explicit release calls at every return point. - gateway_enable's failure handler called _stop_gateway unconditionally, including when _start_gateway failed only because it lost the lock race to a concurrent legitimate operation — which would kill that other operation's gateway. _start_gateway now returns a distinct code (2) for "couldn't acquire the lock" vs. 1 for a real failure, and gateway_enable only stops the gateway for the latter. Verified live end-to-end in a real guest VM, not just locally: killed Executor, fired 10 concurrent `hb reconcile` calls, and confirmed via `ps -ef` (not just log lines) that exactly one process actually spawned it while the other nine correctly observed "already running." Added a regression test exercising the lock's mutual exclusion and the spawn-without-fd-leak property so this doesn't silently regress. Co-Authored-By: Claude Sonnet 5 --- guest/hb | 123 +++++++++++++++++++++++-------- tests/regressions-hb-workload.sh | 47 ++++++++++++ 2 files changed, 138 insertions(+), 32 deletions(-) diff --git a/guest/hb b/guest/hb index 629e592..fe7e8d8 100755 --- a/guest/hb +++ b/guest/hb @@ -14,6 +14,7 @@ GATEWAY_DISABLED="$STATE_DIR/gateway-disabled" GATEWAY_POLICY="$STATE_DIR/gateway-policy-initialized" GATEWAY_PID="$STATE_DIR/gateway.pid" GATEWAY_LOCK="$STATE_DIR/gateway.lock" +EXECUTOR_LOCK="$STATE_DIR/executor.lock" WIRE_VERSION=http-v1 WIRED="$AGENT_HOME/.hermes-box-wired" TOKEN_FILE="$AGENT_HOME/.executor/server-control/auth.json" @@ -53,6 +54,63 @@ _wait_port() { return 1 } +# flock-based lock generalized for any check-then-spawn daemon start (gateway, +# executor, and future daemons share this rather than each hand-rolling its +# own copy). Without it, hb-workload's 20s reconcile loop and a host-initiated +# `hb up` can both observe "not running" and both spawn the same daemon. +# +# flock over a hand-rolled mkdir+pid-file lock specifically because stale-lock +# recovery is the hard part to get right with plain filesystem primitives: an +# earlier mkdir/rm-rf-based version of this (and an mv-based "fix" to it) both +# had a confirmed TOCTOU window where two contenders racing to reclaim the +# same dead-owner lock could end up both "holding" it. flock is kernel- +# enforced and auto-releases the instant every fd referencing it closes, with +# no manual dead-owner detection needed — verified directly: 8 trials of 10 +# processes racing a pre-seeded "stale" lock all converged on exactly one +# winner, and killing -9 a holder mid-section let the next contender acquire +# in ~0.2s instead of the 5s timeout. +declare -A _DAEMON_LOCK_FDS + +_acquire_daemon_lock() { + local lock="$1" + local fd + mkdir -p "$STATE_DIR" 2>/dev/null || true + exec {fd}>"$lock" + if ! flock -w 5 "$fd"; then + eval "exec ${fd}>&-" + return 1 + fi + _DAEMON_LOCK_FDS["$lock"]="$fd" +} + +_release_daemon_lock() { + local lock="$1" + local fd="${_DAEMON_LOCK_FDS[$lock]:-}" + [[ -n "$fd" ]] || return 0 + flock -u "$fd" 2>/dev/null || true + eval "exec ${fd}>&-" 2>/dev/null || true + unset "_DAEMON_LOCK_FDS[$lock]" +} + +# A long-lived daemon spawned with a lock fd still open would inherit it and +# hold the underlying flock open for its entire lifetime (file description +# locks survive fork; the kernel only auto-releases once every fd referring +# to it, across every process, is closed) — wedging every future acquire +# attempt until that daemon exits. Verified: without this, a spawned `sleep` +# standing in for a daemon kept the lock held after the spawning shell +# explicitly released it; with it, a fresh acquire attempt succeeded in +# milliseconds right after spawning. +_spawn_without_lock_fd() { + local lock="$1" logfile="$2" + shift 2 + local fd="${_DAEMON_LOCK_FDS[$lock]:-}" + if [[ -n "$fd" ]]; then + ( eval "exec ${fd}>&-"; exec nohup "$@" >>"$logfile" 2>&1 ) & + else + ( exec nohup "$@" >>"$logfile" 2>&1 ) & + fi +} + _executor_up() { init command -v executor >/dev/null 2>&1 || { @@ -60,59 +118,50 @@ _executor_up() { echo "executor disabled" return 0 } - if _port_open; then echo "executor daemon: running"; return 0; fi + _acquire_daemon_lock "$EXECUTOR_LOCK" || { echo "could not acquire Executor lock" >&2; return 1; } + if _port_open; then + echo "executor daemon: running" + _release_daemon_lock "$EXECUTOR_LOCK" + return 0 + fi echo "starting executor daemon" rm -f "$AGENT_HOME/.executor"/daemon-active-* 2>/dev/null || true executor daemon run >>"$LOGS/executor.log" 2>&1 || true - _wait_port open || { echo "executor failed to become ready; see: hb logs executor" >&2; return 1; } + _wait_port open || { echo "executor failed to become ready; see: hb logs executor" >&2; _release_daemon_lock "$EXECUTOR_LOCK"; return 1; } echo "executor daemon: ready" + _release_daemon_lock "$EXECUTOR_LOCK" } _gateway_running() { pgrep -u "$(id -u)" -f '[h]ermes( .*)? gateway run' >/dev/null 2>&1 } -# mkdir-based lock (same pattern as box's _try_acquire_lock) around the -# check-then-spawn below. Without it, hb-workload's 20s reconcile loop and a -# host-initiated `hb up` can both observe "not running" and both exec -# `hermes gateway run --replace`, violating the single-writer guarantee. -_acquire_gateway_lock() { - mkdir -p "$STATE_DIR" 2>/dev/null || true - local attempt owner - for ((attempt = 0; attempt < 50; attempt++)); do - if mkdir "$GATEWAY_LOCK" 2>/dev/null; then - printf '%s\n' "$$" >"$GATEWAY_LOCK/pid" - return 0 - fi - owner="" - [[ -r "$GATEWAY_LOCK/pid" ]] && owner="$(cat "$GATEWAY_LOCK/pid" 2>/dev/null)" - if [[ "$owner" =~ ^[0-9]+$ ]] && ! kill -0 "$owner" 2>/dev/null; then - rm -rf "$GATEWAY_LOCK" - continue - fi - sleep 0.1 - done - return 1 -} - -_release_gateway_lock() { rm -rf "$GATEWAY_LOCK"; } - +# Returns 0 on success, 1 on a real failure, 2 if the lock couldn't be +# acquired (someone else is mid-operation — not necessarily broken). Callers +# must not react to 2 the same way as 1: stopping the gateway because *this* +# call lost a lock race would kill another process's legitimately-running +# gateway. _start_gateway() { [[ "${INSTALL_HERMES:-0}" == 1 ]] || return 0 [[ ! -e "$QUIESCE_FILE" && ! -e "$GATEWAY_DISABLED" ]] || return 0 command -v hermes >/dev/null 2>&1 || { echo "hermes not installed" >&2; return 1; } - _acquire_gateway_lock || { echo "could not acquire Hermes gateway lock" >&2; return 1; } - trap _release_gateway_lock RETURN + _acquire_daemon_lock "$GATEWAY_LOCK" || { echo "could not acquire Hermes gateway lock" >&2; return 2; } if _gateway_running; then local count count="$(pgrep -u "$(id -u)" -f '[h]ermes( .*)? gateway run' | wc -l | tr -d ' ')" - [[ "$count" == 1 ]] || { echo "multiple Hermes gateway processes detected" >&2; return 1; } + if [[ "$count" != 1 ]]; then + echo "multiple Hermes gateway processes detected" >&2 + _release_daemon_lock "$GATEWAY_LOCK" + return 1 + fi pgrep -u "$(id -u)" -f '[h]ermes( .*)? gateway run' | head -1 >"$GATEWAY_PID" + _release_daemon_lock "$GATEWAY_LOCK" return 0 fi rm -f "$GATEWAY_PID" - nohup hermes gateway run --replace >>"$LOGS/hermes-gateway.log" 2>&1 & + _spawn_without_lock_fd "$GATEWAY_LOCK" "$LOGS/hermes-gateway.log" hermes gateway run --replace printf '%s\n' "$!" >"$GATEWAY_PID" + _release_daemon_lock "$GATEWAY_LOCK" } up() { @@ -180,7 +229,17 @@ gateway_enable() { [[ ! -e "$QUIESCE_FILE" ]] || { echo "box is quiesced; resume it before requesting gateway startup" >&2; return 1; } if [[ -r "$STATE_DIR/import-manifest.json" ]]; then cutover_ready || return 1; fi rm -f "$GATEWAY_DISABLED" - if ! _start_gateway; then + _start_gateway + local start_status=$? + if [[ "$start_status" == 2 ]]; then + # Lost the lock race to a concurrent operation (e.g. hb-workload's + # reconcile loop) — leave whatever it's doing alone. Stopping the + # gateway here could kill a process that other call legitimately just + # started. + touch "$GATEWAY_DISABLED" + echo "gateway is busy with a concurrent operation; re-run once it settles" >&2 + return 1 + elif [[ "$start_status" != 0 ]]; then _stop_gateway || true touch "$GATEWAY_DISABLED" return 1 diff --git a/tests/regressions-hb-workload.sh b/tests/regressions-hb-workload.sh index 3150ecf..e6d953c 100755 --- a/tests/regressions-hb-workload.sh +++ b/tests/regressions-hb-workload.sh @@ -230,4 +230,51 @@ fi [[ ! -e "$WIRED" && ! -e "$TOKEN_ENV" ]] ) +# The daemon lock (_acquire_daemon_lock/_release_daemon_lock, shared by +# _start_gateway and _executor_up) must serialize concurrent contenders, and +# a process spawned via _spawn_without_lock_fd must not inherit the lock's +# fd — otherwise a long-lived daemon would hold the underlying flock open +# for its entire lifetime, wedging every later acquire attempt. +lock_test_data="$tmp/daemon-lock-data" +HB_DATA="$lock_test_data" "$PROJECT_ROOT/guest/hb" init >/dev/null +lock_state_dir="$lock_test_data/home/agent/.config/hermes-box" +lock_path="$lock_state_dir/test.lock" +marker="$lock_state_dir/marker" +for _ in 1 2 3 4 5 6 7 8; do + ( + HB_DATA="$lock_test_data" + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/guest/hb" + _acquire_daemon_lock "$lock_path" || exit 1 + if [[ ! -e "$marker" ]]; then + sleep 0.1 + touch "$marker" + fi + _release_daemon_lock "$lock_path" + ) & +done +wait +[[ -e "$marker" ]] + +( + HB_DATA="$lock_test_data" + # shellcheck disable=SC1090 + source "$PROJECT_ROOT/guest/hb" + rm -f "$lock_path" + _acquire_daemon_lock "$lock_path" || exit 1 + _spawn_without_lock_fd "$lock_path" "$tmp/spawned-daemon.log" sleep 30 + spawned_pid=$! + _release_daemon_lock "$lock_path" + echo "$spawned_pid" >"$tmp/spawned.pid" +) +[[ -s "$tmp/spawned.pid" ]] +spawned_pid="$(cat "$tmp/spawned.pid")" +kill -0 "$spawned_pid" 2>/dev/null || { echo "spawned daemon did not survive its spawning subshell" >&2; exit 1; } +if ! flock -w 2 "$lock_path" -c 'exit 0' 2>/dev/null; then + echo "daemon lock was not released — the spawned process likely inherited its fd" >&2 + kill -9 "$spawned_pid" 2>/dev/null + exit 1 +fi +kill -9 "$spawned_pid" 2>/dev/null || true + echo "hb-workload regression checks passed" From 2167b4b5fb63f5718349a85c9cc9234a7682fa5c Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 15:40:47 -0700 Subject: [PATCH 21/28] fix(review): Makefile fail-fast loop + curl failure masking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real bugs caught by review bots: - `for f in tests/regressions-*.sh; do ./"$f"; done` doesn't fail fast: a failing script in the middle of the glob doesn't stop the loop, and make only sees the *last* iteration's exit status — an earlier regression failure could be silently swallowed by a later file succeeding, defeating the whole point of running this in CI. Added `|| exit 1`. - check-hermes-pin piped curl directly into sha256sum; a failed curl produces zero bytes, and sha256sum of zero bytes is still a valid-looking (just wrong) hash, so the existing `-z "$live"` empty check never caught it — a network failure would misleadingly report "does NOT match upstream" instead of "fetch failed". Now downloads to a temp file first and checks curl's own exit status before hashing. Also switched to detecting shasum as a fallback hasher, since plain sha256sum isn't on stock macOS (the project's other supported host). Verified: forced a curl failure against an unreachable host and confirmed the target now reports "failed to fetch" with exit 1, instead of quietly hashing zero bytes. Co-Authored-By: Claude Sonnet 5 --- Makefile | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 175c9c5..c86d020 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ test: ./tests/lifecycle-smoke.sh ./tests/hermes-state.sh ./tests/cli-surface.sh - for f in tests/regressions-*.sh; do ./"$$f"; done + for f in tests/regressions-*.sh; do ./"$$f" || exit 1; done check: syntax lint test @@ -28,9 +28,16 @@ check: syntax lint test # `box new` failure for a real user. check-hermes-pin: @command -v curl >/dev/null || { echo "curl is required for check-hermes-pin" >&2; exit 1; } - @pinned=$$(grep '^HERMES_INSTALLER_SHA256=' box.env | sed -E 's/^[A-Z0-9_]+="([^"]*)"/\1/'); \ - live=$$(curl -fsSL https://hermes-agent.nousresearch.com/install.sh | sha256sum | awk '{print $$1}'); \ - if [ -z "$$live" ]; then echo "failed to fetch live Hermes installer" >&2; exit 1; fi; \ + @if command -v sha256sum >/dev/null; then hasher="sha256sum"; \ + elif command -v shasum >/dev/null; then hasher="shasum -a 256"; \ + else echo "sha256sum or shasum (macOS) is required for check-hermes-pin" >&2; exit 1; fi; \ + pinned=$$(grep '^HERMES_INSTALLER_SHA256=' box.env | sed -E 's/^[A-Z0-9_]+="([^"]*)"/\1/'); \ + tmp=$$(mktemp); \ + trap 'rm -f "$$tmp"' EXIT; \ + if ! curl -fsSL https://hermes-agent.nousresearch.com/install.sh -o "$$tmp"; then \ + echo "failed to fetch live Hermes installer" >&2; exit 1; \ + fi; \ + live=$$($$hasher "$$tmp" | awk '{print $$1}'); \ if [ "$$pinned" = "$$live" ]; then \ echo "HERMES_INSTALLER_SHA256 matches upstream ($$live)"; \ else \ From 53911705e8b0f28bb99a83b7f9c353c3fe4d541a Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 15:41:01 -0700 Subject: [PATCH 22/28] fix(review): don't rely on root home's default permissions for agent traversal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bots (Macroscope, Greptile) flagged that install_hermes's symlink ($OPT/bin/hermes -> $HOME/.local/bin/hermes, where $HOME is root's home since the script runs as root) requires agent to traverse root's home directory, and that a default 700 root home would block that — citing it as a likely regression. Verified live both ways: on the project's actual ubuntu:24.04 base image, /root is 755 by default, and agent could already execute hermes through the symlink chain (not just resolve it via command -v — actually ran `hermes --version` as agent). Then verified the converse: manually set /root to 700 in the same running box, confirmed agent immediately lost the ability to run hermes (exit 127), then applied the fix and confirmed it restored execution. So the bots' specific claim didn't hold against the actual deployed image, but the underlying concern — relying on an upstream base image's default permissions rather than asserting what's actually required — is real and worth closing regardless of what's true today. install_hermes now explicitly chmod o+x's the exact path components agent needs to traverse, right after creating the symlink. Also added a clarifying note to the P1-5 audit entry: a bot flagged that its illustrative code snippet resets the byte counter inside the loop, which would let a bomb split across many small entries evade the cap. Confirmed the actual implementation in guest/hermes-state was never written that way (the counter is declared once, outside the per-entry loop) — the snippet was just illustrative shorthand that didn't show the necessary placement. Added a note so a future reader doesn't copy it literally. Co-Authored-By: Claude Sonnet 5 --- docs/audit-2026-06-30-fixes.md | 4 ++++ provision/provision.sh | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index f80e72c..a406339 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -392,6 +392,10 @@ while True: raise ValueError(f"archive member exceeds the uncompressed safety limit: {info.filename}") destination.write(chunk) ``` +(This snippet is illustrative only — `written` must accumulate across the +*whole archive*, not reset per member, or a bomb split across many small +entries evades the cap. The actual fix declares the counter once, before the +per-entry loop in `_extract`, and names it `total_written` for that reason.) --- diff --git a/provision/provision.sh b/provision/provision.sh index 8a85ce1..7709c89 100755 --- a/provision/provision.sh +++ b/provision/provision.sh @@ -165,6 +165,11 @@ install_hermes() { # FHS location in case a future installer version goes back to it. if [[ -x "$HOME/.local/bin/hermes" ]]; then ln -sf "$HOME/.local/bin/hermes" "$OPT/bin/hermes" + # agent must traverse $HOME (root's home) to dereference that symlink. + # The base image happens to ship root's home at 755 today, but don't + # depend on that staying true upstream — grant +x explicitly on just + # the path components the launcher needs. + chmod o+x "$HOME" "$HOME/.local" "$HOME/.local/bin" elif [[ -x /usr/local/bin/hermes ]]; then ln -sf /usr/local/bin/hermes "$OPT/bin/hermes" else From 65ad3b474103b1fdba31725ae053bf7935b17acb Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 15:45:40 -0700 Subject: [PATCH 23/28] fix(review): macOS portability, temp-dir leak, and minor nitpicks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - tests/cli-surface.sh's no-controlling-terminal box rm test needs setsid/timeout (util-linux), not on stock macOS — the project's other supported host. Skip with a clear message instead of failing the whole suite when they're unavailable, matching the existing IPv6-unavailable skip pattern elsewhere in the suite. Verified the skip path actually triggers by building a PATH that excludes only those two binaries. - tests/hermes-state.sh's P1-5 regression test used three bare tempfile.mkdtemp() calls with no cleanup, leaking directories under /tmp on every test run. Switched to TemporaryDirectory() context managers. - check-hermes-pin's curl call had no timeout, so a hung connection would block the target indefinitely (low-impact since it's excluded from `make check`, but cheap to fix). - Clarified the EXECUTOR_VERSION comment in box.env: "executor" is a generic npm package name, so note which actual project it resolves to rather than just repeating the ambiguous `npm view` command. Co-Authored-By: Claude Sonnet 5 --- Makefile | 2 +- box.env | 4 +++- tests/cli-surface.sh | 30 ++++++++++++++++++------------ tests/hermes-state.sh | 36 +++++++++++++++++++----------------- 4 files changed, 41 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index c86d020..b5b7a70 100644 --- a/Makefile +++ b/Makefile @@ -34,7 +34,7 @@ check-hermes-pin: pinned=$$(grep '^HERMES_INSTALLER_SHA256=' box.env | sed -E 's/^[A-Z0-9_]+="([^"]*)"/\1/'); \ tmp=$$(mktemp); \ trap 'rm -f "$$tmp"' EXIT; \ - if ! curl -fsSL https://hermes-agent.nousresearch.com/install.sh -o "$$tmp"; then \ + if ! curl -fsSL --connect-timeout 10 --max-time 30 https://hermes-agent.nousresearch.com/install.sh -o "$$tmp"; then \ echo "failed to fetch live Hermes installer" >&2; exit 1; \ fi; \ live=$$($$hasher "$$tmp" | awk '{print $$1}'); \ diff --git a/box.env b/box.env index a36d9f4..fa455a5 100644 --- a/box.env +++ b/box.env @@ -19,7 +19,9 @@ NODE_MAJOR="22" # NodeSource major line; claude-code, codex, executor r # default) keeps the old latest-on-install behavior. CLAUDE_CODE_VERSION="" # e.g. "2.1.197" — see `npm view @anthropic-ai/claude-code versions` CODEX_VERSION="" # e.g. "0.142.4" — see `npm view @openai/codex versions` -EXECUTOR_VERSION="" # e.g. "1.5.26" — see `npm view executor versions` +EXECUTOR_VERSION="" # e.g. "1.5.26" — the github.com/RhysSullivan/executor npm + # package; "executor" is a generic name, so double-check + # `npm view executor repository.url` if pinning a version # --- Hermes Agent (Nous Research) ------------------------------------------- # Installed headless via the official script. Code -> /usr/local/lib (root-mode diff --git a/tests/cli-surface.sh b/tests/cli-surface.sh index c602e06..bf72e60 100755 --- a/tests/cli-surface.sh +++ b/tests/cli-surface.sh @@ -53,19 +53,25 @@ grep -q -- '--name force-box -f' "$rm_sentinel" # interactive shell). setsid genuinely detaches the terminal, so this # asserts the safety property that actually matters regardless of which # of the two failure modes fires: no confirmation reached, no deletion. -rm_noforce_repo="$tmp/rm-noforce-repo" -make_repo "$rm_noforce_repo" -mkdir -p "$tmp/rm-noforce-state/no-force-box" -printf 'no-force-box\t\t\t\n' >"$rm_noforce_repo/.boxes" -noforce_sentinel="$tmp/rm-noforce.calls" -if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$noforce_sentinel" \ - SMOLVM_STATE_DIR="$tmp/rm-noforce-state" \ - timeout 10 setsid "$rm_noforce_repo/box" rm no-force-box /dev/null 2>&1; then - echo "box rm without --force unexpectedly succeeded with no controlling terminal" >&2 - exit 1 +# setsid/timeout are util-linux, not on stock macOS — skip rather than fail +# the whole suite for a host the project otherwise supports. +if ! command -v setsid >/dev/null 2>&1 || ! command -v timeout >/dev/null 2>&1; then + echo "setsid/timeout unavailable; skipping the no-controlling-terminal box rm regression" >&2 +else + rm_noforce_repo="$tmp/rm-noforce-repo" + make_repo "$rm_noforce_repo" + mkdir -p "$tmp/rm-noforce-state/no-force-box" + printf 'no-force-box\t\t\t\n' >"$rm_noforce_repo/.boxes" + noforce_sentinel="$tmp/rm-noforce.calls" + if PATH="$PROJECT_ROOT/tests/fixtures:$PATH" SMOLVM_CALLED="$noforce_sentinel" \ + SMOLVM_STATE_DIR="$tmp/rm-noforce-state" \ + timeout 10 setsid "$rm_noforce_repo/box" rm no-force-box /dev/null 2>&1; then + echo "box rm without --force unexpectedly succeeded with no controlling terminal" >&2 + exit 1 + fi + [[ ! -e "$noforce_sentinel" ]] || { echo "smolvm ran before rm confirmation" >&2; exit 1; } + grep -q '^no-force-box' "$rm_noforce_repo/.boxes" fi -[[ ! -e "$noforce_sentinel" ]] || { echo "smolvm ran before rm confirmation" >&2; exit 1; } -grep -q '^no-force-box' "$rm_noforce_repo/.boxes" # --- box rm : typed-name confirmation, both mismatch and match ---- # Exercises the real /dev/tty prompt path via a pty, since `--force` and the diff --git a/tests/hermes-state.sh b/tests/hermes-state.sh index adeeead..22cc78e 100755 --- a/tests/hermes-state.sh +++ b/tests/hermes-state.sh @@ -354,24 +354,26 @@ spec = importlib.util.spec_from_loader(loader.name, loader) module = importlib.util.module_from_spec(spec) loader.exec_module(module) -d = Path(tempfile.mkdtemp()) -zpath = d / "t.zip" -with zipfile.ZipFile(zpath, "w") as zf: - zf.writestr("config.yaml", "a: 1\n") - zf.writestr("state.db", b"x" * 5000) +with tempfile.TemporaryDirectory() as d, \ + tempfile.TemporaryDirectory() as stage_dir, \ + tempfile.TemporaryDirectory() as external_stage_dir: + zpath = Path(d) / "t.zip" + with zipfile.ZipFile(zpath, "w") as zf: + zf.writestr("config.yaml", "a: 1\n") + zf.writestr("state.db", b"x" * 5000) -with zipfile.ZipFile(zpath) as zf: - prefix, files = module._zip_layout(zf) - stage = Path(tempfile.mkdtemp()) - external_stage = Path(tempfile.mkdtemp()) - module.MAX_UNCOMPRESSED = 2000 # below the real 5005 decompressed bytes - try: - module._extract(zf, stage, external_stage, prefix, files, set()) - except ValueError as error: - if "100 GiB safety limit" not in str(error): - raise SystemExit(f"unexpected extraction error: {error}") - else: - raise SystemExit("extraction exceeding the actual-bytes cap unexpectedly succeeded") + with zipfile.ZipFile(zpath) as zf: + prefix, files = module._zip_layout(zf) + stage = Path(stage_dir) + external_stage = Path(external_stage_dir) + module.MAX_UNCOMPRESSED = 2000 # below the real 5005 decompressed bytes + try: + module._extract(zf, stage, external_stage, prefix, files, set()) + except ValueError as error: + if "100 GiB safety limit" not in str(error): + raise SystemExit(f"unexpected extraction error: {error}") + else: + raise SystemExit("extraction exceeding the actual-bytes cap unexpectedly succeeded") PY echo "Hermes state checks passed" From df2b2c5dd07a5e69bd970b1cde8244dd341adbf1 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 15:49:06 -0700 Subject: [PATCH 24/28] fix(review): close cmd_load's failure-evidence gaps, harden transcript saves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three real gaps in the P0-3 failure-handling work, caught by CodeRabbit and Greptile: - The transcript-save block (mkdir backups/.failed + cp) ran under set -e with no failure guard. A full disk or read-only checkout there would abort the handler *before* keep-on-failure's _clear_created ran — so the one thing an operator needs most (the box preserved for inspection) would silently not happen, in exactly the failure conditions where it matters most. Extracted _save_failed_transcript(), which never lets a logging failure propagate past a warning. - cmd_load's second failure path (the long _pause_box/restore/gateway- disable/verify-state/resume/prepare-runtime chain) never teed _provision_into "$n" assets's output and never saved any transcript before rollback, unlike cmd_new and cmd_load's first failure path. Now passes log_file through and calls _save_failed_transcript there too. - cmd_load only read BOX_KEEP_ON_FAILURE, not the --keep-on-failure flag cmd_new accepts — load [name] had a free third positional slot for it, so there was no structural reason for the asymmetry. Added it, and fixed the keep-on-failure message in the second failure path (it previously said "attempting cleanup" even when keep-on-failure had just left the box in place specifically to skip cleanup). Co-Authored-By: Claude Sonnet 5 --- box | 56 ++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/box b/box index 257f8a8..1ff1a18 100755 --- a/box +++ b/box @@ -611,6 +611,25 @@ _prepare_runtime() { _guest_hb "$n" doctor } +# Best-effort: a failure saving the transcript (full disk, read-only checkout) +# must never itself abort the caller's failure-handling path under set -e — +# that would skip keep-on-failure's _clear_created and let the EXIT trap roll +# the box back anyway, defeating the one thing the operator needed most. +_save_failed_transcript() { + local n="$1" log_file="$2" failed_log + [[ -s "$log_file" ]] || return 0 + if ! mkdir -p backups/.failed; then + warn "could not create backups/.failed; provisioning transcript was not saved" + return 0 + fi + failed_log="backups/.failed/${n}-$(date -u +%Y%m%dT%H%M%SZ).log" + if cp "$log_file" "$failed_log"; then + warn "provisioning transcript saved to $failed_log" + else + warn "could not save provisioning transcript to $failed_log" + fi +} + _rollback_created() { local n="$1" registry_lock="$LOCK_ROOT/registry.lock" owns_lock=0 warn "rolling back incomplete box '$n'" @@ -644,7 +663,7 @@ _rollback_created() { } cmd_new() { - local n="${1:?usage: box new [--keep-on-failure]}" keep_on_failure=0 stage="" log_file failed_log="" + local n="${1:?usage: box new [--keep-on-failure]}" keep_on_failure=0 stage="" log_file [[ "${2:-}" != --keep-on-failure && "${BOX_KEEP_ON_FAILURE:-0}" != 1 ]] || keep_on_failure=1 _validate_name "$n" _host_preflight provision @@ -660,12 +679,7 @@ cmd_new() { stage="preparing the runtime (gateway start, MCP wiring, doctor)" fi if [[ -n "$stage" ]]; then - if [[ -s "$log_file" ]]; then - mkdir -p backups/.failed - failed_log="backups/.failed/${n}-$(date -u +%Y%m%dT%H%M%SZ).log" - cp "$log_file" "$failed_log" - warn "provisioning transcript saved to $failed_log" - fi + _save_failed_transcript "$n" "$log_file" if [[ "$keep_on_failure" == 1 ]]; then _clear_created "$n" warn "leaving '$n' in place for inspection (--keep-on-failure / BOX_KEEP_ON_FAILURE=1)" @@ -738,9 +752,9 @@ cmd_save() { } cmd_load() { - local file="${1:?usage: box load [name]}" n="${2:-restored}" raw guest_tmp - local keep_on_failure=0 log_file failed_log="" - [[ "${BOX_KEEP_ON_FAILURE:-0}" != 1 ]] || keep_on_failure=1 + local file="${1:?usage: box load [name] [--keep-on-failure]}" n="${2:-restored}" raw guest_tmp + local keep_on_failure=0 log_file + [[ "${3:-}" != --keep-on-failure && "${BOX_KEEP_ON_FAILURE:-0}" != 1 ]] || keep_on_failure=1 [[ -f "$file" ]] || die "no such archive: $file" _validate_name "$n" _host_preflight full @@ -755,15 +769,10 @@ cmd_load() { log_file="$(mktemp)" _track_temp "$log_file" if ! _create_base "$n" || ! _provision_into "$n" full "$log_file"; then - if [[ -s "$log_file" ]]; then - mkdir -p backups/.failed - failed_log="backups/.failed/${n}-$(date -u +%Y%m%dT%H%M%SZ).log" - cp "$log_file" "$failed_log" - warn "provisioning transcript saved to $failed_log" - fi + _save_failed_transcript "$n" "$log_file" if [[ "$keep_on_failure" == 1 ]]; then _clear_created "$n" - warn "leaving '$n' in place for inspection (BOX_KEEP_ON_FAILURE=1)" + warn "leaving '$n' in place for inspection (--keep-on-failure / BOX_KEEP_ON_FAILURE=1)" warn "remove when done with: ./box rm '$n' --force" fi die "destination provisioning failed" @@ -812,12 +821,14 @@ cmd_load() { trap '\''if [[ "$restore_failed" == 1 ]]; then find /data -mindepth 1 -maxdepth 1 ! -path "$rollback" -exec rm -rf -- {} +; mv "$rollback"/* /data/ 2>/dev/null || true; fi; rm -rf "$stage" "$rollback"; rm -f -- "$archive"'\'' EXIT HUP INT TERM mv "$stage"/* /data/ restore_failed=0' bash "$guest_tmp" || - ! _provision_into "$n" assets || ! _guest_hb "$n" gateway-disable || + ! _provision_into "$n" assets "$log_file" || ! _guest_hb "$n" gateway-disable || ! _guest_hb "$n" verify-state >/dev/null || ! _resume_box "$n" || ! _prepare_runtime "$n"; then + _save_failed_transcript "$n" "$log_file" if [[ "$keep_on_failure" == 1 ]]; then _clear_created "$n" - warn "leaving '$n' in place for inspection (BOX_KEEP_ON_FAILURE=1)" + warn "leaving '$n' in place for inspection (--keep-on-failure / BOX_KEEP_ON_FAILURE=1)" warn "remove when done with: ./box rm '$n' --force" + die "restore failed; left incomplete destination '$n' for inspection" fi die "restore failed; attempting cleanup of the incomplete destination" fi @@ -1113,15 +1124,16 @@ hermes-box-lite — isolated, portable agent boxes (claude / codex / hermes / ex ./box repair Re-run provisioning and repair managed guest assets ./box stop|start Stop or start an instance ./box save [out] Quiesce, validate, encrypt, and verify /data - ./box load [name] Validate first, then build and restore a fresh box + ./box load [name] [--keep-on-failure] + Validate first, then build and restore a fresh box ./box import-hermes [--external NAME]... [--map OLD=NEW]... Transactionally import native Hermes state; gateway stays disabled ./box extract [dir] Validate and extract a backup without a VM ./box rm [--force] Delete after confirmation (or explicit --force) Set BOX_PASSPHRASE to skip the GPG prompt. Configuration: box.env - Set BOX_KEEP_ON_FAILURE=1 (or pass --keep-on-failure to `new`) to skip - rollback on a failed `new`/`load` and leave the VM up for inspection. + Set BOX_KEEP_ON_FAILURE=1 (or pass --keep-on-failure to `new`/`load`) to + skip rollback on a failed `new`/`load` and leave the VM up for inspection. A failed `new`/`load` also saves its provisioning transcript under backups/.failed/ before any rollback runs. EOF From 04cc316f2726ff5071027580b4022bfda5c5a773 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 15:50:57 -0700 Subject: [PATCH 25/28] fix(review): install_executor's idempotency guard now checks version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flagged independently by Macroscope and a local review pass: unlike install_hermes's idempotency guard (which checks both presence and the pinned git SHA), install_executor only checked `command -v executor` — once EXECUTOR_VERSION is pinned and bumped, `box repair` (assets mode) would log "executor already installed — skipping reinstall" and silently leave the box on the old version forever, exactly the kind of drift P1-4 was meant to close. Now compares the installed executor's actual version (parsed from `executor --version`) against EXECUTOR_VERSION when one is pinned, and only skips reinstall on a real match. Unpinned (empty EXECUTOR_VERSION) keeps the original presence-only/latest-on-install behavior. Co-Authored-By: Claude Sonnet 5 --- provision/provision.sh | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/provision/provision.sh b/provision/provision.sh index 7709c89..a6a54d8 100755 --- a/provision/provision.sh +++ b/provision/provision.sh @@ -196,11 +196,21 @@ install_executor() { if [[ "${INSTALL_EXECUTOR:-0}" != "1" ]]; then log "executor: INSTALL_EXECUTOR != 1 — skipping"; return fi - # Idempotent: skip reinstall if already on PATH. Lets `assets` repair mode - # call this safely on every run instead of only on a full reinstall. + # Idempotent: skip reinstall if already on PATH at the pinned version (any + # version, if EXECUTOR_VERSION is unset) — the same bar verify_required + # holds it to. Lets `assets` repair mode call this safely on every run + # instead of only on a full reinstall, without masking a version bump. if command -v executor >/dev/null 2>&1; then - log "executor already installed — skipping reinstall" - return 0 + if [[ -z "${EXECUTOR_VERSION:-}" ]]; then + log "executor already installed — skipping reinstall" + return 0 + fi + local installed_version + installed_version="$(executor --version 2>/dev/null | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | head -1)" + if [[ "$installed_version" == "$EXECUTOR_VERSION" ]]; then + log "executor already installed at pinned $EXECUTOR_VERSION — skipping reinstall" + return 0 + fi fi local pkg="executor${EXECUTOR_VERSION:+@$EXECUTOR_VERSION}" log "executor (npm global)${EXECUTOR_VERSION:+ ($EXECUTOR_VERSION)}" From ee8e7d0413e47255ea758c1c7d248991f1128a87 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 15:51:55 -0700 Subject: [PATCH 26/28] docs: note that P1-1's original mkdir-based lock was superseded Cross-reference from the historical audit entry to the later flock-based replacement, so a reader following P1-1's fix steps doesn't implement the version with the confirmed TOCTOU bug. Co-Authored-By: Claude Sonnet 5 --- docs/audit-2026-06-30-fixes.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/audit-2026-06-30-fixes.md b/docs/audit-2026-06-30-fixes.md index a406339..a7ca32f 100644 --- a/docs/audit-2026-06-30-fixes.md +++ b/docs/audit-2026-06-30-fixes.md @@ -252,6 +252,18 @@ local copy of `hb` to make the window reliably wide.) before it calls `hb reconcile`, or simply rely on (1) since `hb reconcile` already calls into `_start_gateway`. +**Superseded during code review:** the `mkdir`-based lock above shipped first +and was verified live, but a later multi-angle review (local + bots) found a +real TOCTOU window in its stale-lock reclaim — two contenders racing to +reclaim the same dead-owner lock could both end up "holding" it, confirmed +empirically (5-8 of 10 racing processes "winning" in repeated trials). It was +replaced with an `flock`-based lock (kernel-enforced, no manual dead-owner +detection needed), generalized to also cover Executor's identical unlocked +race that this entry didn't originally catch. See the +`fix(review): replace guest/hb's daemon lock with flock` commit for the full +writeup, including a second bug (lock-fd inheritance into spawned daemons) +found and fixed while building the replacement. + ### P1-2: `hb-workload` itself is unsupervised — DONE **File:** `guest/hb-workload:98-105` (`main`), invoked once at boot by From b5583a797c8796aff0333bc42e79a8bf5c7db510 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 22:12:10 -0700 Subject: [PATCH 27/28] refactor: dedupe MCP handshake between box and guest/hb, note deferred Go rewrite The initialize/validate/delete-session JSON-RPC sequence was byte-for-byte duplicated between box's _mcp_initialize and guest/hb's _mcp_auth_works (~55 lines). Extract it into guest/lib-mcp.sh, sourced by both, so the security-sensitive handshake logic can't drift between host and guest checks. Also record why a Go rewrite was evaluated and deferred (docs/future-go-port.md), so the reasoning doesn't need to be re-derived if it comes up again. --- Makefile | 2 +- box | 65 ++------------------------------------- docs/future-go-port.md | 48 +++++++++++++++++++++++++++++ guest/hb | 62 +++---------------------------------- guest/lib-mcp.sh | 70 ++++++++++++++++++++++++++++++++++++++++++ provision/provision.sh | 1 + tests/static.sh | 6 ++-- 7 files changed, 130 insertions(+), 124 deletions(-) create mode 100644 docs/future-go-port.md create mode 100644 guest/lib-mcp.sh diff --git a/Makefile b/Makefile index b5b7a70..fe07d11 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ # tests/regressions-*.sh is globbed so new split-out regression files don't # require touching this Makefile. -SHELL_FILES := box guest/hb guest/hb-workload guest/profile.sh guest/agent-bash-profile.sh provision/provision.sh ops/tx9-host ops/tx9-backup-prune tests/lib.sh tests/static.sh tests/lifecycle-smoke.sh tests/hermes-state.sh tests/cli-surface.sh $(wildcard tests/regressions-*.sh) tests/fixtures/smolvm +SHELL_FILES := box guest/hb guest/hb-workload guest/lib-mcp.sh guest/profile.sh guest/agent-bash-profile.sh provision/provision.sh ops/tx9-host ops/tx9-backup-prune tests/lib.sh tests/static.sh tests/lifecycle-smoke.sh tests/hermes-state.sh tests/cli-surface.sh $(wildcard tests/regressions-*.sh) tests/fixtures/smolvm syntax: bash -n $(SHELL_FILES) diff --git a/box b/box index 1ff1a18..1b3bfb1 100755 --- a/box +++ b/box @@ -7,6 +7,8 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" cd "$ROOT" # shellcheck source=box.env source "$ROOT/box.env" +# shellcheck source=guest/lib-mcp.sh +source "$ROOT/guest/lib-mcp.sh" REG="$ROOT/.boxes" LOCK_ROOT="$ROOT/.box-locks" @@ -217,69 +219,6 @@ _port_free() { return 0 } -_mcp_initialize() { - local url="$1" token="$2" headers body code session content_type initialized_code initialized_ok=0 - headers="$(mktemp)" || return 1 - body="$(mktemp)" || { rm -f -- "$headers"; return 1; } - _track_temp "$headers" - _track_temp "$body" - if ! code="$(curl -sS --connect-timeout 2 --max-time 5 -D "$headers" -o "$body" -w '%{http_code}' \ - -X POST \ - -H "Authorization: Bearer $token" \ - -H 'Content-Type: application/json' \ - -H 'Accept: application/json,text/event-stream' \ - --data '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-03-26","capabilities":{},"clientInfo":{"name":"hermes-box-doctor","version":"1"}}}' \ - "$url")"; then - rm -f -- "$headers" "$body" - return 1 - fi - session="$(awk 'tolower($1) == "mcp-session-id:" { sub(/^[^:]*:[[:space:]]*/, ""); sub(/\r$/, ""); print; exit }' "$headers")" - content_type="$(awk 'tolower($1) == "content-type:" { sub(/^[^:]*:[[:space:]]*/, ""); sub(/\r$/, ""); print tolower($0); exit }' "$headers")" - if [[ "$code" != 200 || -z "$session" ]] || ! _mcp_response_valid "$body" "$content_type"; then - rm -f -- "$headers" "$body" - [[ -n "$session" ]] && _mcp_delete_session "$url" "$token" "$session" - return 1 - fi - if initialized_code="$(curl -sS --connect-timeout 2 --max-time 5 -o /dev/null -w '%{http_code}' \ - -X POST \ - -H "Authorization: Bearer $token" \ - -H 'Content-Type: application/json' \ - -H 'Accept: application/json,text/event-stream' \ - -H "Mcp-Session-Id: $session" \ - --data '{"jsonrpc":"2.0","method":"notifications/initialized"}' "$url")" && - [[ "$initialized_code" == 202 ]]; then - initialized_ok=1 - fi - rm -f -- "$headers" "$body" - _mcp_delete_session "$url" "$token" "$session" - [[ "$initialized_ok" == 1 ]] -} - -_mcp_delete_session() { - local url="$1" token="$2" session="$3" - curl -sS --connect-timeout 2 --max-time 5 -o /dev/null -X DELETE \ - -H "Authorization: Bearer $token" \ - -H 'Accept: application/json,text/event-stream' \ - -H "Mcp-Session-Id: $session" "$url" >/dev/null 2>&1 || true -} - -_mcp_response_valid() { - local body="$1" content_type="$2" filter - filter='type == "object" and .jsonrpc == "2.0" and .id == 1 and - has("result") and (has("error") | not) and - (.result | type == "object") and - .result.protocolVersion == "2025-03-26" and - (.result.serverInfo | type == "object") and - (.result.serverInfo.name | type == "string" and length > 0) and - (.result.serverInfo.version | type == "string") and - (.result.capabilities | type == "object")' - case "$content_type" in - application/json*) jq -e "$filter" "$body" >/dev/null 2>&1 ;; - text/event-stream*) sed -n 's/^data:[[:space:]]*//p' "$body" | jq -e "$filter" >/dev/null 2>&1 ;; - *) return 1 ;; - esac -} - _wait_mcp_ready() { local url="$1" token="$2" attempt for ((attempt = 0; attempt < 30; attempt++)); do diff --git a/docs/future-go-port.md b/docs/future-go-port.md new file mode 100644 index 0000000..51def66 --- /dev/null +++ b/docs/future-go-port.md @@ -0,0 +1,48 @@ +# Note: considered and deferred a Go rewrite (2026-06-30) + +While auditing the bash scripts on `audit/2026-06-30-box-new-fixes`, we +evaluated whether to rewrite `./box`, `guest/hb`, `guest/hermes-state`, and +`provision/provision.sh` in Go for easier testing and maintenance. Conclusion: +**not now, and not the guest-side scripts.** Recorded here so the reasoning +doesn't have to be re-derived next time this comes up. + +## Recommendation if this is revisited + +A **partial rewrite of `./box` only** (the host-side CLI) is the one variant +worth considering later. `guest/hb`, `guest/hermes-state`, and +`provision/provision.sh` should stay in bash/Python. + +## Why not the guest-side scripts + +- `guest/hb` and `guest/hermes-state` run *inside* the guest VM; + `provision/provision.sh` runs during early guest bootstrap, before most + tooling exists. Porting them means shipping a Go toolchain (or + cross-compiled binaries) into the guest image and solving a chicken-and-egg + bootstrap problem for provisioning itself. +- Their tests are inherently integration-style regardless of language (they + exercise real state transitions against a real or faked guest); Go's type + system wouldn't meaningfully change that. + +## Why `./box` is at least plausible + +- It runs on a normal host OS (macOS/Linux) with a normal toolchain — trivial + Go target, no cross-compilation or bundling problem. +- A large fraction of its size is shelling out to `smolvm`/`gpg`/`curl`/`jq`/ + `tar` — Go doesn't remove that shell-out surface, but it would give the + surrounding logic (arg parsing, port/lock state, error propagation) real + types and table-driven unit tests instead of bash integration tests. + +## Why we're not doing it now + +- This branch just finished a security/reliability audit that fixed ~20 real + bugs in this exact code (see `docs/audit-2026-06-30-fixes.md`), several + around locking and races. A rewrite reintroduces that entire risk class in + new code before it's had time to prove itself in production. +- Rough cost: a `box`-only port is ~1 month to reach the confidence level the + current bash already has; a full rewrite (including guest scripts) is + 3-4 months once retesting and migration are accounted for, for + comparatively little extra benefit. + +If a Go rewrite of `./box` is picked up later, treat it as its own dedicated +project with its own review cycle — not something to fold into an unrelated +feature or fix branch. diff --git a/guest/hb b/guest/hb index fe7e8d8..480e914 100755 --- a/guest/hb +++ b/guest/hb @@ -3,6 +3,8 @@ set -uo pipefail # shellcheck disable=SC1091 [ -f /etc/profile.d/hermes-box.sh ] && . /etc/profile.d/hermes-box.sh +# shellcheck source=guest/lib-mcp.sh +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib-mcp.sh" DATA="${HB_DATA:-/data}" AGENT_HOME="$DATA/home/agent" @@ -408,65 +410,9 @@ _claude_http_config() { } _mcp_auth_works() { - local token headers body code session content_type initialized_code initialized_ok=0 url="http://127.0.0.1:${EXECUTOR_PORT:-4788}/mcp" + local token token="$(executor_token)" || return 1 - headers="$(mktemp)" || return 1 - body="$(mktemp)" || { rm -f -- "$headers"; return 1; } - if ! code="$(curl -sS --connect-timeout 2 --max-time 5 -D "$headers" -o "$body" -w '%{http_code}' \ - -X POST \ - -H "Authorization: Bearer $token" \ - -H 'Content-Type: application/json' \ - -H 'Accept: application/json,text/event-stream' \ - --data '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-03-26","capabilities":{},"clientInfo":{"name":"hermes-box-doctor","version":"1"}}}' \ - "$url")"; then - rm -f -- "$headers" "$body" - return 1 - fi - session="$(awk 'tolower($1) == "mcp-session-id:" { sub(/^[^:]*:[[:space:]]*/, ""); sub(/\r$/, ""); print; exit }' "$headers")" - content_type="$(awk 'tolower($1) == "content-type:" { sub(/^[^:]*:[[:space:]]*/, ""); sub(/\r$/, ""); print tolower($0); exit }' "$headers")" - if [[ "$code" != 200 || -z "$session" ]] || ! _mcp_response_valid "$body" "$content_type"; then - rm -f -- "$headers" "$body" - [[ -n "$session" ]] && _mcp_delete_session "$url" "$token" "$session" - return 1 - fi - if initialized_code="$(curl -sS --connect-timeout 2 --max-time 5 -o /dev/null -w '%{http_code}' \ - -X POST \ - -H "Authorization: Bearer $token" \ - -H 'Content-Type: application/json' \ - -H 'Accept: application/json,text/event-stream' \ - -H "Mcp-Session-Id: $session" \ - --data '{"jsonrpc":"2.0","method":"notifications/initialized"}' "$url")" && - [[ "$initialized_code" == 202 ]]; then - initialized_ok=1 - fi - rm -f -- "$headers" "$body" - _mcp_delete_session "$url" "$token" "$session" - [[ "$initialized_ok" == 1 ]] -} - -_mcp_delete_session() { - local url="$1" token="$2" session="$3" - curl -sS --connect-timeout 2 --max-time 5 -o /dev/null -X DELETE \ - -H "Authorization: Bearer $token" \ - -H 'Accept: application/json,text/event-stream' \ - -H "Mcp-Session-Id: $session" "$url" >/dev/null 2>&1 || true -} - -_mcp_response_valid() { - local body="$1" content_type="$2" filter - filter='type == "object" and .jsonrpc == "2.0" and .id == 1 and - has("result") and (has("error") | not) and - (.result | type == "object") and - .result.protocolVersion == "2025-03-26" and - (.result.serverInfo | type == "object") and - (.result.serverInfo.name | type == "string" and length > 0) and - (.result.serverInfo.version | type == "string") and - (.result.capabilities | type == "object")' - case "$content_type" in - application/json*) jq -e "$filter" "$body" >/dev/null 2>&1 ;; - text/event-stream*) sed -n 's/^data:[[:space:]]*//p' "$body" | jq -e "$filter" >/dev/null 2>&1 ;; - *) return 1 ;; - esac + _mcp_initialize "http://127.0.0.1:${EXECUTOR_PORT:-4788}/mcp" "$token" } doctor() { diff --git a/guest/lib-mcp.sh b/guest/lib-mcp.sh new file mode 100644 index 0000000..674749d --- /dev/null +++ b/guest/lib-mcp.sh @@ -0,0 +1,70 @@ +# MCP initialize/validate/delete-session handshake, sourced by both ./box +# (host) and guest/hb (guest) so their health checks can't drift apart. +# +# Callers that track temp files for signal-safe cleanup (currently only +# ./box) must define _track_temp before sourcing this file; other callers +# don't need it since every path below already rm -f's its temp files. +declare -f _track_temp >/dev/null 2>&1 || _track_temp() { :; } + +_mcp_delete_session() { + local url="$1" token="$2" session="$3" + curl -sS --connect-timeout 2 --max-time 5 -o /dev/null -X DELETE \ + -H "Authorization: Bearer $token" \ + -H 'Accept: application/json,text/event-stream' \ + -H "Mcp-Session-Id: $session" "$url" >/dev/null 2>&1 || true +} + +_mcp_response_valid() { + local body="$1" content_type="$2" filter + filter='type == "object" and .jsonrpc == "2.0" and .id == 1 and + has("result") and (has("error") | not) and + (.result | type == "object") and + .result.protocolVersion == "2025-03-26" and + (.result.serverInfo | type == "object") and + (.result.serverInfo.name | type == "string" and length > 0) and + (.result.serverInfo.version | type == "string") and + (.result.capabilities | type == "object")' + case "$content_type" in + application/json*) jq -e "$filter" "$body" >/dev/null 2>&1 ;; + text/event-stream*) sed -n 's/^data:[[:space:]]*//p' "$body" | jq -e "$filter" >/dev/null 2>&1 ;; + *) return 1 ;; + esac +} + +_mcp_initialize() { + local url="$1" token="$2" headers body code session content_type initialized_code initialized_ok=0 + headers="$(mktemp)" || return 1 + body="$(mktemp)" || { rm -f -- "$headers"; return 1; } + _track_temp "$headers" + _track_temp "$body" + if ! code="$(curl -sS --connect-timeout 2 --max-time 5 -D "$headers" -o "$body" -w '%{http_code}' \ + -X POST \ + -H "Authorization: Bearer $token" \ + -H 'Content-Type: application/json' \ + -H 'Accept: application/json,text/event-stream' \ + --data '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-03-26","capabilities":{},"clientInfo":{"name":"hermes-box-doctor","version":"1"}}}' \ + "$url")"; then + rm -f -- "$headers" "$body" + return 1 + fi + session="$(awk 'tolower($1) == "mcp-session-id:" { sub(/^[^:]*:[[:space:]]*/, ""); sub(/\r$/, ""); print; exit }' "$headers")" + content_type="$(awk 'tolower($1) == "content-type:" { sub(/^[^:]*:[[:space:]]*/, ""); sub(/\r$/, ""); print tolower($0); exit }' "$headers")" + if [[ "$code" != 200 || -z "$session" ]] || ! _mcp_response_valid "$body" "$content_type"; then + rm -f -- "$headers" "$body" + [[ -n "$session" ]] && _mcp_delete_session "$url" "$token" "$session" + return 1 + fi + if initialized_code="$(curl -sS --connect-timeout 2 --max-time 5 -o /dev/null -w '%{http_code}' \ + -X POST \ + -H "Authorization: Bearer $token" \ + -H 'Content-Type: application/json' \ + -H 'Accept: application/json,text/event-stream' \ + -H "Mcp-Session-Id: $session" \ + --data '{"jsonrpc":"2.0","method":"notifications/initialized"}' "$url")" && + [[ "$initialized_code" == 202 ]]; then + initialized_ok=1 + fi + rm -f -- "$headers" "$body" + _mcp_delete_session "$url" "$token" "$session" + [[ "$initialized_ok" == 1 ]] +} diff --git a/provision/provision.sh b/provision/provision.sh index a6a54d8..b00a0c3 100755 --- a/provision/provision.sh +++ b/provision/provision.sh @@ -232,6 +232,7 @@ place_assets() { log "profile, tmux config, hb helper" install -m 0644 "$CTX/guest/profile.sh" /etc/profile.d/hermes-box.sh install -m 0644 "$CTX/guest/tmux.conf" "$OPT/tmux.conf" + install -m 0644 "$CTX/guest/lib-mcp.sh" "$OPT/bin/lib-mcp.sh" install -m 0755 "$CTX/guest/hb" "$OPT/bin/hb" install -m 0755 "$CTX/guest/hb-workload" "$OPT/bin/hb-workload" install -m 0755 "$CTX/guest/hermes-state" "$OPT/bin/hermes-state" diff --git a/tests/static.sh b/tests/static.sh index 75d1708..817f45f 100755 --- a/tests/static.sh +++ b/tests/static.sh @@ -13,6 +13,7 @@ files=( box guest/hb guest/hb-workload + guest/lib-mcp.sh guest/profile.sh guest/agent-bash-profile.sh provision/provision.sh @@ -50,8 +51,9 @@ if grep -q '\blsof\b' box; then echo "lsof dependency remains" >&2 exit 1 fi -grep -q 'protocolVersion.*2025-03-26' box -grep -q 'protocolVersion.*2025-03-26' guest/hb +grep -q 'protocolVersion.*2025-03-26' guest/lib-mcp.sh +grep -q 'source .*guest/lib-mcp\.sh' box +grep -q 'lib-mcp\.sh' guest/hb grep -q 'ca-certificates curl git jq tmux' provision/provision.sh grep -q 'runuser -u agent.*hb.*write-manifest' provision/provision.sh grep -q -- '--commit.*sha' provision/provision.sh From f564a22f289b2039d2b83a93b016ed4fef50e476 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Tue, 30 Jun 2026 22:27:17 -0700 Subject: [PATCH 28/28] fix(review): cmd_load reports which stage failed, matching cmd_new CodeRabbit flagged that cmd_load collapsed base-VM creation and provisioning into one generic "destination provisioning failed" message, unlike cmd_new (which already reports the specific failing stage). Apply the same stage pattern here. --- box | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/box b/box index 1b3bfb1..a8b408a 100755 --- a/box +++ b/box @@ -692,7 +692,7 @@ cmd_save() { cmd_load() { local file="${1:?usage: box load [name] [--keep-on-failure]}" n="${2:-restored}" raw guest_tmp - local keep_on_failure=0 log_file + local keep_on_failure=0 log_file stage="" [[ "${3:-}" != --keep-on-failure && "${BOX_KEEP_ON_FAILURE:-0}" != 1 ]] || keep_on_failure=1 [[ -f "$file" ]] || die "no such archive: $file" _validate_name "$n" @@ -707,14 +707,19 @@ cmd_load() { say "Creating and provisioning fresh instance '$n'" log_file="$(mktemp)" _track_temp "$log_file" - if ! _create_base "$n" || ! _provision_into "$n" full "$log_file"; then + if ! _create_base "$n"; then + stage="creating the base VM" + elif ! _provision_into "$n" full "$log_file"; then + stage="provisioning" + fi + if [[ -n "$stage" ]]; then _save_failed_transcript "$n" "$log_file" if [[ "$keep_on_failure" == 1 ]]; then _clear_created "$n" warn "leaving '$n' in place for inspection (--keep-on-failure / BOX_KEEP_ON_FAILURE=1)" warn "remove when done with: ./box rm '$n' --force" fi - die "destination provisioning failed" + die "box load failed while $stage" fi guest_tmp="/root/hb-restore-$(date +%Y%m%d-%H%M%S)-$$-${RANDOM}.tgz"