Compare commits
6 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 28ba8593b8 | |||
| 6981450110 | |||
| 703db166e7 | |||
| aa74294a7d | |||
| 5b2666e3a2 | |||
| 062a7027ab |
@@ -0,0 +1,46 @@
|
|||||||
|
---
|
||||||
|
paths: ["internal/backup/**", "internal/pbs/**", "internal/pbsdr/**", "internal/dr/**"]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Backup, PBS and DR
|
||||||
|
|
||||||
|
`internal/backup/` is the vzdump runner, restore-test scheduler and report store. `internal/pbs/` is
|
||||||
|
the fingerprint-pinned PBS-API client plus the verify maintenance loop. `internal/pbsdr/` and
|
||||||
|
`internal/dr/` carry the DR tier and recipe halves.
|
||||||
|
|
||||||
|
## The three PBS laws
|
||||||
|
|
||||||
|
1. **Set-only.** `pvesm remove` **DELETES the encryption key**. Re-apply configuration; never remove
|
||||||
|
and re-add a PBS storage to change it.
|
||||||
|
2. **Secret on stdin.** A token secret is passed on stdin, never as an argv the process table shows.
|
||||||
|
3. **Verify the pin BEFORE consuming the secret.** A fingerprint check after the secret has been sent
|
||||||
|
protects nothing.
|
||||||
|
|
||||||
|
## Verify is server-side, and its default skips the work
|
||||||
|
|
||||||
|
The agent drives verification **remotely** via the PBS API; `proxmox-backup-client` has **no** verify
|
||||||
|
subcommand. `POST .../verify` defaults to **`ignore-verified=true`, which SKIPS already-verified
|
||||||
|
snapshots** — send `ignore-verified=false` to actually re-read and detect corruption. A verify that
|
||||||
|
skipped everything reports success.
|
||||||
|
|
||||||
|
## Presence is not success
|
||||||
|
|
||||||
|
A timestamp recording an **attempt** is not evidence of a **result**. Where a status field travels
|
||||||
|
beside a timestamp, the verdict must consult **both** — or the timestamp must record only successes.
|
||||||
|
Ask of any timestamp: *what exactly must have happened for this to be set?* If the answer is "we
|
||||||
|
tried", it cannot answer "did it work".
|
||||||
|
|
||||||
|
**Corollary:** when a verdict changes which field it counts from, the alarm text changes with it.
|
||||||
|
Leaving a message reading `last run 8h ago` while alarming on a six-day-old **success** turns a true
|
||||||
|
alarm into one the operator dismisses.
|
||||||
|
|
||||||
|
## Prune is server-side now
|
||||||
|
|
||||||
|
`DatastoreBackup` carries **no** `Datastore.Prune`. Boxes set `keep_last: 0` and the off-site endpoint
|
||||||
|
runs the prune jobs. **Box tokens stay write-only — never widen that grant** (R-89).
|
||||||
|
|
||||||
|
<!--
|
||||||
|
The ignore-verified default is the sharpest instance of the "absent log line" class in this repo: a
|
||||||
|
verify that silently skipped every snapshot completes fast, exits clean, and reports the same shape
|
||||||
|
as one that read every byte.
|
||||||
|
-->
|
||||||
@@ -0,0 +1,26 @@
|
|||||||
|
---
|
||||||
|
paths: ["internal/capability/**", "internal/storage/**", "internal/localapi/**", "internal/hub/**", "internal/guesthook/**"]
|
||||||
|
---
|
||||||
|
|
||||||
|
# A health check issues no block I/O
|
||||||
|
|
||||||
|
No `statfs`, no `getdents`, no read, write or `fsync` — **not even behind a timeout**.
|
||||||
|
|
||||||
|
A probe that touches a wedged device enters uninterruptible sleep, survives `SIGKILL`, and cannot be
|
||||||
|
recovered until the device returns or the host reboots — so `systemctl restart` hangs too. A timeout
|
||||||
|
protects the caller's control flow and nothing else: the blocked thread remains.
|
||||||
|
|
||||||
|
**Liveness is decided from `/proc` and the kernel's own state**, never by reading or writing the
|
||||||
|
filesystem.
|
||||||
|
|
||||||
|
<!--
|
||||||
|
Measured, R-117 spike §6.3 (felhom.eu/documentation/audits/SPIKE-r117-bind-liveness-2026-07-30.md):
|
||||||
|
a probe stayed in D state 3m50s after kill -9; a buffered write with no fsync blocked too (O_CREAT
|
||||||
|
needs journal access); and statfs/getdents returned HEALTHY on a namespace that EIOs every byte —
|
||||||
|
fast, and wrong.
|
||||||
|
|
||||||
|
This rule used to be duplicated verbatim in felhom-agent/CLAUDE.md with a note explaining that
|
||||||
|
felhom.eu/CLAUDE.md "does not load in an agent-only session". That reasoning was correct before
|
||||||
|
path-scoped rules existed. The single source is now felhom.eu/CLAUDE.md "Code quality rules"; this
|
||||||
|
file is the scoped copy that loads exactly where health checks are written. (2026-08-06)
|
||||||
|
-->
|
||||||
@@ -0,0 +1,44 @@
|
|||||||
|
---
|
||||||
|
paths: ["internal/localapi/**", "internal/authz/**", "internal/guesthook/**"]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Local API, authz and guest hooks — the per-guest blast radius
|
||||||
|
|
||||||
|
`internal/localapi/` is the narrow per-guest local API: token store, disks/format, guest binds,
|
||||||
|
controller swap, stale-lock recovery, pinned self-signed leaf. `internal/authz/` is the operator
|
||||||
|
signed-op verifier (SSHSIG) plus the durable nonce store. `internal/guesthook/` installs the
|
||||||
|
pre-start self-heal hookscript.
|
||||||
|
|
||||||
|
> **Overlap note:** `health-checks.md` also matches `internal/localapi/**` and
|
||||||
|
> `internal/guesthook/**`. That is deliberate — both rules apply there and both load. Neither
|
||||||
|
> supersedes the other.
|
||||||
|
|
||||||
|
## Scoping is the whole security property
|
||||||
|
|
||||||
|
This API is reachable **from inside a customer guest**. Every route must be scoped to the guest that
|
||||||
|
called it — a route that can name another guest's id has escaped its blast radius. Fail **safe to
|
||||||
|
protected**: an unrecognised or unresolvable caller gets less access, never more.
|
||||||
|
|
||||||
|
## Replay protection must survive a restart
|
||||||
|
|
||||||
|
**`authz.MemoryNonceStore` on a real host is a defect** — replay protection dies on restart. Use
|
||||||
|
`authz.FileNonceStore`. The memory store exists for tests.
|
||||||
|
|
||||||
|
## The token is a hash on disk, plaintext only at mint
|
||||||
|
|
||||||
|
The store keeps **hashes**. The plaintext token exists in exactly one place, `bootstrap.json` on the
|
||||||
|
PVE host — so a "read the token" step means reading that file, and a lost token is re-minted, never
|
||||||
|
recovered.
|
||||||
|
|
||||||
|
## Binds can brick guest boot
|
||||||
|
|
||||||
|
| Do not | Because | Use |
|
||||||
|
|---|---|---|
|
||||||
|
| `GuestBinder.AttachBind`/`DetachBind` (per-drive `pct set -mpN`) | legacy model; a missing bind source can **brick guest boot** (C1) | `AttachDrive`/`DetachDrive` (intermediary model) |
|
||||||
|
| `isHostMountpoint` to reconcile bind state | a boolean cannot converge stacked double-binds (the `/mnt` doubling bug) | `countHostMounts` normalization inside `AttachDrive` |
|
||||||
|
|
||||||
|
<!--
|
||||||
|
Why fail-safe-to-protected rather than fail-closed: this API also carries the recovery paths. A hard
|
||||||
|
refusal on an unresolvable caller would make a half-broken guest unrecoverable through the very
|
||||||
|
interface built to recover it. Less access, never none.
|
||||||
|
-->
|
||||||
@@ -0,0 +1,44 @@
|
|||||||
|
---
|
||||||
|
paths: ["internal/proxmox/**", "internal/reconcile/**", "internal/signedjobs/**"]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Proxmox — the API contract, and how destructive work is gated
|
||||||
|
|
||||||
|
`internal/proxmox/` is the API-first `Client` plus the fenced root-CLI `Privileged`.
|
||||||
|
`internal/reconcile/` is the reconcile engine, reversibility gate, op journal and crash recovery.
|
||||||
|
`internal/signedjobs/` holds the operator-signed destructive executors (wipe, decommission).
|
||||||
|
|
||||||
|
## A 200 on the POST is not success
|
||||||
|
|
||||||
|
**Every mutating op is async**: it returns a **UPID**, and `WaitTask` must assert
|
||||||
|
`exitstatus == "OK"`. Authorization can fail at *task execution* long after the HTTP call returned
|
||||||
|
200. Treating the POST's status as the result is how a failed destroy reads as a successful one.
|
||||||
|
|
||||||
|
## The privsep token gotcha
|
||||||
|
|
||||||
|
A `--privsep 1` token's rights are the **intersection** of the backing user's permissions **and** the
|
||||||
|
token's own ACLs. The role must be granted on **both** or every call 403s. The same intersection rule
|
||||||
|
bites on PBS (`token ∩ user`).
|
||||||
|
|
||||||
|
## TLS
|
||||||
|
|
||||||
|
**SHA-256 leaf-cert pinning** against the self-signed host cert. **No insecure default**, ever. The
|
||||||
|
pin is the raw leaf-DER sha — the SAN is never checked, so a cert rotation changes the pin and the
|
||||||
|
agent must be re-pinned.
|
||||||
|
|
||||||
|
## The destructive path — never the direct call
|
||||||
|
|
||||||
|
| Do not | Because | Use |
|
||||||
|
|---|---|---|
|
||||||
|
| `Client.DestroyLXC` / `Vzdump` / `SetConfig` ad-hoc | skips classification, signature, per-guest serialization, crash recovery | `reconcile.Engine` paths / `RunSignedJob`; queue via `Queue.Submit` |
|
||||||
|
| add a method to `proxmox.Privileged` | breaks the 3-exception root-CLI fence (`routing_test.go`) | `proxmox.Runner` + a new sudoers `Cmnd_Alias` + `validate.go`-style checks |
|
||||||
|
| treat `ListLXC` output as "guests we own" | audit A1 — pre-v0.62.0 the stale-lock reaper did exactly this, contained only by the pool-scoped token | intersect with `Client.Pool` membership (`staleLockController.Guests()`); **fail safe on read failure** |
|
||||||
|
|
||||||
|
Full trap table: `REUSE.md` §3. Every guest joins the `felhom` pool — `VM.Audit` comes from the
|
||||||
|
`/pool` grant, not from a per-guest ACL.
|
||||||
|
|
||||||
|
<!--
|
||||||
|
The fence is not stylistic. It is what makes this component auditable: two types, one of which can
|
||||||
|
only speak HTTP and one of which can only shell out, with a test asserting neither crosses. A single
|
||||||
|
convenience method on Privileged that also makes an HTTP call would end that property silently.
|
||||||
|
-->
|
||||||
@@ -0,0 +1,49 @@
|
|||||||
|
---
|
||||||
|
paths: ["internal/storage/**", "internal/escrow/**"]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Storage and escrow — format safety and zero-knowledge recovery
|
||||||
|
|
||||||
|
`internal/storage/` is the storage observer, durable IDs, role/claim classifiers, `SudoHostOps` and
|
||||||
|
the watchdog. `internal/escrow/` is the PBS-key escrow with its zero-knowledge recovery code.
|
||||||
|
|
||||||
|
> **Overlap note:** `health-checks.md` also matches `internal/storage/**`. Deliberate — both rules
|
||||||
|
> apply there and both load.
|
||||||
|
|
||||||
|
## Never format the device you inspected
|
||||||
|
|
||||||
|
**AGENT-001 is a TOCTOU:** acting on the caller's `req.Device` (or any remembered `/dev` path) after
|
||||||
|
inspection lets `/dev` re-enumeration retarget the node to a **different physical disk**. Format the
|
||||||
|
**re-resolved** device — `Server.reresolveWipe` / `reresolveBlank`.
|
||||||
|
|
||||||
|
**Never exec raw `mkfs.*`** (including `Binaries.MkfsExt4`/`MkfsXfs`): sudoers no longer allowlists
|
||||||
|
raw mkfs, and going direct bypasses the claim filter and the wrapper's re-checks. Use
|
||||||
|
`SudoHostOps.Format`, which routes through `felhom-mkfs-guarded`.
|
||||||
|
|
||||||
|
## The two durable-ID schemes refuse each other
|
||||||
|
|
||||||
|
They are not interchangeable, and each returns a `binding_mismatch` for the other's scheme:
|
||||||
|
|
||||||
|
| Purpose | Scheme | Resolver |
|
||||||
|
|---|---|---|
|
||||||
|
| wipe confirmation | `byid:` / `byuuid:` | `ResolveDurableDevice`, `DiskInfo.WipeDurableID` |
|
||||||
|
| enrolled-storage remount | `uuid:` | `ResolveStorageDevice` |
|
||||||
|
|
||||||
|
Using `DiskInfo.DurableID` (a `uuid:`) as a wipe-confirmation id is F20-BUG2.
|
||||||
|
|
||||||
|
## Drive data is never taken by force
|
||||||
|
|
||||||
|
Plain `umount` only — **never `-l`, never `-f`**, and never any format operation under
|
||||||
|
`/mnt/felhom-drives`.
|
||||||
|
|
||||||
|
## Escrow is zero-knowledge, and a fetch failure is not a wrong code
|
||||||
|
|
||||||
|
The server holds no client key; a no-key restore fails with `missing key`. **A fetch failure must
|
||||||
|
never be reported as a wrong recovery code** — that told a customer their correct code was bad, in
|
||||||
|
hundredths of a second, when checking a code actually takes about one. Distinguish "we could not
|
||||||
|
reach the store" from "the code did not match", always.
|
||||||
|
|
||||||
|
<!--
|
||||||
|
The escrow recovery-code "flake" was a REAL defect, not a flake. "Known flake, re-run" needs evidence
|
||||||
|
before it is said out loud — that phrase cost this project a real finding once.
|
||||||
|
-->
|
||||||
+151
@@ -1,3 +1,154 @@
|
|||||||
|
## v0.128.0 — the escrow seed is asserted every tick, not remembered once (2026-08-08, R-221)
|
||||||
|
|
||||||
|
**A rebuilt box could not run the escrow ceremony at all, and there was no way forward from inside
|
||||||
|
the product.** The preflight refuses on `escrow.pbs_storage_id`; the pbsdr bridge writes that key;
|
||||||
|
and it wrote it from exactly one place — `finishConverged`, reached only on the paths that actually
|
||||||
|
converge.
|
||||||
|
|
||||||
|
**The two things live in different places and die at different times.** The convergence marker is
|
||||||
|
host-side (`<agent-state>/pbsdr/marker.json`). The key it seeds is in `agent.json`, which
|
||||||
|
`step_agent_config` renders from `base = {}` unless an explicit `--preserve-from` is passed
|
||||||
|
(`felhom.eu/scripts/felhom-host-install.sh:2396`, the render at `:2449`, the `O_TRUNC` write at
|
||||||
|
`:2579`; the flag at `:1246`, defaulting empty at `:256`) — **and the render never writes an
|
||||||
|
`escrow` section at all.** So a rebuild keeps the marker and takes the key: same descriptor, same
|
||||||
|
hash, early return, and the seed never runs again into a config that no longer has it.
|
||||||
|
|
||||||
|
**Fixed by asserting rather than remembering.** `Apply` now re-asserts the seed *before* the
|
||||||
|
idempotent early return. `seedEscrowStorageID` is unchanged and still never clobbers a different
|
||||||
|
existing value — an operator's own choice outranks the descriptor's, with a warning naming both.
|
||||||
|
|
||||||
|
**The early return is KEPT.** It exists so a converged box does not re-run Proxmox operations every
|
||||||
|
60 s, and `TestSeedReasserted_OnConvergedTick_WithZeroProxmoxCalls` asserts **zero** recorded runner
|
||||||
|
calls on that tick — so a "fix" that simply deleted the return would fail. Cost of the re-assert: one
|
||||||
|
small file read plus a JSON parse per tick, no exec, no network, and an early return once the value
|
||||||
|
matches.
|
||||||
|
|
||||||
|
**A seed failure can never un-converge the box:** Warn plus a message on the published status,
|
||||||
|
exactly as `finishConverged` does it — no marker write, no state change. Pinned by
|
||||||
|
`TestSeedReassertFailure_DoesNotUnconverge`.
|
||||||
|
|
||||||
|
Tests drive the real `Apply` with a real temp-dir `agent.json` and a call-recording runner; calling
|
||||||
|
`seedEscrowStorageID` directly cannot see the early return, which IS the defect. The production
|
||||||
|
wiring (`pbsdr.NewManager(..., cfg.SourcePath, ...)`) is asserted by walking `main.go`'s **AST**, not
|
||||||
|
by `strings.Contains`, which a commented-out call also satisfies.
|
||||||
|
|
||||||
|
Red-proofs: removing the new call makes Scenario A fail on today's tree; removing the early return
|
||||||
|
makes the zero-Proxmox-calls assertion fail.
|
||||||
|
|
||||||
|
## (no version bump) — a comment that claimed the hub reads a field it has no field for (2026-08-08, R-260)
|
||||||
|
|
||||||
|
Comment-only; no behaviour, no wire change, nothing to rebuild.
|
||||||
|
|
||||||
|
`HostReport.SelfUpdatePending` / `SelfUpdatePendingVersion` carried the sentence *"The hub reads an
|
||||||
|
absent field as pending=false, the correct default."* **The hub has no field for either**, so it reads
|
||||||
|
nothing — present or absent — and `encoding/json` discards them on arrival. The sentence described an
|
||||||
|
intent rather than the code and read as settled for long enough that a class sweep had to find it.
|
||||||
|
|
||||||
|
The emission is correct and stays: the agent reports the truth, and the fault is entirely in the
|
||||||
|
receiving. The missing consumer is tracked as **R-264** (OPEN), and
|
||||||
|
`felhom.eu/scripts/wire_contract_gate.py` now refuses any NEW field of this shape while recording the
|
||||||
|
existing ones as explicit, reasoned allowlist entries rather than silence.
|
||||||
|
|
||||||
|
## v0.127.0 — a mount Felhom itself made is not "something else" (2026-08-06, R-220)
|
||||||
|
|
||||||
|
**After a rebuild the customer's own drives could not be re-attached, and the refusal named an action
|
||||||
|
they could not perform.** `GET /api/disks/candidates` returned `initialize: [], attach: []` while both
|
||||||
|
drives sat there, and the deploy refused with *"choose an attached drive from the list"* — a list that
|
||||||
|
was empty. Measured live **three times**: CAMPAIGN-11 Phase 1, and twice on the R-201 re-walk.
|
||||||
|
|
||||||
|
**The mechanism.** Enrolment mounts a drive **twice** — at the managed `/mnt/felhom-drives/<name>` and
|
||||||
|
at the raw `/mnt/<name>` it creates on the host. **The host survives a guest rebuild; the controller's
|
||||||
|
registry does not.** So `classifyClaim` saw a mount outside the managed prefix and correctly concluded
|
||||||
|
"claimed by something else" — about Felhom's own mount.
|
||||||
|
|
||||||
|
**The fix is CORROBORATED, not a widened prefix.** A mountpoint outside `/mnt/felhom-drives` is
|
||||||
|
forgiven **only when the same device is ALSO mounted under the managed path** — a pairing that only
|
||||||
|
Felhom's own enrolment produces. A disk another system is using, at `/srv/data` or `/media/x` or even
|
||||||
|
`/mnt/someone-elses-disk`, has no such counterpart and **is still refused**. That fence has its own
|
||||||
|
test, and its red-proof shows an over-wide fix offering `/mnt/someone-elses-disk` for formatting.
|
||||||
|
|
||||||
|
**Read from `/proc/mounts`, deliberately.** The lsblk invocation is pinned **verbatim** in
|
||||||
|
`configs/felhom-agent.sudoers` (`lsblk -J -o NAME,FSTYPE,PTTYPE,MOUNTPOINT /dev/*`), so switching it to
|
||||||
|
the plural `MOUNTPOINTS` would have meant shipping a sudoers change with the binary — a far larger
|
||||||
|
blast radius than this finding warrants. `/proc/mounts` is world-readable: **no sudo, no new allowlisted
|
||||||
|
command, no config change.**
|
||||||
|
|
||||||
|
**Fail-safe:** an unreadable mount table corroborates **nothing**, so the device classifies exactly as
|
||||||
|
it did before this change — refused. "We could not corroborate" must never read as "it is ours".
|
||||||
|
|
||||||
|
Tests: `claim_r220_test.go` — the own-drive case, the foreign-mount fence over four paths, and the
|
||||||
|
corroboration itself (both mounts required; a lone raw mount vouches for nothing; another device's
|
||||||
|
managed mount does not vouch for this one; an unreadable table corroborates nothing).
|
||||||
|
**Red-proofs:** removing the exemption refuses the customer's own drive again
|
||||||
|
(*"device is mounted at /mnt/adatok (sdb)"*); over-widening it to any `/mnt/*` path breaks the fence.
|
||||||
|
|
||||||
|
## docs — CLAUDE.md becomes a core plus path-scoped rules (2026-08-06, R-229 leg (b)) — no version bump
|
||||||
|
|
||||||
|
**Documentation only. No Go changed, nothing built, nothing deployed.** `go build`/`vet`/`test` green
|
||||||
|
and unchanged.
|
||||||
|
|
||||||
|
**175 -> 99 effective lines** (207 -> 103 raw, 14,093 -> 6,267 bytes). The release/publish-train
|
||||||
|
section was the largest block and the `felhom-build-deploy` skill already carries the procedure, so
|
||||||
|
the core points at it instead of restating a table that drifts from the script. The package layout
|
||||||
|
went the same way as the controller's: `REUSE.md` and the tree are its home, and the per-package
|
||||||
|
annotations that were doing real work moved into the rule file for the area they describe rather than
|
||||||
|
being deleted.
|
||||||
|
|
||||||
|
**New:** `.claude/rules/{proxmox,localapi,backup,storage}.md`, all `paths:`-scoped, all <=46 effective
|
||||||
|
lines, joining the existing `health-checks.md`.
|
||||||
|
|
||||||
|
**Kept in the core deliberately** — it is the only part re-injected after `/compact`: the root-CLI
|
||||||
|
fence and its three named exceptions (breaching it is how this component stops being auditable), the
|
||||||
|
destructive-op gate, the prove-ownership rule from audit A1, the gate entry point, the F9
|
||||||
|
live-validation fence, trunk-based with its revert-and-report escape hatch, and the end-of-session
|
||||||
|
checklist.
|
||||||
|
|
||||||
|
**Glob overlap, stated rather than silently resolved:** `health-checks.md` matches
|
||||||
|
`internal/{localapi,guesthook}/**` and `internal/storage/**`, which `localapi.md` and `storage.md`
|
||||||
|
also match. Both rules load in those directories and neither supersedes the other; each new file says
|
||||||
|
so in its own text so a reader who sees two rules fire is not left guessing which wins.
|
||||||
|
|
||||||
|
## docs — the "CI is still owed" claim was stale; corrected (2026-08-06, R-229 part 2) — no version bump
|
||||||
|
|
||||||
|
**One sentence, no code.** This file asserted that continuous integration was still owed
|
||||||
|
(`felhom.eu` `OPEN-ITEMS.md` R-168). **R-168 was CLOSED on 2026-08-02** — a Gitea Actions runner
|
||||||
|
re-runs each repo's gate entry point on every push and emails the operator on failure. Found while
|
||||||
|
confirming this session's own push by run ID, which is the check that caught it.
|
||||||
|
|
||||||
|
The same stale sentence was in four instruction files across all four repos and is corrected in all
|
||||||
|
four. In `felhom-agent/CLAUDE.md` it **contradicted the same file's release section**, which already
|
||||||
|
said R-168 mails the failure — a contradiction inside one instruction file, which is the exact class
|
||||||
|
the R-229 work exists to find.
|
||||||
|
|
||||||
|
## docs — expired and contradictory blocks removed from CLAUDE.md (2026-08-06, R-229) — no version bump
|
||||||
|
|
||||||
|
**Documentation and gate registration only. No Go changed, nothing built, nothing deployed.**
|
||||||
|
|
||||||
|
Surgical corrections; the file was deliberately **not** restructured (that is deferred, R-229).
|
||||||
|
|
||||||
|
- **Deleted the expired TEMPORARY block.** It read *"felhom-pve is at a remote site (until
|
||||||
|
~2026-08-02) … Delete this block on return"* and was still being read as current fact on
|
||||||
|
**2026-08-06**, four days past its own deadline — while `felhom-controller/CLAUDE.md` asserted the
|
||||||
|
opposite. The location-independence fact worth keeping (`localapi` binds `169.254.253.1:8443` on
|
||||||
|
`vmbr9` since the R-50 island migration) moved to an HTML comment.
|
||||||
|
- **Every component version literal is gone** from effective text, including
|
||||||
|
`felhom-agent --version → 0.115.0` and the `go.mod` Go directive. Versions change several times a
|
||||||
|
day; ask the hub's `/hosts` + `/configs` or the box.
|
||||||
|
- The drill-VM claim and the host addresses now point at `documentation/operations/nodes.md`, which
|
||||||
|
already stated both correctly. **This file's drill-VM claim was the correct one** — confirmed by
|
||||||
|
`qm list` on demo-hp.
|
||||||
|
- The R-115/R-188/R-186 release **narratives** moved to an HTML comment and to the
|
||||||
|
`felhom-build-deploy` skill; the **directives** stayed (never hand-roll the build; the
|
||||||
|
build → tag → publish → push order; reproducible `-trimpath -buildvcs=false`).
|
||||||
|
- The health-check block-I/O rule became `.claude/rules/health-checks.md`, scoped to the five
|
||||||
|
packages where health checks are written. It had been duplicated from `felhom.eu/CLAUDE.md` *with a
|
||||||
|
note explaining that that file does not load in an agent-only session* — correct reasoning, made
|
||||||
|
obsolete by path-scoped rules.
|
||||||
|
|
||||||
|
`agent_gates.py` now registers **`instructions`** (shared, `felhom.eu/scripts/`, never copied).
|
||||||
|
|
||||||
|
Full accounting: `felhom.eu/documentation/audits/LEDGER-instruction-trim-2026-08-06.md`.
|
||||||
|
|
||||||
## v0.126.0 — a fetch failure is not a wrong recovery code (2026-08-06, R-224)
|
## v0.126.0 — a fetch failure is not a wrong recovery code (2026-08-06, R-224)
|
||||||
|
|
||||||
**A hub the agent could not reach was reported to the customer as a bad recovery code.** Measured live
|
**A hub the agent could not reach was reported to the customer as a bad recovery code.** Measured live
|
||||||
|
|||||||
@@ -1,216 +1,103 @@
|
|||||||
# CLAUDE.md — `felhom-agent`
|
# CLAUDE.md — `felhom-agent`
|
||||||
|
|
||||||
> Loads when Claude Code touches this repo. Stable orientation only — **current state lives in
|
> Stable orientation only — **current state lives in `CONTEXT.md` and the top of `CHANGELOG.md`**,
|
||||||
> `CONTEXT.md` and the top of `CHANGELOG.md`**, never here. Cross-repo orientation: workspace-root
|
> never here. Cross-repo conventions (artifact taxonomy, access, clean-tree gate, secrets,
|
||||||
> `/mnt/5_hdd/felhom.eu/git/CLAUDE.md`.
|
> CHANGELOG/REPORT): workspace-root `/mnt/5_hdd/felhom.eu/git/CLAUDE.md`. Path-scoped detail:
|
||||||
|
> `.claude/rules/`.
|
||||||
|
|
||||||
## What this repo is
|
## What this repo is
|
||||||
|
|
||||||
`felhom-agent` is the operator-tier **host agent** that runs on each Proxmox host and owns **all**
|
The operator-tier **host agent**, one per Proxmox host, owning **all** Proxmox interaction:
|
||||||
Proxmox interaction: provision/restore guests, host storage, backup/restore orchestration, the hub
|
provision/restore guests, host storage, backup/restore orchestration, the hub control loop, and a
|
||||||
control loop, and a narrow per-guest local API. It is the **most privilege-sensitive** component.
|
narrow per-guest local API. It is the **most privilege-sensitive component in the system**.
|
||||||
|
|
||||||
- Renamed former `proxmox-controller` repo.
|
- Renamed from `proxmox-controller`.
|
||||||
- **Distinct from `felhom-controller`** — that is the *in-guest* controller (Docker-only, no Proxmox
|
- **Distinct from `felhom-controller`** — that is the *in-guest* controller, Docker-only, holding no
|
||||||
creds). Do not confuse them.
|
Proxmox credentials. Do not confuse them.
|
||||||
- Control plane, not data plane: if the agent dies, apps keep serving; only management degrades.
|
- **Control plane, not data plane:** if the agent dies, apps keep serving; only management degrades.
|
||||||
|
- Pure Go stdlib + `golang.org/x/crypto`. No web frameworks.
|
||||||
|
|
||||||
## Read before writing code
|
## Doing X → read Y
|
||||||
|
|
||||||
- **`REUSE.md`** — canonical helpers, format-safety guards, traps, seams. Check it first; update it
|
| Doing | Read |
|
||||||
in the same commit that changes a shared helper or pattern.
|
|---|---|
|
||||||
- `CONTEXT.md` (current state + open threads) and the top `CHANGELOG.md` entry (authoritative history).
|
| writing any new code | `REUSE.md` — helpers, format-safety guards, traps, seams |
|
||||||
- Design doc: `felhom.eu/documentation/architecture/03-host-agent.md` (locked). Platform facts:
|
| needing current state / open threads | `CONTEXT.md` + the top `CHANGELOG.md` entry |
|
||||||
`felhom.eu/documentation/proxmox-platform.md` + `tests/phase{0,1-2,3,4}-findings.md`.
|
| Proxmox, reconcile or signed jobs | loads itself: `.claude/rules/proxmox.md` |
|
||||||
|
| local API, authz or guest hooks | loads itself: `.claude/rules/localapi.md` |
|
||||||
|
| backup, PBS or DR | loads itself: `.claude/rules/backup.md` |
|
||||||
|
| storage or escrow | loads itself: `.claude/rules/storage.md` |
|
||||||
|
| writing a health check | loads itself: `.claude/rules/health-checks.md` |
|
||||||
|
| **release, build, publish, deploy, verify a version** | the **`felhom-build-deploy`** skill — **never hand-roll it** |
|
||||||
|
| writing or reviewing a test, fixing a bug | the **`felhom-testing`** skill |
|
||||||
|
| host addresses, break-glass, node facts | `felhom.eu/documentation/operations/nodes.md` — never restate them |
|
||||||
|
| which box may I break | `felhom.eu/documentation/runbooks/target-selection.md` |
|
||||||
|
| what version is live anywhere | ask the hub (`/hosts`, `/configs`) or the box — **never a doc** |
|
||||||
|
| the authoritative design | `felhom.eu/documentation/architecture/03-host-agent.md` (locked) |
|
||||||
|
|
||||||
## Layout (verified against the tree)
|
## The root-CLI fence — API-first, exactly three exceptions
|
||||||
|
|
||||||
```
|
This is in the core because breaching it is how this component stops being auditable.
|
||||||
cmd/felhom-agent/ main + flags + --selftest modes + the daemon entry
|
|
||||||
cmd/felhom-opsign/ offline operator signing CLI (SSHSIG)
|
|
||||||
internal/authz/ operator signed-op verifier (SSHSIG) + durable FileNonceStore
|
|
||||||
internal/backup/ vzdump backup runner + restore-test scheduler + report store
|
|
||||||
internal/capability/ live sudo-policy capability probe (degradation visibility)
|
|
||||||
internal/config/ JSON config + FELHOM_AGENT_* env overlay; secrets redacted (Redacted())
|
|
||||||
internal/desired/ hub desired-state syncer (envelope observer)
|
|
||||||
internal/escrow/ PBS-key escrow (zero-knowledge recovery code)
|
|
||||||
internal/guesthook/ pre-start self-heal hookscript install
|
|
||||||
internal/hub/ daemon: HostReport collector + Bearer client + resilient Loop
|
|
||||||
internal/lanresolver/ split-horizon DNS on guest IP change (dnsmasq RESTART, not reload)
|
|
||||||
internal/localapi/ per-guest local API: token store, disks/format, guest binds, controller swap,
|
|
||||||
stale-lock recovery, pinned self-signed leaf
|
|
||||||
internal/log/ slog setup
|
|
||||||
internal/pbs/ PBS-API client (fingerprint-pinned) + verify maintenance loop
|
|
||||||
internal/provision/ guest bootstrap back-half (token mint → bootstrap.json → pct bind)
|
|
||||||
internal/proxmox/ API-first Client + fenced root-CLI Privileged + UPID WaitTask
|
|
||||||
internal/reconcile/ reconcile engine + reversibility gate + op journal + crash recovery
|
|
||||||
internal/signedjobs/ operator-signed destructive executors (wipe, decommission)
|
|
||||||
internal/storage/ storage observer + durable ids + role/claim classifiers + SudoHostOps + watchdog
|
|
||||||
```
|
|
||||||
|
|
||||||
## Build / run
|
|
||||||
|
|
||||||
- Module `gitea.dooplex.hu/admin/felhom-agent`; binary `felhom-agent` (`cmd/felhom-agent/`).
|
|
||||||
- **Pure Go stdlib + `golang.org/x/crypto` only** — no web frameworks. `go.mod` directive go 1.25.0;
|
|
||||||
DooPlex (192.168.0.180, where CC runs) has the Go toolchain and is on the same LAN as the demo
|
|
||||||
host — build and run live tests locally.
|
|
||||||
- Version via `-ldflags "-X main.version=<v>"`; `--version` flag. Bump on meaningful changes + CHANGELOG entry.
|
|
||||||
- **Full build/deploy/publish runbook: use the `felhom-build-deploy` skill.** Summary:
|
|
||||||
|
|
||||||
> **Clean-tree gate before any build:** `git status --porcelain` must be empty and
|
|
||||||
> `git rev-parse HEAD` must equal `git rev-parse origin/main` in the repo being built. An unpushed
|
|
||||||
> change does not exist — never build a dirty or unpushed tree. The `git pull` in the build step
|
|
||||||
> stays (it is a no-op when you work in this tree, and load-bearing if anything was pushed from
|
|
||||||
> elsewhere).
|
|
||||||
|
|
||||||
> **RELEASING IS ONE COMMAND, AND IT PUBLISHES (R-115).** There used to be a raw `go build` line
|
|
||||||
> here and a *separate* "Publish" row, so publishing was a step someone had to remember — and it was
|
|
||||||
> **forgotten three times in five days**, the last leaving agent v0.120.0 deployed on both demo hosts
|
|
||||||
> and undownloadable, where a documented-path reinstall would have silently downgraded them while
|
|
||||||
> reporting success. Do not hand-roll the build: the script also creates the `v<version>` git TAG
|
|
||||||
> that `felhom-host-install.sh` fetches this version's sixteen config files from (R-183), and it
|
|
||||||
> verifies by an **independent download** rather than trusting the publish step's own output.
|
|
||||||
> `scripts/publish-agent.sh` still exists and is still correct — the release script CALLS it rather
|
|
||||||
> than reimplementing it.
|
|
||||||
>
|
|
||||||
> **THE ORDER IS build → tag LOCALLY → publish → push tag, and each step protects something (R-188,
|
|
||||||
> R-186).** The tag is created before the publish so the build and the tag describe the same commit;
|
|
||||||
> it is *pushed* after, because the push is what wakes CI (`on: [push]`) and a tag visible before its
|
|
||||||
> package makes the published-versions gate correctly fail a correct release — it did, on roughly
|
|
||||||
> every second release, and R-168 sends that failure to you by mail. The invariant the old order
|
|
||||||
> protected is asserted directly instead: the gate now also refuses a **published version with no
|
|
||||||
> tag**. If the push fails after a successful publish the script says so loudly and prints the
|
|
||||||
> one-line recovery; if the *publish* fails it removes the local-only tag so a retry is clean.
|
|
||||||
>
|
|
||||||
> **A RELEASED BINARY IS INDEPENDENTLY VERIFIABLE (R-186).** The build uses `-trimpath
|
|
||||||
> -buildvcs=false` so the same source produces the same bytes whether or not the tag exists yet —
|
|
||||||
> before this, a rebuild could not reproduce the sha you were vouching. To check any published
|
|
||||||
> version yourself:
|
|
||||||
>
|
|
||||||
> ```bash
|
|
||||||
> V=0.122.0
|
|
||||||
> git checkout "v$V" && go build -trimpath -buildvcs=false -ldflags "-X main.version=$V" \
|
|
||||||
> -o /tmp/felhom-agent-check ./cmd/felhom-agent
|
|
||||||
> sha256sum /tmp/felhom-agent-check
|
|
||||||
> curl -fsSL "https://gitea.dooplex.hu/api/packages/admin/generic/felhom-agent/$V/felhom-agent" | sha256sum
|
|
||||||
> ```
|
|
||||||
>
|
|
||||||
> The two hashes must match. `publish-agent.sh`'s fallback build uses the **same** flags — it used to
|
|
||||||
> force `CGO_ENABLED=0` and produce a 74 KB-smaller binary for the same version; if either build line
|
|
||||||
> ever changes, change both or one version name means two binaries again.
|
|
||||||
|
|
||||||
| Step | Where | One-liner |
|
|
||||||
|---|---|---|
|
|
||||||
| **Release** (build + tag + publish + verify) | DooPlex (local) | `GITEA_USER=admin GITEA_TOKEN=<tok> scripts/release-agent.sh <ver>` — refuses a dirty/unpushed tree and refuses to re-release an existing version |
|
|
||||||
| Copy | local → felhom-pve | `scp /tmp/felhom-agent-<v> felhom-pve:/tmp/` (one hop) |
|
|
||||||
| Deploy | felhom-pve | backup `.bak-<old>` → `install -m0755` → `systemctl restart felhom-agent` (non-root `felhom-agent` user, config `/etc/felhom-agent/agent.json`) |
|
|
||||||
| Ship configs | felhom-pve | sudoers (`/etc/sudoers.d/felhom-agent`) + guarded-mkfs wrapper WITH the binary when `configs/` changed |
|
|
||||||
| **Verify** (anyone, any time) | anywhere with the repo + Go | `git checkout v<ver> && go build -trimpath -buildvcs=false -ldflags "-X main.version=<ver>" -o /tmp/a ./cmd/felhom-agent && sha256sum /tmp/a` — must equal `curl -fsSL <pkg-url> \| sha256sum` |
|
|
||||||
| **Vouch** | hub operator UI | Configs → Day-0 artifacts. **Deliberately NOT automated** — vouching is what points machines at a version, and it stays your act (prove-then-vouch) |
|
|
||||||
| Verify | felhom-pve | `felhom-agent --version` + journal (clean ReassertGuestBinds, no capability degradation) |
|
|
||||||
|
|
||||||
## Proxmox model (the load-bearing rules)
|
|
||||||
|
|
||||||
- **API-first** via a scoped `FelhomAgent` token. Raw root-CLI is **fenced to exactly 3 exceptions**:
|
- **API-first** via a scoped `FelhomAgent` token. Raw root-CLI is **fenced to exactly 3 exceptions**:
|
||||||
keyctl `pct create` (golden image), USB mount/fstab, SMART/sensors. `Client` never shells out;
|
keyctl `pct create` (golden image), USB mount/fstab, SMART/sensors.
|
||||||
`Privileged` never makes HTTP calls (asserted by `routing_test.go`). Keep that fence.
|
- **`Client` never shells out; `Privileged` never makes HTTP calls** — asserted by `routing_test.go`.
|
||||||
- **Every mutating op is async** → returns a UPID → `WaitTask` asserts `exitstatus == "OK"`. A 200 on
|
Adding a method to `proxmox.Privileged` breaks the fence; use `proxmox.Runner` plus a new sudoers
|
||||||
the POST is **not** success; authorization can fail at task execution.
|
`Cmnd_Alias` and `validate.go`-style checks (`REUSE.md` §3).
|
||||||
- **TLS:** SHA-256 leaf-cert pinning (self-signed host cert). No insecure default.
|
- **Destructive ops go through the reconcile gate / signed-jobs path.** Never call
|
||||||
- **Privsep token gotcha:** a `--privsep 1` token's rights = intersection of the backing user's perms
|
`Client.DestroyLXC` / `Vzdump` / `SetConfig` ad-hoc — that skips classification, signature,
|
||||||
AND the token's ACLs — the role must be granted on **both**, or every call 403s.
|
per-guest serialization and crash recovery.
|
||||||
- Destructive ops go through the reconcile gate / signed-jobs path — never call `Client.DestroyLXC`/
|
- **Ownership must be PROVEN, never assumed.** A raw `ListLXC` list is not "guests the agent owns";
|
||||||
`Vzdump`/`SetConfig` ad-hoc (REUSE.md §3).
|
intersect with `Client.Pool` membership and fail safe on a read failure (audit A1).
|
||||||
|
|
||||||
## Demo host (for live tests)
|
## Gates — ONE entry point
|
||||||
|
|
||||||
Node **`demo-felhom`**, API `https://192.168.0.162:8006`. SSH alias `felhom-pve` (root@pam) —
|
**Run `python3 scripts/agent_gates.py` from the repo root after ANY change here.** It runs this
|
||||||
available to CC as plain `ssh felhom-pve`. A **second demo node `demo-hp`** (HP t740, node name
|
repo's gates — `reuse_refs_check` and `instructions_gate`, both the **shared** copies in
|
||||||
`felhom-host`, `ssh demo-hp` — no baked key; break-glass root via hub `host_recovery/demo-hp-bb76ea` +
|
`felhom.eu/scripts/`, never copied into this repo (a copy recreates the drift they detect; an absent
|
||||||
`sshpass`) is the **designated drill+build VM host** per the 2026-07-25 operator ruling, and that ruling
|
sibling clone FAILS). `--fast` selects the gates touching no network and no container runtime; today
|
||||||
is **realized** — it hosts drill VM `300` (`drill-r50`), so **start there**, not on DooPlex. (The
|
that is all of them. **A missing gate is a FAILURE, never a skip.**
|
||||||
historical golden-bake `drill.qcow2` still lives on DooPlex and is a bake fixture, not a drill target.)
|
|
||||||
**Which box is safe to break, and what may be done to each:
|
|
||||||
`felhom.eu/documentation/runbooks/target-selection.md`** — read it before any destructive test. Both
|
|
||||||
nodes + the break-glass recipe: `felhom.eu/documentation/operations/nodes.md`. The agent pins the served leaf cert — verify the
|
|
||||||
fingerprint still matches before a live run. Selftest modes (run locally on DooPlex, pointed at the
|
|
||||||
demo API): `--selftest[=read|task|hub|storage|backup|restore-test|pbs-verify]`; no flag = the daemon.
|
|
||||||
|
|
||||||
> **TEMPORARY — felhom-pve is at a remote site (until ~2026-08-02).** The home-LAN literal
|
**The pre-push hook** (`.githooks/pre-push`) runs it with `--fast` and refuses a failing push. It is
|
||||||
> `192.168.0.162` is NOT reachable from DooPlex for the duration. Access via Tailscale:
|
**per-clone** — switch it on once with `git config core.hooksPath .githooks`, and a manual run WARNS
|
||||||
> felhom-pve = 100.70.170.35; the `Host felhom-pve` entry in `~/.ssh/config` on DooPlex already
|
when this clone is unarmed. `git push --no-verify` bypasses it deliberately; **say so in the session
|
||||||
> points there (the direct-LAN path stays available as `Host felhom-pve-lan`). Delete this block on
|
report when you use it** — CI re-runs the same entry point on every push and **emails the operator on
|
||||||
> return. All documented `ssh felhom-pve` / `pct exec` workflows are unchanged. Path is **direct**
|
failure**, so a bypass is noticed even though it is not blocked (R-168, CLOSED 2026-08-02).
|
||||||
> (not DERP), ~37 ms rtt per hop. At the remote site the host is on **DHCP**; re-check its address
|
|
||||||
> rather than trusting one written here (`ip -br addr show vmbr0` — it read `192.168.0.162/24` on
|
|
||||||
> 2026-07-30, and `felhom-pve-lan` from DooPlex is still `No route to host`). Details + findings:
|
|
||||||
> `felhom.eu/documentation/audits/AUDIT-vacation-remote-ops-2026-07-20.md`
|
|
||||||
>
|
|
||||||
> **The "agent does not run at the remote site" warning this block used to carry is RETRACTED
|
|
||||||
> (2026-07-30) — it was true before R-50 and is false now.** `localapi` no longer binds a LAN literal:
|
|
||||||
> since the R-50 island migration (2026-07-25) it binds `169.254.253.1:8443` on `vmbr9`, which is
|
|
||||||
> location-independent by design, and `proxmox.endpoint` is `https://127.0.0.1:8006`. Verified live:
|
|
||||||
> `systemctl is-active felhom-agent` → `active`, `felhom-agent --version` → 0.115.0, and the per-guest
|
|
||||||
> local API answered `GET /disks` over the island. No config edit and no Viktor GO are outstanding.
|
|
||||||
|
|
||||||
> **Legacy: Windows workstation.** Until 2026-07-19 CC ran on Windows 11; `pct` commands over SSH
|
<!--
|
||||||
> needed `export MSYS_NO_PATHCONV=1`, and every remote command used
|
WHY ONE ENTRY POINT (2026-08-02, R-29): a census of all gates across the four repos found every check
|
||||||
> `SSH=/c/Windows/System32/OpenSSH/ssh.exe`. Agent deploy was a two-hop copy via the Windows box
|
a CLAUDE.md names was passing, and two of the four nobody is told to run were failing. This repo was
|
||||||
> (`cygpath -w` for the local scp path; CRLF hazard on config files).
|
the extreme case — nothing ran against it at all, and 90 cited paths were checked by no one.
|
||||||
|
-->
|
||||||
|
|
||||||
|
## Live validation — the fence
|
||||||
|
|
||||||
|
Exercise the **SERVER-SIDE PIPELINE** a real user triggers, end-to-end. **The forbidden shortcut is
|
||||||
|
BYPASSING it** — the F9 episode was a raw guest-attach with hand-set state, and it proved nothing.
|
||||||
|
|
||||||
|
`claude-in-chrome` is NOT available on DooPlex. Invoking the exact endpoint the UI invokes is an
|
||||||
|
acceptable proxy — **say which method was used**. Low-level mechanism tests where the direct call IS
|
||||||
|
the mechanism are exempt.
|
||||||
|
|
||||||
## Conventions
|
## Conventions
|
||||||
|
|
||||||
### Trunk-based — no branches
|
- **Trunk-based — no branches.** All shippable work commits directly to `main`; `main` equals what is
|
||||||
|
deployed. Report-only artifacts (audits, findings, fixspecs) go to `felhom.eu/documentation/`.
|
||||||
All shippable work commits **directly to `main`**; `main` equals what is deployed.
|
- **Unattended escape hatch:** if a fix cannot be cleanly verified and shipped, **revert and report**
|
||||||
- Report-only artifacts (audits, findings, fixspecs) → `felhom.eu/documentation/` (`audits/`, `backlog/`).
|
— never park it on a branch.
|
||||||
- Risky/supervised fixes are spec'd, then implemented **during the supervised session, on `main`**.
|
- **Logging**: the slog logger fans out to journald (configured level) plus the always-DEBUG
|
||||||
- Unattended escape hatch: if a fix can't be cleanly verified/shipped, revert + report — never park on a branch.
|
`applog.Ring` (remote pulls). English, keys-never-values, durations on outcomes. Full rules:
|
||||||
|
|
||||||
> **In every repository where you make a change, update both files in that repo:**
|
|
||||||
> - **`CHANGELOG.md`** — cumulative log, newest on top.
|
|
||||||
> - **`REPORT.md`** — **overwrite** with the most recent implementation/validation summary only.
|
|
||||||
>
|
|
||||||
> **Never write secrets** into any committed file — reference them as "stored out-of-band".
|
|
||||||
|
|
||||||
- Code quality: verify generated code for bugs/edge cases; add debug logging; **ask rather than
|
|
||||||
guess** when you'd otherwise invent input/output.
|
|
||||||
- **A health check issues no block I/O** — no `statfs`, no `getdents`, no read, write or `fsync`, **not
|
|
||||||
even behind a timeout**. Liveness is decided from `/proc` and kernel state. The full rule + the
|
|
||||||
measurement lives in `felhom.eu/CLAUDE.md` "Code quality rules"; it is repeated here because health
|
|
||||||
checks are written in THIS repo and that file does not load in an agent-only session. R-117 spike §6.3.
|
|
||||||
- Update `REUSE.md` if you added/changed/deprecated a shared helper or pattern (same commit).
|
|
||||||
- **Run `python3 scripts/agent_gates.py` from the repo root after ANY change in this repo.** It is
|
|
||||||
the ONE entry point for this repo's gates. Today it runs one — `reuse_refs_check` over this
|
|
||||||
repo's `REUSE.md` — and it exists at one gate on purpose: a census on 2026-08-02 found that every
|
|
||||||
check a `CLAUDE.md` names was passing and two of the four nobody is told to run were failing, and
|
|
||||||
this repo was the extreme case, with nothing running against it at all and 90 cited paths checked
|
|
||||||
by no one. It grows when the agent grows a second check. `--fast` selects the gates that touch no
|
|
||||||
network and no container runtime; today that is all of them. A missing gate is a FAILURE, never a
|
|
||||||
skip. **The shared `reuse_refs_check.py` lives in `felhom.eu/scripts/` and is never copied here**
|
|
||||||
— a copy would recreate the drift it detects; an absent sibling clone FAILS the gate.
|
|
||||||
**The pre-push hook** (`.githooks/pre-push`) runs it with `--fast` and refuses a failing push. It
|
|
||||||
is per-clone — switch it on once with `git config core.hooksPath .githooks`, and a manual run
|
|
||||||
WARNS when this clone is unarmed. `git push --no-verify` bypasses it deliberately; **say so in the
|
|
||||||
session report when you use it.** Both facts are why CI is still owed (`OPEN-ITEMS.md` R-168).
|
|
||||||
- Testing doctrine (non-hollow tests, red-proofs, seams): use the `felhom-testing` skill.
|
|
||||||
- **Logging**: the slog logger fans out to journald (configured level) + the always-DEBUG `applog.Ring`
|
|
||||||
(remote pulls) — English, keys-never-values, durations on outcomes; full rules in
|
|
||||||
`felhom.eu/documentation/runbooks/logging-conventions.md`.
|
`felhom.eu/documentation/runbooks/logging-conventions.md`.
|
||||||
|
- Update `REUSE.md` in the same commit that adds, changes or deprecates a shared helper or pattern.
|
||||||
|
|
||||||
### Live validation
|
## End-of-session checklist
|
||||||
|
|
||||||
Exercise the SERVER-SIDE PIPELINE a real user triggers, end-to-end. The forbidden shortcut is
|
- **`CHANGELOG.md`** (cumulative, newest on top) and **`REPORT.md`** (overwritten with this run only)
|
||||||
BYPASSING it (the F9 episode: raw guest-attach + hand-set state). Invoking the exact endpoint the UI
|
— in every repo touched.
|
||||||
invokes is an acceptable proxy when a browser isn't available — say which method was used. Low-level
|
- **`CONTEXT.md`** — decisions, state, what is next.
|
||||||
mechanism tests where the direct call IS the mechanism are exempt.
|
- **`REUSE.md`** — if a shared helper or pattern moved.
|
||||||
|
- **A finding goes in `felhom.eu/documentation/backlog/OPEN-ITEMS.md` first**, never only in a report
|
||||||
## Workflow & artifacts
|
or an audit.
|
||||||
|
- **Confirm your own last push's CI run went green, by run ID** — CI mails on failure, which is a PUSH
|
||||||
- Implement **`TASK.md` / `TASK-*.md`** specs (when placed as `TASK.md` or told to), then push +
|
signal; this is the PULL check that catches a lost or unread mail. An unchecked green is an
|
||||||
CHANGELOG + REPORT.md.
|
assumption, not an observation.
|
||||||
- **`RUNBOOK-*.md`** — an operational procedure. CC executes the steps it has access and capability
|
|
||||||
for, including live validation on the demo Proxmox host (CC has root@felhom-pve SSH + the
|
|
||||||
felhom-agent token). Mark a step HUMAN only when it genuinely needs physical presence, a real-world
|
|
||||||
decision, or credentials CC truly lacks. Judgment still applies: confirm before irreversible ops on
|
|
||||||
real customer data — demo scratch guests are fair game.
|
|
||||||
|
|||||||
@@ -1,53 +1,41 @@
|
|||||||
# REPORT — felhom-agent v0.126.0: a fetch failure is not a wrong recovery code (R-224)
|
# REPORT — felhom-agent v0.127.0: a mount Felhom made is not foreign (R-220)
|
||||||
|
|
||||||
**Scope: this repo's half of R-224.** The controller half ships as felhom-controller v0.202.0.
|
**Scope: the host half of R-220.** The customer-facing refusal message is the controller's half and
|
||||||
|
ships as felhom-controller v0.203.0.
|
||||||
## Why the agent changed at all
|
|
||||||
|
|
||||||
The task that commissioned this work scoped `felhom-agent` as **untouched**. It could not be. Its
|
|
||||||
Scenario A (a hub outage must not blame the customer's code) and Scenario C (a genuine mistype must be
|
|
||||||
told to re-check the ten words) are **mutually unsatisfiable** while this agent answers both with one
|
|
||||||
HTTP 400 and one sentence. No value available to the controller separates them. The task's own §5
|
|
||||||
anticipates this — *"if the step is not recoverable from the value, make it so, and say what that
|
|
||||||
cost"* — and §4.3 says the source outranks the register's recorded shape. **The cost is this version,
|
|
||||||
a publish, and a `MinAgent` coupling on the controller side.**
|
|
||||||
|
|
||||||
## What changed
|
## What changed
|
||||||
|
|
||||||
| File | Change |
|
| File | Change |
|
||||||
|---|---|
|
|---|---|
|
||||||
| `internal/escrow/recover.go` | new `ErrBundleFetch` sentinel; the fetch leg joins it with `%w: %w` so the cause survives for the operator log |
|
| `internal/storage/claim.go` | `claimFacts.felhomOwnedMounts`; `classifyClaim` forgives a non-managed mountpoint **only when corroborated**; `felhomOwnedMounts()` + `procMounts()` |
|
||||||
| `internal/localapi/escrow_recover.go` | new `case errors.Is(err, escrow.ErrBundleFetch)` → **502** with its own words; the `default` now carries only the wrong-code case and drops the "or" |
|
| `internal/storage/hostops.go` | `mountTable` seam (nil ⇒ real `/proc/mounts`) |
|
||||||
| `internal/escrow/recover_test.go` | three new tests; the pre-existing `FetchErrorIsDistinct` re-pointed from a string to the sentinel, with the reason it failed to protect |
|
| `internal/storage/claim_r220_test.go` | new — the own-drive case, the fence, and the corroboration's four edges |
|
||||||
| `internal/localapi/escrow_recover_class_test.go` | new — the consequence-level test: four situations, four statuses |
|
|
||||||
|
|
||||||
**Four statuses:** `502` fetch failed (the code was **not used**) · `400` fetched and refused ·
|
## The shape chosen, and why (§7.3)
|
||||||
`404` no bundle · `409` bundle predates the field.
|
|
||||||
|
|
||||||
## The finding this turned up
|
**Candidate (b): the claimed check distinguishes a mount Felhom made from a foreign one** — the task
|
||||||
|
called it "nearer the truth" and it is, because the host and its knowledge survive the rebuild while
|
||||||
|
the guest's registry does not. Candidate (a) — having the rebuild path clear the raw mounts — would
|
||||||
|
have made correctness depend on a cleanup step running, and a cleanup that does not run leaves exactly
|
||||||
|
today's defect.
|
||||||
|
|
||||||
**A green test named the defect and did not prevent it.** `TestRecoverOffsiteRepoPassword_FetchErrorIsDistinct`
|
**The discriminator is corroboration, not a path prefix**: the same device must ALSO be mounted under
|
||||||
has asserted since v0.125.0 that *"the operator must not be sent to re-read their recovery code because
|
`/mnt/felhom-drives`. Only enrolment produces that pairing.
|
||||||
the hub was unreachable"*. It passed throughout, because it checked this package's error **string** one
|
|
||||||
layer below the local-api `default` that did the merging — and a string is not something a caller can
|
**`/proc/mounts` rather than `lsblk MOUNTPOINTS`**, because the lsblk invocation is pinned verbatim in
|
||||||
branch on. **Mechanism asserted, consequence unpinned**; the project's own rule names this exact case.
|
the sudoers file; changing it would have coupled this fix to a config rollout. `/proc/mounts` is
|
||||||
It is also a comment-vs-code entry: `recover.go`'s header said the errors were *"DISTINCT on purpose"*
|
world-readable and needs neither.
|
||||||
and named **three** situations while a fourth was silently folded into one of them.
|
|
||||||
|
|
||||||
## Green gate
|
## Green gate
|
||||||
|
|
||||||
`go build ./...` clean · `go vet ./...` clean · `go test ./...` → **29 packages ok** ·
|
`go build` · `go vet` clean · `go test ./...` → **29 packages ok** · `agent_gates.py --fast` → all OK.
|
||||||
`python3 scripts/agent_gates.py --fast` → all gates OK.
|
|
||||||
|
|
||||||
**Red-proofs, each demonstrated failing then restored:**
|
| Red-proof | Result |
|
||||||
|
|
||||||
| Mutation | Result |
|
|
||||||
|---|---|
|
|---|---|
|
||||||
| remove the `%w: %w` join (pre-R-224 wrap) | `FetchFailureIsClassifiedAsFetch` **FAILS** |
|
| remove the `felhomOwnedMounts` exemption | **FAILS** — "device is mounted at /mnt/adatok (sdb)", the pre-fix refusal |
|
||||||
| delete the `ErrBundleFetch` handler case | fetch answers `400 "the recovery code did not open the sealed bundle"` — **the exact defect**, and both status tests **FAIL** |
|
| over-widen the exemption to any `/mnt/*` | **FAILS** — "/mnt/someone-elses-disk was offered for formatting" |
|
||||||
|
|
||||||
## Not changed
|
## Not changed
|
||||||
|
|
||||||
No Proxmox surface, no privileged path, no report/hub contract, no config schema. The route's
|
No sudoers, no allowlisted command, no PVE surface, no format path. Every other claim signal
|
||||||
success path, its scoping and its R-handling discipline (`R = ""` on both paths, never logged, never
|
(system disk, read-only, LVM PV, ZFS member, member FSTYPEs, empty-topology backstop) is untouched.
|
||||||
persisted) are untouched.
|
|
||||||
|
|||||||
@@ -68,8 +68,14 @@ type HostReport struct {
|
|||||||
// report is stored opaquely hub-side, so these additive fields need no hub-schema change.
|
// report is stored opaquely hub-side, so these additive fields need no hub-schema change.
|
||||||
// Both are `omitempty` (the Wireguard precedent): in the steady state (no update in flight)
|
// Both are `omitempty` (the Wireguard precedent): in the steady state (no update in flight)
|
||||||
// they are absent — which keeps the cross-repo host-report golden contract byte-stable without
|
// they are absent — which keeps the cross-repo host-report golden contract byte-stable without
|
||||||
// a hub change. They appear only while an update is pending. The hub reads an absent field as
|
// a hub change. They appear only while an update is pending.
|
||||||
// pending=false, the correct default.
|
//
|
||||||
|
// ⚠ CORRECTED 2026-08-08 (R-260). This comment used to end "The hub reads an absent field as
|
||||||
|
// pending=false, the correct default." THE HUB HAS NO FIELD FOR EITHER OF THESE, so it reads
|
||||||
|
// nothing — present or absent — and encoding/json discards them on arrival. The sentence
|
||||||
|
// described an intent, not the code, and it read as settled for long enough that a sweep had to
|
||||||
|
// find it. The emission is correct and stays; the missing consumer is tracked as R-264, and
|
||||||
|
// `felhom.eu/scripts/wire_contract_gate.py` now refuses any NEW field of this shape.
|
||||||
SelfUpdatePending bool `json:"selfupdate_pending,omitempty"`
|
SelfUpdatePending bool `json:"selfupdate_pending,omitempty"`
|
||||||
SelfUpdatePendingVersion string `json:"selfupdate_pending_version,omitempty"`
|
SelfUpdatePendingVersion string `json:"selfupdate_pending_version,omitempty"`
|
||||||
|
|
||||||
|
|||||||
@@ -283,7 +283,36 @@ func (m *Manager) Apply(ctx context.Context, fetched bool, block *hub.WirePBSDR)
|
|||||||
h := descriptorHash(block)
|
h := descriptorHash(block)
|
||||||
cf := m.loadConsumedFailed()
|
cf := m.loadConsumedFailed()
|
||||||
if mk := m.loadMarker(); mk != nil && mk.Hash == h && (cf == nil || cf.Hash != h) {
|
if mk := m.loadMarker(); mk != nil && mk.Hash == h && (cf == nil || cf.Hash != h) {
|
||||||
m.setStatus(&hub.PBSDRStatus{State: mk.State, StorageID: block.StorageID, Namespace: block.Namespace, AppliedAt: mk.AppliedAt})
|
// R-221: RE-ASSERT THE SEED, DO NOT REMEMBER IT. The marker records that this descriptor
|
||||||
|
// converged; it says nothing about whether the file the seed writes still exists.
|
||||||
|
//
|
||||||
|
// The two live in different places and die at different times. The marker is host-side
|
||||||
|
// (`<agent-state>/pbsdr/`, markerPath above); the seed's target is `agent.json`, and the
|
||||||
|
// installer's `step_agent_config` renders that file from `base = {}` unless an explicit
|
||||||
|
// `--preserve-from` is passed — it NEVER writes an `escrow` section — then replaces it with
|
||||||
|
// O_TRUNC (felhom-host-install.sh:2396, :2449, :2579; the flag is :1246, defaulting empty at
|
||||||
|
// :256). So a rebuild leaves the marker and takes the seed, the hash still matches, this
|
||||||
|
// branch returns, and `escrow.pbs_storage_id` is never written again. The customer then
|
||||||
|
// cannot run the escrow ceremony AT ALL: handleEscrowPreflight fails the `pbs_storage_id`
|
||||||
|
// row and the wizard refuses, with no way forward from inside the product.
|
||||||
|
//
|
||||||
|
// A rebuild is only the case that was measured. The same hole opens for a hand-edited or
|
||||||
|
// restored config, which is the honest reason this is a seam fix rather than an installer
|
||||||
|
// fix — the seed must be a thing the loop asserts, not a thing it did once.
|
||||||
|
//
|
||||||
|
// COST: this runs on the converged path, i.e. every tick (60 s) forever. It is one small
|
||||||
|
// file read plus a JSON parse — no exec, no network, no Proxmox call — and seedEscrowStorageID
|
||||||
|
// returns early once the value matches. That is the whole reason it is affordable here.
|
||||||
|
//
|
||||||
|
// IT MUST NEVER UN-CONVERGE THE BOX: a failure is a Warn plus a message on the published
|
||||||
|
// status, exactly as finishConverged does it. No marker write, no state change, no retry
|
||||||
|
// storm — the early return below still happens either way.
|
||||||
|
msg := ""
|
||||||
|
if err := m.seedEscrowStorageID(block.StorageID); err != nil {
|
||||||
|
msg = "escrow.pbs_storage_id seed failed: " + err.Error() + " (set it manually before the ceremony)"
|
||||||
|
m.logger.Warn("pbsdr: " + msg)
|
||||||
|
}
|
||||||
|
m.setStatus(&hub.PBSDRStatus{State: mk.State, StorageID: block.StorageID, Namespace: block.Namespace, AppliedAt: mk.AppliedAt, Message: msg})
|
||||||
return // idempotent: this exact descriptor already converged
|
return // idempotent: this exact descriptor already converged
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,232 @@
|
|||||||
|
package pbsdr
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"encoding/json"
|
||||||
|
"go/ast"
|
||||||
|
"go/parser"
|
||||||
|
"go/printer"
|
||||||
|
"go/token"
|
||||||
|
"io"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
|
||||||
|
)
|
||||||
|
|
||||||
|
// R-221 — the escrow seed must be ASSERTED on every converged tick, not remembered.
|
||||||
|
//
|
||||||
|
// These tests drive the real Apply() with a real temp-dir agent.json and a call-recording runner.
|
||||||
|
// Calling seedEscrowStorageID directly would prove nothing: the defect IS the early return in
|
||||||
|
// Apply, and a test that steps around it cannot see it.
|
||||||
|
|
||||||
|
// convergedMarker writes a marker whose hash matches the block, i.e. puts the manager on exactly
|
||||||
|
// the idempotent path where the seed used to be skipped.
|
||||||
|
func convergedMarker(t *testing.T, m *Manager, block *hub.WirePBSDR, state string) {
|
||||||
|
t.Helper()
|
||||||
|
if err := m.writeState(m.markerPath(), marker{
|
||||||
|
Hash: descriptorHash(block), State: state, AppliedAt: "2026-08-08T00:00:00Z",
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("write marker: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func escrowStorageID(t *testing.T, cfgPath string) string {
|
||||||
|
t.Helper()
|
||||||
|
raw, err := os.ReadFile(cfgPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("read config: %v", err)
|
||||||
|
}
|
||||||
|
var doc struct {
|
||||||
|
Escrow struct {
|
||||||
|
PBSStorageID string `json:"pbs_storage_id"`
|
||||||
|
} `json:"escrow"`
|
||||||
|
}
|
||||||
|
if err := json.Unmarshal(raw, &doc); err != nil {
|
||||||
|
t.Fatalf("parse config: %v", err)
|
||||||
|
}
|
||||||
|
return doc.Escrow.PBSStorageID
|
||||||
|
}
|
||||||
|
|
||||||
|
// drBlock mirrors the existing valid fixture (manager_test.go descriptor()) so that validate()
|
||||||
|
// passes and Apply actually reaches the marker check — the branch these tests are about.
|
||||||
|
func drBlock(storageID string) *hub.WirePBSDR {
|
||||||
|
return &hub.WirePBSDR{
|
||||||
|
Enabled: true, StorageID: storageID, PBSTunnelIP: "10.77.0.1",
|
||||||
|
Datastore: "felhom-offsite", Namespace: "peti", TokenID: "felhom@pbs!peti",
|
||||||
|
Fingerprint: testFP,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// SCENARIO A — the seed is re-asserted on a converged box, and nothing else happens.
|
||||||
|
//
|
||||||
|
// THIS IS THE TEST THAT MATTERS. It must fail against the pre-R-221 tree; if it passes there, it is
|
||||||
|
// not testing the defect and that is the finding.
|
||||||
|
func TestSeedReasserted_OnConvergedTick_WithZeroProxmoxCalls(t *testing.T) {
|
||||||
|
r := &fakeRunner{}
|
||||||
|
st := &fakeStorage{found: true, active: []bool{true}}
|
||||||
|
c := &fakeConsumer{}
|
||||||
|
m, cfgPath := newTestManager(t, r, st, c)
|
||||||
|
|
||||||
|
block := drBlock("felhom-pbs-dr")
|
||||||
|
convergedMarker(t, m, block, "applied")
|
||||||
|
|
||||||
|
// the shape a rebuild leaves behind: escrow section present, pbs_storage_id GONE
|
||||||
|
if got := escrowStorageID(t, cfgPath); got != "" {
|
||||||
|
t.Fatalf("precondition: config already carries a storage id %q", got)
|
||||||
|
}
|
||||||
|
|
||||||
|
m.Apply(context.Background(), true, block)
|
||||||
|
|
||||||
|
if got := escrowStorageID(t, cfgPath); got != "felhom-pbs-dr" {
|
||||||
|
t.Errorf("the converged tick did not re-assert the seed: escrow.pbs_storage_id = %q, want %q.\n"+
|
||||||
|
"This is R-221: the marker survives a rebuild, the descriptor hash still matches, the early "+
|
||||||
|
"return fires and the seed never runs into the config that no longer has it — so the customer "+
|
||||||
|
"cannot run the escrow ceremony at all.", got, "felhom-pbs-dr")
|
||||||
|
}
|
||||||
|
|
||||||
|
// ...and the idempotent path is STILL idempotent. This assertion is not decorative: without it
|
||||||
|
// a "fix" that simply deletes the early return would pass the line above.
|
||||||
|
if calls := r.recorded(); len(calls) != 0 {
|
||||||
|
t.Errorf("a converged tick must execute ZERO Proxmox commands; got %d: %+v", len(calls), calls)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// SCENARIO B — an operator's own different value survives, and the warning names both.
|
||||||
|
func TestSeedReasserted_NeverClobbersAnOperatorValue(t *testing.T) {
|
||||||
|
r := &fakeRunner{}
|
||||||
|
m, cfgPath := newTestManager(t, r, &fakeStorage{found: true, active: []bool{true}}, &fakeConsumer{})
|
||||||
|
|
||||||
|
if err := os.WriteFile(cfgPath, []byte(
|
||||||
|
`{"log_level":"info","escrow":{"posture":"zero_knowledge","pbs_storage_id":"operator-chosen"},`+
|
||||||
|
`"custom_unknown":{"keep":1}}`), 0o600); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
block := drBlock("hub-chosen")
|
||||||
|
convergedMarker(t, m, block, "applied")
|
||||||
|
|
||||||
|
m.Apply(context.Background(), true, block)
|
||||||
|
|
||||||
|
if got := escrowStorageID(t, cfgPath); got != "operator-chosen" {
|
||||||
|
t.Errorf("a value a person put there was overwritten by the descriptor: got %q, want %q", got, "operator-chosen")
|
||||||
|
}
|
||||||
|
// unknown keys must still round-trip
|
||||||
|
raw, _ := os.ReadFile(cfgPath)
|
||||||
|
if !strings.Contains(string(raw), "custom_unknown") {
|
||||||
|
t.Error("an unknown config key was dropped by the re-assert")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// SCENARIO C — the ceremony preflight's live read sees the re-asserted value with NO restart.
|
||||||
|
//
|
||||||
|
// The preflight itself lives in internal/localapi and reads the file through config.Load; what this
|
||||||
|
// asserts is the half that belongs to this package: after a converged tick, THE FILE ON DISK carries
|
||||||
|
// the id, so any live re-read is green. The daemon is never restarted in this test because it is
|
||||||
|
// never started — which is the point.
|
||||||
|
func TestSeedReasserted_IsVisibleOnDiskImmediately(t *testing.T) {
|
||||||
|
r := &fakeRunner{}
|
||||||
|
m, cfgPath := newTestManager(t, r, &fakeStorage{found: true, active: []bool{true}}, &fakeConsumer{})
|
||||||
|
block := drBlock("felhom-pbs-dr")
|
||||||
|
convergedMarker(t, m, block, "applied")
|
||||||
|
|
||||||
|
// preflight's predicate BEFORE: storageID == "" → the row is NOT OK
|
||||||
|
if escrowStorageID(t, cfgPath) != "" {
|
||||||
|
t.Fatal("precondition")
|
||||||
|
}
|
||||||
|
m.Apply(context.Background(), true, block)
|
||||||
|
// preflight's predicate AFTER, from the same file the ceremony subprocess loads
|
||||||
|
if id := escrowStorageID(t, cfgPath); id == "" {
|
||||||
|
t.Error("the preflight row would still be NOT OK after a converged tick")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A seed failure must NEVER un-converge the box: no marker rewrite, no state change, and the status
|
||||||
|
// still reports the marker's converged state — with the failure surfaced as a message.
|
||||||
|
func TestSeedReassertFailure_DoesNotUnconverge(t *testing.T) {
|
||||||
|
r := &fakeRunner{}
|
||||||
|
m, cfgPath := newTestManager(t, r, &fakeStorage{found: true, active: []bool{true}}, &fakeConsumer{})
|
||||||
|
block := drBlock("felhom-pbs-dr")
|
||||||
|
convergedMarker(t, m, block, "applied")
|
||||||
|
markerBefore, err := os.ReadFile(m.markerPath())
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// make the seed fail in a way it cannot recover from: unparseable config
|
||||||
|
if err := os.WriteFile(cfgPath, []byte(`{ this is not json`), 0o600); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
m.Apply(context.Background(), true, block)
|
||||||
|
|
||||||
|
after, err := os.ReadFile(m.markerPath())
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("the marker was removed by a seed failure: %v", err)
|
||||||
|
}
|
||||||
|
if string(after) != string(markerBefore) {
|
||||||
|
t.Error("a seed failure rewrote the convergence marker — it must not touch state")
|
||||||
|
}
|
||||||
|
if calls := r.recorded(); len(calls) != 0 {
|
||||||
|
t.Errorf("a seed failure must not trigger Proxmox work; got %+v", calls)
|
||||||
|
}
|
||||||
|
st := m.Status()
|
||||||
|
if st == nil || st.State != "applied" {
|
||||||
|
t.Errorf("a seed failure must leave the box converged; status = %+v", st)
|
||||||
|
}
|
||||||
|
if st != nil && !strings.Contains(st.Message, "seed failed") {
|
||||||
|
t.Errorf("a seed failure must be surfaced on the status, got message %q", st.Message)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// SEAM WIRING — production must construct the manager with the live config path, or the whole seed
|
||||||
|
// leg is inert. Three shipped defects in this project were fully green while their seam was never
|
||||||
|
// wired, so this walks main.go's AST for the actual call rather than grepping: a commented-out call
|
||||||
|
// satisfies strings.Contains, and an AST walk cannot see a comment.
|
||||||
|
func TestProductionWiring_NewManagerGetsTheLiveConfigPath(t *testing.T) {
|
||||||
|
path := filepath.Join("..", "..", "cmd", "felhom-agent", "main.go")
|
||||||
|
fset := token.NewFileSet()
|
||||||
|
f, err := parser.ParseFile(fset, path, nil, 0) // comments not even collected
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("parse main.go: %v", err)
|
||||||
|
}
|
||||||
|
found := false
|
||||||
|
ast.Inspect(f, func(n ast.Node) bool {
|
||||||
|
call, ok := n.(*ast.CallExpr)
|
||||||
|
if !ok {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
sel, ok := call.Fun.(*ast.SelectorExpr)
|
||||||
|
if !ok || sel.Sel.Name != "NewManager" {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
if pkg, ok := sel.X.(*ast.Ident); !ok || pkg.Name != "pbsdr" {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
// signature: (runner, px, hubc, stateDir, secretDir, configPath, logger)
|
||||||
|
if len(call.Args) < 6 {
|
||||||
|
t.Errorf("pbsdr.NewManager called with %d args, expected >= 6", len(call.Args))
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
var buf strings.Builder
|
||||||
|
if err := printNode(&buf, fset, call.Args[5]); err != nil {
|
||||||
|
t.Fatalf("print arg: %v", err)
|
||||||
|
}
|
||||||
|
got := buf.String()
|
||||||
|
if !strings.Contains(got, "SourcePath") {
|
||||||
|
t.Errorf("pbsdr.NewManager's configPath argument is %q, which is not the live config path.\n"+
|
||||||
|
"With an empty or wrong path seedEscrowStorageID returns nil immediately and the entire "+
|
||||||
|
"R-221 fix is inert while every test above still passes.", got)
|
||||||
|
}
|
||||||
|
found = true
|
||||||
|
return false
|
||||||
|
})
|
||||||
|
if !found {
|
||||||
|
t.Error("no pbsdr.NewManager call found in main.go — the manager is not constructed in production")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func printNode(w io.Writer, fset *token.FileSet, n ast.Node) error {
|
||||||
|
return printer.Fprint(w, fset, n)
|
||||||
|
}
|
||||||
@@ -53,7 +53,11 @@ type claimFacts struct {
|
|||||||
nodes []claimNode // the whole disk + its children (partitions)
|
nodes []claimNode // the whole disk + its children (partitions)
|
||||||
lvmPV bool // pvs (authoritative): the disk / a partition is an LVM physical volume
|
lvmPV bool // pvs (authoritative): the disk / a partition is an LVM physical volume
|
||||||
zfsMember bool // zpool (authoritative): the disk / a partition is a ZFS pool member
|
zfsMember bool // zpool (authoritative): the disk / a partition is a ZFS pool member
|
||||||
gatherErr string // non-empty ⇒ a REQUIRED read failed ⇒ fail-safe CLAIMED
|
// felhomOwnedMounts (R-220) — mountpoints OUTSIDE /mnt/felhom-drives that are nevertheless Felhom's
|
||||||
|
// OWN, corroborated from the host mount table: the same device is also mounted at the managed path.
|
||||||
|
// Empty means "nothing corroborated", which is the fail-safe direction.
|
||||||
|
felhomOwnedMounts map[string]bool
|
||||||
|
gatherErr string // non-empty ⇒ a REQUIRED read failed ⇒ fail-safe CLAIMED
|
||||||
}
|
}
|
||||||
|
|
||||||
// classifyClaim is the pure guard verdict. unclaimed=true ONLY when the device is provably free for
|
// classifyClaim is the pure guard verdict. unclaimed=true ONLY when the device is provably free for
|
||||||
@@ -81,7 +85,20 @@ func classifyClaim(f claimFacts) (unclaimed bool, reason string) {
|
|||||||
if memberFSTypes[n.fstype] {
|
if memberFSTypes[n.fstype] {
|
||||||
return false, "device holds a " + n.fstype + " (" + n.name + ")"
|
return false, "device holds a " + n.fstype + " (" + n.name + ")"
|
||||||
}
|
}
|
||||||
if n.mountpoint != "" && !underFelhomDrives(n.mountpoint) {
|
// ── R-220 — A MOUNT FELHOM ITSELF MADE IS NOT "SOMETHING ELSE". ───────────────────────
|
||||||
|
//
|
||||||
|
// Enrolment mounts a drive TWICE: at the managed path `/mnt/felhom-drives/<name>` and at the
|
||||||
|
// raw `/mnt/<name>` it creates on the host. The host — and therefore that raw mount — survives
|
||||||
|
// a guest rebuild, while the controller's registry does not. So after a rebuild the customer's
|
||||||
|
// own drives looked foreign, `attach` returned an empty list, and the refusal told them to
|
||||||
|
// choose from it. Measured live three times (CAMPAIGN-11 Phase 1, and the R-201 re-walk twice);
|
||||||
|
// unmounting only the raw mounts flipped `attach: []` to both drives every time.
|
||||||
|
//
|
||||||
|
// The fence this must NOT breach: a disk genuinely in use by something else stays refused. So
|
||||||
|
// the exemption is not "any /mnt/* path" — it is CORROBORATED: the same device must ALSO be
|
||||||
|
// mounted at Felhom's managed path, which is a state only Felhom's own enrolment produces.
|
||||||
|
// A foreign disk at /srv/data or /media/x has no such counterpart and is still refused.
|
||||||
|
if n.mountpoint != "" && !underFelhomDrives(n.mountpoint) && !f.felhomOwnedMounts[n.mountpoint] {
|
||||||
return false, "device is mounted at " + n.mountpoint + " (" + n.name + ")"
|
return false, "device is mounted at " + n.mountpoint + " (" + n.name + ")"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -154,6 +171,8 @@ func (h *SudoHostOps) gatherClaimFacts(ctx context.Context, device string) claim
|
|||||||
return f
|
return f
|
||||||
}
|
}
|
||||||
f.nodes = nodes
|
f.nodes = nodes
|
||||||
|
// R-220: corroborate which non-managed mountpoints are nevertheless Felhom's own.
|
||||||
|
f.felhomOwnedMounts = felhomOwnedMounts(device, nodes, h.mountTable)
|
||||||
|
|
||||||
// LVM PV (authoritative). pvs installed but erroring ⇒ fail-safe claimed; absent ⇒ rely on lsblk's
|
// LVM PV (authoritative). pvs installed but erroring ⇒ fail-safe claimed; absent ⇒ rely on lsblk's
|
||||||
// LVM2_member FSTYPE (already in nodes).
|
// LVM2_member FSTYPE (already in nodes).
|
||||||
@@ -285,3 +304,71 @@ func (h *SudoHostOps) zfsMembers(ctx context.Context, nodes []claimNode, wholeDi
|
|||||||
}
|
}
|
||||||
return false, nil
|
return false, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// mountTableSource yields the host mount table as (device, mountpoint) pairs. A seam so the R-220
|
||||||
|
// corroboration is unit-testable without a host. nil ⇒ the real /proc/mounts.
|
||||||
|
type mountTableSource func() ([][2]string, error)
|
||||||
|
|
||||||
|
// procMounts reads /proc/mounts — WORLD-READABLE, so this needs no sudo and no allowlisted command.
|
||||||
|
// That matters: the lsblk invocation is pinned verbatim in the sudoers file
|
||||||
|
// (`lsblk -J -o NAME,FSTYPE,PTTYPE,MOUNTPOINT /dev/*`), so switching it to the plural MOUNTPOINTS
|
||||||
|
// would have meant shipping a sudoers change with the binary — a far larger blast radius than this
|
||||||
|
// finding warrants. Reading the mount table directly sidesteps that entirely.
|
||||||
|
func procMounts() ([][2]string, error) {
|
||||||
|
data, err := os.ReadFile("/proc/mounts")
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
var out [][2]string
|
||||||
|
for _, line := range strings.Split(string(data), "\n") {
|
||||||
|
fields := strings.Fields(line)
|
||||||
|
if len(fields) < 2 {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
// /proc/mounts escapes spaces as \040; unescape so a path with a space still compares.
|
||||||
|
out = append(out, [2]string{fields[0], strings.ReplaceAll(fields[1], `\040`, " ")})
|
||||||
|
}
|
||||||
|
return out, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// felhomOwnedMounts returns the mountpoints of `device` (and its children) that sit OUTSIDE
|
||||||
|
// /mnt/felhom-drives but are still Felhom's own, corroborated by the same device also being mounted
|
||||||
|
// UNDER /mnt/felhom-drives. That pairing is what enrolment produces and nothing else does.
|
||||||
|
//
|
||||||
|
// ⚠ FAIL-SAFE: an unreadable mount table returns an EMPTY set, never a permissive one. The device then
|
||||||
|
// classifies exactly as it did before R-220 — refused — because "we could not corroborate" must never
|
||||||
|
// read as "it is ours".
|
||||||
|
func felhomOwnedMounts(device string, nodes []claimNode, src mountTableSource) map[string]bool {
|
||||||
|
if src == nil {
|
||||||
|
src = procMounts
|
||||||
|
}
|
||||||
|
table, err := src()
|
||||||
|
if err != nil {
|
||||||
|
return nil // unreadable ⇒ corroborate nothing
|
||||||
|
}
|
||||||
|
// Every device name this disk answers to: the whole disk and each child node.
|
||||||
|
devs := map[string]bool{device: true}
|
||||||
|
if wd, ok := wholeDiskOf(device); ok {
|
||||||
|
devs[wd] = true
|
||||||
|
}
|
||||||
|
for _, n := range nodes {
|
||||||
|
devs["/dev/"+n.name] = true
|
||||||
|
}
|
||||||
|
// A device is Felhom-managed only if it is mounted under the managed prefix.
|
||||||
|
managed := map[string]bool{}
|
||||||
|
for _, row := range table {
|
||||||
|
if devs[row[0]] && underFelhomDrives(row[1]) {
|
||||||
|
managed[row[0]] = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if len(managed) == 0 {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
owned := map[string]bool{}
|
||||||
|
for _, row := range table {
|
||||||
|
if managed[row[0]] && !underFelhomDrives(row[1]) {
|
||||||
|
owned[path.Clean(row[1])] = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return owned
|
||||||
|
}
|
||||||
|
|||||||
@@ -0,0 +1,102 @@
|
|||||||
|
package storage
|
||||||
|
|
||||||
|
import "testing"
|
||||||
|
|
||||||
|
// ── R-220 — A MOUNT FELHOM ITSELF MADE IS NOT "SOMETHING ELSE" ──────────────────────────────────
|
||||||
|
//
|
||||||
|
// Enrolment mounts a drive twice: at `/mnt/felhom-drives/<name>` and at the raw `/mnt/<name>` it
|
||||||
|
// creates on the host. The host survives a guest rebuild; the controller's registry does not. So after
|
||||||
|
// a rebuild the customer's own drives read as claimed-by-something-else, `attach` came back empty, and
|
||||||
|
// the refusal told them to pick from the empty list. Measured three times live.
|
||||||
|
//
|
||||||
|
// The fence: a disk genuinely in use elsewhere must STILL be refused. These assert both directions.
|
||||||
|
|
||||||
|
// ── SCENARIO E — the customer's own drive is offered again after a rebuild ───────────────────────
|
||||||
|
//
|
||||||
|
// RED-PROOF: drop `&& !f.felhomOwnedMounts[n.mountpoint]` from classifyClaim — the pre-R-220 check —
|
||||||
|
// and this FAILS with the drive refused and the list empty again.
|
||||||
|
func TestClassifyClaim_R220_FelhomsOwnRawMountIsNotForeign(t *testing.T) {
|
||||||
|
f := claimFacts{
|
||||||
|
device: "/dev/sdb", wholeDisk: "/dev/sdb", wholeDiskOK: true,
|
||||||
|
nodes: []claimNode{{name: "sdb", fstype: "ext4", mountpoint: "/mnt/adatok"}},
|
||||||
|
// corroborated: the SAME device is also mounted at the managed path
|
||||||
|
felhomOwnedMounts: map[string]bool{"/mnt/adatok": true},
|
||||||
|
}
|
||||||
|
unclaimed, reason := classifyClaim(f)
|
||||||
|
if !unclaimed {
|
||||||
|
t.Fatalf("R-220 RETURNED: the customer's own drive is refused after a rebuild — %q", reason)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── SCENARIO F — a genuinely foreign mount is STILL refused ──────────────────────────────────────
|
||||||
|
//
|
||||||
|
// RED-PROOF: over-widen the fix to exempt any /mnt/* path (or to skip the mountpoint check entirely)
|
||||||
|
// and this FAILS — a disk another system is using would be offered for formatting.
|
||||||
|
func TestClassifyClaim_R220_ForeignMountIsStillRefused(t *testing.T) {
|
||||||
|
for _, mp := range []string{"/srv/data", "/media/photos", "/mnt/someone-elses-disk", "/var/lib/other"} {
|
||||||
|
f := claimFacts{
|
||||||
|
device: "/dev/sdb", wholeDisk: "/dev/sdb", wholeDiskOK: true,
|
||||||
|
nodes: []claimNode{{name: "sdb", fstype: "ext4", mountpoint: mp}},
|
||||||
|
felhomOwnedMounts: nil, // nothing corroborated it as ours
|
||||||
|
}
|
||||||
|
unclaimed, reason := classifyClaim(f)
|
||||||
|
if unclaimed {
|
||||||
|
t.Fatalf("THE FENCE BROKE: a disk mounted at %s was offered for formatting", mp)
|
||||||
|
}
|
||||||
|
if reason == "" {
|
||||||
|
t.Fatalf("a refusal must carry a reason (%s)", mp)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The corroboration itself: it must require BOTH mounts of the SAME device, and fail safe.
|
||||||
|
func TestFelhomOwnedMounts_RequiresTheManagedCounterpart(t *testing.T) {
|
||||||
|
nodes := []claimNode{{name: "sdb"}}
|
||||||
|
|
||||||
|
t.Run("both mounts present -> the raw one is ours", func(t *testing.T) {
|
||||||
|
src := func() ([][2]string, error) {
|
||||||
|
return [][2]string{
|
||||||
|
{"/dev/sdb", "/mnt/adatok"},
|
||||||
|
{"/dev/sdb", "/mnt/felhom-drives/adatok"},
|
||||||
|
}, nil
|
||||||
|
}
|
||||||
|
got := felhomOwnedMounts("/dev/sdb", nodes, src)
|
||||||
|
if !got["/mnt/adatok"] {
|
||||||
|
t.Fatal("the raw enrolment mount was not recognised as Felhom's own")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("only the raw mount -> corroborates NOTHING", func(t *testing.T) {
|
||||||
|
src := func() ([][2]string, error) {
|
||||||
|
return [][2]string{{"/dev/sdb", "/mnt/adatok"}}, nil
|
||||||
|
}
|
||||||
|
if got := felhomOwnedMounts("/dev/sdb", nodes, src); len(got) != 0 {
|
||||||
|
t.Fatalf("a lone /mnt/<name> mount must corroborate nothing, got %v", got)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("a DIFFERENT device under the managed path does not vouch for this one", func(t *testing.T) {
|
||||||
|
src := func() ([][2]string, error) {
|
||||||
|
return [][2]string{
|
||||||
|
{"/dev/sdb", "/srv/data"},
|
||||||
|
{"/dev/sdc", "/mnt/felhom-drives/mentes"}, // someone else's, not sdb's
|
||||||
|
}, nil
|
||||||
|
}
|
||||||
|
if got := felhomOwnedMounts("/dev/sdb", nodes, src); got["/srv/data"] {
|
||||||
|
t.Fatal("another device's managed mount vouched for a foreign one")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("an unreadable mount table corroborates NOTHING (fail-safe)", func(t *testing.T) {
|
||||||
|
src := func() ([][2]string, error) { return nil, errRead }
|
||||||
|
if got := felhomOwnedMounts("/dev/sdb", nodes, src); len(got) != 0 {
|
||||||
|
t.Fatalf("an unreadable mount table must corroborate nothing, got %v", got)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
var errRead = errNoMountTable{}
|
||||||
|
|
||||||
|
type errNoMountTable struct{}
|
||||||
|
|
||||||
|
func (errNoMountTable) Error() string { return "mount table unreadable" }
|
||||||
@@ -164,6 +164,9 @@ type SudoHostOps struct {
|
|||||||
// UNPRIVILEGED read (`systemctl is-failed`) — seam-injected so the reassert's F10 reset-failed path
|
// UNPRIVILEGED read (`systemctl is-failed`) — seam-injected so the reassert's F10 reset-failed path
|
||||||
// is unit-testable without a real systemd. Default set in NewSudoHostOps.
|
// is unit-testable without a real systemd. Default set in NewSudoHostOps.
|
||||||
unitFailed func(ctx context.Context, unit string) bool
|
unitFailed func(ctx context.Context, unit string) bool
|
||||||
|
// mountTable (R-220) yields the host mount table for the "is this mount Felhom's own?"
|
||||||
|
// corroboration. nil ⇒ the real /proc/mounts; tests inject.
|
||||||
|
mountTable mountTableSource
|
||||||
}
|
}
|
||||||
|
|
||||||
// SudoHostOpsConfig configures a SudoHostOps.
|
// SudoHostOpsConfig configures a SudoHostOps.
|
||||||
|
|||||||
@@ -41,10 +41,13 @@ import sys
|
|||||||
|
|
||||||
ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||||
SHARED_REUSE = os.path.join(os.path.dirname(ROOT), "felhom.eu", "scripts", "reuse_refs_check.py")
|
SHARED_REUSE = os.path.join(os.path.dirname(ROOT), "felhom.eu", "scripts", "reuse_refs_check.py")
|
||||||
|
SHARED_INSTRUCTIONS = os.path.join(
|
||||||
|
os.path.dirname(ROOT), "felhom.eu", "scripts", "instructions_gate.py")
|
||||||
|
|
||||||
# (label, absolute script path, args, fast)
|
# (label, absolute script path, args, fast)
|
||||||
GATES = [
|
GATES = [
|
||||||
("reuse-refs", SHARED_REUSE, [ROOT], True),
|
("reuse-refs", SHARED_REUSE, [ROOT], True),
|
||||||
|
("instructions", SHARED_INSTRUCTIONS, [ROOT], True),
|
||||||
("published", os.path.join(ROOT, "scripts", "check-published-versions.py"), [], False),
|
("published", os.path.join(ROOT, "scripts", "check-published-versions.py"), [], False),
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user