diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d6b4df..b94d71d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,40 @@ +## v0.211.0 — the wall a rebuilt box could not get past (2026-08-10, R-280) — MinAgent 0.127.0 + +**R-280 — after a reinstall the data drive can be re-attached, and the page stops promising a click +that does not exist.** Measured on the rebuilt demo-hp 2026-08-09: the restore page diagnosed the +situation perfectly, said *„Ez két kattintás"*, and pointed at a picker holding nothing. It was zero +clicks; getting past it needed an internal path no customer could produce. + +**Why the obvious fix would not have worked, kept here because it cost the session an hour.** The +agent builds BOTH `initialize` and `attach` from its unclaimed-DISK scan, and widening that scan is +the fix the finding proposed. But the filesystem a rebuilt box must re-register is an **in-guest** +one — on demo-hp `/mnt/sys_drive`, the guest's own 70 GB data volume, which is what the escape hatch +actually registered. The agent enumerates HOST block devices and would have offered the 1 TB NVMe +(the `felhom-backup` target): the wrong drive, non-destructively attached, customer data still +unreachable. **So this ships in the controller and the agent is unchanged** — MinAgent stays 0.127.0. + +- **`attach` now also carries the controller's own mounted-but-unregistered filesystems** + (`internal/web/attach_sources.go`), read from its own mount table — the controller runs in-guest + with `/mnt` bind-mounted in, so what it can see is what it can register. `initialize` is passed + through **untouched**: the format wizard's system/backup protection lives in the agent's scan and + is not widened by a single line here (pinned by `TestMergeAttachCandidates_InitializeIsUntouched`, + whose red-proof put the customer's data volume in the FORMAT list). +- **A union, not a replacement.** The agent's entries serve the case this wizard was built for — a + fresh external drive carrying a filesystem, not yet mounted — which a mount table cannot report + precisely because it is not mounted. Dropping them would fix the reinstall and break the USB. +- **These candidates are REGISTERED in place, never mounted** (`POST /api/storage/register-mounted`). + The posted path is re-derived server-side and refused if it is not currently offered, so the route + cannot register an arbitrary directory. +- **The „két kattintás" sentence is now conditional on the picker being non-empty**, and the false + branch says what is true and names a route. Both branches are render-tested. +- **Exclusions, each with a reason in the code:** the guest rootfs at `/mnt`, the intermediary-model + parent `/mnt/felhom-drives`, tmpfs/overlay, anything outside `/mnt/`, non-ext4/xfs, already- + registered paths — and **any bind alias of the rootfs, excluded by DEVICE**, because a bind + republishes a filesystem under a second path and registering that one would put app data on the + box's own root. That last guard was found by trying to build the live reproduction, not by review. +- **Fail-safe:** an unreadable mount table yields an EMPTY list, never a permissive one — and because + the page gates its promise on that list, "we could not look" renders as "we cannot offer this". + ## v0.210.0 — two pictures that were not true (2026-08-08, R-259 / R-258) — MinAgent 0.127.0 Both are the same shape: something the box already knows, drawn as its opposite. diff --git a/controller/internal/agentapi/client.go b/controller/internal/agentapi/client.go index a006219..69ef681 100644 --- a/controller/internal/agentapi/client.go +++ b/controller/internal/agentapi/client.go @@ -424,6 +424,11 @@ type DiskCandidate struct { Mountable bool `json:"mountable"` MountSource string `json:"mount_source,omitempty"` DurableID string `json:"durable_id,omitempty"` + // AlreadyMounted marks a candidate the CONTROLLER contributed from its own mount table (R-280), + // not one the agent scanned. The agent NEVER sets it. Its action is REGISTER the existing + // mountpoint — sending it down the device-attach path would try to mount an in-guest path as if + // it were a raw device. See web/attach_sources.go for why the agent's scan cannot supply these. + AlreadyMounted bool `json:"already_mounted,omitempty"` } // CandidatesResult mirrors GET /disks/candidates: disks free to enroll, split into initialize (all diff --git a/controller/internal/web/agent_disk_handlers.go b/controller/internal/web/agent_disk_handlers.go index 80257ed..cc3ed77 100644 --- a/controller/internal/web/agent_disk_handlers.go +++ b/controller/internal/web/agent_disk_handlers.go @@ -126,8 +126,18 @@ func (s *Server) agentDisksListHandler(w http.ResponseWriter, r *http.Request) { // agentDiskCandidatesHandler proxies GET /api/disks/candidates → agent GET /disks/candidates (Impl-2b): // the raw-device scan (Impl-2a) that feeds the enrollment wizards. The agent's unclaimed-disk filter -// already excludes claimed/OS/enrolled disks (fail-safe), so the controller passes the list through -// untouched — no controller-side filtering. +// already excludes claimed/OS/enrolled disks (fail-safe), so `initialize` passes through UNTOUCHED — +// no controller-side filtering, and the system/backup drives it hides from the format wizard stay +// hidden. +// +// R-280: `attach` additionally carries the controller's own mounted-but-unregistered filesystems. +// The agent's scan alone left a rebuilt box with an empty picker under a sentence promising „két +// kattintás", because the drive that must be re-registered is an in-guest filesystem no host-disk +// scan can see. Attaching is non-destructive, so this list is additive by nature — it can only ever +// offer MORE places to put data back, never a new way to erase any. Why the union rather than a +// replacement: the agent's entries serve the case this endpoint was built for — a fresh external +// drive that already carries a filesystem and is not yet mounted — which the mount table cannot +// report precisely because it is not mounted. Dropping them would fix the reinstall and break the USB. func (s *Server) agentDiskCandidatesHandler(w http.ResponseWriter, r *http.Request) { client, err := s.agentClient() if err != nil { @@ -140,7 +150,35 @@ func (s *Server) agentDiskCandidatesHandler(w http.ResponseWriter, r *http.Reque writeDiskJSON(w, http.StatusBadGateway, false, err.Error(), nil) return } - writeDiskJSON(w, http.StatusOK, true, "", resp) + writeDiskJSON(w, http.StatusOK, true, "", mergeAttachCandidates(resp, s.attachableStores())) +} + +// mergeAttachCandidates adds the mounted-but-unregistered stores to `attach` and returns the result. +// `initialize` is passed through untouched — the ONE line that keeps the format wizard's protection +// intact, and the reason this is a separate function rather than two appends at the call site: it can +// be tested, and a change to it fails a test instead of shipping. +func mergeAttachCandidates(resp agentapi.CandidatesResult, stores []mountedStore) agentapi.CandidatesResult { + resp.Attach = append(resp.Attach, mountedStoreCandidates(stores)...) + return resp +} + +// mountedStoreCandidates renders mounted-but-unregistered stores in the picker's shape. MountSource +// carries the mountpoint (the thing the register action needs); Device is display only. +func mountedStoreCandidates(stores []mountedStore) []agentapi.DiskCandidate { + out := make([]agentapi.DiskCandidate, 0, len(stores)) + for _, m := range stores { + out = append(out, agentapi.DiskCandidate{ + Device: m.Device, + FSType: m.FSType, + MountSource: m.Path, + DataBearing: true, + Mountable: true, + // Size is deliberately absent: measuring it means statfs on a possibly-wedged device + // inside a request handler, and a picker entry is actionable without it. + AlreadyMounted: true, + }) + } + return out } // sortDisksForView orders the agent's disk list deterministically (user-data → system → backup → diff --git a/controller/internal/web/attach_sources.go b/controller/internal/web/attach_sources.go new file mode 100644 index 0000000..f91ac16 --- /dev/null +++ b/controller/internal/web/attach_sources.go @@ -0,0 +1,163 @@ +package web + +import ( + "os" + "path" + "sort" + "strings" +) + +// R-280 — where the `attach` list comes from, and why it is NOT the agent's disk scan. +// +// After a reinstall the customer's drives survive but their REGISTRATION does not, and every restore +// then refuses. The restore page diagnosed that correctly and sent the customer to a page offering +// nothing: `GET /disks/candidates` returned `{"initialize":[],"attach":[]}`. +// +// The agent builds BOTH lists from its unclaimed-DISK scan (felhom-agent +// internal/localapi/disks.go handleDiskCandidates → storage.ListCandidateDisks). That filter is +// CORRECT for `initialize` — never offer to format a disk in use — and over-broad for `attach`, +// which is non-destructive. +// +// But widening the agent's scan would still not fix it, and that is the part worth writing down: +// the filesystem that must be re-registered after a reinstall is an IN-GUEST one. On the rebuilt +// demo-hp the drive the customer needed was `/mnt/sys_drive` (the guest's own data volume), and the +// escape hatch that unblocked everything registered exactly that path. The agent's host-disk scan +// cannot see it — it enumerates HOST block devices, and would have offered the 1 TB NVMe (the +// felhom-backup target) instead: the wrong drive, non-destructively attached, and the customer's +// data still not reachable. +// +// So the attach source is the controller's OWN mount table. The controller runs in-guest with /mnt +// bind-mounted in, so the filesystems it can see ARE the ones it can register — the source and the +// action finally agree. +// +// These candidates are ALREADY MOUNTED, so the action is REGISTER, never mount-a-device. That is why +// they carry AlreadyMounted: the wizard must not send them down the device-attach path, which would +// try to mount an in-guest path as if it were a raw device. + +// mountedFSTypes are the on-disk filesystems a mounted store may carry. Deliberately the same pair +// the agent calls attach-mountable (storage.mountableFSTypes) and the init flow offers +// (validFSTypes) — a third dialect here is how the three lists drift apart. +var mountedFSTypes = map[string]bool{"ext4": true, "xfs": true} + +// managedDrivesParent is the intermediary-model PARENT. Children of it are real drives; the parent +// itself is the container that holds them and is never a storage destination of its own. +const managedDrivesParent = "/mnt/felhom-drives" + +// mountedStore is one already-mounted filesystem the controller can register as a storage location. +type mountedStore struct { + Path string // the in-guest mountpoint, e.g. /mnt/sys_drive — what gets registered, verbatim + Device string // backing device, for display only ("/dev/mapper/pve-vm--9201--disk--1") + FSType string +} + +// parseMountTable turns mount-table text (/proc/mounts format) into (device, mountpoint, fstype) +// rows. Space escaping (\040) is undone so a path with a space still compares — the same handling +// the agent's own procMounts does. +func parseMountTable(text string) [][3]string { + var out [][3]string + for _, line := range strings.Split(text, "\n") { + fields := strings.Fields(line) + if len(fields) < 3 { + continue + } + out = append(out, [3]string{ + fields[0], + strings.ReplaceAll(fields[1], `\040`, " "), + fields[2], + }) + } + return out +} + +// mountedUnregisteredStores returns the mounted filesystems under /mnt that are NOT already in the +// storage registry — the drives a customer can attach (register) without anything being erased. +// +// ⚠ FAIL-SAFE: an unreadable mount table yields an EMPTY list, never a permissive one. "We could not +// look" must never render as "here is what you may attach" — and, because the caller gates the +// „two kattintás" sentence on this list being non-empty, an empty list makes the page say so plainly +// rather than promise a click that does not exist. +func mountedUnregisteredStores(mountsText string, registered map[string]bool) []mountedStore { + rows := parseMountTable(mountsText) + + // Devices that back the box's OWN root. A bind mount republishes a filesystem under a second + // path, and a bind of the rootfs at /mnt/ looks exactly like a data drive to everything + // below — offering it would invite the customer to store app data on the root filesystem and + // fill it. Excluded by DEVICE, so no alias can smuggle it back in under a different path. + rootDevices := map[string]bool{} + for _, row := range rows { + if w := path.Clean(row[1]); w == "/" || w == "/mnt" { + rootDevices[row[0]] = true + } + } + + var out []mountedStore + seen := map[string]bool{} + for _, row := range rows { + dev, where, fstype := row[0], path.Clean(row[1]), row[2] + + if rootDevices[dev] { + continue + } + + // A real block device only. Excludes overlay/tmpfs/proc/sysfs and every virtual mount. + if !strings.HasPrefix(dev, "/dev/") { + continue + } + // An on-disk filesystem we can actually hand to apps. + if !mountedFSTypes[fstype] { + continue + } + // The storage convention is /mnt/. `/mnt` itself is the guest rootfs mount, not a drive. + if !strings.HasPrefix(where, "/mnt/") || where == "/mnt" { + continue + } + // The intermediary-model parent holds drives; it is not one. + if where == managedDrivesParent { + continue + } + // Already registered → not a candidate. This is what makes a healthy box render exactly as + // before: its store is registered, so it never appears here (Scenario D). + if registered[where] { + continue + } + // /proc/mounts lists a mountpoint once per mount event; a bind or a re-mount would otherwise + // produce the same path twice in the picker. + if seen[where] { + continue + } + seen[where] = true + out = append(out, mountedStore{Path: where, Device: dev, FSType: fstype}) + } + // Deterministic order — the picker must not reshuffle between reloads. + sort.Slice(out, func(i, j int) bool { return out[i].Path < out[j].Path }) + return out +} + +// readMountTable reads the controller's own mount table. Returns "" on failure, which +// mountedUnregisteredStores turns into an empty (never permissive) list. +func readMountTable() string { + b, err := os.ReadFile("/proc/mounts") + if err != nil { + return "" + } + return string(b) +} + +// registeredStoragePaths is the set of paths already in the registry, for the exclusion above. +func (s *Server) registeredStoragePaths() map[string]bool { + out := map[string]bool{} + if s.settings == nil { + return out + } + for _, sp := range s.settings.GetStoragePaths() { + out[path.Clean(sp.Path)] = true + } + return out +} + +// attachableStores is the one derivation both the candidates endpoint and the restore page's +// precondition read, so the picker and the sentence pointing at it cannot disagree — the same +// single-derivation rule R-252 applied to HasRestoreDestination. +func (s *Server) attachableStores() []mountedStore { + return mountedUnregisteredStores(readMountTable(), s.registeredStoragePaths()) +} diff --git a/controller/internal/web/attach_sources_test.go b/controller/internal/web/attach_sources_test.go new file mode 100644 index 0000000..da11c67 --- /dev/null +++ b/controller/internal/web/attach_sources_test.go @@ -0,0 +1,252 @@ +package web + +import ( + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/agentapi" +) + +// R-280 — THE DRIVE CAN BE RE-ATTACHED AFTER A REINSTALL. +// +// Measured on the rebuilt demo-hp (2026-08-09): the restore page diagnosed the situation correctly +// and then sent the customer to a picker that was empty. `GET /disks/candidates` answered +// `{"initialize":[],"attach":[]}` because BOTH lists come from the agent's unclaimed-DISK scan, and +// the box's NVMe is claimed (it is the felhom-backup target). Getting past it needed an internal path +// no customer could produce. +// +// These tests pin the SOURCE of the attach list, and the mount table below is the real one from that +// box — /mnt/sys_drive is the guest data volume the escape hatch had to register by hand. + +// demoHPMounts is guest 9201's actual mount table on the rebuilt demo-hp, trimmed to the rows that +// matter. Keeping the real shape means the fixture cannot quietly diverge from the box. +const demoHPMounts = `proc /proc proc rw,relatime 0 0 +/dev/mapper/pve-vm--9201--disk--0 /mnt ext4 rw,relatime,stripe=16 0 0 +/dev/mapper/pve-root /mnt/felhom-drives ext4 rw,relatime,errors=remount-ro 0 0 +/dev/mapper/pve-vm--9201--disk--1 /mnt/sys_drive ext4 rw,relatime,stripe=16 0 0 +tmpfs /dev/shm tmpfs rw,nosuid,nodev 0 0 +overlay /var/lib/docker/overlay2/x/merged overlay rw,relatime 0 0 +` + +// ── SCENARIO A — a mounted, unregistered filesystem on a CLAIMED disk is offered ──────────────── +// +// RED-PROOF (the one that matters): revert the attach list to the unclaimed-disk scan — i.e. make +// mountedUnregisteredStores return nil, or drop the `resp.Attach = append(...)` line in +// agentDiskCandidatesHandler. This fails with `attach candidates: 0`, which IS yesterday's wall: +// an empty picker under a sentence promising two clicks. +func TestMountedUnregisteredStores_OffersTheGuestDataVolume(t *testing.T) { + got := mountedUnregisteredStores(demoHPMounts, map[string]bool{}) + + if len(got) != 1 { + t.Fatalf("attach candidates: %d, want 1 (/mnt/sys_drive) — got %+v", len(got), got) + } + if got[0].Path != "/mnt/sys_drive" { + t.Errorf("offered %q, want /mnt/sys_drive — the drive the rebuilt box could not re-attach", got[0].Path) + } + if got[0].FSType != "ext4" { + t.Errorf("fstype %q, want ext4", got[0].FSType) + } + if got[0].Device != "/dev/mapper/pve-vm--9201--disk--1" { + t.Errorf("device %q — the backing device is shown to the customer and must be the real one", got[0].Device) + } +} + +// The disk is CLAIMED — that is the whole point. The claim filter is a property of the agent's scan, +// and this source deliberately does not consult it, because attaching erases nothing. This pins that +// the mount table alone decides, so re-introducing a claim check here would fail. +func TestMountedUnregisteredStores_ClaimedDiskIsStillOffered(t *testing.T) { + // pve-vm--9201--disk--1 is LVM on the OS disk: claimed by every definition the agent uses. + got := mountedUnregisteredStores(demoHPMounts, map[string]bool{}) + if len(got) == 0 { + t.Fatal("a claimed-but-mounted filesystem was filtered out — attaching is non-destructive, " + + "and this filter is exactly what made the picker empty on the rebuilt box") + } +} + +// ── SCENARIO D — a normal box with a registered store is unchanged ────────────────────────────── + +// RED-PROOF: drop the `registered[where]` exclusion and this fails with the already-registered store +// offered for attaching a second time. +func TestMountedUnregisteredStores_RegisteredStoreIsNotOffered(t *testing.T) { + got := mountedUnregisteredStores(demoHPMounts, map[string]bool{"/mnt/sys_drive": true}) + if len(got) != 0 { + t.Errorf("a healthy box offered %+v — its store is registered, so the picker must be empty "+ + "and the page byte-identical to before this change", got) + } +} + +// ── The exclusions, each with the reason it exists ────────────────────────────────────────────── + +func TestMountedUnregisteredStores_Exclusions(t *testing.T) { + cases := []struct { + name, table, why string + }{ + { + "guest rootfs at /mnt", + "/dev/mapper/pve-vm--9201--disk--0 /mnt ext4 rw 0 0\n", + "/mnt is the guest rootfs mount, not a drive — offering it would register the box's own root", + }, + { + "the managed parent itself", + "/dev/mapper/pve-root /mnt/felhom-drives ext4 rw 0 0\n", + "the intermediary-model parent holds drives; it is not one", + }, + { + "tmpfs", + "tmpfs /mnt/scratch tmpfs rw 0 0\n", + "a RAM filesystem would silently lose the customer's data on reboot", + }, + { + "overlay", + "overlay /mnt/ovl overlay rw 0 0\n", + "not a real block device", + }, + { + "outside /mnt", + "/dev/sdb1 /srv/data ext4 rw 0 0\n", + "the storage convention is /mnt/; registering outside it is the manual-add path", + }, + { + "unsupported fs", + "/dev/sdb1 /mnt/win ntfs rw 0 0\n", + "ntfs is data-bearing but not one the stack hands to apps", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := mountedUnregisteredStores(c.table, map[string]bool{}); len(got) != 0 { + t.Errorf("offered %+v — %s", got, c.why) + } + }) + } +} + +// FAIL-SAFE: an unreadable mount table yields nothing, never everything. Paired with the caller's +// non-empty assertion, "we could not look" renders as "we cannot offer this", never as a promise. +func TestMountedUnregisteredStores_UnreadableTableOffersNothing(t *testing.T) { + if got := mountedUnregisteredStores("", map[string]bool{}); len(got) != 0 { + t.Errorf("an unreadable mount table produced %+v — it must produce nothing", got) + } +} + +// A mountpoint listed twice (bind / re-mount) must appear once, or the picker shows a duplicate. +func TestMountedUnregisteredStores_DeduplicatesMountpoints(t *testing.T) { + table := "/dev/sdb1 /mnt/data ext4 rw 0 0\n/dev/sdb1 /mnt/data ext4 rw,remount 0 0\n" + if got := mountedUnregisteredStores(table, map[string]bool{}); len(got) != 1 { + t.Errorf("got %d entries, want 1 — a re-mount must not double the picker row", len(got)) + } +} + +// ── SCENARIO C — nothing attachable, said plainly ─────────────────────────────────────────────── + +// RED-PROOF: remove the `{{if .HasAttachDestination}}` conditional from backups_restore.html and this +// fails on the first assertion — the „két kattintás" promise returns over an empty picker, which is +// the exact sentence that cost the rehearsal its time. +func TestRestorePage_NothingAttachable_DoesNotPromiseTwoClicks(t *testing.T) { + d := restoreData() + d["NoRestoreDestination"] = true + d["HasAttachDestination"] = false + html := renderBackupPage(t, "backups_restore", d) + + if strings.Contains(html, "két kattintás") { + t.Error("R-280: the page still promises „két kattintás" + + "\" while there is nothing to click — it is zero clicks, and the customer cannot get past it") + } + if !strings.Contains(html, "Csatolható meghajtót viszont most nem látunk") { + t.Error("R-280: the page does not say plainly that there is nothing to attach") + } + if !strings.Contains(html, "üzemeltető") { + t.Error("R-280: a refusal with no route is the R-252 defect again — it must name what to do instead") + } +} + +// ── SCENARIO A, rendered — the promise is kept only when it is true ───────────────────────────── + +func TestRestorePage_Attachable_KeepsTheTwoClicksRoute(t *testing.T) { + d := restoreData() + d["NoRestoreDestination"] = true + d["HasAttachDestination"] = true + html := renderBackupPage(t, "backups_restore", d) + + if !strings.Contains(html, "két kattintás") { + t.Error("with a real destination the original instruction must survive — this change narrows " + + "a false promise, it does not remove a true one") + } + if !strings.Contains(html, `href="/storage"`) { + t.Error("the instruction no longer routes to the picker") + } +} + +// ── SCENARIO B — `initialize` is left exactly as it was ───────────────────────────────────────── +// +// The format wizard hides system and backup drives, and it says so on the page. That protection is a +// property of the agent's unclaimed scan, and the mounted-store source must never reach it. +// +// RED-PROOF: switch the initialize list to the new source too — in mergeAttachCandidates, add +// `resp.Initialize = append(resp.Initialize, mountedStoreCandidates(stores)...)`. This fails with the +// guest data volume offered for FORMATTING, which is the protection breaking in the open. +func TestMergeAttachCandidates_InitializeIsUntouched(t *testing.T) { + agentSaid := agentapi.CandidatesResult{ + VMID: 9201, + Initialize: []agentapi.DiskCandidate{{Device: "/dev/sdd", FSType: ""}}, + Attach: []agentapi.DiskCandidate{{Device: "/dev/sdd", MountSource: "/dev/sdd1", FSType: "ext4"}}, + } + stores := []mountedStore{{Path: "/mnt/sys_drive", Device: "/dev/mapper/pve-vm--9201--disk--1", FSType: "ext4"}} + + got := mergeAttachCandidates(agentSaid, stores) + + // initialize: byte-for-byte the agent's list. + if len(got.Initialize) != 1 || got.Initialize[0].Device != "/dev/sdd" { + t.Fatalf("initialize was modified: %+v — the format wizard's system/backup protection lives "+ + "in the agent's unclaimed scan, and widening it is how a customer is offered their own "+ + "data drive to format", got.Initialize) + } + for _, c := range got.Initialize { + if c.AlreadyMounted { + t.Errorf("a mounted store reached the FORMAT list: %+v", c) + } + } + + // attach: the agent's entry survives (the fresh-USB case) AND the mounted store is added. + if len(got.Attach) != 2 { + t.Fatalf("attach has %d entries, want 2 (agent's + the mounted store) — got %+v", len(got.Attach), got.Attach) + } + var foundMounted bool + for _, c := range got.Attach { + if c.MountSource == "/mnt/sys_drive" { + foundMounted = true + if !c.AlreadyMounted { + t.Error("the mounted store is not flagged already_mounted — the wizard would send it " + + "down the device-attach path and try to mount an in-guest path as a raw device") + } + } + } + if !foundMounted { + t.Error("the mounted store did not reach attach — this is the rebuilt box's empty picker") + } + if got.Attach[0].Device != "/dev/sdd" { + t.Error("the agent's own attach entry was dropped — a fresh external drive with a filesystem " + + "on it is exactly what this wizard was built for, and the mount table cannot report it") + } +} + +// A bind mount republishes a filesystem under a second path. A bind of the guest ROOTFS under +// /mnt/ is indistinguishable from a data drive by path alone — and registering it would put +// app data on the root filesystem. +// +// RED-PROOF: drop the `rootDevices[dev]` exclusion and this fails with /mnt/rootcopy offered. +func TestMountedUnregisteredStores_RootfsAliasIsNotOffered(t *testing.T) { + table := demoHPMounts + "/dev/mapper/pve-vm--9201--disk--0 /mnt/rootcopy ext4 rw 0 0\n" + got := mountedUnregisteredStores(table, map[string]bool{}) + + for _, m := range got { + if m.Path == "/mnt/rootcopy" { + t.Error("a bind of the guest rootfs was offered as an attachable store — registering it " + + "would store the customer's app data on the box's own root filesystem") + } + } + // The real drive on a DIFFERENT device must survive the new exclusion. + if len(got) != 1 || got[0].Path != "/mnt/sys_drive" { + t.Errorf("the genuine data volume was lost to the rootfs guard: %+v", got) + } +} diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index a4f117c..4a551dd 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -1053,6 +1053,11 @@ func (s *Server) backupsRestoreHandler(w http.ResponseWriter, r *http.Request) { // predicate so the page and the resolver cannot disagree. FALSE on a healthy box, where the // template renders exactly as before (Scenario E). data["NoRestoreDestination"] = !s.backupMgr.HasRestoreDestination() + // R-280: the notice above told the customer this was „két kattintás" and pointed at a picker + // that was empty, so it was zero clicks. The promise is now conditional on the destination it + // points at actually having something in it — and when it does not, the page says so and names + // what to do instead. Same derivation the picker uses, so the two cannot disagree. + data["HasAttachDestination"] = len(s.attachableStores()) > 0 } s.executeTemplate(w, r, "backups_restore", data) } diff --git a/controller/internal/web/storage_handlers.go b/controller/internal/web/storage_handlers.go index b229c2e..75b4606 100644 --- a/controller/internal/web/storage_handlers.go +++ b/controller/internal/web/storage_handlers.go @@ -373,6 +373,8 @@ func (s *Server) ServeStorageAPI(w http.ResponseWriter, r *http.Request) { s.handleStorageImpact(w, r) case r.URL.Path == "/api/storage/register" && r.Method == http.MethodPost: s.handleStorageRegister(w, r) + case r.URL.Path == "/api/storage/register-mounted" && r.Method == http.MethodPost: + s.handleStorageRegisterMounted(w, r) // E-2 Parts 3+4. State drives the degraded banner and the OFFER; assign is the offer's // ACCEPTANCE and the ONLY writer of the role — registration above deliberately does not set it. case r.URL.Path == "/api/storage/backup-target" && r.Method == http.MethodGet: @@ -865,6 +867,53 @@ func (s *Server) handleStorageRegister(w http.ResponseWriter, r *http.Request) { writeDiskJSON(w, http.StatusOK, true, "", map[string]any{"registered": true, "where": stable, "raw": req.Where}) } +// handleStorageRegisterMounted registers an ALREADY-mounted, unregistered filesystem verbatim +// (R-280). It is the action behind an `already_mounted` attach candidate, and it is the route the +// rebuilt demo-hp needed: the escape hatch that unblocked that box registered `/mnt/sys_drive`, an +// in-guest path, and nothing in the dashboard offered it. +// +// It differs from handleStorageRegister deliberately: that one takes the agent's RAW /mnt/ host +// mount and registers the STABLE /mnt/felhom-drives/ the intermediary model binds it to. These +// candidates are not agent drives and have no stable twin — the mountpoint IS the path to register, +// so translating it would register a directory that does not exist. +// +// The posted path is NOT trusted: it is matched against the freshly re-derived set of mounted, +// unregistered filesystems. A path that is not currently offered is refused, so this cannot be used +// to register an arbitrary directory. +func (s *Server) handleStorageRegisterMounted(w http.ResponseWriter, r *http.Request) { + var req struct { + Path string `json:"path"` + Label string `json:"label"` + SetDefault bool `json:"set_default"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeDiskJSON(w, http.StatusBadRequest, false, "érvénytelen kérés", nil) + return + } + want := path.Clean(strings.TrimSpace(req.Path)) + var match *mountedStore + for _, m := range s.attachableStores() { + if m.Path == want { + found := m + match = &found + break + } + } + if match == nil { + // Names a reason the customer can act on, and a route — never a bare refusal. + writeDiskJSON(w, http.StatusBadRequest, false, + "Ez a meghajtó most nem csatolható — lehet, hogy már regisztrálva van, vagy időközben lecsatolódott. Frissítsd az oldalt, és nézd meg a Tárhely → Meghajtók listát.", nil) + return + } + if err := s.registerStoragePath(match.Path, req.Label, req.SetDefault); err != nil { + s.logger.Printf("[WARN] [web] mounted-store register %s failed: %v", match.Path, err) + writeDiskJSON(w, http.StatusBadGateway, false, err.Error(), nil) + return + } + s.logger.Printf("[INFO] [web] storage path registered (already-mounted store): %s", match.Path) + writeDiskJSON(w, http.StatusOK, true, "", map[string]any{"registered": true, "where": match.Path}) +} + func (s *Server) handleStorageAttach(w http.ResponseWriter, r *http.Request) { var req storageProvReq if err := json.NewDecoder(r.Body).Decode(&req); err != nil { diff --git a/controller/internal/web/templates/backups_restore.html b/controller/internal/web/templates/backups_restore.html index 2f33aa9..6876aeb 100644 --- a/controller/internal/web/templates/backups_restore.html +++ b/controller/internal/web/templates/backups_restore.html @@ -71,12 +71,24 @@ repository the whole time. Installed-ness is a property OF a row, never a filter on it. --> + {{if .NoRestoreDestination}}

