diff --git a/CHANGELOG.md b/CHANGELOG.md index 086f3a9..efbcb10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,42 @@ All notable changes to **felhom-agent** are recorded here. Update on every code change that gets pushed. +## v0.12.0 — slice 8C Phase A: disk endpoints + data-bearing classifier gate + mkfs executor (2026-06-10) + +The agent half of slice 8C, Phase A (additive). Adds the host disk-management endpoints the +controller's disk UI drives — with the **8C security invariant**: the agent decides +data-bearing-ness by **inspecting the actual device** (agent-internal evidence), NEVER from the +caller's claim. A compromised controller asserting "this drive is blank" cannot wipe a data-bearing +drive. (Controller rewire + disk-subsystem retirement + de-privilege are Phases B/C, `felhom-controller`.) + +### Added +- **`internal/storage` — `mkfs` executor + data-bearing inspection.** `SudoHostOps.Format(device, + fstype)` (device-pinned, `ValidateBlockDevice`+`ValidateFSType`, narrow `FELHOM_FORMAT` sudoers — + `mkfs.ext4 -F` / `mkfs.xfs -f` on a `/dev/*` path the agent fine-validates first). + `SudoHostOps.InspectDevice(device)` → `DeviceProbe` (filesystem signature via `blkid -p`, partition + table / partitions / mount via `lsblk -J`). **`DeviceProbe.DataBearing()` is conservative**: any + signature / partition table / partition / mount — OR a probe that did not read cleanly — is + data-bearing (fail-safe; an unreadable device is never called blank). +- **`internal/localapi` — the §6 disk endpoints**, all self-scoped (token→guest; cross-guest 403): + - `GET /disks` — host drives + a **data-bearing flag** (UI hint). Read-only/benign. + - `POST /disks/assign` — attach a drive as a mount (benign, additive → `EnsureMount`). Self-serve. + - `POST /disks/eject` — safe-unmount (benign, data preserved) + the **dependent guests** that + mount it (so the controller can warn which apps lose that storage). + - `POST /disks/format` — **the security centerpiece**: the agent **inspects the device itself**; + blank → benign → `mkfs`; **data-bearing → ClassStorageWipe → the slice-4 gate → refused + `pending_signature`** (the operator-signed completion is slice 10). The caller's claim is + ignored — only a device the agent reads as blank is formatted. +- `storageGateAdapter` bridges the format path to the slice-4 reversibility gate (no new gate/crypto). + +### Tests +- localapi (security matrix): blank device → **mkfs called, gate not consulted**; a **data-bearing + device → 403, mkfs NEVER called**, gate consulted (`pending_signature`); an **ambiguous/unprobed + device → treated destructive** (fail-safe); even a gate that *allows* does not format data-bearing + in 8C; assign → `EnsureMount`; eject → `Unmount` + dependent guests; cross-guest → 403; bad + device/fstype → 400; unconfigured → 503. +- storage: `ValidateBlockDevice`/`ValidateFSType` (whitelist + injection rejection); `InspectDevice` + blank/filesystem/partition-table/mounted/failed-probe-fail-safe; `Format` invokes the right `mkfs.*`. + ## v0.11.0 — slice 8B: app-consistent backup — /backup/due policy + /backup/status phases (2026-06-10) The agent half of slice 8B (doc 03 §8). Turns the 8A thin backup stubs into the real policy the diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index e47f5b2..e3661ef 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -40,7 +40,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.11.0" +var version = "0.12.0" func main() { var ( @@ -337,7 +337,7 @@ func runDaemon(cfg config.Config, logger *slog.Logger) int { // host still reports/reconciles). The leaf is generated+persisted once so its pin is stable. localServers := 0 var localTokens *localapi.TokenStore - localSrv := buildLocalAPIServer(cfg, px, backupStore, observer, logger, &localTokens) + localSrv := buildLocalAPIServer(cfg, px, backupStore, observer, hostOps, gate, logger, &localTokens) if localTokens != nil { defer localTokens.Close() } @@ -462,7 +462,7 @@ func buildRestoreTestScheduler(cfg config.Config, px *proxmox.Client, engine *re // leaf (stable fingerprint). Any failure DISABLES the server (returns nil) WITHOUT crashing the // daemon — the host still reports/reconciles; only the controller channel is unavailable until // fixed. The opened token store is returned via outTokens so the caller can Close it. -func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.Store, observer *storage.Observer, logger *slog.Logger, outTokens **localapi.TokenStore) *localapi.Server { +func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.Store, observer *storage.Observer, hostOps storage.HostOps, gate *reconcile.Gate, logger *slog.Logger, outTokens **localapi.TokenStore) *localapi.Server { if !cfg.LocalAPI.Enabled() { return nil } @@ -493,7 +493,11 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St Storage: observer, Tokens: tokens, BackupCadence: cfg.Backup.BackupCadence(), - Logger: logger, + // Disk management (slice 8C): the privileged host surface + the data-bearing wipe gate. + Disks: hostOps, + DiskGate: storageGateAdapter{gate: gate, hostID: cfg.Hub.HostID}, + Guests2: px, + Logger: logger, }) if err != nil { logger.Warn("daemon: local-api disabled (server build)", "err", err) @@ -502,6 +506,23 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St return srv } +// storageGateAdapter bridges the local-API disk-format path to the slice-4 reversibility gate +// (8C): a data-bearing format is a ClassStorageWipe destructive op, so with no operator signature +// the gate returns pending_signature and the agent refuses. The signed completion is slice 10. +type storageGateAdapter struct { + gate *reconcile.Gate + hostID string +} + +func (a storageGateAdapter) AuthorizeWipe(device string) (bool, string) { + params, _ := json.Marshal(map[string]string{"device": device, "op": "format"}) + dec := a.gate.Authorize( + reconcile.IntentForStorageDestructive(reconcile.ClassStorageWipe, a.hostID, device, params, reconcile.SourceOneShotJob), + nil, // no operator signature in 8C → pending_signature + ) + return dec.Allowed, string(dec.Reason) +} + // reconcileJournalPath chooses the op-journal path: a `journal.log` sibling of the // configured nonce store (both are durable agent state), falling back to the standard // host state dir when the nonce store is unset. diff --git a/configs/felhom-agent.sudoers b/configs/felhom-agent.sudoers index 68b8bad..981994f 100644 --- a/configs/felhom-agent.sudoers +++ b/configs/felhom-agent.sudoers @@ -36,4 +36,15 @@ Cmnd_Alias FELHOM_PROVISION = \ /usr/bin/chown -R 100000\:100000 /var/lib/felhom-agent/guests/*, \ /usr/sbin/pct set [0-9]* -mp[0-9]* /var/lib/felhom-agent/guests/* -felhom-agent ALL=(root) NOPASSWD: FELHOM_MOUNT, FELHOM_DISK, FELHOM_PROVISION +# 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. +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/* + +felhom-agent ALL=(root) NOPASSWD: FELHOM_MOUNT, FELHOM_DISK, FELHOM_PROVISION, FELHOM_FORMAT diff --git a/internal/localapi/disks.go b/internal/localapi/disks.go new file mode 100644 index 0000000..dad9b69 --- /dev/null +++ b/internal/localapi/disks.go @@ -0,0 +1,260 @@ +package localapi + +import ( + "context" + "net/http" + "strings" + + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" + "gitea.dooplex.hu/admin/felhom-agent/internal/storage" +) + +// Disk management (slice 8C, doc 03 §6). The controller's disk-management UX stays in the +// controller; EXECUTION is the agent's. The security centerpiece: the agent decides +// data-bearing-ness by INSPECTING THE ACTUAL DEVICE (agent-internal evidence), never from the +// caller's claim — a compromised controller asserting "this drive is blank" cannot wipe a +// data-bearing drive. Benign ops (list/assign/eject/format-blank) execute self-serve; a +// data-bearing format is classified destructive → the gate refuses it `pending_signature` (the +// operator-signed completion is slice 10). + +// DiskOps is the privileged host-storage surface the disk endpoints need. Satisfied by +// *storage.SudoHostOps. Optional — the endpoints report "not configured" when absent. +type DiskOps interface { + EnsureMount(ctx context.Context, spec storage.MountSpec) error + Unmount(ctx context.Context, where string) error + Format(ctx context.Context, device, fstype string) error + InspectDevice(ctx context.Context, device string) (storage.DeviceProbe, error) +} + +// StorageGate authorizes a DESTRUCTIVE storage op (a data-bearing wipe/format) through the +// slice-4 reversibility gate. Satisfied by an adapter over reconcile.Gate in main.go. In 8C an +// unsigned destructive op returns (false, "pending_signature"); the signed path is slice 10. +type StorageGate interface { + AuthorizeWipe(device string) (allowed bool, reason string) +} + +// GuestLister lists the host's guests (to map a mount to the guests that depend on it for the +// eject warning). Satisfied by *proxmox.Client. +type GuestLister interface { + ListLXC(ctx context.Context) ([]proxmox.Guest, error) +} + +// ---- handlers --------------------------------------------------------------------------- + +// DiskInfo is one host drive with its data-bearing flag (for the UI). +type DiskInfo struct { + Name string `json:"name"` // PVE storage id + Type string `json:"type"` // local-dir | usb | lvmthin | … + State string `json:"state"` // attached | disconnected + BackingDevice string `json:"backing_device"` // /dev/sdb1, … ("" for network/lvm) + MountPath string `json:"mount_path"` + Class string `json:"class"` // fast | slow | "" + DataBearing bool `json:"data_bearing"` // agent device-inspection verdict (UI hint) + DataReason string `json:"data_reason,omitempty"` +} + +// handleDisks lists the host's drives + data-bearing flags (read-only/benign). +func (s *Server) handleDisks(w http.ResponseWriter, r *http.Request, vmid int) { + if s.disks == nil { + writeErr(w, http.StatusServiceUnavailable, "disk management not configured on this host") + return + } + targets, err := s.storage.Observe(r.Context()) + if err != nil { + writeErr(w, http.StatusBadGateway, "could not read storage view") + return + } + out := make([]DiskInfo, 0, len(targets)) + for _, t := range targets { + di := DiskInfo{ + Name: t.Name, Type: t.Type, State: t.State, + BackingDevice: t.BackingDevice, MountPath: t.MountPath, Class: t.ClassHint, + } + // Inspect the backing device for the UI's data-bearing hint (the authoritative check + // is re-run at format time on the actual device). + if t.BackingDevice != "" { + if probe, perr := s.disks.InspectDevice(r.Context(), t.BackingDevice); perr == nil { + di.DataBearing = probe.DataBearing() + di.DataReason = probe.Reason() + } else { + di.DataBearing = true // fail-safe + di.DataReason = "could not inspect device" + } + } + out = append(out, di) + } + writeOK(w, map[string]any{"vmid": vmid, "disks": out}) +} + +type assignRequest struct { + VMID int `json:"vmid"` + UUID string `json:"uuid"` + Where string `json:"where"` + FSType string `json:"fstype"` + Options string `json:"options"` +} + +// handleDiskAssign attaches a drive as a host mount (benign, additive → EnsureMount). Self-serve. +func (s *Server) handleDiskAssign(w http.ResponseWriter, r *http.Request, vmid int) { + if s.disks == nil { + writeErr(w, http.StatusServiceUnavailable, "disk management not configured on this host") + return + } + var req assignRequest + if !decodeBody(w, r, &req) { + return + } + if !s.scopedFromBody(w, req.VMID, vmid, r.URL.Path) { + return + } + // EnsureMount validates uuid/where/fstype/options itself (storage/validate.go). + if err := s.disks.EnsureMount(r.Context(), storage.MountSpec{ + Name: req.UUID, UUID: req.UUID, Where: req.Where, FSType: req.FSType, Options: req.Options, + }); err != nil { + s.logger.Error("local-api: disk assign", "vmid", vmid, "where", req.Where, "err", err) + writeErr(w, http.StatusBadRequest, "assign failed: "+err.Error()) + return + } + writeOK(w, map[string]any{"vmid": vmid, "assigned": req.Where}) +} + +type ejectRequest struct { + VMID int `json:"vmid"` + Where string `json:"where"` +} + +// handleDiskEject safe-unmounts a host mount (benign — data preserved, re-attachable) and returns +// the guests that depend on it so the controller can warn which apps lose that storage. +func (s *Server) handleDiskEject(w http.ResponseWriter, r *http.Request, vmid int) { + if s.disks == nil { + writeErr(w, http.StatusServiceUnavailable, "disk management not configured on this host") + return + } + var req ejectRequest + if !decodeBody(w, r, &req) { + return + } + if !s.scopedFromBody(w, req.VMID, vmid, r.URL.Path) { + return + } + if strings.TrimSpace(req.Where) == "" { + writeErr(w, http.StatusBadRequest, "where (mountpoint) is required") + return + } + dependents := s.dependentGuests(r.Context(), req.Where) + if err := s.disks.Unmount(r.Context(), req.Where); err != nil { + s.logger.Error("local-api: disk eject", "vmid", vmid, "where", req.Where, "err", err) + writeErr(w, http.StatusBadRequest, "eject failed: "+err.Error()) + return + } + writeOK(w, map[string]any{"vmid": vmid, "ejected": req.Where, "dependent_guests": dependents}) +} + +type formatRequest struct { + VMID int `json:"vmid"` + Device string `json:"device"` + FSType string `json:"fstype"` + // NOTE: any caller-supplied "blank"/"force" claim is deliberately IGNORED — the agent + // inspects the device itself (8C invariant). +} + +// FormatResponse is POST /disks/format. +type FormatResponse struct { + VMID int `json:"vmid"` + Device string `json:"device"` + Formatted bool `json:"formatted"` + DataBearing bool `json:"data_bearing"` + Reason string `json:"reason"` +} + +// handleDiskFormat is the security centerpiece. The agent INSPECTS the device; if it is +// data-bearing it is classified destructive and the gate refuses it `pending_signature` — the +// caller's claim is never trusted. Only a device the agent itself reads as blank is formatted. +func (s *Server) handleDiskFormat(w http.ResponseWriter, r *http.Request, vmid int) { + if s.disks == nil || s.diskGate == nil { + writeErr(w, http.StatusServiceUnavailable, "disk management not configured on this host") + return + } + var req formatRequest + if !decodeBody(w, r, &req) { + return + } + if !s.scopedFromBody(w, req.VMID, vmid, r.URL.Path) { + return + } + if err := storage.ValidateBlockDevice(req.Device); err != nil { + writeErr(w, http.StatusBadRequest, err.Error()) + return + } + if err := storage.ValidateFSType(req.FSType); err != nil { + writeErr(w, http.StatusBadRequest, err.Error()) + return + } + + // AGENT-INTERNAL device inspection — NEVER the caller's claim. + probe, err := s.disks.InspectDevice(r.Context(), req.Device) + if err != nil { + s.logger.Error("local-api: format device inspect", "device", req.Device, "err", err) + // inspect error → fail-safe data-bearing (probe.DataBearing() is true on !Probed) + } + if probe.DataBearing() { + // Destructive: route through the gate. With no operator signature (8C) → pending_signature. + allowed, reason := s.diskGate.AuthorizeWipe(req.Device) + s.logger.Warn("local-api: refusing format of a data-bearing device", + "vmid", vmid, "device", req.Device, "why", probe.Reason(), "gate", reason) + if !allowed { + writeStatus(w, http.StatusForbidden, false, + FormatResponse{VMID: vmid, Device: req.Device, Formatted: false, DataBearing: true, Reason: probe.Reason()}, + "device is data-bearing — format requires operator authorization ("+reason+")") + return + } + // A signed completion would land here in slice 10; 8C never reaches it (gate refuses unsigned). + writeErr(w, http.StatusForbidden, "data-bearing format is not supported in this slice") + return + } + + // Blank device → benign → mkfs. + if err := s.disks.Format(r.Context(), req.Device, req.FSType); err != nil { + s.logger.Error("local-api: format", "vmid", vmid, "device", req.Device, "err", err) + writeErr(w, http.StatusBadGateway, "format failed: "+err.Error()) + return + } + writeOK(w, FormatResponse{VMID: vmid, Device: req.Device, Formatted: true, DataBearing: false, Reason: "blank device formatted " + req.FSType}) +} + +// dependentGuests returns the VMIDs whose config has a mount whose storage backs the ejected +// mount path — best-effort (a scan failure yields an empty list; the eject still proceeds). +func (s *Server) dependentGuests(ctx context.Context, where string) []int { + if s.guestList == nil { + return nil + } + guests, err := s.guestList.ListLXC(ctx) + if err != nil { + s.logger.Warn("local-api: eject dependent-scan: list guests", "err", err) + return nil + } + // Map each storage id whose mount path == `where` (from the storage view) → dependents. + storeForPath := map[string]bool{} + if targets, err := s.storage.Observe(ctx); err == nil { + for _, t := range targets { + if t.MountPath == where { + storeForPath[t.Name] = true + } + } + } + var out []int + for _, g := range guests { + cfg, err := s.guests.GuestConfig(ctx, g.VMID) + if err != nil { + continue + } + for _, mp := range cfg.MountPoints() { + store, mpPath, _ := parseMount(mp) + if storeForPath[store] || mpPath == where { + out = append(out, g.VMID) + break + } + } + } + return out +} diff --git a/internal/localapi/disks_test.go b/internal/localapi/disks_test.go new file mode 100644 index 0000000..4e9cad7 --- /dev/null +++ b/internal/localapi/disks_test.go @@ -0,0 +1,303 @@ +package localapi + +import ( + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "strings" + "sync" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/hub" + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" + "gitea.dooplex.hu/admin/felhom-agent/internal/storage" +) + +// ---- fakes ------------------------------------------------------------------------------ + +type fakeDiskOps struct { + mu sync.Mutex + probe storage.DeviceProbe // returned by InspectDevice (Device filled per call) + inspectErr error + formatCalls []string + mountCalls []storage.MountSpec + unmountCalls []string +} + +func (f *fakeDiskOps) InspectDevice(_ context.Context, device string) (storage.DeviceProbe, error) { + p := f.probe + p.Device = device + return p, f.inspectErr +} +func (f *fakeDiskOps) Format(_ context.Context, device, _ string) error { + f.mu.Lock() + f.formatCalls = append(f.formatCalls, device) + f.mu.Unlock() + return nil +} +func (f *fakeDiskOps) EnsureMount(_ context.Context, spec storage.MountSpec) error { + f.mu.Lock() + f.mountCalls = append(f.mountCalls, spec) + f.mu.Unlock() + return nil +} +func (f *fakeDiskOps) Unmount(_ context.Context, where string) error { + f.mu.Lock() + f.unmountCalls = append(f.unmountCalls, where) + f.mu.Unlock() + return nil +} +func (f *fakeDiskOps) formatted() []string { f.mu.Lock(); defer f.mu.Unlock(); return append([]string(nil), f.formatCalls...) } + +type fakeGate struct { + allowed bool + reason string + calls []string +} + +func (g *fakeGate) AuthorizeWipe(device string) (bool, string) { + g.calls = append(g.calls, device) + return g.allowed, g.reason +} + +type fakeGuestList struct{ guests []proxmox.Guest } + +func (f fakeGuestList) ListLXC(context.Context) ([]proxmox.Guest, error) { return f.guests, nil } + +// newDiskServer builds a server wired with the 8C disk deps (token A → guest 8200). +func newDiskServer(t *testing.T, d *fakeDiskOps, g *fakeGate, sv StorageView, gl GuestLister) http.Handler { + t.Helper() + if sv == nil { + sv = fakeStorage{} + } + srv, err := NewServer(Options{ + ListenAddr: "127.0.0.1:0", + Guests: &fakeGuests{}, + Backups: &fakeBackups{}, + Store: &fakeStore{}, + Storage: sv, + Tokens: staticTokens{"A": 8200, "B": 9300}, + Disks: d, + DiskGate: g, + Guests2: gl, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }) + if err != nil { + t.Fatalf("new server: %v", err) + } + srv.baseCtx = context.Background() + return srv.Handler() +} + +// ---- the security centerpiece ----------------------------------------------------------- + +// A blank device (the agent's own probe says blank) is formatted — mkfs called, gate NOT consulted. +func TestFormat_BlankDevice_Formats(t *testing.T) { + d := &fakeDiskOps{probe: storage.DeviceProbe{Probed: true}} // blank: Probed, nothing set + g := &fakeGate{allowed: false, reason: "pending_signature"} + h := newDiskServer(t, d, g, nil, nil) + + w := do(t, h, "POST", "/disks/format", "A", `{"device":"/dev/sdb","fstype":"ext4"}`) + if w.Code != http.StatusOK { + t.Fatalf("blank format: got %d want 200 (%s)", w.Code, w.Body.String()) + } + if got := d.formatted(); len(got) != 1 || got[0] != "/dev/sdb" { + t.Fatalf("mkfs not called for blank device: %v", got) + } + if len(g.calls) != 0 { + t.Fatal("gate was consulted for a blank-device format (should be benign)") + } +} + +// THE HEADLINE 8C TEST: a caller asks to format a DATA-BEARING device. The agent inspects the +// device itself, classifies it destructive, the gate refuses pending_signature, and **mkfs is +// NEVER called** — the caller's intent cannot wipe data-bearing storage. +func TestFormat_DataBearingDevice_RefusedNoMkfs(t *testing.T) { + d := &fakeDiskOps{probe: storage.DeviceProbe{Probed: true, HasFilesystem: true, FSType: "ext4"}} + g := &fakeGate{allowed: false, reason: "pending_signature"} + h := newDiskServer(t, d, g, nil, nil) + + w := do(t, h, "POST", "/disks/format", "A", `{"device":"/dev/sdb","fstype":"ext4"}`) + if w.Code != http.StatusForbidden { + t.Fatalf("data-bearing format: got %d want 403", w.Code) + } + if got := d.formatted(); len(got) != 0 { + t.Fatalf("mkfs WAS called on a data-bearing device — security invariant violated: %v", got) + } + if len(g.calls) != 1 || g.calls[0] != "/dev/sdb" { + t.Fatalf("gate not consulted for the destructive format: %v", g.calls) + } + if !strings.Contains(w.Body.String(), "operator authorization") { + t.Fatalf("response did not signal operator-authorization needed: %s", w.Body.String()) + } +} + +// Fail-safe: a device the agent could NOT reliably inspect (Probed=false) is treated as +// data-bearing → refused, mkfs not called. +func TestFormat_AmbiguousProbe_TreatedDestructive(t *testing.T) { + d := &fakeDiskOps{probe: storage.DeviceProbe{Probed: false}} // probe failed → DataBearing()=true + g := &fakeGate{allowed: false, reason: "pending_signature"} + h := newDiskServer(t, d, g, nil, nil) + + w := do(t, h, "POST", "/disks/format", "A", `{"device":"/dev/sdb","fstype":"ext4"}`) + if w.Code != http.StatusForbidden { + t.Fatalf("ambiguous device: got %d want 403", w.Code) + } + if got := d.formatted(); len(got) != 0 { + t.Fatalf("mkfs called on an unprobed device: %v", got) + } +} + +// Even a (hypothetical) gate that ALLOWS does not format a data-bearing device in 8C (the signed +// completion is slice 10). +func TestFormat_DataBearing_GateAllows_StillNoMkfsIn8C(t *testing.T) { + d := &fakeDiskOps{probe: storage.DeviceProbe{Probed: true, HasPartitionTable: true}} + g := &fakeGate{allowed: true, reason: "signed"} + h := newDiskServer(t, d, g, nil, nil) + w := do(t, h, "POST", "/disks/format", "A", `{"device":"/dev/sdb","fstype":"ext4"}`) + if w.Code != http.StatusForbidden { + t.Fatalf("got %d want 403 (8C never formats data-bearing)", w.Code) + } + if len(d.formatted()) != 0 { + t.Fatal("mkfs called on a data-bearing device even though 8C must refuse") + } +} + +func TestFormat_RejectsBadDeviceOrFSType(t *testing.T) { + d := &fakeDiskOps{probe: storage.DeviceProbe{Probed: true}} + h := newDiskServer(t, d, &fakeGate{}, nil, nil) + if w := do(t, h, "POST", "/disks/format", "A", `{"device":"/dev/../etc","fstype":"ext4"}`); w.Code != http.StatusBadRequest { + t.Fatalf("bad device: got %d want 400", w.Code) + } + if w := do(t, h, "POST", "/disks/format", "A", `{"device":"/dev/sdb","fstype":"ntfs"}`); w.Code != http.StatusBadRequest { + t.Fatalf("bad fstype: got %d want 400", w.Code) + } + if len(d.formatted()) != 0 { + t.Fatal("formatted despite invalid input") + } +} + +// ---- assign / eject --------------------------------------------------------------------- + +func TestAssign_EnsureMount(t *testing.T) { + d := &fakeDiskOps{} + h := newDiskServer(t, d, &fakeGate{}, nil, nil) + w := do(t, h, "POST", "/disks/assign", "A", `{"uuid":"1234-ABCD","where":"/mnt/data","fstype":"ext4"}`) + if w.Code != http.StatusOK { + t.Fatalf("assign: got %d (%s)", w.Code, w.Body.String()) + } + d.mu.Lock() + defer d.mu.Unlock() + if len(d.mountCalls) != 1 || d.mountCalls[0].Where != "/mnt/data" || d.mountCalls[0].UUID != "1234-ABCD" { + t.Fatalf("EnsureMount not called correctly: %+v", d.mountCalls) + } +} + +func TestEject_UnmountAndDependents(t *testing.T) { + d := &fakeDiskOps{} + // storage view: target "bulk" is mounted at /mnt/bulk + sv := fakeStorage{targets: []hub.StorageTarget{{Name: "bulk", MountPath: "/mnt/bulk"}}} + // guest 8200 mounts storage "bulk"; guest 9300 does not + gl := fakeGuestList{guests: []proxmox.Guest{{VMID: 8200}, {VMID: 9300}}} + h := newDiskServerWithGuestConfigs(t, d, sv, gl, map[int]map[string]string{ + 8200: {"mp0": "bulk:200,mp=/mnt/media,backup=0"}, + 9300: {"mp0": "local-lvm:8,mp=/var,backup=1"}, + }) + + w := do(t, h, "POST", "/disks/eject", "A", `{"where":"/mnt/bulk"}`) + if w.Code != http.StatusOK { + t.Fatalf("eject: got %d (%s)", w.Code, w.Body.String()) + } + d.mu.Lock() + unmounts := append([]string(nil), d.unmountCalls...) + d.mu.Unlock() + if len(unmounts) != 1 || unmounts[0] != "/mnt/bulk" { + t.Fatalf("Unmount not called: %v", unmounts) + } + var resp struct { + Data struct { + DependentGuests []int `json:"dependent_guests"` + } `json:"data"` + } + _ = json.Unmarshal(w.Body.Bytes(), &resp) + found := false + for _, v := range resp.Data.DependentGuests { + if v == 8200 { + found = true + } + if v == 9300 { + t.Fatal("9300 listed as dependent but it does not mount bulk") + } + } + if !found { + t.Fatalf("dependent guest 8200 not reported: %v", resp.Data.DependentGuests) + } +} + +// ---- auth / config ---------------------------------------------------------------------- + +func TestDisks_CrossGuest403(t *testing.T) { + h := newDiskServer(t, &fakeDiskOps{probe: storage.DeviceProbe{Probed: true}}, &fakeGate{}, nil, nil) + if w := do(t, h, "GET", "/disks?vmid=9300", "A", ""); w.Code != http.StatusForbidden { + t.Fatalf("cross-guest /disks: got %d want 403", w.Code) + } + if w := do(t, h, "POST", "/disks/format", "A", `{"vmid":9300,"device":"/dev/sdb","fstype":"ext4"}`); w.Code != http.StatusForbidden { + t.Fatalf("cross-guest format: got %d want 403", w.Code) + } +} + +func TestDisks_NotConfigured(t *testing.T) { + // a server with no disk deps → 503 on disk endpoints + srv, err := NewServer(Options{ + ListenAddr: "127.0.0.1:0", Guests: &fakeGuests{}, Backups: &fakeBackups{}, + Store: &fakeStore{}, Storage: fakeStorage{}, Tokens: staticTokens{"A": 8200}, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }) + if err != nil { + t.Fatal(err) + } + srv.baseCtx = context.Background() + h := srv.Handler() + if w := do(t, h, "POST", "/disks/format", "A", `{"device":"/dev/sdb","fstype":"ext4"}`); w.Code != http.StatusServiceUnavailable { + t.Fatalf("unconfigured format: got %d want 503", w.Code) + } +} + +// ---- helpers ---------------------------------------------------------------------------- + +// newDiskServerWithGuestConfigs is like newDiskServer but with a fakeGuests that returns the given +// per-vmid mount maps (so eject's dependent-scan can resolve). +func newDiskServerWithGuestConfigs(t *testing.T, d *fakeDiskOps, sv StorageView, gl GuestLister, mounts map[int]map[string]string) http.Handler { + t.Helper() + fg := &fakeGuestsCfg{mounts: mounts} + srv, err := NewServer(Options{ + ListenAddr: "127.0.0.1:0", Guests: fg, Backups: &fakeBackups{}, Store: &fakeStore{}, + Storage: sv, Tokens: staticTokens{"A": 8200, "B": 9300}, + Disks: d, DiskGate: &fakeGate{}, Guests2: gl, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }) + if err != nil { + t.Fatal(err) + } + srv.baseCtx = context.Background() + return srv.Handler() +} + +// fakeGuestsCfg returns per-vmid mountpoints from GuestConfig. +type fakeGuestsCfg struct{ mounts map[int]map[string]string } + +func (f *fakeGuestsCfg) GuestConfig(_ context.Context, vmid int) (proxmox.GuestConfig, error) { + extra := map[string]json.RawMessage{} + for k, v := range f.mounts[vmid] { + b, _ := json.Marshal(v) + extra[k] = b + } + return proxmox.GuestConfig{Extra: extra}, nil +} +func (f *fakeGuestsCfg) Snapshot(context.Context, int, string, string) (string, error) { return "", nil } +func (f *fakeGuestsCfg) Rollback(context.Context, int, string) (string, error) { return "", nil } +func (f *fakeGuestsCfg) WaitTask(context.Context, string, proxmox.WaitOptions) (proxmox.TaskStatus, error) { + return proxmox.TaskStatus{ExitStatus: "OK"}, nil +} diff --git a/internal/localapi/server.go b/internal/localapi/server.go index 90116b2..6d1ca20 100644 --- a/internal/localapi/server.go +++ b/internal/localapi/server.go @@ -65,7 +65,13 @@ type Options struct { // is "due" when no successful backup is recorded OR the newest one is older than this. 0 → a // safe default (24h). The hub-served policy is slice 10; this is the agent-local cadence. BackupCadence time.Duration - Logger *slog.Logger + // Disk management (slice 8C) — OPTIONAL. When Disks + DiskGate are set, the /disks endpoints + // are served; otherwise they report "not configured". DiskGate authorizes the destructive + // (data-bearing) format path; Guests lists guests for the eject dependent-warning. + Disks DiskOps + DiskGate StorageGate + Guests2 GuestLister + Logger *slog.Logger } // defaultBackupCadence is the fallback /backup/due window when none is configured. @@ -104,6 +110,10 @@ type Server struct { logger *slog.Logger now func() time.Time + disks DiskOps // slice 8C (optional) + diskGate StorageGate // slice 8C (optional) + guestList GuestLister // slice 8C (optional) + jobsMu sync.Mutex jobs map[int]*backupJob // per-guest backup job state (slice 8B) @@ -133,10 +143,13 @@ func NewServer(o Options) (*Server, error) { store: o.Store, storage: o.Storage, tokens: o.Tokens, - cadence: cadence, - logger: o.Logger, - now: func() time.Time { return time.Now().UTC() }, - jobs: map[int]*backupJob{}, + cadence: cadence, + logger: o.Logger, + now: func() time.Time { return time.Now().UTC() }, + disks: o.Disks, + diskGate: o.DiskGate, + guestList: o.Guests2, + jobs: map[int]*backupJob{}, }, nil } @@ -150,6 +163,11 @@ func (s *Server) Handler() http.Handler { mux.HandleFunc("GET /backup/due", s.withGuest(s.handleBackupDue)) mux.HandleFunc("GET /backup/status", s.withGuest(s.handleBackupStatus)) mux.HandleFunc("GET /restore-test/status", s.withGuest(s.handleRestoreTestStatus)) + // Disk management (slice 8C) — self-scoped; format routes through the data-bearing classifier+gate. + mux.HandleFunc("GET /disks", s.withGuest(s.handleDisks)) + mux.HandleFunc("POST /disks/assign", s.withGuest(s.handleDiskAssign)) + mux.HandleFunc("POST /disks/eject", s.withGuest(s.handleDiskEject)) + mux.HandleFunc("POST /disks/format", s.withGuest(s.handleDiskFormat)) return mux } diff --git a/internal/storage/hostops.go b/internal/storage/hostops.go index 7c2c24c..8efb4cf 100644 --- a/internal/storage/hostops.go +++ b/internal/storage/hostops.go @@ -2,6 +2,7 @@ package storage import ( "context" + "encoding/json" "fmt" "log/slog" "os" @@ -34,6 +35,55 @@ type HostOps interface { // ThinPoolMetadata returns the lvmthin pool's metadata-used fraction (0..1) via lvs. // ok=false when it cannot be read (the field stays null in the report). ThinPoolMetadata(ctx context.Context, vg, pool string) (fraction float64, ok bool) + // InspectDevice probes a block device for data-bearing evidence (filesystem signature, + // partition table, partitions, mounted) — the AGENT-INTERNAL evidence the 8C classifier + // uses, NEVER the caller's claim. Conservative: a failed/ambiguous probe → DataBearing() + // true (fail-safe). This is the read that decides whether a format is benign or destructive. + InspectDevice(ctx context.Context, device string) (DeviceProbe, error) + // Format runs mkfs. on a (validated) device. DESTRUCTIVE to whatever is on the + // device — the caller MUST have classified it non-data-bearing AND/OR routed it through the + // gate first; HostOps only performs an already-authorized format. + Format(ctx context.Context, device, fstype string) error +} + +// DeviceProbe is the result of inspecting a block device for data-bearing evidence (8C). The +// agent decides data-bearing-ness from THIS (its own device read), never from the caller's claim. +type DeviceProbe struct { + Device string `json:"device"` + Probed bool `json:"probed"` // false = the probe failed/was ambiguous → treat as data-bearing + HasFilesystem bool `json:"has_filesystem"` // a filesystem signature (blkid TYPE / USAGE) + HasPartitionTable bool `json:"has_partition_table"` // a partition table (blkid PTTYPE) + HasPartitions bool `json:"has_partitions"` // child partitions present (lsblk) + Mounted bool `json:"mounted"` // currently mounted somewhere + FSType string `json:"fstype,omitempty"` +} + +// DataBearing is the conservative verdict: any signature / partition table / partition / mount — +// OR a probe that did not complete cleanly — makes the device data-bearing. Only a device that +// probed cleanly AND shows none of those is considered blank (benign to format). +func (p DeviceProbe) DataBearing() bool { + if !p.Probed { + return true // fail-safe: never call an unprobed device blank + } + return p.HasFilesystem || p.HasPartitionTable || p.HasPartitions || p.Mounted +} + +// Reason returns a short human string for why the device is data-bearing (for the UI/audit). +func (p DeviceProbe) Reason() string { + switch { + case !p.Probed: + return "device could not be reliably inspected" + case p.Mounted: + return "device is mounted" + case p.HasFilesystem: + return "device has a " + p.FSType + " filesystem" + case p.HasPartitionTable: + return "device has a partition table" + case p.HasPartitions: + return "device has partitions" + default: + return "device is blank" + } } // MountSpec describes a persistent by-UUID mount. @@ -52,6 +102,10 @@ type Binaries struct { Install string Smartctl string 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) } func (b Binaries) withDefaults() Binaries { @@ -67,6 +121,18 @@ func (b Binaries) withDefaults() Binaries { if b.Lvs == "" { b.Lvs = "/usr/sbin/lvs" } + if b.Blkid == "" { + b.Blkid = "/usr/sbin/blkid" + } + if b.Lsblk == "" { + b.Lsblk = "/usr/bin/lsblk" + } + if b.MkfsExt4 == "" { + b.MkfsExt4 = "/usr/sbin/mkfs.ext4" + } + if b.MkfsXfs == "" { + b.MkfsXfs = "/usr/sbin/mkfs.xfs" + } return b } @@ -210,6 +276,87 @@ func (h *SudoHostOps) ThinPoolMetadata(ctx context.Context, vg, pool string) (fl return parseThinPoolMetadata(out) } +// InspectDevice probes a device for data-bearing evidence (8C). It runs `blkid -p -o export` +// (the reliable signature probe) for filesystem/partition-table signatures and `lsblk -J` for +// child partitions + mount state. The verdict defaults to data-bearing on ANY read failure +// (Probed=false), so a compromised caller cannot get a data-bearing device declared blank. +func (h *SudoHostOps) InspectDevice(ctx context.Context, device string) (DeviceProbe, error) { + if err := ValidateBlockDevice(device); err != nil { + return DeviceProbe{Device: device}, err // Probed=false → DataBearing()=true + } + probe := DeviceProbe{Device: device} + + // blkid -p -o export is the authoritative on-disk SIGNATURE probe. Its OUTPUT is the signal: + // any TYPE/PTTYPE/USAGE line is positive data-bearing evidence. Its exit code is NOT relied + // on (blkid exits 2 on a blank device) — output presence is what matters. A broken/empty + // blkid simply adds no positive evidence; lsblk (below) is the read-success authority. + bout, _, _ := h.runner.Run(ctx, h.bins.Blkid, "-p", "-o", "export", device) + for k, v := range parseBlkidExport(bout) { + switch k { + case "TYPE": + probe.HasFilesystem = true + probe.FSType = v + case "PTTYPE": + probe.HasPartitionTable = true + case "USAGE": + if v != "" { + probe.HasFilesystem = true // filesystem/raid/crypto member = data-bearing + } + } + } + + // lsblk -J is the READ-SUCCESS authority + the partition/mount view. It exits 0 on any valid + // device (blank or not), so a clean parse means the agent reliably read the device. If lsblk + // fails, Probed stays false → DataBearing()=true (fail-safe — never call a device blank on a + // failed read). + lout, _, lerr := h.runner.Run(ctx, h.bins.Lsblk, "-J", "-o", "NAME,FSTYPE,PTTYPE,MOUNTPOINT", device) + if lerr == nil { + probe.Probed = true + hasChildren, mounted, fstype, pttype := parseLsblkDevice(lout) + if hasChildren { + probe.HasPartitions = true + } + if mounted { + probe.Mounted = true + } + if fstype != "" { + probe.HasFilesystem = true + if probe.FSType == "" { + probe.FSType = fstype + } + } + if pttype != "" { + probe.HasPartitionTable = true + } + } + return probe, nil +} + +// Format runs mkfs. on a validated device. The caller is responsible for authorization +// (8C: only after classifying the device non-data-bearing, or via a slice-10 operator signature). +func (h *SudoHostOps) Format(ctx context.Context, device, fstype string) error { + if err := ValidateBlockDevice(device); err != nil { + return err + } + 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 + } + h.logger.Info("storage: formatted device", "device", device, "fstype", fstype) + return nil +} + // run execs an allow-listed command with a fixed arg vector and wraps a nonzero exit. func (h *SudoHostOps) run(ctx context.Context, name string, args ...string) error { _, stderr, err := h.runner.Run(ctx, name, args...) @@ -239,6 +386,48 @@ func trim(b []byte) string { return s } +// parseBlkidExport parses `blkid -p -o export` output (KEY=value lines) into a map. +func parseBlkidExport(out []byte) map[string]string { + m := map[string]string{} + for _, line := range strings.Split(string(out), "\n") { + line = strings.TrimSpace(line) + if i := strings.IndexByte(line, '='); i > 0 { + m[line[:i]] = line[i+1:] + } + } + return m +} + +// lsblkDevice mirrors the `lsblk -J` device shape (only the fields we read). +type lsblkDevice struct { + Name string `json:"name"` + FSType string `json:"fstype"` + PTType string `json:"pttype"` + MountPoint string `json:"mountpoint"` + Children []lsblkDevice `json:"children"` +} + +// parseLsblkDevice parses `lsblk -J -o NAME,FSTYPE,PTTYPE,MOUNTPOINT ` for the top device: +// whether it has child partitions, is mounted (itself or any child), and its fstype/pttype. +func parseLsblkDevice(out []byte) (hasChildren, mounted bool, fstype, pttype string) { + var doc struct { + BlockDevices []lsblkDevice `json:"blockdevices"` + } + if json.Unmarshal(out, &doc) != nil || len(doc.BlockDevices) == 0 { + return false, false, "", "" + } + d := doc.BlockDevices[0] + fstype, pttype = d.FSType, d.PTType + hasChildren = len(d.Children) > 0 + mounted = d.MountPoint != "" + for _, c := range d.Children { + if c.MountPoint != "" { + mounted = true + } + } + return hasChildren, mounted, fstype, pttype +} + // NoopHostOps is the safe fallback when the privileged surface is unavailable or declined // (a missing sudoers entry must degrade with a clear warning, not crash — slice notes). It // reports SMART as UNKNOWN, no thin-pool metadata, and errors on any write (so a benign @@ -257,3 +446,11 @@ func (n NoopHostOps) SMART(context.Context, string) (hub.SmartSummary, error) { func (n NoopHostOps) ThinPoolMetadata(context.Context, string, string) (float64, bool) { return 0, false } +func (n NoopHostOps) InspectDevice(_ context.Context, device string) (DeviceProbe, error) { + // Probed=false → DataBearing()=true: with no privileged surface we MUST NOT call any device + // blank (fail-safe — a format would then be refused as destructive). + return DeviceProbe{Device: device}, nil +} +func (n NoopHostOps) Format(context.Context, string, string) error { + return fmt.Errorf("storage: privileged HostOps not configured; cannot format") +} diff --git a/internal/storage/hostops_disk_test.go b/internal/storage/hostops_disk_test.go new file mode 100644 index 0000000..9223f38 --- /dev/null +++ b/internal/storage/hostops_disk_test.go @@ -0,0 +1,185 @@ +package storage + +import ( + "context" + "errors" + "strings" + "testing" +) + +// scriptedRunner returns canned stdout/stderr/err per command name (last arg = device). +type scriptedRunner struct { + calls [][]string + outputs map[string][]byte // keyed by binary basename + errs map[string]error +} + +func (r *scriptedRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) { + r.calls = append(r.calls, append([]string{name}, args...)) + base := name + if i := strings.LastIndexByte(name, '/'); i >= 0 { + base = name[i+1:] + } + return r.outputs[base], nil, r.errs[base] +} + +func (r *scriptedRunner) ran(substr string) bool { + for _, c := range r.calls { + if strings.Contains(strings.Join(c, " "), substr) { + return true + } + } + return false +} + +func newSudo(r *scriptedRunner) *SudoHostOps { + return NewSudoHostOps(SudoHostOpsConfig{Runner: r}) +} + +// ---- validators ------------------------------------------------------------------------- + +func TestValidateBlockDevice(t *testing.T) { + ok := []string{"/dev/sdb", "/dev/sdb1", "/dev/nvme0n1", "/dev/nvme0n1p2", "/dev/vdb3"} + for _, d := range ok { + if err := ValidateBlockDevice(d); err != nil { + t.Errorf("expected %q valid: %v", d, err) + } + } + bad := []string{"/dev/disk/by-uuid/x", "/dev/../etc/passwd", "/dev/sdb; rm -rf /", "/etc/shadow", "/dev/mapper/x", "sdb", ""} + for _, d := range bad { + if err := ValidateBlockDevice(d); err == nil { + t.Errorf("expected %q rejected", d) + } + } +} + +func TestValidateFSType(t *testing.T) { + for _, f := range []string{"ext4", "xfs"} { + if err := ValidateFSType(f); err != nil { + t.Errorf("expected %q valid", f) + } + } + for _, f := range []string{"ntfs", "vfat", "ext4 ", "", "ext4;ls"} { + if err := ValidateFSType(f); err == nil { + t.Errorf("expected %q rejected", f) + } + } +} + +// ---- InspectDevice (data-bearing detection) --------------------------------------------- + +func TestInspect_Blank(t *testing.T) { + // blkid finds nothing (empty output); lsblk reads cleanly and shows a bare disk. + r := &scriptedRunner{ + outputs: map[string][]byte{ + "blkid": nil, + "lsblk": []byte(`{"blockdevices":[{"name":"sdb","fstype":null,"pttype":null,"mountpoint":null}]}`), + }, + errs: map[string]error{"blkid": errors.New("exit status 2")}, // blkid exits non-zero on blank + } + p, err := newSudo(r).InspectDevice(context.Background(), "/dev/sdb") + if err != nil { + t.Fatal(err) + } + if !p.Probed { + t.Fatal("expected a clean probe (lsblk read cleanly)") + } + if p.DataBearing() { + t.Fatalf("blank device classified data-bearing: %+v", p) + } +} + +func TestInspect_HasFilesystem(t *testing.T) { + r := &scriptedRunner{ + outputs: map[string][]byte{ + "blkid": []byte("DEVNAME=/dev/sdb\nTYPE=ext4\nUSAGE=filesystem\n"), + "lsblk": []byte(`{"blockdevices":[{"name":"sdb","fstype":"ext4","pttype":null,"mountpoint":null}]}`), + }, + } + p, _ := newSudo(r).InspectDevice(context.Background(), "/dev/sdb") + if !p.DataBearing() || !p.HasFilesystem || p.FSType != "ext4" { + t.Fatalf("filesystem not detected: %+v", p) + } +} + +func TestInspect_HasPartitionTable(t *testing.T) { + r := &scriptedRunner{ + outputs: map[string][]byte{ + "blkid": []byte("DEVNAME=/dev/sdb\nPTTYPE=gpt\n"), + "lsblk": []byte(`{"blockdevices":[{"name":"sdb","pttype":"gpt","children":[{"name":"sdb1","fstype":"ext4"}]}]}`), + }, + } + p, _ := newSudo(r).InspectDevice(context.Background(), "/dev/sdb") + if !p.DataBearing() || !p.HasPartitionTable || !p.HasPartitions { + t.Fatalf("partition table/children not detected: %+v", p) + } +} + +func TestInspect_Mounted(t *testing.T) { + r := &scriptedRunner{ + outputs: map[string][]byte{ + "blkid": []byte("TYPE=xfs\n"), + "lsblk": []byte(`{"blockdevices":[{"name":"sdb","fstype":"xfs","mountpoint":"/mnt/data"}]}`), + }, + } + p, _ := newSudo(r).InspectDevice(context.Background(), "/dev/sdb") + if !p.Mounted || !p.DataBearing() { + t.Fatalf("mounted not detected: %+v", p) + } +} + +// A probe that fails to read cleanly must be conservative (data-bearing). +func TestInspect_FailedProbe_FailSafe(t *testing.T) { + r := &scriptedRunner{ + outputs: map[string][]byte{"blkid": nil, "lsblk": nil}, + errs: map[string]error{"blkid": errors.New("blkid broke"), "lsblk": errors.New("lsblk broke")}, + } + p, _ := newSudo(r).InspectDevice(context.Background(), "/dev/sdb") + if p.Probed { + t.Fatal("a broken probe must not be 'Probed'") + } + if !p.DataBearing() { + t.Fatal("a broken probe must be treated as data-bearing (fail-safe)") + } +} + +func TestInspect_RejectsBadDevice(t *testing.T) { + if _, err := newSudo(&scriptedRunner{}).InspectDevice(context.Background(), "/dev/../etc"); err == nil { + t.Fatal("expected a bad device to be rejected before any exec") + } +} + +// ---- Format (mkfs) ---------------------------------------------------------------------- + +func TestFormat_Ext4(t *testing.T) { + r := &scriptedRunner{} + if err := newSudo(r).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) + } +} + +func TestFormat_Xfs(t *testing.T) { + r := &scriptedRunner{} + if err := newSudo(r).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) + } +} + +func TestFormat_RejectsBadArgs(t *testing.T) { + r := &scriptedRunner{} + if err := newSudo(r).Format(context.Background(), "/dev/disk/by-uuid/x", "ext4"); err == nil { + t.Fatal("expected bad device rejected") + } + if err := newSudo(r).Format(context.Background(), "/dev/sdb", "ntfs"); err == nil { + t.Fatal("expected bad fstype rejected") + } + if len(r.calls) != 0 { + t.Fatalf("mkfs ran despite invalid input: %v", r.calls) + } +} diff --git a/internal/storage/observe_test.go b/internal/storage/observe_test.go index b349116..3a093bc 100644 --- a/internal/storage/observe_test.go +++ b/internal/storage/observe_test.go @@ -177,6 +177,10 @@ func (f *fakeHostOps) ThinPoolMetadata(_ context.Context, vg, pool string) (floa v, ok := f.metaByPool[vg+"/"+pool] return v, ok } +func (f *fakeHostOps) InspectDevice(_ context.Context, device string) (DeviceProbe, error) { + return DeviceProbe{Device: device, Probed: true}, nil +} +func (f *fakeHostOps) Format(context.Context, string, string) error { return nil } func TestObserve_EnrichesSMARTAndThinPoolMetadata(t *testing.T) { api := &fakeStorageAPI{ diff --git a/internal/storage/validate.go b/internal/storage/validate.go index 7ad509f..7b33f79 100644 --- a/internal/storage/validate.go +++ b/internal/storage/validate.go @@ -25,6 +25,16 @@ var ( // smartctl is run against. Anything else is refused. reSMARTDevice = regexp.MustCompile(`^/dev/(sd[a-z]+|nvme[0-9]+n[0-9]+|hd[a-z]+|vd[a-z]+)$`) + // Block device for inspection / mkfs: a raw disk OR a partition under /dev. Like the SMART + // whitelist but also allows the trailing partition number (sda1, nvme0n1p2, vdb3). No + // by-* symlinks, no device-mapper, no traversal. This is the mkfs/inspect target — it is + // validated AND the agent device-inspects it before any destructive decision (8C). + reBlockDevice = regexp.MustCompile(`^/dev/(sd[a-z]+[0-9]*|nvme[0-9]+n[0-9]+(p[0-9]+)?|hd[a-z]+[0-9]*|vd[a-z]+[0-9]*)$`) + + // Filesystem types the agent will mkfs. Deliberately tiny — the sudoers mkfs entries are + // per-fstype binaries (mkfs.ext4 / mkfs.xfs), so this set MUST match those entries. + reFSType = regexp.MustCompile(`^(ext4|xfs)$`) + // LVM VG / pool names: LVM permits [A-Za-z0-9._+-]; we forbid leading '-' (would look // like a flag) and cap the length. reLVMName = regexp.MustCompile(`^[A-Za-z0-9_+.][A-Za-z0-9_+.-]*$`) @@ -106,6 +116,25 @@ func ValidateSMARTDevice(device string) error { return nil } +// ValidateBlockDevice accepts only a raw disk or partition path under /dev (the mkfs / inspect +// target). The same strict-whitelist discipline as ValidateSMARTDevice: no symlinks, no +// device-mapper, no traversal — refused before any command is built. +func ValidateBlockDevice(device string) error { + if !reBlockDevice.MatchString(device) { + return fmt.Errorf("storage: refusing to operate on non-whitelisted block device %q", device) + } + return nil +} + +// ValidateFSType accepts only a filesystem type the agent is configured to mkfs (ext4|xfs). The +// set MUST match the per-fstype sudoers entries. +func ValidateFSType(fstype string) error { + if !reFSType.MatchString(fstype) { + return fmt.Errorf("storage: unsupported filesystem type %q (want ext4|xfs)", fstype) + } + return nil +} + // ValidateLVMName accepts an LVM VG or LV (pool) name. func ValidateLVMName(name string) error { if name == "" {