From c1d04c28c1a5ad6c7cc3fd54f13c542e2aef339f Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Fri, 12 Jun 2026 15:38:55 +0200 Subject: [PATCH] =?UTF-8?q?agent=20v0.25.0:=20slice=2010=20P2=20=E2=80=94?= =?UTF-8?q?=20bind=20enrolled=20user-data=20drives=20into=20the=20guest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /disks/guest-attach binds an enrolled drive's felhom-data namespace into the guest (Model A: felhom-data is the bind source mounted at /mnt/, so only Felhom's namespace crosses in). GuestBinder does mkdir+chown(100000)+pct set (RW bind) via the fenced runner. Idempotent, free-slot selection, path-validated. Spike-proven on 9201. Pairs with controller P2C + golden /mnt:rslave (P2B). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 24 +++++++++ cmd/felhom-agent/main.go | 17 ++++-- internal/localapi/disks.go | 94 +++++++++++++++++++++++++++++++++ internal/localapi/disks_test.go | 90 +++++++++++++++++++++++++++++++ internal/localapi/guestbind.go | 79 +++++++++++++++++++++++++++ internal/localapi/server.go | 15 ++++-- 6 files changed, 311 insertions(+), 8 deletions(-) create mode 100644 internal/localapi/guestbind.go diff --git a/CHANGELOG.md b/CHANGELOG.md index c6743ad..5d0b844 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,30 @@ All notable changes to **felhom-agent** are recorded here. Update on every code change that gets pushed. +## v0.25.0 — slice 10 P2: bind enrolled user-data drives into the guest (passthrough) (2026-06-12) + +External user-data drives are mounted on the HOST but were never passed INTO the guest (diagnosed +Branch A), so apps silently wrote to the rootfs and the controller couldn't see them. This adds the +guest passthrough. Spike-proven on 9201 first (see REPORT / the usb-passthrough-spike findings): +`pct set` **bind form** (host path, never `storage:size`), `chown` to the guest base (idmap not clean +for mixed-ownership data), `shared:49` propagation host↔guest automatic. + +- **`POST /disks/guest-attach` (`internal/localapi`)** — self-scoped (vmid from token). Binds an + enrolled drive's **felhom-data namespace** into the guest at `/mnt/` (**Model A**: the + felhom-data dir is the bind source mounted AT `/mnt/`, so only Felhom's namespace crosses into + the guest — the customer's other data on the drive never does). Idempotent (returns the existing slot + if already bound); picks the lowest free `mpN`; validates `where` is `/mnt/` (no traversal). +- **`GuestBinder` (`internal/localapi/guestbind.go`)** — the host-root steps over the fenced + `proxmox.Runner` (same pattern as the provision back-half's bind): `mkdir -p /felhom-data` → + `chown 100000:100000` the namespace ROOT (not -R; per-app subdirs are chowned at deploy) → `pct set + -mpN /felhom-data,mp=/mnt/` (RW bind). The namespace is created fresh + uniformly + owned, which sidesteps the drive's pre-existing mixed-ownership data entirely. +- **Tests** — `TestGuestAttach_*`: free-slot selection (mp0 when mp9 taken), idempotency (no re-bind + + `already:true`), bad-path rejection (traversal/non-/mnt/multi-component), not-configured 503. + +Pairs with felhom-controller P2C (enroll triggers attach) + the golden's `/mnt:rslave` controller bind +(P2B). Self-heal reconcile (P3) and dual-role (P4) follow. + ## v0.24.0 — role-gate the eject path (system/backup mounts are unmount-protected at the agent) (2026-06-12) Closes the eject gap in the storage-authorization redesign: `POST /disks/eject` now **refuses to diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 44aef9e..80c34d7 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -43,7 +43,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.24.0" +var version = "0.25.0" func main() { var ( @@ -554,6 +554,13 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St } logger.Info("local-api leaf ready", "fingerprint_sha256", fp, "cert", cfg.LocalAPI.CertPath()) runner := backup.NewBackupRunner(px, cfg.Backup.LocalBackupTarget, "", "felhom local-api", logger) + // Guest data-drive passthrough (slice 10 P2): a root-CLI runner for the `pct set` bind + chown + // (same fenced ExecRunner the host-storage + provision back-half use). + gaMode := proxmox.RunnerMode(cfg.Privileged.Mode) + if gaMode == "" { + gaMode = proxmox.RunnerSudo + } + guestBinder := localapi.NewGuestBinder(&proxmox.ExecRunner{Mode: gaMode, SudoPath: cfg.Privileged.SudoPath}, logger) srv, err := localapi.NewServer(localapi.Options{ ListenAddr: cfg.LocalAPI.ListenAddr, Cert: cert, @@ -564,9 +571,11 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St Tokens: tokens, BackupCadence: cfg.Backup.BackupCadence(), // 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, + Disks: hostOps, + DiskGate: storageGateAdapter{gate: gate, hostID: cfg.Hub.HostID}, + Guests2: px, + GuestAttach: guestBinder, // slice 10 P2: bind enrolled data drives into the guest + // Host metrics (slice 9): the shared collector serves GET /host/metrics — a fresh host + // per-storage view to the customer's monitoring page (reuses the slice-4 collector). HostMetrics: collector, diff --git a/internal/localapi/disks.go b/internal/localapi/disks.go index 5036a69..c898aa5 100644 --- a/internal/localapi/disks.go +++ b/internal/localapi/disks.go @@ -3,6 +3,7 @@ package localapi import ( "context" "net/http" + "strconv" "strings" "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" @@ -60,6 +61,12 @@ type GuestLister interface { ListLXC(ctx context.Context) ([]proxmox.Guest, error) } +// GuestAttacher binds an enrolled user-data drive's felhom-data namespace into a guest as an RW bind +// mount (slice 10 P2, Model A). Satisfied by *GuestBinder. The handler picks the slot + dedups. +type GuestAttacher interface { + AttachBind(ctx context.Context, vmid int, mountKey, where string) error +} + // ---- handlers --------------------------------------------------------------------------- // DiskInfo is one host drive with its data-bearing flag (for the UI). @@ -203,6 +210,93 @@ func (s *Server) handleDiskEject(w http.ResponseWriter, r *http.Request, vmid in writeOK(w, map[string]any{"vmid": vmid, "ejected": req.Where, "dependent_guests": dependents}) } +type guestAttachRequest struct { + VMID int `json:"vmid"` + Where string `json:"where"` // the host mount path of the enrolled drive (e.g. /mnt/felhom-usb) +} + +// handleDiskGuestAttach binds an enrolled user-data drive's felhom-data namespace into THIS guest as +// an RW bind mount (slice 10 P2, Model A). Self-scoped (the vmid is the token's). Idempotent: if a +// mountpoint already binds `where`, it returns the existing slot without re-attaching. The drive must +// already be mounted on the host at `where` (the enroll flow's assign did that) — this only adds the +// guest passthrough. The customer's non-felhom data on the drive is NOT exposed (only felhom-data). +func (s *Server) handleDiskGuestAttach(w http.ResponseWriter, r *http.Request, vmid int) { + if s.guestAttach == nil { + writeErr(w, http.StatusServiceUnavailable, "guest passthrough not configured on this host") + return + } + var req guestAttachRequest + if !decodeBody(w, r, &req) { + return + } + if !s.scopedFromBody(w, req.VMID, vmid, r.URL.Path) { + return + } + where := strings.TrimSpace(req.Where) + if !validGuestMountPath(where) { + writeErr(w, http.StatusBadRequest, "where must be an absolute /mnt/ path (no traversal)") + return + } + // Read the guest config for idempotency + free-slot selection. + cfg, err := s.guests.GuestConfig(r.Context(), vmid) + if err != nil { + s.logger.Error("local-api: guest-attach guest config", "vmid", vmid, "err", err) + writeErr(w, http.StatusBadGateway, "could not read guest config") + return + } + mounts := cfg.MountPoints() + // Idempotency: already bound at `where`? (a bind's mp= equals the guest path). + for key, spec := range mounts { + if _, mp, _ := parseMount(spec); mp == where { + s.logger.Info("local-api: guest-attach idempotent (already bound)", "vmid", vmid, "where", where, "slot", key) + writeOK(w, map[string]any{"vmid": vmid, "attached": where, "slot": key, "already": true}) + return + } + } + slot, ok := freeMountSlot(mounts) + if !ok { + writeErr(w, http.StatusConflict, "no free mountpoint slot on the guest") + return + } + if err := s.guestAttach.AttachBind(r.Context(), vmid, slot, where); err != nil { + s.logger.Error("local-api: guest-attach", "vmid", vmid, "where", where, "slot", slot, "err", err) + writeErr(w, http.StatusBadGateway, "guest-attach failed: "+err.Error()) + return + } + writeOK(w, map[string]any{"vmid": vmid, "attached": where, "slot": slot}) +} + +// validGuestMountPath accepts an absolute /mnt/ path with no traversal (the enroll convention +// root). Mirrors the controller's mount-name discipline so a hostile `where` can't escape /mnt. +func validGuestMountPath(p string) bool { + if !strings.HasPrefix(p, "/mnt/") || strings.Contains(p, "..") { + return false + } + rest := strings.TrimPrefix(p, "/mnt/") + if rest == "" || strings.ContainsAny(rest, "/ \t") { + return false // exactly one path component under /mnt + } + for _, c := range rest { + if (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_' || c == '-' { + continue + } + return false + } + return true +} + +// freeMountSlot returns the lowest mpN (0..255) not present in the guest's current mountpoints. The +// bootstrap mount (mp9) and any existing data mounts are already in `mounts`, so they're skipped. +func freeMountSlot(mounts map[string]string) (string, bool) { + for i := 0; i <= 255; i++ { + key := "mp" + strconv.Itoa(i) + if _, used := mounts[key]; !used { + return key, true + } + } + return "", false +} + type formatRequest struct { VMID int `json:"vmid"` Device string `json:"device"` diff --git a/internal/localapi/disks_test.go b/internal/localapi/disks_test.go index c90714a..889817c 100644 --- a/internal/localapi/disks_test.go +++ b/internal/localapi/disks_test.go @@ -372,6 +372,96 @@ func TestEject_RoleGated(t *testing.T) { d4.mu.Unlock() } +// ---- guest data-drive passthrough (slice 10 P2) ----------------------------------------- + +type fakeGuestAttacher struct { + mu sync.Mutex + calls []struct { + vmid int + slot, where string + } +} + +func (f *fakeGuestAttacher) AttachBind(_ context.Context, vmid int, mountKey, where string) error { + f.mu.Lock() + defer f.mu.Unlock() + f.calls = append(f.calls, struct { + vmid int + slot, where string + }{vmid, mountKey, where}) + return nil +} +func (f *fakeGuestAttacher) count() int { f.mu.Lock(); defer f.mu.Unlock(); return len(f.calls) } + +func newAttachServer(t *testing.T, ga GuestAttacher, mounts map[int]map[string]string) http.Handler { + t.Helper() + srv, err := NewServer(Options{ + ListenAddr: "127.0.0.1:0", Guests: &fakeGuestsCfg{mounts: mounts}, Backups: &fakeBackups{}, + Store: &fakeStore{}, Storage: fakeStorage{}, Tokens: staticTokens{"A": 8200, "B": 9300}, + GuestAttach: ga, Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }) + if err != nil { + t.Fatal(err) + } + srv.baseCtx = context.Background() + return srv.Handler() +} + +// A first attach picks the lowest free slot (mp0; mp9 bootstrap is taken) and calls the binder. +func TestGuestAttach_PicksFreeSlotAndBinds(t *testing.T) { + ga := &fakeGuestAttacher{} + h := newAttachServer(t, ga, map[int]map[string]string{ + 8200: {"mp9": "/var/lib/.../bootstrap,mp=/etc/felhom-bootstrap,ro=1"}, + }) + w := do(t, h, "POST", "/disks/guest-attach", "A", `{"where":"/mnt/felhom-usb"}`) + if w.Code != http.StatusOK { + t.Fatalf("attach: got %d want 200 (%s)", w.Code, w.Body.String()) + } + if ga.count() != 1 || ga.calls[0].slot != "mp0" || ga.calls[0].where != "/mnt/felhom-usb" || ga.calls[0].vmid != 8200 { + t.Fatalf("AttachBind not called with mp0/where/vmid: %+v", ga.calls) + } +} + +// An already-bound drive is idempotent: returns the existing slot, binder NOT called again. +func TestGuestAttach_Idempotent(t *testing.T) { + ga := &fakeGuestAttacher{} + h := newAttachServer(t, ga, map[int]map[string]string{ + 8200: {"mp0": "/mnt/felhom-usb/felhom-data,mp=/mnt/felhom-usb"}, + }) + w := do(t, h, "POST", "/disks/guest-attach", "A", `{"where":"/mnt/felhom-usb"}`) + if w.Code != http.StatusOK { + t.Fatalf("idempotent attach: got %d want 200 (%s)", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), `"already":true`) { + t.Fatalf("expected already:true: %s", w.Body.String()) + } + if ga.count() != 0 { + t.Fatalf("AttachBind must NOT be called for an already-bound drive: %+v", ga.calls) + } +} + +// A hostile/invalid where is refused with no binder call. +func TestGuestAttach_RejectsBadPath(t *testing.T) { + ga := &fakeGuestAttacher{} + h := newAttachServer(t, ga, map[int]map[string]string{8200: {}}) + for _, bad := range []string{`{"where":"/mnt/../etc"}`, `{"where":"/etc/passwd"}`, `{"where":"/mnt/a/b"}`, `{"where":""}`} { + if w := do(t, h, "POST", "/disks/guest-attach", "A", bad); w.Code != http.StatusBadRequest { + t.Fatalf("bad where %s: got %d want 400", bad, w.Code) + } + } + if ga.count() != 0 { + t.Fatal("binder called for an invalid path") + } +} + +// Not configured (no GuestAttach dep) → 503. +func TestGuestAttach_NotConfigured(t *testing.T) { + h := newAttachServer(t, nil, map[int]map[string]string{8200: {}}) + if w := do(t, h, "POST", "/disks/guest-attach", "A", `{"where":"/mnt/felhom-usb"}`); w.Code != http.StatusServiceUnavailable { + t.Fatalf("unconfigured: got %d want 503", w.Code) + } +} + // ---- auth / config ---------------------------------------------------------------------- func TestDisks_CrossGuest403(t *testing.T) { diff --git a/internal/localapi/guestbind.go b/internal/localapi/guestbind.go new file mode 100644 index 0000000..411bb43 --- /dev/null +++ b/internal/localapi/guestbind.go @@ -0,0 +1,79 @@ +package localapi + +import ( + "context" + "fmt" + "log/slog" + "strconv" + + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" +) + +// Guest data-drive passthrough (slice 10 P2, Model A). An enrolled external user-data drive is +// mounted on the HOST at /mnt/; this binds its felhom-data NAMESPACE into the guest so the +// in-guest controller + apps can use it. Confinement is the inner (host→guest) bind: only +// /felhom-data crosses into the guest — the customer's other data on the drive never does. +// +// Model A: the felhom-data dir is bound AT the guest's /mnt/ (so the guest's /mnt/ IS the +// felhom-data namespace; `findmnt` shows /dev/sdXN[/felhom-data], which the controller's mount strip +// already handles). The bind is RW (NOT ro=1 like the bootstrap mount). The namespace is chowned to +// the unprivileged-LXC base so the guest reads it as root-owned (per-app subdirs are chowned to the +// app's mapped UID at deploy — NOT here). Spike-proven on 9201 (see usb-passthrough-spike memory). + +// guestMappedRoot is the unprivileged-LXC idmap base — guest root (UID 0) == host UID 100000. chowning +// the namespace to this makes the guest see it as root:root, writable by the in-guest controller. +const guestMappedRoot = "100000:100000" + +// felhomDataNS is the Felhom-managed namespace directory created on every external data drive. Only +// this subtree is exposed to the guest (matches the controller's appbackup.FelhomDataDir). +const felhomDataNS = "felhom-data" + +// GuestBinder attaches a host data-drive's felhom-data namespace into a guest as an RW bind mount via +// `pct set` (a root@pam op — same fenced Runner the provision back-half uses for its bind). It does +// NOT make HTTP calls; the slot selection + idempotency live in the handler (which has the guest +// config). Satisfies localapi.GuestAttacher. +type GuestBinder struct { + runner proxmox.Runner + logger *slog.Logger +} + +// NewGuestBinder builds a binder over the given root-CLI runner. +func NewGuestBinder(r proxmox.Runner, logger *slog.Logger) *GuestBinder { + if logger == nil { + logger = slog.Default() + } + return &GuestBinder{runner: r, logger: logger} +} + +// AttachBind creates + chowns /felhom-data on the host and binds it into the guest at +// (Model A). mountKey is the chosen guest slot ("mp3"). Idempotency + slot choice are the caller's +// (it reads the guest config); this performs the host-root steps only. +func (b *GuestBinder) AttachBind(ctx context.Context, vmid int, mountKey, where string) error { + src := where + "/" + felhomDataNS // host source = the felhom-data namespace on the drive + // 1. Ensure the namespace dir exists (idempotent; created fresh + uniformly owned, so the drive's + // pre-existing mixed-ownership customer data is never touched). + if err := b.run(ctx, "mkdir", "-p", src); err != nil { + return fmt.Errorf("guest-attach: create namespace %s: %w", src, err) + } + // 2. chown the namespace ROOT to the guest base (NOT -R: per-app subdirs are chowned at deploy). + if err := b.run(ctx, "chown", guestMappedRoot, src); err != nil { + return fmt.Errorf("guest-attach: chown namespace %s: %w", src, err) + } + // 3. Bind it into the guest at `where`, RW. Bind form (host path), NEVER storage:size (that volume + // form would create a fresh empty disk and lose the existing data). + spec := fmt.Sprintf("%s,mp=%s", src, where) + if err := b.run(ctx, "pct", "set", strconv.Itoa(vmid), "-"+mountKey, spec); err != nil { + return fmt.Errorf("guest-attach: pct set %s: %w", spec, err) + } + b.logger.Info("guest-attach: data drive bound into guest", + "vmid", vmid, "slot", mountKey, "source", src, "guest_path", where) + return nil +} + +func (b *GuestBinder) run(ctx context.Context, name string, args ...string) error { + _, stderr, err := b.runner.Run(ctx, name, args...) + if err != nil { + return fmt.Errorf("%s: %w: %s", name, err, string(stderr)) + } + return nil +} diff --git a/internal/localapi/server.go b/internal/localapi/server.go index 3bbe32c..421a698 100644 --- a/internal/localapi/server.go +++ b/internal/localapi/server.go @@ -81,6 +81,9 @@ type Options struct { Disks DiskOps DiskGate StorageGate Guests2 GuestLister + // GuestAttach binds an enrolled user-data drive's felhom-data namespace into the guest (slice 10 + // P2, Model A). OPTIONAL — when nil, POST /disks/guest-attach reports "not configured". + GuestAttach GuestAttacher // HostReader is the root-free host topology reader used to classify a device/mount's protection // ROLE (it backs SystemDisks for the eject role-gate + the /disks role hints). OPTIONAL — when nil // it defaults to the production *storage.ProcHostReader. Injectable so the role-gate is testable. @@ -132,10 +135,11 @@ 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) - host storage.HostReader // role classification source (optional; defaults to ProcHostReader) + disks DiskOps // slice 8C (optional) + diskGate StorageGate // slice 8C (optional) + guestList GuestLister // slice 8C (optional) + guestAttach GuestAttacher // slice 10 P2 (optional) + host storage.HostReader // role classification source (optional; defaults to ProcHostReader) hostMetrics HostMetricsProvider // slice 9 (optional) hostID string // slice 10B: for the data-bearing-format pending-op hint @@ -175,6 +179,7 @@ func NewServer(o Options) (*Server, error) { disks: o.Disks, diskGate: o.DiskGate, guestList: o.Guests2, + guestAttach: o.GuestAttach, host: o.HostReader, hostMetrics: o.HostMetrics, hostID: o.HostID, @@ -200,6 +205,8 @@ func (s *Server) Handler() http.Handler { 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)) + // Guest data-drive passthrough (slice 10 P2): bind an enrolled drive's felhom-data namespace in. + mux.HandleFunc("POST /disks/guest-attach", s.withGuest(s.handleDiskGuestAttach)) return mux }