docs: felhom-agent CLAUDE.md becomes a core plus path-scoped rules (R-229 leg b)
gates / gates (push) Successful in 8s
gates / gates (push) Successful in 8s
175 -> 99 effective lines. New .claude/rules/{proxmox,localapi,backup,storage}.md alongside the
existing health-checks.md. The release section points at the felhom-build-deploy skill rather than
restating a table that drifts from the script; the layout section's per-package annotations moved
into the rule file for their area instead of being deleted.
Kept in the core because it is the only part re-injected after /compact: the root-CLI fence and its
three exceptions, the destructive-op gate, prove-ownership (audit A1), the gate entry point, the F9
live-validation fence, and the checklist.
health-checks.md overlaps localapi.md and storage.md on three globs -- deliberate, both load,
stated in each file. Go build/vet/test green and unchanged.
This commit is contained in:
@@ -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,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.
|
||||
-->
|
||||
Reference in New Issue
Block a user