diff --git a/CHANGELOG.md b/CHANGELOG.md index dfeed51..d9d6735 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,32 @@ +## v0.54.0 — format-safety foundation: unclaimed-disk guard + guarded-mkfs wrapper (2026-07-01) + +Impl-1 (SPIKE-drive-enrollment-2026-07-01). Hardens the destructive `Format`/mkfs path BEFORE the +enrollment feature: today `Format` delegates authorization to its caller and only checks `DataBearing` +(has-data), which is insufficient — the OS disk is data-bearing yet catastrophic — and the sudoers +permits `mkfs /dev/*`. Two independent, layered guards: + +- **Part A — mandatory unclaimed-disk guard inside `Format` (the primary safety).** New + `internal/storage/claim.go`: `classifyClaim` (pure) + `gatherClaimFacts` refuse to format any device + not provably UNCLAIMED — reusing `SystemDisks` (OS disk) + lsblk FSTYPE (LVM2_member/zfs_member/ + linux_raid_member/crypto_LUKS/swap) + foreign mounts + read-only + authoritative `pvs`/`zpool`. A + Felhom-owned mount under `/mnt/felhom-drives` is NOT a foreign claim (re-init stays allowed; the + DataBearing wipe-confirm still gates data loss). **FAIL-SAFE: any read error / undeterminable topology + → CLAIMED → refuse.** The guard is in `Format` (lowest layer), not the handler, so no caller can + bypass it. Read-only sudoers additions: `pvs`, `zpool status` (in `FELHOM_DISK`). +- **Part B — guarded-mkfs wrapper below the agent.** `configs/felhom-mkfs-guarded.sh` (root, 0755) is + now the ONLY mkfs path the sudoers allows (`FELHOM_FORMAT` no longer allowlists raw `mkfs.*`). It + re-checks the cheap catastrophic cases (system disk / LVM PV / foreign mount) and refuses — so even + an agent bug/compromise can't mkfs the OS disk. `Format` execs the wrapper (` `) via + `Binaries.MkfsGuarded`. +- **Tests:** `claim_test.go` — table-driven `classifyClaim` (every claim signal + fail-safe + the two + allow cases) incl. the **red-proof** (a claimed, non-data-bearing OS disk: removing the isSystem check + flips it to allowed → test fails, proving the guard adds safety beyond `DataBearing`); Format-guard + integration tests (refuses system disk / LVM member, allows unclaimed → wrapper invoked); capability + manifest updated (mkfs sample → the wrapper). `go build`/`vet`/`test ./...` clean. +- **Deferred to Impl-3:** a raw disk passed through to ANOTHER VM looks unused to the host — a host-level + filter can't detect it; the operator gate (shared-box mode) closes that. Impl-1 closes everything + host-visible (a strict improvement over today's no-guard state). + ## v0.53.0 — restore guests INTO the felhom pool (pool-scoped-ACL enabler) (2026-07-01) Colleague-safety batch #4 phase b (agent half). Enables the agent token to be scoped from `/` to diff --git a/README.md b/README.md index 0afd97e..bfc6d2f 100644 --- a/README.md +++ b/README.md @@ -250,6 +250,13 @@ of `/` (blast-radius containment on a shared host) — the restore is how a fres that scoped token (`VM.Allocate`+`Pool.Allocate` at `/pool/felhom`). Layout + validation: `felhom.eu/documentation/audits/SPIKE-pool-scoped-acl-2026-07-01.md`. +**Format safety (v0.54.0, Impl-1):** `Format` (mkfs) is gated by a mandatory **unclaimed-disk guard** +(`internal/storage/claim.go`) — it refuses any device not provably free for Felhom (OS disk, LVM PV, +ZFS/mdraid member, foreign mount, read-only; fail-safe on any read error), independent of `DataBearing`. +Below the agent, mkfs runs ONLY through `configs/felhom-mkfs-guarded.sh` (the sole mkfs the sudoers +permits), which re-checks the catastrophic cases as root. The pool-scoped token does NOT touch mkfs +(sudo op) — the filter + wrapper are the guard. See `SPIKE-drive-enrollment-2026-07-01.md`. + ## Process model Native Go binary, systemd service, **non-root** `felhom-agent` service user holding the scoped token, diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 9a2cc9c..8e08f90 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -45,7 +45,7 @@ import ( // version is the agent version. Overridable at build time with // -ldflags "-X main.version="; defaults to the in-repo CHANGELOG version. -var version = "0.53.0" +var version = "0.54.0" // runGuestHook is the PVE pre-start hook body (`felhom-agent guest-hook `). On the // pre-start phase it creates placeholder dirs for any absent bind-mount source so the guest always boots diff --git a/configs/felhom-agent.sudoers b/configs/felhom-agent.sudoers index 2ef7655..e924f89 100644 --- a/configs/felhom-agent.sudoers +++ b/configs/felhom-agent.sudoers @@ -24,7 +24,9 @@ Cmnd_Alias FELHOM_DISK = \ /usr/sbin/smartctl -a -j /dev/nvme[0-9]*n[0-9]*, \ /usr/sbin/smartctl -a -j /dev/vd[a-z]*, \ /usr/sbin/smartctl -a -j /dev/hd[a-z]*, \ - /usr/sbin/lvs --reportformat json --units b -o lv_name\,data_percent\,metadata_percent -- * + /usr/sbin/lvs --reportformat json --units b -o lv_name\,data_percent\,metadata_percent -- *, \ + /usr/sbin/pvs --reportformat json --noheadings -o pv_name, \ + /usr/sbin/zpool status -P # Provisioning back-half (slice 8A, doc 03 §6): populate a guest's bootstrap config mount # host-side (internal/provision). These are host-root ops the API token cannot do — a bind mount @@ -37,16 +39,15 @@ Cmnd_Alias FELHOM_PROVISION = \ /usr/sbin/pct set [0-9]* -mp[0-9]* /var/lib/felhom-agent/guests/*, \ /usr/sbin/pct set [0-9]* -onboot 1 -# Disk inspection + format (slice 8C). blkid/lsblk read the device's data-bearing evidence (the -# agent decides data-bearing-ness from THIS, never the caller's claim); mkfs.* formats a device the -# agent already classified blank (a data-bearing format is refused pending an operator signature). -# The agent fine-validates the device path (ValidateBlockDevice: raw disk / partition under /dev -# only) + fstype before any exec — the wildcard is the coarse allowlist, the agent is the fine gate. +# Disk inspection + format (slice 8C + Impl-1). blkid/lsblk read the device's data-bearing evidence +# (the agent decides data-bearing-ness from THIS, never the caller's claim). Format goes ONLY through +# felhom-mkfs-guarded (Impl-1 Part B): raw mkfs.* is NO LONGER allowlisted, so even a bad agent cannot +# mkfs the OS disk — the wrapper re-checks the catastrophic cases (system disk / LVM PV / foreign mount) +# as root and refuses, and the agent's unclaimed-disk filter (claim.go) is the primary guard above it. Cmnd_Alias FELHOM_FORMAT = \ /usr/sbin/blkid -p -o export /dev/*, \ /usr/bin/lsblk -J -o NAME\,FSTYPE\,PTTYPE\,MOUNTPOINT /dev/*, \ - /usr/sbin/mkfs.ext4 -F /dev/*, \ - /usr/sbin/mkfs.xfs -f /dev/* + /usr/local/sbin/felhom-mkfs-guarded /dev/* * # LAN split-horizon resolver (internal/lanresolver): the agent manages a host-side dnsmasq that # answers *. with each guest's live LAN IP. install only ever writes felhom-*.conf diff --git a/configs/felhom-mkfs-guarded.sh b/configs/felhom-mkfs-guarded.sh new file mode 100644 index 0000000..c3fa168 --- /dev/null +++ b/configs/felhom-mkfs-guarded.sh @@ -0,0 +1,66 @@ +#!/bin/bash +#=============================================================================== +# felhom-mkfs-guarded — the ONLY mkfs path the felhom-agent sudoers permits (Impl-1 Part B, +# SPIKE-drive-enrollment-2026-07-01 §SQ3). Defense-in-depth BELOW the agent: even a buggy or +# compromised agent cannot mkfs a catastrophic target through this — it re-checks, as root, the +# cheap catastrophic cases (OS/system disk, LVM physical volume, a foreign mount) and refuses. +# +# The agent's full unclaimed-disk filter (internal/storage/claim.go) is the PRIMARY guard; this +# wrapper is a deliberately minimal, auditable second gate. It is NOT the place for the full filter. +# +# Usage: felhom-mkfs-guarded +#=============================================================================== +set -euo pipefail + +die() { echo "felhom-mkfs-guarded: REFUSED: $*" >&2; exit 1; } + +dev="${1:-}"; fstype="${2:-}" +[[ -n "$dev" && -n "$fstype" ]] || die "usage: felhom-mkfs-guarded " + +# The device must be a REAL block-device node under /dev — no symlink (e.g. /dev/disk/by-*), no traversal. +[[ "$dev" == /dev/* ]] || die "device must be under /dev ($dev)" +[[ "$dev" != *..* ]] || die "path traversal ($dev)" +[[ -b "$dev" ]] || die "not a block device ($dev)" +[[ ! -L "$dev" ]] || die "device must be a real node, not a symlink ($dev)" + +# Whole-disk of the target (a partition's parent, else the disk itself). +pk="$(lsblk -ndo PKNAME "$dev" 2>/dev/null || true)" +whole="$dev"; [[ -n "$pk" ]] && whole="/dev/$pk" + +# 1) OS/system disk — does the target's whole-disk back /, /boot or /boot/efi? +while read -r src mnt _rest; do + case "$mnt" in + /|/boot|/boot/efi) + spk="$(lsblk -ndo PKNAME "$src" 2>/dev/null || true)" + swhole="$src"; [[ -n "$spk" ]] && swhole="/dev/$spk" + [[ "$swhole" == "$whole" || "$src" == "$dev" || "$src" == "$whole" ]] && die "system/OS disk ($dev backs $mnt)" + ;; + esac +done < /proc/mounts + +# 2) LVM physical volume anywhere on the target disk or its partitions. +if command -v pvs >/dev/null 2>&1; then + while read -r pv; do + pv="${pv//[[:space:]]/}"; [[ -z "$pv" ]] && continue + pvpk="$(lsblk -ndo PKNAME "$pv" 2>/dev/null || true)" + pvwhole="$pv"; [[ -n "$pvpk" ]] && pvwhole="/dev/$pvpk" + [[ "$pvwhole" == "$whole" ]] && die "device holds an LVM physical volume ($pv)" + done < <(pvs --noheadings -o pv_name 2>/dev/null || true) +fi + +# 3) mounted OUTSIDE Felhom's own drive area = a live foreign filesystem → catastrophic. Mounts under +# /mnt/felhom-drives are our own drives (the agent detaches before a re-init) → allowed. +while read -r mp; do + [[ -z "$mp" ]] && continue + case "$mp" in + /mnt/felhom-drives|/mnt/felhom-drives/*) : ;; + *) die "device (or a partition) is mounted at $mp ($dev)" ;; + esac +done < <(lsblk -nro MOUNTPOINT "$whole" 2>/dev/null || true) + +# Passed the catastrophic checks → format. exec so the mkfs exit status is the wrapper's. +case "$fstype" in + ext4) exec /usr/sbin/mkfs.ext4 -F "$dev" ;; + xfs) exec /usr/sbin/mkfs.xfs -f "$dev" ;; + *) die "unsupported fstype ($fstype)" ;; +esac diff --git a/internal/capability/manifest.go b/internal/capability/manifest.go index c279ee8..a191b16 100644 --- a/internal/capability/manifest.go +++ b/internal/capability/manifest.go @@ -57,8 +57,8 @@ var manifest = []Capability{ // ---- Disk inspect / format gate (Critical: the data-bearing classifier + format) ---- {"disk-blkid", "disk data-bearing classify (format gate)", "/usr/sbin/blkid", []string{"-p", "-o", "export", "/dev/sda"}, true}, {"disk-lsblk", "disk topology read (format gate)", "/usr/bin/lsblk", []string{"-J", "-o", "NAME,FSTYPE,PTTYPE,MOUNTPOINT", "/dev/sda"}, true}, - {"disk-mkfs-ext4", "blank-device format (ext4)", "/usr/sbin/mkfs.ext4", []string{"-F", "/dev/sda"}, true}, - {"disk-mkfs-xfs", "blank-device format (xfs)", "/usr/sbin/mkfs.xfs", []string{"-f", "/dev/sda"}, false}, + {"disk-mkfs-ext4", "guarded format (ext4)", "/usr/local/sbin/felhom-mkfs-guarded", []string{"/dev/sda", "ext4"}, true}, + {"disk-mkfs-xfs", "guarded format (xfs)", "/usr/local/sbin/felhom-mkfs-guarded", []string{"/dev/sda", "xfs"}, false}, {"disk-smart", "disk SMART health read", "/usr/sbin/smartctl", []string{"-a", "-j", "/dev/sda"}, false}, {"disk-lvs", "thin-pool usage read", "/usr/sbin/lvs", []string{"--reportformat", "json", "--units", "b", "-o", "lv_name,data_percent,metadata_percent", "--", "pve/data"}, false}, diff --git a/internal/storage/claim.go b/internal/storage/claim.go new file mode 100644 index 0000000..0527ed3 --- /dev/null +++ b/internal/storage/claim.go @@ -0,0 +1,269 @@ +package storage + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path" + "path/filepath" + "strings" +) + +// The "unclaimed-disk" filter (Impl-1, SPIKE-drive-enrollment-2026-07-01 §SQ2). It answers a single +// safety-critical question: is a block device provably FREE for Felhom to format, i.e. NOT claimed by +// the OS or by anything but Felhom's own drives? It is the mandatory guard `Format` runs before mkfs +// (the sudoers permits `mkfs /dev/*`, so the agent's code — this filter — is the real destructive-op +// guard; the pool-scoped token ACL does not touch a sudo mkfs). +// +// FAIL-SAFE is the rule: the filter returns unclaimed ONLY when it can positively read every required +// signal AND none indicates a claim. Any read error, undeterminable topology, or any claim signal → +// CLAIMED → refuse. It never offers a disk it cannot prove is free. + +// felhomDrivesPrefix — a mount under here is one of Felhom's OWN managed drives, which is NOT a foreign +// claim: re-initialising our own drive stays allowed (the DataBearing wipe-confirm still gates the data +// loss). A mount anywhere else is a foreign claim → refuse. +const felhomDrivesPrefix = "/mnt/felhom-drives" + +// memberFSTypes are lsblk/blkid FSTYPE values meaning the device is a MEMBER of a higher-level +// construct (LVM/ZFS/mdraid/LUKS) or active swap — always a claim, never a plain formattable data disk. +var memberFSTypes = map[string]bool{ + "LVM2_member": true, + "zfs_member": true, + "linux_raid_member": true, + "crypto_LUKS": true, + "swap": true, +} + +// claimNode is one block node — the whole disk or a partition/child — with the signals we classify on. +type claimNode struct { + name string + fstype string + mountpoint string +} + +// claimFacts is the gathered evidence for one candidate device. classifyClaim is a PURE function of it +// (so the decision logic is fully unit-testable from fixtures, with no host). +type claimFacts struct { + device string + wholeDisk string + wholeDiskOK bool + isSystem bool + readonly bool + nodes []claimNode // the whole disk + its children (partitions) + lvmPV bool // pvs (authoritative): the disk / a partition is an LVM physical volume + zfsMember bool // zpool (authoritative): the disk / a partition is a ZFS pool member + gatherErr string // non-empty ⇒ a REQUIRED read failed ⇒ fail-safe CLAIMED +} + +// classifyClaim is the pure guard verdict. unclaimed=true ONLY when the device is provably free for +// Felhom to format; every claim signal / error / ambiguity ⇒ unclaimed=false with a human reason. +func classifyClaim(f claimFacts) (unclaimed bool, reason string) { + if f.gatherErr != "" { + return false, "could not determine device claims: " + f.gatherErr + } + if !f.wholeDiskOK { + return false, "undeterminable device topology (not a recognizable raw disk)" + } + if f.isSystem { + return false, "system/OS disk" + } + if f.readonly { + return false, "read-only device" + } + if f.lvmPV { + return false, "device holds an LVM physical volume" + } + if f.zfsMember { + return false, "device is a ZFS pool member" + } + for _, n := range f.nodes { + if memberFSTypes[n.fstype] { + return false, "device holds a " + n.fstype + " (" + n.name + ")" + } + if n.mountpoint != "" && !underFelhomDrives(n.mountpoint) { + return false, "device is mounted at " + n.mountpoint + " (" + n.name + ")" + } + } + return true, "unclaimed" +} + +// underFelhomDrives reports whether a mountpoint is one of Felhom's own managed drive mounts. +func underFelhomDrives(mp string) bool { + mp = path.Clean(mp) // mountpoints are always unix paths — path.Clean, not filepath.Clean (Windows tests) + return mp == felhomDrivesPrefix || strings.HasPrefix(mp, felhomDrivesPrefix+"/") +} + +// deviceUnclaimed gathers the claim evidence for device and returns the pure verdict. +func (h *SudoHostOps) deviceUnclaimed(ctx context.Context, device string) (bool, string) { + return classifyClaim(h.gatherClaimFacts(ctx, device)) +} + +// gatherClaimFacts reads every claim signal via host-visible reads (NOT the PVE token): the OS-disk set +// (mount table), the whole-disk lsblk tree (member FSTYPE + mountpoints — the authoritative backbone), +// the /sys read-only flag, and — when their tools are installed — the authoritative LVM (pvs) and ZFS +// (zpool) sources. A tool that is genuinely ABSENT contributes "no claim of that kind" (lsblk still +// detects the member FSTYPE); a tool that is present but ERRORS is a fail-safe CLAIMED. +func (h *SudoHostOps) gatherClaimFacts(ctx context.Context, device string) claimFacts { + f := claimFacts{device: device} + wd, ok := wholeDiskOf(device) + f.wholeDisk, f.wholeDiskOK = wd, ok + + // OS/system disk — fail-safe inside (unknown topology / unknown system set ⇒ system). + sys, sysKnown := SystemDisks(h.host) + f.isSystem = isSystemBacked(device, sys, sysKnown) + if !ok { + return f // classifyClaim refuses on !wholeDiskOK + } + + // Read-only flag (/sys/block//ro is world-readable). + if ro, rerr := readWholeDiskRO(wd); rerr != nil { + f.gatherErr = "read-only flag unreadable" + return f + } else { + f.readonly = ro + } + + // lsblk tree on the WHOLE disk — REQUIRED (error ⇒ claimed). Reuses the allowlisted command. + lout, _, lerr := h.runner.Run(ctx, h.bins.Lsblk, "-J", "-o", "NAME,FSTYPE,PTTYPE,MOUNTPOINT", wd) + if lerr != nil { + f.gatherErr = "lsblk failed" + return f + } + nodes, perr := parseLsblkNodes(lout) + if perr != nil { + f.gatherErr = "lsblk parse failed" + return f + } + f.nodes = nodes + + // LVM PV (authoritative). pvs installed but erroring ⇒ fail-safe claimed; absent ⇒ rely on lsblk's + // LVM2_member FSTYPE (already in nodes). + if h.binaryPresent(h.bins.Pvs) { + pvset, err := h.lvmPVSet(ctx) + if err != nil { + f.gatherErr = "pvs failed" + return f + } + for _, n := range nodes { + if pvset["/dev/"+n.name] { + f.lvmPV = true + } + } + if pvset[wd] { + f.lvmPV = true + } + } + + // ZFS members (authoritative). Absent ⇒ no ZFS on this host; present but erroring ⇒ claimed. + if h.binaryPresent(h.bins.Zpool) { + member, err := h.zfsMembers(ctx, nodes, wd) + if err != nil { + f.gatherErr = "zpool failed" + return f + } + f.zfsMember = member + } + return f +} + +// binaryPresent reports whether an absolute binary path exists (distinguishes "tool not installed" +// from "tool present but errored" — only the latter is a fail-safe CLAIMED). +func (h *SudoHostOps) binaryPresent(path string) bool { + if path == "" { + return false + } + _, err := os.Stat(path) + return err == nil +} + +// readWholeDiskRO reads /sys/block//ro ("1" ⇒ read-only). Whole-disk path e.g. /dev/sdd. A +// package var so tests can stub the /sys read (the only host read in the gather that isn't a stubbable +// runner/HostReader call). +var readWholeDiskRO = func(wholeDisk string) (bool, error) { + name := filepath.Base(wholeDisk) + b, err := os.ReadFile(filepath.Join("/sys/block", name, "ro")) + if err != nil { + return false, err + } + return strings.TrimSpace(string(b)) == "1", nil +} + +// --- lsblk tree parsing (per-node FSTYPE + mountpoint for the disk and every child) --- + +type lsblkDev struct { + Name string `json:"name"` + FSType string `json:"fstype"` + MountPoint string `json:"mountpoint"` + Children []lsblkDev `json:"children"` +} + +// parseLsblkNodes flattens `lsblk -J` output into every node (the disk + all descendants). +func parseLsblkNodes(out []byte) ([]claimNode, error) { + var doc struct { + BlockDevices []lsblkDev `json:"blockdevices"` + } + if err := json.Unmarshal(out, &doc); err != nil { + return nil, err + } + var nodes []claimNode + var walk func(d lsblkDev) + walk = func(d lsblkDev) { + nodes = append(nodes, claimNode{name: d.Name, fstype: d.FSType, mountpoint: d.MountPoint}) + for _, c := range d.Children { + walk(c) + } + } + for _, d := range doc.BlockDevices { + walk(d) + } + return nodes, nil +} + +// lvmPVSet returns the set of PV device paths (canonicalized to whole-disk where possible is done by +// the caller; here we return the raw pv_name paths as pvs reports them, e.g. /dev/sda3 or /dev/sdb). +func (h *SudoHostOps) lvmPVSet(ctx context.Context) (map[string]bool, error) { + out, stderr, err := h.runner.Run(ctx, h.bins.Pvs, "--reportformat", "json", "--noheadings", "-o", "pv_name") + if err != nil { + return nil, fmt.Errorf("pvs: %w: %s", err, trim(stderr)) + } + var doc struct { + Report []struct { + PV []struct { + PVName string `json:"pv_name"` + } `json:"pv"` + } `json:"report"` + } + if jerr := json.Unmarshal(out, &doc); jerr != nil { + return nil, jerr + } + set := map[string]bool{} + for _, r := range doc.Report { + for _, pv := range r.PV { + if n := strings.TrimSpace(pv.PVName); n != "" { + set[n] = true + } + } + } + return set, nil +} + +// zfsMembers reports whether any of the device's nodes (or the whole disk) is a ZFS pool member, by +// scanning `zpool status -P` (which prints full /dev paths). Conservative substring match on the node +// device paths — a false positive only ever REFUSES (safe direction). +func (h *SudoHostOps) zfsMembers(ctx context.Context, nodes []claimNode, wholeDisk string) (bool, error) { + out, stderr, err := h.runner.Run(ctx, h.bins.Zpool, "status", "-P") + if err != nil { + return false, fmt.Errorf("zpool: %w: %s", err, trim(stderr)) + } + text := string(out) + if strings.Contains(text, wholeDisk) { + return true, nil + } + for _, n := range nodes { + if n.name != "" && strings.Contains(text, "/dev/"+n.name) { + return true, nil + } + } + return false, nil +} diff --git a/internal/storage/claim_test.go b/internal/storage/claim_test.go new file mode 100644 index 0000000..d9e8039 --- /dev/null +++ b/internal/storage/claim_test.go @@ -0,0 +1,155 @@ +package storage + +import ( + "context" + "strings" + "testing" +) + +// classifyClaim is the pure guard verdict — the load-bearing safety logic. Table-driven over every +// claim signal + the fail-safe, plus the two ALLOW cases (clean disk, and re-init of our own drive). +func TestClassifyClaim(t *testing.T) { + base := func() claimFacts { + return claimFacts{device: "/dev/sdd", wholeDisk: "/dev/sdd", wholeDiskOK: true, + nodes: []claimNode{{name: "sdd", fstype: "", mountpoint: ""}, {name: "sdd1", fstype: "ntfs", mountpoint: ""}}} + } + cases := []struct { + name string + mutate func(f *claimFacts) + wantUnclaimed bool + reasonHas string + }{ + {"clean unclaimed disk (ntfs, unmounted)", func(f *claimFacts) {}, true, "unclaimed"}, + {"blank disk", func(f *claimFacts) { f.nodes = []claimNode{{name: "sdd"}} }, true, "unclaimed"}, + {"our own drive re-init (mounted under /mnt/felhom-drives)", func(f *claimFacts) { + f.nodes = []claimNode{{name: "sdd"}, {name: "sdd1", fstype: "ext4", mountpoint: "/mnt/felhom-drives/felhom-flash"}} + }, true, "unclaimed"}, + {"system/OS disk (non-data-bearing) — RED-PROOF vs DataBearing", func(f *claimFacts) { + f.isSystem = true + f.nodes = []claimNode{{name: "sda"}} // blank: DataBearing would say benign; the guard must still refuse + }, false, "system/OS disk"}, + {"LVM PV (pvs)", func(f *claimFacts) { f.lvmPV = true }, false, "LVM physical volume"}, + {"LVM2_member via lsblk fstype", func(f *claimFacts) { + f.nodes = []claimNode{{name: "sdd"}, {name: "sdd1", fstype: "LVM2_member"}} + }, false, "LVM2_member"}, + {"ZFS pool member (zpool)", func(f *claimFacts) { f.zfsMember = true }, false, "ZFS pool member"}, + {"zfs_member via lsblk fstype", func(f *claimFacts) { + f.nodes = []claimNode{{name: "sdd", fstype: "zfs_member"}} + }, false, "zfs_member"}, + {"mdraid member", func(f *claimFacts) { + f.nodes = []claimNode{{name: "sdd1", fstype: "linux_raid_member"}} + }, false, "linux_raid_member"}, + {"active swap signature", func(f *claimFacts) { + f.nodes = []claimNode{{name: "sdd1", fstype: "swap"}} + }, false, "swap"}, + {"foreign mount (outside /mnt/felhom-drives)", func(f *claimFacts) { + f.nodes = []claimNode{{name: "sdd1", fstype: "ext4", mountpoint: "/srv/data"}} + }, false, "mounted at /srv/data"}, + {"read-only device", func(f *claimFacts) { f.readonly = true }, false, "read-only"}, + {"undeterminable topology", func(f *claimFacts) { f.wholeDiskOK = false }, false, "undeterminable"}, + {"fail-safe: gather error", func(f *claimFacts) { f.gatherErr = "lsblk failed" }, false, "could not determine"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := base() + tc.mutate(&f) + gotUnclaimed, reason := classifyClaim(f) + if gotUnclaimed != tc.wantUnclaimed { + t.Fatalf("unclaimed=%v want %v (reason %q)", gotUnclaimed, tc.wantUnclaimed, reason) + } + if !strings.Contains(reason, tc.reasonHas) { + t.Errorf("reason %q missing %q", reason, tc.reasonHas) + } + }) + } +} + +func TestParseLsblkNodes(t *testing.T) { + out := []byte(`{"blockdevices":[{"name":"sdd","fstype":null,"mountpoint":null,"children":[{"name":"sdd1","fstype":"ntfs","mountpoint":null}]}]}`) + nodes, err := parseLsblkNodes(out) + if err != nil { + t.Fatalf("parse: %v", err) + } + if len(nodes) != 2 || nodes[0].name != "sdd" || nodes[1].name != "sdd1" || nodes[1].fstype != "ntfs" { + t.Fatalf("nodes = %+v", nodes) + } + if _, err := parseLsblkNodes([]byte("not json")); err == nil { + t.Error("expected a parse error on garbage") + } +} + +// --- Format guard integration: the guard is wired into Format and actually gates mkfs. --- + +// newGuardOps builds a SudoHostOps with stubbed reads: a scriptRunner for lsblk/mkfs, a fakeHostReader +// for SystemDisks, Pvs/Zpool empty (→ binaryPresent=false → skipped), and readWholeDiskRO stubbed. +func newGuardOps(sr *scriptRunner, mounts []Mount) *SudoHostOps { + return &SudoHostOps{ + runner: sr, + // Pvs/Zpool empty ⇒ binaryPresent=false ⇒ skipped (deterministic across OSes; lsblk carries the signals). + bins: Binaries{Lsblk: "/usr/bin/lsblk", MkfsGuarded: "/usr/local/sbin/felhom-mkfs-guarded"}, + host: &fakeHostReader{mounts: mounts}, + logger: quietLogger(), + } +} + +func mkfsCalled(sr *scriptRunner) bool { + for _, c := range sr.calls { + if strings.Contains(c[0], "mkfs") { + return true + } + } + return false +} + +func TestFormatGuard_RefusesSystemDisk(t *testing.T) { + defer stubRO(false)() + sr := &scriptRunner{out: map[string][]byte{ + "/usr/bin/lsblk": []byte(`{"blockdevices":[{"name":"sda","children":[{"name":"sda2","mountpoint":"/"}]}]}`), + }} + // / on /dev/sda2 → SystemDisks resolves {/dev/sda}; formatting /dev/sda must be refused. + ops := newGuardOps(sr, []Mount{{Device: "/dev/sda2", MountPoint: "/"}}) + err := ops.Format(context.Background(), "/dev/sda", "ext4") + if err == nil || !strings.Contains(err.Error(), "system/OS disk") { + t.Fatalf("want system-disk refusal, got %v", err) + } + if mkfsCalled(sr) { + t.Fatal("mkfs was invoked on a claimed device — guard failed") + } +} + +func TestFormatGuard_RefusesLVMMember(t *testing.T) { + defer stubRO(false)() + sr := &scriptRunner{out: map[string][]byte{ + "/usr/bin/lsblk": []byte(`{"blockdevices":[{"name":"sdd","children":[{"name":"sdd1","fstype":"LVM2_member"}]}]}`), + }} + ops := newGuardOps(sr, []Mount{{Device: "/dev/sda2", MountPoint: "/"}}) + err := ops.Format(context.Background(), "/dev/sdd", "ext4") + if err == nil || !strings.Contains(err.Error(), "LVM2_member") { + t.Fatalf("want LVM-member refusal, got %v", err) + } + if mkfsCalled(sr) { + t.Fatal("mkfs invoked on an LVM member — guard failed") + } +} + +func TestFormatGuard_AllowsUnclaimed(t *testing.T) { + defer stubRO(false)() + sr := &scriptRunner{out: map[string][]byte{ + "/usr/bin/lsblk": []byte(`{"blockdevices":[{"name":"sdd","children":[{"name":"sdd1","fstype":"ntfs"}]}]}`), + }} + // /dev/sdd is not the system disk, not a member, not mounted → guard passes → mkfs runs. + ops := newGuardOps(sr, []Mount{{Device: "/dev/sda2", MountPoint: "/"}}) + if err := ops.Format(context.Background(), "/dev/sdd", "ext4"); err != nil { + t.Fatalf("unclaimed disk must format, got %v", err) + } + if !mkfsCalled(sr) { + t.Fatal("mkfs was NOT invoked on an unclaimed disk — guard over-refused") + } +} + +// stubRO overrides the /sys read-only probe for the duration of a test; the returned func restores it. +func stubRO(ro bool) func() { + orig := readWholeDiskRO + readWholeDiskRO = func(string) (bool, error) { return ro, nil } + return func() { readWholeDiskRO = orig } +} diff --git a/internal/storage/hostops.go b/internal/storage/hostops.go index 4364206..4f5a6dc 100644 --- a/internal/storage/hostops.go +++ b/internal/storage/hostops.go @@ -104,8 +104,11 @@ type Binaries struct { Lvs string Blkid string // device signature probe (8C data-bearing detection) Lsblk string // partition/mount topology (8C) - MkfsExt4 string // 8C format executor (ext4) - MkfsXfs string // 8C format executor (xfs) + MkfsExt4 string // 8C format executor (ext4) — now invoked by the guarded wrapper, not the agent directly + MkfsXfs string // 8C format executor (xfs) — now invoked by the guarded wrapper, not the agent directly + MkfsGuarded string // Impl-1 Part B: the guarded-mkfs wrapper the agent execs (device+fstype) + Pvs string // Impl-1 claim filter: LVM physical-volume enumeration (read-only) + Zpool string // Impl-1 claim filter: ZFS pool member enumeration (read-only) } func (b Binaries) withDefaults() Binaries { @@ -133,6 +136,15 @@ func (b Binaries) withDefaults() Binaries { if b.MkfsXfs == "" { b.MkfsXfs = "/usr/sbin/mkfs.xfs" } + if b.MkfsGuarded == "" { + b.MkfsGuarded = "/usr/local/sbin/felhom-mkfs-guarded" + } + if b.Pvs == "" { + b.Pvs = "/usr/sbin/pvs" + } + if b.Zpool == "" { + b.Zpool = "/usr/sbin/zpool" + } return b } @@ -145,6 +157,7 @@ type SudoHostOps struct { bins Binaries unitDir string // where enabled units live (e.g. /etc/systemd/system) stageDir string // agent-owned staging dir for unit files before install + host HostReader // root-free reads (mount table) for the Impl-1 Format claim guard logger *slog.Logger } @@ -154,6 +167,7 @@ type SudoHostOpsConfig struct { Bins Binaries UnitDir string // default /etc/systemd/system StageDir string // default /units; must be agent-writable + Host HostReader // default NewProcHostReader(); the Format claim guard's mount-table read Logger *slog.Logger } @@ -171,11 +185,16 @@ func NewSudoHostOps(cfg SudoHostOpsConfig) *SudoHostOps { if logger == nil { logger = slog.Default() } + host := cfg.Host + if host == nil { + host = NewProcHostReader() + } return &SudoHostOps{ runner: cfg.Runner, bins: cfg.Bins.withDefaults(), unitDir: unitDir, stageDir: stageDir, + host: host, logger: logger, } } @@ -423,17 +442,19 @@ func (h *SudoHostOps) Format(ctx context.Context, device, fstype string) error { if err := ValidateFSType(fstype); err != nil { return err } - switch fstype { - case "ext4": - if err := h.run(ctx, h.bins.MkfsExt4, "-F", device); err != nil { - return fmt.Errorf("storage: mkfs.ext4 %s: %w", device, err) - } - case "xfs": - if err := h.run(ctx, h.bins.MkfsXfs, "-f", device); err != nil { - return fmt.Errorf("storage: mkfs.xfs %s: %w", device, err) - } - default: - return fmt.Errorf("storage: unsupported fstype %q", fstype) // unreachable after Validate + // MANDATORY unclaimed-disk guard (Impl-1): mkfs is destructive and the sudoers permits `mkfs /dev/*`, + // so THIS check — not the caller's authorization and not `DataBearing` (the OS disk is data-bearing) — + // is the real guard. Evaluated on the device as passed (the caller re-resolves the durable-id first). + // Fail-safe: anything not provably unclaimed (incl. any read error) is refused BEFORE any mkfs. + if ok, reason := h.deviceUnclaimed(ctx, device); !ok { + h.logger.Warn("storage: REFUSING format — device is claimed", "device", device, "reason", reason) + return fmt.Errorf("storage: refusing to format %s: %s", device, reason) + } + // Part B: mkfs runs through the guarded wrapper (the sudoers allowlists ONLY the wrapper, not raw + // mkfs) — a second, below-the-agent gate that re-checks the catastrophic cases even against an agent + // bug. The wrapper takes and picks/execs the right mkfs. + if err := h.run(ctx, h.bins.MkfsGuarded, device, fstype); err != nil { + return fmt.Errorf("storage: guarded mkfs %s (%s): %w", device, fstype, err) } h.logger.Info("storage: formatted device", "device", device, "fstype", fstype) return nil diff --git a/internal/storage/hostops_disk_test.go b/internal/storage/hostops_disk_test.go index 6d0661a..80f5c2b 100644 --- a/internal/storage/hostops_disk_test.go +++ b/internal/storage/hostops_disk_test.go @@ -156,26 +156,46 @@ func TestInspect_RejectsBadDevice(t *testing.T) { // ---- Format (mkfs) ---------------------------------------------------------------------- +// Format now (Impl-1) runs the unclaimed guard then execs the guarded wrapper ( ), +// not raw mkfs. These use the stubbed guard (unclaimed /dev/sdb; system disk = /dev/sda). func TestFormat_Ext4(t *testing.T) { - r := &scriptedRunner{} - if err := newSudo(r).Format(context.Background(), "/dev/sdb", "ext4"); err != nil { + defer stubRO(false)() + sr := &scriptRunner{out: map[string][]byte{ + "/usr/bin/lsblk": []byte(`{"blockdevices":[{"name":"sdb"}]}`), + }} + ops := newGuardOps(sr, []Mount{{Device: "/dev/sda2", MountPoint: "/"}}) + if err := ops.Format(context.Background(), "/dev/sdb", "ext4"); err != nil { t.Fatal(err) } - if !r.ran("mkfs.ext4 -F /dev/sdb") { - t.Fatalf("mkfs.ext4 not invoked correctly: %v", r.calls) + if !srRan(sr, "/usr/local/sbin/felhom-mkfs-guarded /dev/sdb ext4") { + t.Fatalf("guarded wrapper not invoked correctly: %v", sr.calls) } } func TestFormat_Xfs(t *testing.T) { - r := &scriptedRunner{} - if err := newSudo(r).Format(context.Background(), "/dev/nvme0n1p1", "xfs"); err != nil { + defer stubRO(false)() + sr := &scriptRunner{out: map[string][]byte{ + "/usr/bin/lsblk": []byte(`{"blockdevices":[{"name":"nvme0n1","children":[{"name":"nvme0n1p1"}]}]}`), + }} + ops := newGuardOps(sr, []Mount{{Device: "/dev/sda2", MountPoint: "/"}}) + if err := ops.Format(context.Background(), "/dev/nvme0n1p1", "xfs"); err != nil { t.Fatal(err) } - if !r.ran("mkfs.xfs -f /dev/nvme0n1p1") { - t.Fatalf("mkfs.xfs not invoked correctly: %v", r.calls) + if !srRan(sr, "/usr/local/sbin/felhom-mkfs-guarded /dev/nvme0n1p1 xfs") { + t.Fatalf("guarded wrapper not invoked correctly: %v", sr.calls) } } +// srRan reports whether the scriptRunner recorded a call whose joined form contains substr. +func srRan(sr *scriptRunner, substr string) bool { + for _, c := range sr.calls { + if strings.Contains(strings.Join(c, " "), substr) { + return true + } + } + return false +} + func TestFormat_RejectsBadArgs(t *testing.T) { r := &scriptedRunner{} if err := newSudo(r).Format(context.Background(), "/dev/disk/by-uuid/x", "ext4"); err == nil {