From 582917d12dda7dcc52fbca446f078156ff2976e7 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sun, 5 Jul 2026 14:45:58 +0200 Subject: [PATCH] =?UTF-8?q?docs(audit):=20SPIKE=20=E2=80=94=20agent=20self?= =?UTF-8?q?-update=20rollback=20mechanics=20(systemd=20257=20semantics=20o?= =?UTF-8?q?n=20felhom-pve)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GO with two design-reshaping surprises: OnFailure= fires on EVERY crash on systemd 257 (rollback triggers at first post-apply crash; marker-guard + idempotency mandatory), and deliberate restarts consume start-limit budget. S1 trap confirmed (default shape never reaches failed — 35 starts/180s); tuned [Unit] StartLimitIntervalSec=120 + Burst=4 → failed in ~20s; [Service] placement is an asymmetric half-applied trap; reset-failed required. S2: all restart variants reliable, detached systemd-run wins. S3: A/B flip atomic under kill -9, all verbs + guards pass. S4: StateDir + sudoers shapes OK. Dummy artifacts only; real agent uptime unchanged (verified). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6 --- .../SPIKE-agent-selfupdate-2026-07-05.md | 200 ++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 documentation/audits/SPIKE-agent-selfupdate-2026-07-05.md diff --git a/documentation/audits/SPIKE-agent-selfupdate-2026-07-05.md b/documentation/audits/SPIKE-agent-selfupdate-2026-07-05.md new file mode 100644 index 0000000..2f20aba --- /dev/null +++ b/documentation/audits/SPIKE-agent-selfupdate-2026-07-05.md @@ -0,0 +1,200 @@ +# SPIKE — agent self-update rollback mechanics (systemd semantics on felhom-pve) + +- **Date:** 2026-07-05 +- **Class:** Spike / empirical. Dummy artifacts only (`felhom-spike-selfupd.service`, + `felhom-spike-rollback.service`, `/usr/local/bin/felhom-spike-agent`, + `/usr/local/sbin/felhom-spike-guarded`, state in `/var/tmp/felhom-spike-selfupd/`) — the real + `felhom-agent` unit/binary/sudoers/StateDir were never modified, stopped, or restarted + (uptime proof in §Cleanup). No production code. Full teardown verified. +- **Host:** felhom-pve (`demo-felhom`, 192.168.0.162), root over SSH. +- **Context baseline:** felhom-agent `main` @ `72d7f05` (v0.69.0, running); unit shape mirrored: + `Type=simple`, `Restart=on-failure`, `RestartSec=5s`, no StartLimit overrides. +- **Verdict: GO** — every mechanism works, **but two systemd-257 behaviours reshape the design**: + (1) `OnFailure=` fires on **every crash**, not only on terminal `failed` (S1 surprise — makes + rollback *faster* but makes the marker-guard + idempotency non-negotiable); (2) **deliberate + restarts count against the start limit** (S2 surprise — the update flow itself can trip the + limit it installs). Findings-for-the-TASK list at the end. + +## S5 — environment facts (everything below is pinned to this stack) + +| Fact | Value | +|---|---| +| systemd | **257 (257.13-1~deb13u1)** | +| OS / PVE | Debian 13.5, pve-manager/9.2.2 (kernel 7.0.2-6-pve) | +| Compiled start-limit defaults | `DefaultStartLimitIntervalSec=10s`, `DefaultStartLimitBurst=5` (commented in system.conf → compiled defaults apply; confirmed live on the spike unit: `StartLimitIntervalUSec=10s / StartLimitBurst=5`) | +| `/usr/local/bin`, `/var/lib/felhom-agent`, `/var/tmp` | **all on `/dev/mapper/pve-root` ext4 — same fs ⇒ atomic `rename(2)` between staging and live is valid** | +| Tools | `sha256sum` `/usr/bin`, `systemd-run` present, `visudo` `/usr/sbin` | +| Real agent at spike start | v0.69.0, MainPID 1705667, ActiveEnterTimestamp `Sat 2026-07-04 21:07:38 CEST` | + +## S1 — start-limit → failed → OnFailure chain + +Dummy unit mirroring the real shape; dummy binary exits 1 while a `CRASH` marker exists, else +sleeps; `OnFailure=felhom-spike-rollback.service` (oneshot appending to `onfailure.log`). + +### S1a — current (default) shape: **hypothesis CONFIRMED + SURPRISE** + +`CRASH` set, unit started, watched 180s: + +``` +StartLimitIntervalUSec=10s StartLimitBurst=5 Restart=on-failure RestartUSec=5s +t=+15s NRestarts=2 ActiveState=activating SubState=auto-restart onfailure.log: 3 +t=+90s NRestarts=17 activating/auto-restart onfailure.log: 18 +t=+180s NRestarts=34 activating/auto-restart onfailure.log: 35 +35 start attempts in 180s; `systemctl is-failed` → "activating" the whole time +``` + +- **CONFIRMED (the suspected trap):** with `RestartSec=5s` and the default 10s/5 limit, restarts + are spaced too far apart to ever trip the limit → the unit **never enters `failed`** and + crash-loops **forever**. With the real agent's current unit shape, a start-limit-based terminal + state does not exist. +- **SURPRISE (design-reshaping):** on systemd 257 **`OnFailure=` fires on EVERY crash** — 35 + fires in 180s, one per exit — even while the unit sits in `activating (auto-restart)`, never + reaching `failed`. The classic mental model ("OnFailure fires once, when restarts are + exhausted") is **wrong on this systemd**. Journal wording per crash: + `Triggering OnFailure= dependencies`. + +**Verdict: NO-GO for the current unit shape as a rollback trigger** — but not for the reason +hypothesized: the trigger DOES fire (repeatedly); what's missing is a terminal state, and what's +mandatory is a rollback unit that tolerates being fired dozens of times. + +### S1b — tuned limits: bounded-time failure + the placement trap + +**Placement trap (version-specific, ASYMMETRIC):** in a drop-in under `[Service]`: + +``` +/etc/systemd/system/felhom-spike-selfupd.service.d/limits.conf:2: + Unknown key 'StartLimitIntervalSec' in section [Service], ignoring. +→ shows StartLimitIntervalUSec=10s (ignored!) StartLimitBurst=4 (ACCEPTED!) +``` + +`StartLimitBurst` is still honoured in `[Service]` (compat) while `StartLimitIntervalSec` is +silently-warned and ignored — a half-applied config. **Both keys MUST live in `[Unit]`.** +With `[Unit]` placement both apply: `StartLimitIntervalUSec=2min, StartLimitBurst=4`. + +**Crash burst with `[Unit]` `StartLimitIntervalSec=120` + `StartLimitBurst=4`:** + +``` +starts at t=0,5,10,15 (4 = burst) → 5th scheduled restart at t=+20s: + "Start request repeated too quickly." → "Failed with result 'exit-code'." +entered FAILED at t=+20s (final: ActiveState=failed NRestarts=4 Result=exit-code) +onfailure fired count: 5 (one per crash ×4 + one on the terminal failed entry) +``` + +- Terminal `failed` in **~20s** — well inside the ≤90s GO bound. +- Note `Result=exit-code` (not `start-limit-hit`) — match on `is-failed`/state, not on Result. +- The "exactly once" expectation is **refuted**: 5 fires. Harmless *iff* the rollback unit is + idempotent + marker-guarded (see findings). + +### S1c — recovery path + +- **`systemctl start` while `failed` (within the limit window) is REFUSED** (exit 1, unit stays + failed) — the start limit still gates manual/rollback starts. **`systemctl reset-failed + ` is REQUIRED** before the post-rollback start (or you wait out the interval). +- **Full chain demo** (rollback oneshot armed to: remove CRASH → `reset-failed` → `start`): + crash → OnFailure fired at the **first** crash (t≈0s, because of the per-crash firing) → + rollback ran → unit **running again at t=+5s** and stayed up. The production rollback will + typically fire and recover on the FIRST crash of a bad binary — tens of seconds *faster* than + the start-limit path, which remains the backstop. + +### S1d — false-trigger check + +OnFailure fired throughout S1a/S1b with **no pending marker in existence**, and (S2 surprise, +below) **also fires when a healthy binary trips the start limit through deliberate restarts**. +→ The production rollback unit's **no-op-without-pending-marker guard is MANDATORY**, and the +rollback action itself must be idempotent (it will be invoked many times per incident). + +**S1 GO** — tuned drop-in values for the real unit (in `[Unit]`, via drop-in): +`StartLimitIntervalSec=120` + `StartLimitBurst=4` → terminal `failed` ≈20s after the first +crash; rollback in practice triggers on the first OnFailure fire (~0–5s). + +## S2 — restarting your own unit from inside its cgroup + +Trigger files make the dummy binary invoke the restart on itself from within the unit's cgroup. + +**First run surprise:** rounds began failing after a few restarts — journal: +`Start request repeated too quickly` ×3. **Deliberate `systemctl restart`s count against +StartLimitBurst**: with 120s/4 armed, the update flow's own restarts (apply-restart + +health-checks + rollback-restart) can trip the limit on a perfectly healthy binary, blocking +further starts AND firing OnFailure. (With `reset-failed` between rounds → 100% reliable.) + +| Variant | Result | Caller survives the call? | +|---|---|---| +| **S2a** direct `systemctl restart` from inside the cgroup | **5/5** — restart completes, new MainPID, clean journal (Stopping→Stopped→Started, no canceled-job noise) | **No** — killed with the cgroup mid-call (its post-call echo never ran); systemd completes the job anyway | +| **S2b** detached: `systemd-run --on-active=2s --timer-property=AccuracySec=100ms systemctl restart …` | **5/5** | **Yes** — `systemd-run` returns 0, agent can log "restart scheduled"; transient timer+service self-clean (no leftovers) | +| **S2c** `systemctl restart --no-block` | **3/3** | No (killed once the restart lands) | + +**S2 GO — winner: S2b (detached transient timer).** All three variants are mechanically +reliable; S2b is preferred because the agent survives to log/report the handoff and the restart +is decoupled from sudo's process lifetime. S2a is an acceptable fallback (systemd finishes a +restart job even when the requester dies). +**Caveat for the TASK:** every deliberate restart consumes start-limit budget — the wrapper's +restart paths should `systemctl reset-failed` first (as the rollback already must), and the flow +should be designed to stay under Burst within the interval. + +## S3 — A/B flip + pending-marker wrapper mechanics + +Dummy wrapper verbs `apply ` / `commit` / `rollback` over +`/usr/local/bin/felhom-spike-agent` (+`.prev`) + `pending.json`; staging + live on the same fs. + +| Probe | Result | Evidence | +|---|---|---| +| **S3a companion — wrong sha** | **PASS (refused, nothing changed)** | `refusing apply: sha mismatch`, exit 1; live sha unchanged; no pending; **no `.prev` created** (the sha check runs before the `.prev` copy — keep this ordering) | +| **S3a — happy path** | **PASS** | apply → live=v2 sha exact, `.prev`=v1 sha exact, pending written; detached restart (S2b) → `v2 started` logged, unit running | +| **S3b — kill -9 mid-swap** | **PASS** | killed between `.prev` copy and rename: live binary still the complete old version (sha equal, parses); no pending; re-run `apply` converged to v3. *Nuance:* the kill landed before the staging-copy existed, so the orphaned-temp case wasn't exercised — a kill between staging-copy and `mv` would leave a harmless `.new.`; the production wrapper should sweep stale temps on entry | +| **S3c — rollback verb** | **PASS** | prev restored **byte-identical** (sha equal), pending cleared, `reset-failed` + restart → unit running the previous version | +| **S3d — commit** | **PASS** | pending cleared, `.prev` retained (manual safety net); second commit = clean no-op (exit 0) | +| **S3e — rollback with NO pending** | **PASS** | `no pending — refusing rollback (no-op)`, exit 0, binary byte-identical, **unit not restarted** — exactly the guard shape the OnFailure-triggered unit needs | + +**S3 GO** — verb semantics as probed survive; atomic same-fs rename guarantee held under kill -9. + +## S4 — permissions + sudoers syntax + +- **S4a PASS:** `sudo -u felhom-agent` mkdir/write/read/remove under + `/var/lib/felhom-agent/selfupdate/` all OK (StateDir 0750 agent-owned; the subdir was removed + after the probe — the only touch near real agent state, agent-owned data only). +- **S4b PASS (both shapes parse):** `visudo -cf` accepts both + `…felhom-selfupdate-guarded apply /var/lib/felhom-agent/selfupdate/* [a-f0-9]*` and the coarse + `… /var/lib/felhom-agent/selfupdate/* *`. Note on semantics (not just syntax): sudoers args + match by fnmatch — `[a-f0-9]*` only pins the FIRST character to the hex class; it is + effectively coarse anyway. Since the wrapper re-verifies the sha itself (S3a companion), the + **coarse `*` form is acceptable and simpler**; keep the `[a-f0-9]*` form only if the mild + first-char narrowing is wanted. + +## Cleanup (verified) + +Both spike units stopped/removed + `daemon-reload` + `reset-failed`; dummy binary/wrapper/state +dir/StateDir-subdir all removed; `systemctl list-units 'felhom-*'` = exactly the pre-spike set +(`felhom-agent.service` running + `felhom-shared-parent.service`); no transient `run-p*` units +left. **Real agent uptime unchanged:** MainPID 1705667, ActiveEnterTimestamp +`Sat 2026-07-04 21:07:38 CEST` — identical before and after. + +## Findings that MUST shape the implementation TASK + +1. **OnFailure fires per crash on systemd 257** (S1a) — the rollback unit will be invoked many + times per incident, including while the main unit is merely `activating (auto-restart)`. It + MUST be: (a) **no-op without the pending marker** (S3e shape proven), (b) **idempotent**, + (c) tolerant of concurrent scheduled auto-restarts. Corollary: rollback happens at the FIRST + post-apply crash (~0–5s) — the start-limit is the backstop, not the trigger. +2. **Start-limit tuning is still required** for a terminal state: drop-in **in `[Unit]`** with + `StartLimitIntervalSec=120` + `StartLimitBurst=4` → `failed` ≈20s after first crash. Without + it the unit crash-loops forever (S1a, 35 starts/180s). +3. **`[Service]` placement is a half-applied trap on 257:** `StartLimitBurst` is accepted, + `StartLimitIntervalSec` is ignored with only a journal warning. Never split; always `[Unit]`. +4. **`reset-failed` is mandatory** before any start of a failed unit inside the limit window + (S1c) — the rollback path AND any deliberate-restart path must run it first. +5. **Deliberate restarts consume start-limit budget** (S2 surprise): the apply→restart→ + health→commit flow must budget its own restarts (reset-failed before deliberate restarts; + don't chain more than Burst restarts within the interval). +6. **Detached restart (S2b) is the winner:** `systemd-run --on-active=2s + --timer-property=AccuracySec=100ms systemctl restart ` — 5/5, caller survives to log + the handoff, transient units self-clean. Direct restart also completes (systemd finishes the + job after the requester dies) — acceptable fallback. +7. **Wrapper ordering proven by S3:** sha-verify BEFORE `.prev` copy; staging-copy + atomic + same-fs `mv`; pending written after the flip; `.prev` retained on commit; sweep stale + `.new.*` temps on entry (the one unexercised crash window). +8. **Match failure on state, not Result:** terminal start-limit failure reports + `Result=exit-code` on this systemd (journal: "Start request repeated too quickly"), not + `start-limit-hit`. +9. **Sudoers:** coarse trailing `*` for the sha arg is fine (fnmatch makes `[a-f0-9]*` + first-char-only anyway; the wrapper is the real gate). Staging under + `/var/lib/felhom-agent/selfupdate/` is writable by the agent user as-is.