From 9e5bbc13d5efc333547b0bf3b0c09c643686e48a Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Wed, 1 Jul 2026 15:14:27 +0200 Subject: [PATCH] =?UTF-8?q?docs:=20SPIKE=20drive-enrollment=20=E2=80=94=20?= =?UTF-8?q?raw-device=20discovery=20+=20safe-to-format=20filter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read-only spike on felhom-pve. Verdicts: (SQ1) customer drives need NOT be PVE storages — track in the intent registry (durable-id + host mount + bind), the dir-storage is legacy. (SQ2) an unclaimed-disk filter (OS/LVM-PV/ZFS/PVE-storage/ VM-CT/mounted/RO/registry signals, fail-safe exclude-on-unknown) returns EXACTLY {/dev/sdd} on the demo, never sda/sdb/sdc. (SQ3) mkfs sudoers is /dev/* — the agent code is the only guard; DataBearing (wipe-confirm) is insufficient (OS disk is data-bearing), so a mandatory agent unclaimed-gate + guarded-mkfs-wrapper sudoers + a shared-box operator gate for self-serve format. (SQ4) new /disks/candidates raw-scan endpoint feeding both wizards, registry-based enrollment. (SQ5) migrate existing drives to the registry. Implementation plan + open risks included. No product code. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../SPIKE-drive-enrollment-2026-07-01.md | 180 ++++++++++++++++++ 1 file changed, 180 insertions(+) create mode 100644 documentation/audits/SPIKE-drive-enrollment-2026-07-01.md diff --git a/documentation/audits/SPIKE-drive-enrollment-2026-07-01.md b/documentation/audits/SPIKE-drive-enrollment-2026-07-01.md new file mode 100644 index 0000000..47cc0ae --- /dev/null +++ b/documentation/audits/SPIKE-drive-enrollment-2026-07-01.md @@ -0,0 +1,180 @@ +# SPIKE — safe visibility + enrollment of non-OS (customer data) drives (2026-07-01) + +**Class:** SPIKE (read-only investigation + design; no product code, no version bump). **Repos read:** +felhom-agent v0.53.0 @ `7639ab5` (read-only). **Test bed:** felhom-pve — 1 OS SSD (`sda`), 2 enrolled +drives (`sdb`=felhom-flash, `sdc`=felhom-usb), 1 free raw device (`sdd`, 59.5G, ntfs, unmounted). + +**The gap (confirmed live):** both enrollment wizards ("initialize new drive" / "attach existing +drive") draw candidates from the agent's `GET /disks` → `storage.Observe()` → **PVE storages only**. +A raw block device that is not yet a PVE storage is invisible to both → a genuinely new drive cannot be +enrolled self-serve. (See `RUNBOOK-usb-enrollment-3bfix-2026-07-01.md` for the live dead-end.) + +--- + +## SQ1 — do customer data drives need to be PVE storages? **NO.** + +Per-consumer analysis (agent source + live demo): + +| Consumer of "a drive" | Needs a PVE storage? | Mechanism | +|---|---|---| +| App data access (in-guest) | **No** | host mount `/mnt/felhom-drives/` → guest bind (`mp8: /mnt/felhom-drives`, no `backup=1`) → `/appdata` | +| Drive-data backup | **No** | cross-drive rsync/restic to paths (not vzdump-to-storage); the drive is a path, not a PVE backup target | +| PBS / vzdump CT snapshot | **No (must NOT)** | the `mp8` drive bind has no `backup=1` → **excluded** from the CT snapshot (verified in 9201's config). Drive data is deliberately out of the CT image. | +| Health / detach detection | **No** | the observer keys off the host mount being present under `/mnt/felhom-drives` (own-mount check), not the PVE storage | +| Agent-view / durable tracking | **No** | the agent **intent registry** already tracks drives by durable-id (`SetEnrolled/Ejected/Decommissioned`, `deriveDurableID` → `byid:`/`uuid:`) — independent of PVE storages | +| Wizard candidacy | **Yes today (the bug)** | candidacy = `Observe()` = PVE storages — the ONLY thing that makes a drive "need" to be a PVE storage, and it's exactly what excludes raw drives | + +**Verdict:** the PVE `dir:` storage entry buys nothing functional for a customer data drive — it is +legacy for the existing two (operator-created out-of-band). Everything a drive does (bind into the +guest, hold appdata, get rsync/restic-backed, be health-tracked) works off the host mount + the intent +registry. Track drives in the **intent registry**, not PVE storage. (The dir-storage's only real use is +making a drive a *vzdump/PBS backup target* — which Felhom does not do for customer drives; it excludes +them from the CT snapshot on purpose.) + +--- + +## SQ2 — the "free-for-Felhom" candidate filter (THE safety artifact) + +A raw-device scan must offer ONLY disks that are positively provable as **unclaimed data candidates**. +Claimed signals derived empirically on the demo (all read-only): + +| Signal | Source | Demo hit | +|---|---|---| +| holds EFI/`/boot`/root | `lsblk MOUNTPOINT` (`/boot/efi`, `/`) | sda | +| is/holds an **LVM PV** | `pvs -o pv_name` (`/dev/sda3`→VG pve) | sda | +| is a **ZFS** member | `zpool status` | none | +| backs a **PVE storage** | `storage.cfg` path/vgname/thinpool resolved onto the disk | sda (local, local-lvm), sdb (felhom-flash), sdc (felhom-usb) | +| backs a **VM/CT disk** | `/etc/pve/{qemu-server,lxc}/*.conf` (all on local-lvm=sda) | sda | +| currently **mounted** | `/proc/mounts` / `lsblk MOUNTPOINT` | sda, sdb (/mnt/felhom-flash), sdc (/mnt/felhom-usb) | +| **read-only** device | `lsblk RO=1` | none | +| an **already-enrolled Felhom drive** | intent registry durable-id match | sdb, sdc | + +**Exclusion set on the demo = {sda, sdb, sdc}. Candidate set = EXACTLY {/dev/sdd}.** The OS disk (sda) +is caught by five independent signals; the two enrolled drives (sdb/sdc) by mount + registry. `sdd` +matches none → the only offer. ✓ (The agent already has `storage.SystemDisks()` — the OS/system-disk +resolver used for role classification — which supplies the EFI/root/LVM-PV part; the filter extends it +with the PVE-storage / VM-CT / mounted / ZFS / RO signals.) + +**Fail-safe rule (mandatory):** a disk is a candidate ONLY if it matches NONE of the claimed signals +AND every signal source read cleanly. Any read error, any ambiguity, anything unclassifiable → +**EXCLUDE**. Never offer a disk the filter cannot POSITIVELY prove unclaimed. + +**Shared-box fragility (must be handled by the impl):** +- More VMs/CTs with disks on other physical disks (not just local-lvm) — the VM/CT-backing scan must + resolve every storage's backing disk, not assume local-lvm. Fragile if a storage type isn't mapped → + fail-safe excludes it. +- Extra LVM VGs / ZFS pools / mdraid / bcache / multipath on the shared box — each is a claimed signal + the scan must check (`pvs`, `zpool`, `/proc/mdstat`, `lsblk TYPE`). Unknown holder type → exclude. +- A disk mounted by the host for its own use (not under `/mnt/felhom-drives`) → excluded by the mount + signal. Good. +- **Net:** on a busy foreign box the filter will correctly narrow, but its correctness depends on + enumerating *all* claim sources; the fail-safe (exclude-on-unknown) is what makes it safe when a + claim source is unrecognized. + +--- + +## SQ3 — format-containment posture (the destructive-capability decision) + +**Blast radius (confirmed):** `configs/felhom-agent.sudoers` `FELHOM_FORMAT = mkfs.ext4 -F /dev/*, +mkfs.xfs -f /dev/*` — a `/dev/*` wildcard, forced mkfs. The agent (felhom-agent user) can mkfs **any** +block device incl. the OS disk `/dev/sda`. The pool-scoped **token ACL is irrelevant** here — format is +a sudo op, outside the token entirely. **The agent's Go code is the only guard.** + +**Existing guard is INSUFFICIENT for a raw-scan path.** Today `Format` is only reachable via a wizard +device that came from `Observe()` (an already-known PVE storage), re-inspected for `DataBearing()` +(FS/partition/mount → needs a wipe-confirm). But `DataBearing` answers "does this device have data," +NOT "is this device claimed." The **OS disk is data-bearing**, so a wipe-confirm alone could authorize +formatting it. A raw-scan path newly allows `Format` on an arbitrary `/dev/sdX` → the wipe-confirm is +not enough. + +**Decision (recommended posture):** +1. **MANDATORY defense-in-depth (agent):** `Format` MUST refuse any device the SQ2 unclaimed-filter + does not positively classify as an unclaimed candidate — evaluated **at format time on the resolved + device** (same re-resolve discipline as the existing wipe TOCTOU fix), regardless of DataBearing, + the wipe-confirm, or what the sudoers permits. The filter, not the sudoers, is the real guard. +2. **Tighten the sudoers if feasible:** replace the raw `mkfs.* /dev/*` allowlist with a **wrapper + script** (`felhom-mkfs-guarded `) that re-runs the unclaimed-filter and refuses + unless the device is an unclaimed candidate, then exec's mkfs — sudoers allowlists only the wrapper, + not raw mkfs. (Device paths are dynamic so a static `/dev/sdX` allowlist isn't viable; a guarded + wrapper is. This moves the guard below the agent process too — defense-in-depth even against an + agent bug.) **Recommended.** +3. **Shared vs dedicated box:** self-serve format of an unclaimed disk is fine on a **dedicated** Felhom + box. On a **shared/foreign** box (the colleague's), recommend a **"shared box" mode** (config flag) + that **disables self-serve format** and requires an **operator gate** for any format — + `felhom-opsign` (the existing operator-signed-op path) or a per-disk operator allowlist. The + unclaimed-filter still applies; the operator gate is the *additional* belt for the shared case where + "unclaimed by PVE" ≠ "the human wants Felhom to eat it." Attach (mount-only, no format) of an + FS-bearing unclaimed disk can stay self-serve even on a shared box (non-destructive). + +--- + +## SQ4 — enrollment architecture (off a raw scan, PVE-storage-free) + +- **New agent endpoint `GET /disks/candidates`** (sibling to `/disks`): runs the SQ2 raw scan + (non-sudo `lsblk -J` enumeration + `blkid -p` probe via the existing FELHOM_FORMAT blkid/lsblk + allowance) cross-referenced against the claimed-signal sources, returning ONLY unclaimed disks with: + size/model/serial/transport, FS/PTTYPE, `DataBearing` verdict + reason, and a **durable-id preview** + (`byid:` from `/dev/disk/by-id` now; `uuid:` after format). Fail-safe: empty on any scan error. +- **Two wizard paths consume it** (controller rewire, `internal/web/agent_disk_handlers.go` + the + init/attach templates): + - *Initialize* → offers unclaimed disks; format (data-bearing → explicit wipe confirm) + mount + bind. + - *Attach* → offers unclaimed disks that already carry a usable FS; mount + bind only (no format). +- **Enrollment records the drive in the intent registry** (durable-id + role=user-data + mount path) — + NOT a PVE storage. Then `format?`→`EnsureMount` (systemd `.mount` by-UUID) → guest bind under + `/mnt/felhom-drives/`. `deriveDurableID` already yields a stable `byid:`/`uuid:` id; the demo's + `sdd` → `byid:usb-Generic-_SD_MMC_MS_PRO_20120926571200000-0:0` (pre-format) → `uuid:` after, + which the existing `ReassertEnrolledMounts` re-attach machinery already keys off. + +--- + +## SQ5 — existing drives (felhom-usb / felhom-flash) migration + +They already have durable-ids and are tracked in the intent registry. Recommendation: +- **Move their tracking fully to the intent-registry model** (they're already there via enroll) and make + the agent-view / candidacy read the registry + raw scan, not `Observe()`. +- **Retire their PVE `dir:` storage entries** (optional, low priority): harmless to leave, but once + candidacy no longer depends on PVE storages the entries are pure legacy. If retired, do it via a + host-op (`pvesm remove`, sudo) — no token priv — and confirm nothing references them as a backup + target first. +- **observe.go / health:** if drive tracking moves off `Observe()`, the detach/health signal must move + to the registry + host-mount check (it already uses the own-mount check; decouple it from the PVE + storage list). Note for the impl: keep `Observe()` for the real PVE storages (local/local-lvm/pbs). + +--- + +## Implementation plan (for the follow-up spec[s]) + +**Agent (risky — touches mkfs):** +1. `internal/storage` — the **unclaimed-disk filter** (extend `SystemDisks` with PVE-storage/VM-CT/ + mounted/ZFS/LVM-PV/RO/registry signals; fail-safe exclude) + a raw-scan producer. +2. `GET /disks/candidates` endpoint (localapi) feeding both wizards. +3. **`Format` hardening:** refuse unless the resolved device passes the unclaimed filter (mandatory). +4. **sudoers:** replace `mkfs.* /dev/*` with a guarded-wrapper allowlist (recommended). +5. Registry-based enrollment (durable-id + role + mount; no PVE storage) reusing EnsureMount/bind. +6. **"shared box" mode** flag → disables self-serve format / requires `felhom-opsign` (SQ3). + +**Controller:** rewire the init/attach wizards to `/disks/candidates`; render probe info + the +wipe-confirm; keep the eject/decommission paths. + +**Migration:** move felhom-usb/felhom-flash tracking to the registry; optionally retire their dir- +storages; decouple health/detach from `Observe()`. + +**Supervised/risky parts:** anything reaching `Format`/mkfs (steps 3–4, 6) and the live migration. +Each needs the same dry-run + read-only-first discipline as 3b, and a live format must be validated +FIRST on the demo's `sdd` (throwaway) with the filter proven to refuse sda/sdb/sdc. + +--- + +## Open risks the implementation MUST resolve before shipping + +1. **The format guard on a shared box** — the single highest risk. Self-serve `mkfs` on a foreign box + with the `/dev/*` sudoers is a foot-cannon; do not ship self-serve format there without the operator + gate (SQ3.3) AND the mandatory agent filter (SQ3.1) AND the guarded-wrapper sudoers (SQ3.2). +2. **Claim-source completeness** — the filter is only as safe as its enumeration of claim sources; an + unrecognized holder (mdraid/bcache/multipath/foreign LVM/ZFS) must fail-safe to EXCLUDE, never offer. +3. **Re-resolve at format time** — resolve the durable-id → device and re-run the filter immediately + before mkfs (TOCTOU: a device letter can move between scan and format). +4. **Existing-drive migration** must not break the live felhom-usb/felhom-flash (health, binds, apps) + during the cutover. + +**No product code shipped. Follow-up = an Implementation spec written from this doc.**