Előbb csatold vissza az adatmeghajtót. A mentéseid megvannak, és a meghajtók is megvannak — újratelepítés után viszont a gép még nem ismeri őket, ezért most nincs hová - visszaállítani. Ez két kattintás: Tárhely → + visszaállítani. + {{if .HasAttachDestination}} + Ez két kattintás: Tárhely → Meghajtók, „Meglévő meghajtó csatolása". Utána gyere vissza ide. + {{else}} + Csatolható meghajtót viszont most nem látunk ezen a gépen, ezért ezt innen nem tudod + elindítani. Ha a meghajtó be van kötve, de nincs csatlakoztatva, az üzemeltető tudja + csatlakoztatni — jelezd neki. A Tárhely → + Meghajtók oldalon látod, mit ismer most a gép. + {{end}}

{{end}} {{if eq .OffsiteStoreState "unreadable"}} diff --git a/controller/internal/web/templates/storage_attach.html b/controller/internal/web/templates/storage_attach.html index 118c33a..04304c6 100644 --- a/controller/internal/web/templates/storage_attach.html +++ b/controller/internal/web/templates/storage_attach.html @@ -23,7 +23,9 @@ — -
+ +
/mnt/ @@ -32,6 +34,10 @@
A meghajtó a /mnt/<név> útvonalra kerül.
+
@@ -49,7 +55,7 @@