agent v0.24.0: role-gate the eject path (system/backup mounts unmount-protected at the agent)

handleDiskEject now resolves the authoritative role of the storage at `where` and refuses 403 (no Unmount) unless it is user-data. Fails safe to protected on ambiguity. Adds roleForMountPath + an injectable HostReader seam for testability. TestEject_RoleGated asserts protected mounts are refused with no Unmount. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-12 09:29:09 +02:00
parent 832b73e6e8
commit 7ae82e1d5d
5 changed files with 153 additions and 9 deletions
@@ -73,6 +73,22 @@ type fakeGuestList struct{ guests []proxmox.Guest }

 func (f fakeGuestList) ListLXC(context.Context) ([]proxmox.Guest, error) { return f.guests, nil }

+// fakeHostReader is a deterministic HostReader fixture for the role classifier: "/" lives on the
+// whole disk /dev/sda, so SystemDisks resolves {/dev/sda} (sysKnown=true) and any storage backed by
+// a different whole-disk (e.g. /dev/sdb1) classifies as user-data without touching the real host.
+type fakeHostReader struct{ mounts []storage.Mount }
+
+func (f fakeHostReader) Mounts() ([]storage.Mount, error)       { return f.mounts, nil }
+func (f fakeHostReader) ResolveUUID(string) (string, bool)      { return "", false }
+func (f fakeHostReader) DeviceExists(string) bool               { return true }
+func (f fakeHostReader) Rotational(string) (bool, bool)         { return false, false }
+func (f fakeHostReader) Removable(string) (bool, bool)          { return false, false }
+
+// sysOnSDA is the default system-disk fixture (root on /dev/sda) used by the disk-server test helpers.
+func sysOnSDA() fakeHostReader {
+	return fakeHostReader{mounts: []storage.Mount{{Device: "/dev/sda1", MountPoint: "/"}}}
+}
+
 // 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()
@@ -89,6 +105,7 @@ func newDiskServer(t *testing.T, d *fakeDiskOps, g *fakeGate, sv StorageView, gl
 		Disks:      d,
 		DiskGate:   g,
 		Guests2:    gl,
+		HostReader: sysOnSDA(),
 		Logger:     slog.New(slog.NewTextHandler(io.Discard, nil)),
 	})
 	if err != nil {
@@ -256,8 +273,9 @@ func TestAssign_EnsureMount(t *testing.T) {

 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"}}}
+	// storage view: USER-DATA target "bulk" (usb on a non-system disk) mounted at /mnt/bulk — the
+	// role-gate permits ejecting it.
+	sv := fakeStorage{targets: []hub.StorageTarget{{Name: "bulk", Type: hub.StorageTypeUSB, BackingDevice: "/dev/sdb1", 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{
@@ -295,6 +313,65 @@ func TestEject_UnmountAndDependents(t *testing.T) {
 	}
 }

+// A2 (security): the eject path is ROLE-GATED at the agent. A system mount (local dir on the OS disk)
+// and a backup mount (PBS) are REFUSED 403 with NO Unmount; only a user-data mount ejects. The UI
+// hiding the button is not the control — a direct API call to unmount a protected storage is refused.
+func TestEject_RoleGated(t *testing.T) {
+	sv := fakeStorage{targets: []hub.StorageTarget{
+		{Name: "bulk", Type: hub.StorageTypeUSB, BackingDevice: "/dev/sdb1", MountPath: "/mnt/bulk"}, // user-data
+		{Name: "local", Type: "local", MountPath: "/var/lib/vz"},                                     // system (builtin dir)
+		{Name: "felhom-pbs", Type: hub.StorageTypePBS, MountPath: "/mnt/pbs"},                         // backup (PBS)
+	}}
+
+	// system mount → refused, no Unmount.
+	d := &fakeDiskOps{}
+	h := newDiskServer(t, d, &fakeGate{}, sv, nil)
+	if w := do(t, h, "POST", "/disks/eject", "A", `{"where":"/var/lib/vz"}`); w.Code != http.StatusForbidden {
+		t.Fatalf("eject system mount: got %d want 403 (%s)", w.Code, w.Body.String())
+	}
+	d.mu.Lock()
+	if len(d.unmountCalls) != 0 {
+		t.Fatalf("Unmount called on a system mount — role-gate bypassed: %v", d.unmountCalls)
+	}
+	d.mu.Unlock()
+
+	// backup mount (PBS) → refused, no Unmount.
+	d2 := &fakeDiskOps{}
+	h2 := newDiskServer(t, d2, &fakeGate{}, sv, nil)
+	if w := do(t, h2, "POST", "/disks/eject", "A", `{"where":"/mnt/pbs"}`); w.Code != http.StatusForbidden {
+		t.Fatalf("eject backup mount: got %d want 403 (%s)", w.Code, w.Body.String())
+	}
+	d2.mu.Lock()
+	if len(d2.unmountCalls) != 0 {
+		t.Fatalf("Unmount called on a backup mount — role-gate bypassed: %v", d2.unmountCalls)
+	}
+	d2.mu.Unlock()
+
+	// user-data mount → ejects (Unmount called once).
+	d3 := &fakeDiskOps{}
+	h3 := newDiskServer(t, d3, &fakeGate{}, sv, nil)
+	if w := do(t, h3, "POST", "/disks/eject", "A", `{"where":"/mnt/bulk"}`); w.Code != http.StatusOK {
+		t.Fatalf("eject user-data mount: got %d want 200 (%s)", w.Code, w.Body.String())
+	}
+	d3.mu.Lock()
+	if len(d3.unmountCalls) != 1 || d3.unmountCalls[0] != "/mnt/bulk" {
+		t.Fatalf("user-data eject did not Unmount /mnt/bulk: %v", d3.unmountCalls)
+	}
+	d3.mu.Unlock()
+
+	// fail-safe: an unknown mount (no storage target) → refused, no Unmount.
+	d4 := &fakeDiskOps{}
+	h4 := newDiskServer(t, d4, &fakeGate{}, sv, nil)
+	if w := do(t, h4, "POST", "/disks/eject", "A", `{"where":"/mnt/unknown"}`); w.Code != http.StatusForbidden {
+		t.Fatalf("eject unresolvable mount: got %d want 403 (fail-safe) (%s)", w.Code, w.Body.String())
+	}
+	d4.mu.Lock()
+	if len(d4.unmountCalls) != 0 {
+		t.Fatalf("Unmount called on an unresolvable mount — fail-safe violated: %v", d4.unmountCalls)
+	}
+	d4.mu.Unlock()
+}
+
 // ---- auth / config ----------------------------------------------------------------------

 func TestDisks_CrossGuest403(t *testing.T) {
@@ -334,7 +411,7 @@ func newDiskServerWithGuestConfigs(t *testing.T, d *fakeDiskOps, sv StorageView,
 	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,
+		Disks: d, DiskGate: &fakeGate{}, Guests2: gl, HostReader: sysOnSDA(),
 		Logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
 	})
 	if err != nil {