fix(localapi): F2 mount-role fallback — enrolled user-data drives ejectable again (v0.73.0)
roleForMountPath resolved role only from the PVE storage view; a bind-mounted RAW enrolled user-data drive is not a PVE storage, so it fail-safe'd to system and the eject/decommission gates 403'd EVERY user-data drive in the standard topology (campaign F2, where=/mnt/teszt_enroll role=system). Add a mount-table fallback mirroring durableIDForMount Impl-2b: device-keyed classification with a whole-disk containment pass (new storage.SameWholeDisk) and the Observe-error early return kept BEFORE the fallback (else a blind view -> permissive). Only roleForMountPath touched. Tests A1/B1/B2/C1-C3 + 3 red-proofs; existing RoleGated tests green unmodified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
This commit is contained in:
@@ -1,3 +1,26 @@
|
||||
## v0.73.0 — F2 mount-role fallback: enrolled user-data drives are ejectable/decommissionable again (2026-07-06)
|
||||
|
||||
Closes campaign finding **F2** (`felhom.eu/documentation/audits/CAMPAIGN-nomercy-2026-07-06.md` +
|
||||
RERUN addendum). `roleForMountPath` (`internal/localapi/disks.go`) resolved a mount's protection role
|
||||
ONLY from the PVE storage view (`Observe`), but a **bind-mounted RAW enrolled user-data drive is not
|
||||
a PVE storage** → no MountPath match → fail-safe `RoleSystem` → the eject/decommission role gates
|
||||
403'd **every user-data drive in the standard topology** (journal: `where=/mnt/teszt_enroll
|
||||
role=system`). Customers could not eject or decommission their own drives.
|
||||
|
||||
- **Fallback** mirrors `durableIDForMount`'s Impl-2b: after the MountPath loop misses (on a SUCCESSFUL
|
||||
Observe), resolve the mount's backing device from the host mount table and classify **device-keyed**
|
||||
— non-`/dev` source (NAS) → system; device on the same whole disk as a KNOWN target → THAT target's
|
||||
role (containment, via new `storage.SameWholeDisk`, whole-disk granularity so a protected disk can't
|
||||
be ejected through this path); else `RoleForRawDevice`. **Fail-safe preserved**: an Observe error
|
||||
returns system BEFORE the fallback (a blind containment pass could label a backup drive user-data —
|
||||
permissive), and a mount-table-read failure or an absent/NAS mount → system.
|
||||
- Scope: **only** `roleForMountPath`; gates, `deviceRole`, `DecommissionExecutor`, `classify.go`,
|
||||
`ReassertGuestBinds` untouched; the `deviceRole`/`roleForMountPath` unification is deferred.
|
||||
- Tests (`f2_role_fallback_test.go`): A1 ejectable + decommission effects; B1 containment→403; B2
|
||||
system-disk→403; C1/C2 fail-safe; C3 Observe-error skips the fallback. Three red-proofs demonstrated
|
||||
(pre-fix→A1 FAIL, containment-skip→B1 FAIL, fallback-on-error→C3 FAIL). Existing RoleGated/
|
||||
Decommission tests green unmodified.
|
||||
|
||||
## v0.72.0 — OOB operator access: rendered operator /32 + dedicated felhom-sshd + port-adaptive belt + oob health (TASK H1) (2026-07-05)
|
||||
|
||||
The agent half of the merged E1+H1 operator-SSH-access feature (hub half = felhom-hub v0.35.0).
|
||||
|
||||
@@ -1015,20 +1015,56 @@ func (s *Server) hostReader() storage.HostReader {
|
||||
// roleForMountPath resolves the AUTHORITATIVE protection role of the storage mounted at `where`, from
|
||||
// the agent's OWN storage view + host topology (never the caller's claim). It mirrors deviceRole but
|
||||
// keys on the mount path (the eject input). It FAILS SAFE to system (most-protected) on any
|
||||
// ambiguity — a view error, or no storage target found at `where` — so an unresolvable eject is
|
||||
// refused rather than silently unmounted.
|
||||
// ambiguity — a view error, an unrecognizable mount source, or `where` absent from both the storage
|
||||
// view and the mount table — so an unresolvable eject is refused rather than silently unmounted.
|
||||
func (s *Server) roleForMountPath(ctx context.Context, where string) storage.DeviceRole {
|
||||
sysDisks, sysKnown := storage.SystemDisks(s.hostReader())
|
||||
targets, err := s.storage.Observe(ctx)
|
||||
if err != nil {
|
||||
return storage.RoleSystem // can't read the view → treat as protected
|
||||
// Can't read the view → protected. The mount-table fallback below must NOT run here: with
|
||||
// the PVE view down its containment pass is blind, and falling through to raw classification
|
||||
// could label a backup-backing device user-data — a PERMISSIVE regression.
|
||||
return storage.RoleSystem
|
||||
}
|
||||
for _, t := range targets {
|
||||
if t.MountPath == where {
|
||||
return storage.RoleForStorage(t.Type, t.BackingDevice, sysDisks, sysKnown)
|
||||
}
|
||||
}
|
||||
return storage.RoleSystem // no storage target at this mount → fail safe to protected
|
||||
// Fallback (campaign F2, 2026-07-06 — mirrors durableIDForMount's Impl-2b): a RAW enrolled
|
||||
// user-data drive is NOT a PVE storage, so it never appears in Observe — which made this
|
||||
// function fail-safe every such mount to `system` and the eject/decommission gates 403 EVERY
|
||||
// user-data drive in the standard topology (journal: where=/mnt/teszt_enroll role=system).
|
||||
// Resolve the mount's backing device from the host mount table, then classify device-keyed:
|
||||
// - non-/dev source (NAS "server:/export", tmpfs, …) → system (never drive-ejectable);
|
||||
// - device on the same whole disk as a KNOWN storage target → THAT target's role (containment:
|
||||
// felhom-usb/felhom-flash are dir storages with backup content — their disk must never
|
||||
// become ejectable through this path);
|
||||
// - otherwise RoleForRawDevice (system-disk membership; fail-safe to system).
|
||||
mounts, merr := s.hostReader().Mounts()
|
||||
if merr != nil {
|
||||
return storage.RoleSystem // can't read the mount table → protected
|
||||
}
|
||||
for _, m := range mounts {
|
||||
if m.MountPoint != where {
|
||||
continue
|
||||
}
|
||||
if !strings.HasPrefix(m.Device, "/dev/") {
|
||||
return storage.RoleSystem // network/virtual source — has its own lifecycle, protected here
|
||||
}
|
||||
// Containment pass over the ALREADY-FETCHED targets (never re-Observe: a racing error there
|
||||
// would degrade to the permissive raw path). Compare at whole-disk granularity: targets carry
|
||||
// the PARTITION as BackingDevice (a dir storage on /dev/sdb1) while a raw enrolled drive mounts
|
||||
// the WHOLE disk (/dev/sdb) — SameWholeDisk normalizes both so a backup disk stays protected.
|
||||
for _, t := range targets {
|
||||
if t.BackingDevice != "" && storage.SameWholeDisk(t.BackingDevice, m.Device) {
|
||||
return storage.RoleForStorage(t.Type, t.BackingDevice, sysDisks, sysKnown)
|
||||
}
|
||||
}
|
||||
// No known target claims this device → classify the raw device by system-disk membership.
|
||||
return storage.RoleForRawDevice(m.Device, sysDisks, sysKnown)
|
||||
}
|
||||
return storage.RoleSystem // no storage target and no mount-table entry → fail safe to protected
|
||||
}
|
||||
|
||||
// deviceRole resolves a device's AUTHORITATIVE protection tier. It prefers a known storage target's
|
||||
|
||||
@@ -0,0 +1,192 @@
|
||||
package localapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/storage"
|
||||
)
|
||||
|
||||
// f2HostReader is a configurable HostReader fixture for the roleForMountPath F2 fallback: it serves a
|
||||
// mount table (device→mountpoint), resolves fs-UUIDs for the decommission bind-prune, and COUNTS
|
||||
// Mounts() calls so a test can prove the fallback did/didn't run (SystemDisks calls Mounts() once per
|
||||
// request; the fallback is a SECOND call — so Observe-error → exactly 1 call proves the fallback was
|
||||
// skipped).
|
||||
type f2HostReader struct {
|
||||
mounts []storage.Mount
|
||||
uuids map[string]string
|
||||
mu sync.Mutex
|
||||
calls int
|
||||
}
|
||||
|
||||
func (f *f2HostReader) Mounts() ([]storage.Mount, error) {
|
||||
f.mu.Lock()
|
||||
f.calls++
|
||||
f.mu.Unlock()
|
||||
return f.mounts, nil
|
||||
}
|
||||
func (f *f2HostReader) ResolveUUID(dev string) (string, bool) { u, ok := f.uuids[dev]; return u, ok }
|
||||
func (f *f2HostReader) DeviceExists(string) bool { return true }
|
||||
func (f *f2HostReader) Rotational(string) (bool, bool) { return false, false }
|
||||
func (f *f2HostReader) Removable(string) (bool, bool) { return false, false }
|
||||
func (f *f2HostReader) mountsCalls() int { f.mu.Lock(); defer f.mu.Unlock(); return f.calls }
|
||||
|
||||
// errStorage is a StorageView whose Observe always fails — for the Observe-error fail-safe (edge C3).
|
||||
type errStorage struct{}
|
||||
|
||||
func (errStorage) Observe(context.Context) ([]hub.StorageTarget, error) {
|
||||
return nil, errors.New("proxmox unreachable")
|
||||
}
|
||||
|
||||
// f2EjectServer builds an eject/decommission-capable server with a custom storage view + host reader.
|
||||
func f2EjectServer(t *testing.T, d *fakeDiskOps, sv StorageView, host storage.HostReader) *Server {
|
||||
t.Helper()
|
||||
srv := newDiskServerRaw(t, d, &fakeGate{}, sv, nil)
|
||||
if host != nil {
|
||||
srv.host = host
|
||||
}
|
||||
return srv
|
||||
}
|
||||
|
||||
// A1 — the campaign shape: a bind-mounted enrolled user-data drive (/mnt/teszt_enroll on /dev/sdb) is
|
||||
// NOT a PVE storage, so it misses the MountPath loop. Pre-fix → fail-safe system → 403 EVERY user-data
|
||||
// drive. Post-fix → the mount-table fallback classifies /dev/sdb (not the system disk /dev/sda) as
|
||||
// user-data → eject 200 + decommission fires its effects.
|
||||
//
|
||||
// COMPANION RED-PROOF: on the pre-fix roleForMountPath (the plain "no target → RoleSystem" body) this
|
||||
// eject returns 403 → the 200 assertion FAILS.
|
||||
func TestRoleForMountPath_F2_BindMountedUserData_Ejectable(t *testing.T) {
|
||||
// storage view: a PBS + a system dir, but NOTHING at /mnt/teszt_enroll (the raw enrolled drive).
|
||||
sv := fakeStorage{targets: []hub.StorageTarget{
|
||||
{Name: "felhom-pbs", Type: hub.StorageTypePBS, MountPath: "/mnt/pbs"},
|
||||
{Name: "local", Type: "local", MountPath: "/var/lib/vz"},
|
||||
}}
|
||||
host := &f2HostReader{
|
||||
mounts: []storage.Mount{
|
||||
{Device: "/dev/sda1", MountPoint: "/"}, // system disk
|
||||
{Device: "/dev/sdb", MountPoint: "/mnt/teszt_enroll"}, // the raw enrolled user-data drive
|
||||
},
|
||||
uuids: map[string]string{"/dev/sdb": "f2236136-ced7"},
|
||||
}
|
||||
|
||||
// eject → 200, and the raw is NOT unmounted (intermediary model).
|
||||
d := &fakeDiskOps{}
|
||||
h := f2EjectServer(t, d, sv, host).Handler()
|
||||
if w := do(t, h, "POST", "/disks/eject", "A", `{"where":"/mnt/teszt_enroll"}`); w.Code != http.StatusOK {
|
||||
t.Fatalf("eject bind-mounted user-data drive: got %d want 200 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
d.mu.Lock()
|
||||
if len(d.unmountCalls) != 0 {
|
||||
t.Fatalf("user-data eject must not unmount the raw drive: %v", d.unmountCalls)
|
||||
}
|
||||
d.mu.Unlock()
|
||||
|
||||
// decommission → 200 + all three effects (intent, bind prune, DetachDrive), no unmount/format.
|
||||
intent := newFakeIntent()
|
||||
intent.SetEnrolled("uuid:f2236136-ced7")
|
||||
gb := tempBindStore(t)
|
||||
_ = gb.Record(8200, "uuid:f2236136-ced7")
|
||||
ga := &fakeGuestAttacher{}
|
||||
d2 := &fakeDiskOps{}
|
||||
srv := decommServer(t, d2, sv, fakeGuestList{}, intent, gb, ga, nil)
|
||||
srv.host = host // inject the teszt_enroll mount table
|
||||
if w := do(t, srv.Handler(), "POST", "/disks/decommission", "A", `{"where":"/mnt/teszt_enroll"}`); w.Code != http.StatusOK {
|
||||
t.Fatalf("decommission bind-mounted user-data drive: got %d want 200 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if got := intent.Get("uuid:f2236136-ced7"); got != storage.IntentDecommissioned {
|
||||
t.Errorf("intent = %q, want decommissioned", got)
|
||||
}
|
||||
if ids := gb.Guests()[8200]; len(ids) != 0 {
|
||||
t.Errorf("guest-bind not pruned: %v", ids)
|
||||
}
|
||||
if len(ga.detachDrives) != 1 || ga.detachDrives[0] != "/mnt/teszt_enroll" {
|
||||
t.Errorf("DetachDrive calls = %v, want [/mnt/teszt_enroll]", ga.detachDrives)
|
||||
}
|
||||
d2.mu.Lock()
|
||||
defer d2.mu.Unlock()
|
||||
if len(d2.unmountCalls) != 0 || len(d2.formatCalls) != 0 {
|
||||
t.Errorf("decommission must not unmount/format: unmount=%v format=%v", d2.unmountCalls, d2.formatCalls)
|
||||
}
|
||||
}
|
||||
|
||||
// B1 (containment) — a mount whose device shares a whole disk with a KNOWN protected target must honor
|
||||
// THAT target's role, not raw-classify. Here /dev/sdb1 backs a lvmthin (→ system) target; a mount on
|
||||
// /dev/sdb1 that misses the MountPath loop must resolve to system (403), NOT user-data.
|
||||
//
|
||||
// COMPANION RED-PROOF: the containment-skipping mutation (fallback = bare RoleForRawDevice(m.Device))
|
||||
// returns user-data for /dev/sdb1 (not the system disk) → this eject returns 200 → the 403 FAILS.
|
||||
func TestRoleForMountPath_F2_Containment(t *testing.T) {
|
||||
sv := fakeStorage{targets: []hub.StorageTarget{
|
||||
// lvmthin on an external disk /dev/sdb → RoleForStorage(lvmthin) = system (default tier).
|
||||
{Name: "extra-lvm", Type: "lvmthin", BackingDevice: "/dev/sdb1", MountPath: "/never/matched"},
|
||||
}}
|
||||
host := &f2HostReader{mounts: []storage.Mount{
|
||||
{Device: "/dev/sda1", MountPoint: "/"},
|
||||
{Device: "/dev/sdb1", MountPoint: "/mnt/onprotecteddisk"},
|
||||
}}
|
||||
d := &fakeDiskOps{}
|
||||
h := f2EjectServer(t, d, sv, host).Handler()
|
||||
if w := do(t, h, "POST", "/disks/eject", "A", `{"where":"/mnt/onprotecteddisk"}`); w.Code != http.StatusForbidden {
|
||||
t.Fatalf("containment: mount on a protected target's disk must be 403, got %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
d.mu.Lock()
|
||||
if len(d.unmountCalls) != 0 {
|
||||
t.Fatalf("containment: no unmount on the refused mount: %v", d.unmountCalls)
|
||||
}
|
||||
d.mu.Unlock()
|
||||
}
|
||||
|
||||
// B2 — a mount on the SYSTEM disk (/dev/sda) is refused: RoleForRawDevice sees it is system-backed.
|
||||
func TestRoleForMountPath_F2_SystemDiskMount(t *testing.T) {
|
||||
sv := fakeStorage{targets: []hub.StorageTarget{{Name: "felhom-pbs", Type: hub.StorageTypePBS, MountPath: "/mnt/pbs"}}}
|
||||
host := &f2HostReader{mounts: []storage.Mount{
|
||||
{Device: "/dev/sda1", MountPoint: "/"},
|
||||
{Device: "/dev/sda3", MountPoint: "/mnt/onsystemdisk"},
|
||||
}}
|
||||
h := f2EjectServer(t, &fakeDiskOps{}, sv, host).Handler()
|
||||
if w := do(t, h, "POST", "/disks/eject", "A", `{"where":"/mnt/onsystemdisk"}`); w.Code != http.StatusForbidden {
|
||||
t.Fatalf("system-disk mount must be 403, got %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// C1/C2 — fail-safe: a mount absent from the table, and a non-/dev (NAS) source, both refuse.
|
||||
func TestRoleForMountPath_F2_FailSafe(t *testing.T) {
|
||||
sv := fakeStorage{targets: []hub.StorageTarget{{Name: "felhom-pbs", Type: hub.StorageTypePBS, MountPath: "/mnt/pbs"}}}
|
||||
host := &f2HostReader{mounts: []storage.Mount{
|
||||
{Device: "/dev/sda1", MountPoint: "/"},
|
||||
{Device: "nas.local:/export", MountPoint: "/mnt/nas"}, // NAS: own lifecycle, never drive-ejectable
|
||||
}}
|
||||
// C1: mount not in the table at all.
|
||||
h := f2EjectServer(t, &fakeDiskOps{}, sv, host).Handler()
|
||||
if w := do(t, h, "POST", "/disks/eject", "A", `{"where":"/mnt/absent"}`); w.Code != http.StatusForbidden {
|
||||
t.Fatalf("absent mount must be 403, got %d", w.Code)
|
||||
}
|
||||
// C2: NAS-backed mount → system (non-/dev source).
|
||||
if w := do(t, h, "POST", "/disks/eject", "A", `{"where":"/mnt/nas"}`); w.Code != http.StatusForbidden {
|
||||
t.Fatalf("NAS mount must be 403 (has its own lifecycle), got %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// C3 — the CRITICAL fail-safe: when Observe() fails the fallback must NOT run (a blind containment
|
||||
// pass could label a backup drive user-data — permissive). Assert 403 AND that Mounts() was called
|
||||
// exactly once (by SystemDisks; the fallback — a second call — was skipped).
|
||||
//
|
||||
// COMPANION RED-PROOF: moving the fallback ahead of the Observe-error return (running it regardless)
|
||||
// classifies /dev/sdb as user-data → this eject returns 200 → the 403 FAILS.
|
||||
func TestRoleForMountPath_F2_ObserveError_NoFallback(t *testing.T) {
|
||||
host := &f2HostReader{mounts: []storage.Mount{
|
||||
{Device: "/dev/sda1", MountPoint: "/"},
|
||||
{Device: "/dev/sdb", MountPoint: "/mnt/teszt_enroll"}, // would classify user-data IF the fallback ran
|
||||
}}
|
||||
h := f2EjectServer(t, &fakeDiskOps{}, errStorage{}, host).Handler()
|
||||
if w := do(t, h, "POST", "/disks/eject", "A", `{"where":"/mnt/teszt_enroll"}`); w.Code != http.StatusForbidden {
|
||||
t.Fatalf("Observe error must fail safe to 403, got %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if c := host.mountsCalls(); c != 1 {
|
||||
t.Fatalf("on Observe error the fallback must be skipped: Mounts() called %d times, want 1 (SystemDisks only)", c)
|
||||
}
|
||||
}
|
||||
@@ -136,3 +136,14 @@ func RoleForRawDevice(device string, sysDisks map[string]bool, sysKnown bool) De
|
||||
}
|
||||
return RoleUserData
|
||||
}
|
||||
|
||||
// SameWholeDisk reports whether two device paths live on the same physical whole disk
|
||||
// (e.g. /dev/sdb and /dev/sdb1). Storage targets carry BOTH granularities in practice (a dir
|
||||
// storage's BackingDevice is the mounted PARTITION, a raw enrolled drive mounts the WHOLE disk),
|
||||
// so containment checks must compare at whole-disk level — mirroring isSystemBacked. False when
|
||||
// either side's whole-disk is unrecognizable (device-mapper/network) — callers fail safe.
|
||||
func SameWholeDisk(a, b string) bool {
|
||||
wa, oka := wholeDiskOf(a)
|
||||
wb, okb := wholeDiskOf(b)
|
||||
return oka && okb && wa == wb
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user