v0.127.0: a mount Felhom itself made is not 'something else' (R-220)
gates / gates (push) Successful in 8s
gates / gates (push) Successful in 8s
After a rebuild the customer's own drives could not be re-attached: candidates returned initialize:[] attach:[] while both drives sat there, and the deploy refused with 'choose an attached drive from the list' — a list that was empty. Measured live three times. Mechanism: enrolment mounts a drive TWICE, at /mnt/felhom-drives/<name> and at the raw /mnt/<name> it creates on the host. The host survives a guest rebuild; the controller's registry does not. So classifyClaim saw a mount outside the managed prefix and concluded 'claimed by something else' — about our own mount. The fix is CORROBORATED, not a widened prefix: a non-managed mountpoint is forgiven only when the SAME device is also mounted under the managed path, a pairing only our enrolment produces. A disk another system uses — /srv/data, /media/x, even /mnt/someone-elses-disk — has no counterpart and is STILL refused, with its own test and a red-proof showing an over-wide fix offering it for formatting. Read from /proc/mounts deliberately: the lsblk invocation is pinned verbatim in configs/felhom-agent.sudoers, so using the plural MOUNTPOINTS would have coupled this to a sudoers rollout. /proc/mounts is world-readable — no sudo, no new allowlisted command, no config change. Fail-safe: an unreadable mount table corroborates NOTHING, so the device classifies exactly as before. 'Could not corroborate' must never read as 'ours'. 29 packages ok, vet clean, agent gates OK.
This commit is contained in:
@@ -53,7 +53,11 @@ type claimFacts struct {
|
||||
nodes []claimNode // the whole disk + its children (partitions)
|
||||
lvmPV bool // pvs (authoritative): the disk / a partition is an LVM physical volume
|
||||
zfsMember bool // zpool (authoritative): the disk / a partition is a ZFS pool member
|
||||
gatherErr string // non-empty ⇒ a REQUIRED read failed ⇒ fail-safe CLAIMED
|
||||
// felhomOwnedMounts (R-220) — mountpoints OUTSIDE /mnt/felhom-drives that are nevertheless Felhom's
|
||||
// OWN, corroborated from the host mount table: the same device is also mounted at the managed path.
|
||||
// Empty means "nothing corroborated", which is the fail-safe direction.
|
||||
felhomOwnedMounts map[string]bool
|
||||
gatherErr string // non-empty ⇒ a REQUIRED read failed ⇒ fail-safe CLAIMED
|
||||
}
|
||||
|
||||
// classifyClaim is the pure guard verdict. unclaimed=true ONLY when the device is provably free for
|
||||
@@ -81,7 +85,20 @@ func classifyClaim(f claimFacts) (unclaimed bool, reason string) {
|
||||
if memberFSTypes[n.fstype] {
|
||||
return false, "device holds a " + n.fstype + " (" + n.name + ")"
|
||||
}
|
||||
if n.mountpoint != "" && !underFelhomDrives(n.mountpoint) {
|
||||
// ── R-220 — A MOUNT FELHOM ITSELF MADE IS NOT "SOMETHING ELSE". ───────────────────────
|
||||
//
|
||||
// Enrolment mounts a drive TWICE: at the managed path `/mnt/felhom-drives/<name>` and at the
|
||||
// raw `/mnt/<name>` it creates on the host. The host — and therefore that raw mount — survives
|
||||
// a guest rebuild, while the controller's registry does not. So after a rebuild the customer's
|
||||
// own drives looked foreign, `attach` returned an empty list, and the refusal told them to
|
||||
// choose from it. Measured live three times (CAMPAIGN-11 Phase 1, and the R-201 re-walk twice);
|
||||
// unmounting only the raw mounts flipped `attach: []` to both drives every time.
|
||||
//
|
||||
// The fence this must NOT breach: a disk genuinely in use by something else stays refused. So
|
||||
// the exemption is not "any /mnt/* path" — it is CORROBORATED: the same device must ALSO be
|
||||
// mounted at Felhom's managed path, which is a state only Felhom's own enrolment produces.
|
||||
// A foreign disk at /srv/data or /media/x has no such counterpart and is still refused.
|
||||
if n.mountpoint != "" && !underFelhomDrives(n.mountpoint) && !f.felhomOwnedMounts[n.mountpoint] {
|
||||
return false, "device is mounted at " + n.mountpoint + " (" + n.name + ")"
|
||||
}
|
||||
}
|
||||
@@ -154,6 +171,8 @@ func (h *SudoHostOps) gatherClaimFacts(ctx context.Context, device string) claim
|
||||
return f
|
||||
}
|
||||
f.nodes = nodes
|
||||
// R-220: corroborate which non-managed mountpoints are nevertheless Felhom's own.
|
||||
f.felhomOwnedMounts = felhomOwnedMounts(device, nodes, h.mountTable)
|
||||
|
||||
// LVM PV (authoritative). pvs installed but erroring ⇒ fail-safe claimed; absent ⇒ rely on lsblk's
|
||||
// LVM2_member FSTYPE (already in nodes).
|
||||
@@ -285,3 +304,71 @@ func (h *SudoHostOps) zfsMembers(ctx context.Context, nodes []claimNode, wholeDi
|
||||
}
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// mountTableSource yields the host mount table as (device, mountpoint) pairs. A seam so the R-220
|
||||
// corroboration is unit-testable without a host. nil ⇒ the real /proc/mounts.
|
||||
type mountTableSource func() ([][2]string, error)
|
||||
|
||||
// procMounts reads /proc/mounts — WORLD-READABLE, so this needs no sudo and no allowlisted command.
|
||||
// That matters: the lsblk invocation is pinned verbatim in the sudoers file
|
||||
// (`lsblk -J -o NAME,FSTYPE,PTTYPE,MOUNTPOINT /dev/*`), so switching it to the plural MOUNTPOINTS
|
||||
// would have meant shipping a sudoers change with the binary — a far larger blast radius than this
|
||||
// finding warrants. Reading the mount table directly sidesteps that entirely.
|
||||
func procMounts() ([][2]string, error) {
|
||||
data, err := os.ReadFile("/proc/mounts")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
var out [][2]string
|
||||
for _, line := range strings.Split(string(data), "\n") {
|
||||
fields := strings.Fields(line)
|
||||
if len(fields) < 2 {
|
||||
continue
|
||||
}
|
||||
// /proc/mounts escapes spaces as \040; unescape so a path with a space still compares.
|
||||
out = append(out, [2]string{fields[0], strings.ReplaceAll(fields[1], `\040`, " ")})
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// felhomOwnedMounts returns the mountpoints of `device` (and its children) that sit OUTSIDE
|
||||
// /mnt/felhom-drives but are still Felhom's own, corroborated by the same device also being mounted
|
||||
// UNDER /mnt/felhom-drives. That pairing is what enrolment produces and nothing else does.
|
||||
//
|
||||
// ⚠ FAIL-SAFE: an unreadable mount table returns an EMPTY set, never a permissive one. The device then
|
||||
// classifies exactly as it did before R-220 — refused — because "we could not corroborate" must never
|
||||
// read as "it is ours".
|
||||
func felhomOwnedMounts(device string, nodes []claimNode, src mountTableSource) map[string]bool {
|
||||
if src == nil {
|
||||
src = procMounts
|
||||
}
|
||||
table, err := src()
|
||||
if err != nil {
|
||||
return nil // unreadable ⇒ corroborate nothing
|
||||
}
|
||||
// Every device name this disk answers to: the whole disk and each child node.
|
||||
devs := map[string]bool{device: true}
|
||||
if wd, ok := wholeDiskOf(device); ok {
|
||||
devs[wd] = true
|
||||
}
|
||||
for _, n := range nodes {
|
||||
devs["/dev/"+n.name] = true
|
||||
}
|
||||
// A device is Felhom-managed only if it is mounted under the managed prefix.
|
||||
managed := map[string]bool{}
|
||||
for _, row := range table {
|
||||
if devs[row[0]] && underFelhomDrives(row[1]) {
|
||||
managed[row[0]] = true
|
||||
}
|
||||
}
|
||||
if len(managed) == 0 {
|
||||
return nil
|
||||
}
|
||||
owned := map[string]bool{}
|
||||
for _, row := range table {
|
||||
if managed[row[0]] && !underFelhomDrives(row[1]) {
|
||||
owned[path.Clean(row[1])] = true
|
||||
}
|
||||
}
|
||||
return owned
|
||||
}
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
package storage
|
||||
|
||||
import "testing"
|
||||
|
||||
// ── R-220 — A MOUNT FELHOM ITSELF MADE IS NOT "SOMETHING ELSE" ──────────────────────────────────
|
||||
//
|
||||
// Enrolment mounts a drive twice: at `/mnt/felhom-drives/<name>` and at the raw `/mnt/<name>` it
|
||||
// creates on the host. The host survives a guest rebuild; the controller's registry does not. So after
|
||||
// a rebuild the customer's own drives read as claimed-by-something-else, `attach` came back empty, and
|
||||
// the refusal told them to pick from the empty list. Measured three times live.
|
||||
//
|
||||
// The fence: a disk genuinely in use elsewhere must STILL be refused. These assert both directions.
|
||||
|
||||
// ── SCENARIO E — the customer's own drive is offered again after a rebuild ───────────────────────
|
||||
//
|
||||
// RED-PROOF: drop `&& !f.felhomOwnedMounts[n.mountpoint]` from classifyClaim — the pre-R-220 check —
|
||||
// and this FAILS with the drive refused and the list empty again.
|
||||
func TestClassifyClaim_R220_FelhomsOwnRawMountIsNotForeign(t *testing.T) {
|
||||
f := claimFacts{
|
||||
device: "/dev/sdb", wholeDisk: "/dev/sdb", wholeDiskOK: true,
|
||||
nodes: []claimNode{{name: "sdb", fstype: "ext4", mountpoint: "/mnt/adatok"}},
|
||||
// corroborated: the SAME device is also mounted at the managed path
|
||||
felhomOwnedMounts: map[string]bool{"/mnt/adatok": true},
|
||||
}
|
||||
unclaimed, reason := classifyClaim(f)
|
||||
if !unclaimed {
|
||||
t.Fatalf("R-220 RETURNED: the customer's own drive is refused after a rebuild — %q", reason)
|
||||
}
|
||||
}
|
||||
|
||||
// ── SCENARIO F — a genuinely foreign mount is STILL refused ──────────────────────────────────────
|
||||
//
|
||||
// RED-PROOF: over-widen the fix to exempt any /mnt/* path (or to skip the mountpoint check entirely)
|
||||
// and this FAILS — a disk another system is using would be offered for formatting.
|
||||
func TestClassifyClaim_R220_ForeignMountIsStillRefused(t *testing.T) {
|
||||
for _, mp := range []string{"/srv/data", "/media/photos", "/mnt/someone-elses-disk", "/var/lib/other"} {
|
||||
f := claimFacts{
|
||||
device: "/dev/sdb", wholeDisk: "/dev/sdb", wholeDiskOK: true,
|
||||
nodes: []claimNode{{name: "sdb", fstype: "ext4", mountpoint: mp}},
|
||||
felhomOwnedMounts: nil, // nothing corroborated it as ours
|
||||
}
|
||||
unclaimed, reason := classifyClaim(f)
|
||||
if unclaimed {
|
||||
t.Fatalf("THE FENCE BROKE: a disk mounted at %s was offered for formatting", mp)
|
||||
}
|
||||
if reason == "" {
|
||||
t.Fatalf("a refusal must carry a reason (%s)", mp)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The corroboration itself: it must require BOTH mounts of the SAME device, and fail safe.
|
||||
func TestFelhomOwnedMounts_RequiresTheManagedCounterpart(t *testing.T) {
|
||||
nodes := []claimNode{{name: "sdb"}}
|
||||
|
||||
t.Run("both mounts present -> the raw one is ours", func(t *testing.T) {
|
||||
src := func() ([][2]string, error) {
|
||||
return [][2]string{
|
||||
{"/dev/sdb", "/mnt/adatok"},
|
||||
{"/dev/sdb", "/mnt/felhom-drives/adatok"},
|
||||
}, nil
|
||||
}
|
||||
got := felhomOwnedMounts("/dev/sdb", nodes, src)
|
||||
if !got["/mnt/adatok"] {
|
||||
t.Fatal("the raw enrolment mount was not recognised as Felhom's own")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("only the raw mount -> corroborates NOTHING", func(t *testing.T) {
|
||||
src := func() ([][2]string, error) {
|
||||
return [][2]string{{"/dev/sdb", "/mnt/adatok"}}, nil
|
||||
}
|
||||
if got := felhomOwnedMounts("/dev/sdb", nodes, src); len(got) != 0 {
|
||||
t.Fatalf("a lone /mnt/<name> mount must corroborate nothing, got %v", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("a DIFFERENT device under the managed path does not vouch for this one", func(t *testing.T) {
|
||||
src := func() ([][2]string, error) {
|
||||
return [][2]string{
|
||||
{"/dev/sdb", "/srv/data"},
|
||||
{"/dev/sdc", "/mnt/felhom-drives/mentes"}, // someone else's, not sdb's
|
||||
}, nil
|
||||
}
|
||||
if got := felhomOwnedMounts("/dev/sdb", nodes, src); got["/srv/data"] {
|
||||
t.Fatal("another device's managed mount vouched for a foreign one")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("an unreadable mount table corroborates NOTHING (fail-safe)", func(t *testing.T) {
|
||||
src := func() ([][2]string, error) { return nil, errRead }
|
||||
if got := felhomOwnedMounts("/dev/sdb", nodes, src); len(got) != 0 {
|
||||
t.Fatalf("an unreadable mount table must corroborate nothing, got %v", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
var errRead = errNoMountTable{}
|
||||
|
||||
type errNoMountTable struct{}
|
||||
|
||||
func (errNoMountTable) Error() string { return "mount table unreadable" }
|
||||
@@ -164,6 +164,9 @@ type SudoHostOps struct {
|
||||
// UNPRIVILEGED read (`systemctl is-failed`) — seam-injected so the reassert's F10 reset-failed path
|
||||
// is unit-testable without a real systemd. Default set in NewSudoHostOps.
|
||||
unitFailed func(ctx context.Context, unit string) bool
|
||||
// mountTable (R-220) yields the host mount table for the "is this mount Felhom's own?"
|
||||
// corroboration. nil ⇒ the real /proc/mounts; tests inject.
|
||||
mountTable mountTableSource
|
||||
}
|
||||
|
||||
// SudoHostOpsConfig configures a SudoHostOps.
|
||||
|
||||
Reference in New Issue
Block a user