diff --git a/CHANGELOG.md b/CHANGELOG.md index d8ee2d8..312e7dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,42 @@ +## v0.124.0 — a lost storage grant repairs itself, and says that it was lost (2026-08-04, R-190) + +**R-190 is a grant that demonstrably worked at 04:44 on 2026-08-03 and was gone by 09:24** — with a +host reinstall, logged `pveum` activity and cluster-log entries all ruled out by measurement. The +cause is still open. The resilience does not have to wait for it. + +**Everything needed already existed and had only ever been called once.** The root wrapper +(`felhom-backup-target-apply grant `), its sudoers vector (`grant *`, any storage id, confirmed +not assumed), and the exact command were all in place — and the `grant` verb had only ever run at +storage CREATION. That is the *built but never wired* shape, in a verb rather than a seam, and it is +this project's seventh instance. + +**What v0.124.0 does:** when the store-grant probe finds the grant absent on a tier the box depends +on, it runs that wrapper and **re-reads once** to confirm — the pbsdr R-22 self-grant shape, including +its restraint: one attempt, one confirmation, and anything still wrong stays loudly wrong. + +**THE RECORD IS THE POINT, AND IT IS THE HALF R-190 IS ACTUALLY ABOUT.** A repair that leaves only +`ok` behind destroys the only evidence a permission vanished, so a recurring loss becomes undetectable +forever — strictly worse than the fault it fixes. So a confirmed repair reports **DEGRADED for exactly +one cycle**, with the explanation in `Feature`: + +``` +backup tier felhom-backup: the agent's storage grant was MISSING and has been AUTOMATICALLY +RESTORED — the tier works now, but a permission that vanished on its own needs investigating (R-190) +``` + +**Nothing new was built to carry it.** The hub's existing ok→degraded→ok edge is the channel — it +alerts and e-mails on the first edge and logs the recovery on the next cycle, so one loss produces +exactly one alert pair. No wire change, no hub change, no new event type. `Feature` carries the text +because that is the field the hub interpolates into the operator's e-mail; `Reason` does not travel. + +**Bounded (Scenario F):** one attempt per tier per hour, in memory. A storage can be unreadable for +reasons an ACL cannot fix, and a re-grant on every report cycle is a repair loop wearing a fix's +clothes. An agent restart re-arms it, which is correct — a restart is exactly when a box should +re-check what it depends on. + +**A failed repair never masks the fault:** the capability stays degraded with the failure in its +reason, and a repair that "succeeded" but did not survive the re-read is reported as needing a human. + ## v0.123.0 — a tier the box cannot READ now says so (2026-08-03, R-185) **The missing permission is one command. The silence was the defect.** On demo-felhom the agent's PVE diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 281d6cf..57ab88b 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -24,6 +24,7 @@ import ( "path/filepath" "strconv" "strings" + "sync" "syscall" "time" @@ -441,15 +442,71 @@ func poolReadStatus(ctx context.Context, px *proxmox.Client) capability.Status { // One exception, so an ordinary configuration is not turned into an alarm: a box with no dedicated // target (`local_backup_target: "local"`, which host-install's own comment calls the DEGRADED // fallback) is not treated as critical for that tier — see storeGrantCritical. -func storeGrantStatuses(ctx context.Context, px *proxmox.Client, cfg config.Config) []capability.Status { +func storeGrantStatuses(ctx context.Context, px *proxmox.Client, cfg config.Config, repair *storeGrantRepairer) []capability.Status { tiers, _ := cfg.Backup.BackupTiers() // warnings are logged where the tiers are armed out := make([]capability.Status, 0, len(tiers)) for _, t := range tiers { - out = append(out, storeGrantStatus(ctx, px, t.TargetID, storeGrantCritical(t.TargetID))) + out = append(out, storeGrantStatus(ctx, px, t.TargetID, storeGrantCritical(t.TargetID), repair)) } return out } +// storeGrantRepairMinInterval bounds how often a single tier's grant may be re-granted (Scenario F). +// +// A storage can be unreadable for reasons an ACL cannot fix — the storage is gone, PVE is wedged, +// the wrapper is missing. Without a bound the probe would re-grant on every report cycle forever: a +// repair loop is a new defect wearing a fix's clothes. One attempt per tier per hour is frequent +// enough that a real loss is repaired within one backup window, and rare enough that a permanent +// fault produces attempts you can count on one hand per day. +const storeGrantRepairMinInterval = time.Hour + +// storeGrantRepairer bounds and records the self-repair. It is deliberately in-memory: an agent +// restart re-arms the repair, which is correct — a restart is exactly when a box should re-check +// everything it depends on. +type storeGrantRepairer struct { + run func(ctx context.Context, name string, args ...string) ([]byte, []byte, error) + log *slog.Logger + mu sync.Mutex + last map[string]time.Time // target id → last ATTEMPT (success or failure) +} + +// mayAttempt reports whether a repair may run now for this target, and records the attempt if so. +func (r *storeGrantRepairer) mayAttempt(target string, now time.Time) bool { + if r == nil || r.run == nil { + return false + } + r.mu.Lock() + defer r.mu.Unlock() + if r.last == nil { + r.last = map[string]time.Time{} + } + if t, ok := r.last[target]; ok && now.Sub(t) < storeGrantRepairMinInterval { + return false + } + r.last[target] = now + return true +} + +// repair runs the EXISTING root wrapper's `grant` verb for this storage. It adds no privileged +// surface: `felhom-backup-target-apply grant *` is already in the sudoers allowlist for any storage +// id (configs/felhom-agent.sudoers), and the verb already grants BOTH the user and the token — a +// privsep token's rights are the intersection, so granting one of the two grants nothing usable. +// +// This is the pbsdr shape (internal/pbsdr/manager.go, the R-22 self-grant): on a refusal, run the +// root wrapper and RE-READ ONCE rather than dead-locking. Its restraint is copied too — one attempt, +// one confirmation, and anything still wrong stays loudly wrong. +func (r *storeGrantRepairer) repair(ctx context.Context, target string) error { + rctx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + _, errOut, err := r.run(rctx, localapi.BackupTargetWrapperPath, "grant", target) + if err != nil { + r.log.Error("store-grant: SELF-REPAIR FAILED — the tier stays unreadable", + "target", target, "err", err, "stderr", strings.TrimSpace(string(errOut))) + return err + } + return nil +} + // storeGrantRequiredPriv is the privilege whose ABSENCE was measured to blind the content listing. // // Measured on demo-felhom 2026-08-03: the two storages that list through the token hold @@ -468,7 +525,7 @@ func storeGrantCritical(targetID string) bool { return targetID != "local" } // storeGrantStatus is one tier's grant probe. It NEVER reports ok when it could not ask: a // self-check that fails open is worse than none, because it converts "I do not know" into "fine". -func storeGrantStatus(ctx context.Context, px *proxmox.Client, targetID string, critical bool) capability.Status { +func storeGrantStatus(ctx context.Context, px *proxmox.Client, targetID string, critical bool, repair *storeGrantRepairer) capability.Status { s := capability.Status{ Name: "pve:store-grant:" + targetID, Feature: "backup tier " + targetID + " readable by the agent (archive listing, restore-test candidacy)", @@ -486,7 +543,93 @@ func storeGrantStatus(ctx context.Context, px *proxmox.Client, targetID string, pctx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() privs, err := px.Permissions(pctx, "/storage/"+targetID) - return storeGrantVerdict(targetID, critical, privs, err) + s = storeGrantVerdict(targetID, critical, privs, err) + if err != nil || s.Status != capability.StatusDegraded { + return s + } + + // ── R-190 mitigation: the grant is missing — repair it, and SAY that it was missing ────────── + // + // R-190 is a grant that demonstrably worked at 04:44 and was gone by 09:24, with a reinstall, + // logged pveum activity and cluster-log entries all ruled out. The cause is still open; the + // resilience does not have to wait for it. Everything needed already exists — the root wrapper, + // its sudoers vector for any storage id, and the exact command — and until now the `grant` verb + // had only ever been called at CREATION. That is the "built but never wired" shape, in a verb + // rather than a seam. + if !repair.mayAttempt(targetID, time.Now()) { + // Bounded (Scenario F): an earlier attempt did not hold and it is too soon to try again. Stay + // degraded and say why — a quiet "we already tried" is how a permanent fault becomes silence. + s.Reason = "the agent token lacks " + storeGrantRequiredPriv + " on /storage/" + targetID + + " and a self-repair was attempted within the last " + storeGrantRepairMinInterval.String() + + " without holding — NOT retrying yet; this needs a human" + return s + } + if rerr := repair.repair(ctx, targetID); rerr != nil { + s.Reason = "the agent token lacks " + storeGrantRequiredPriv + " on /storage/" + targetID + + " and the self-repair FAILED (" + rerr.Error() + ") — this tier's archives are INVISIBLE to the agent" + return s // Scenario E: a failed repair must never mask the degraded state. + } + // Re-read ONCE to confirm, exactly as pbsdr does — the wrapper reporting success is a claim about + // its own write; the grant being readable is a different claim, and it is the one that matters. + cctx, ccancel := context.WithTimeout(ctx, 10*time.Second) + defer ccancel() + privs2, err2 := px.Permissions(cctx, "/storage/"+targetID) + if err2 != nil || privs2[storeGrantRequiredPriv] != 1 { + s.Reason = "the agent token lacks " + storeGrantRequiredPriv + " on /storage/" + targetID + + " and the self-repair did not take (re-read says it is still missing) — this needs a human" + return s + } + + // REPAIRED — and reported as DEGRADED for exactly this one cycle, deliberately. + // + // The tier works again, so "ok" would be true of this instant and would throw away the only + // evidence that anything happened. R-190's own words: the probe sees the STATE, nothing sees the + // TRANSITION. A silent self-repair makes a recurring loss undetectable forever, which is strictly + // worse than the fault it fixes. + // + // §8.5 asked whether the hub's existing degraded↔ok edge suffices before building anything new. + // It does — as a CHANNEL — but only if the agent deliberately reports one degraded cycle: the hub + // alerts and e-mails on the ok→degraded edge and logs the degraded→ok recovery, so one loss + // produces exactly one alert pair and the operator learns of it. NOTHING NEW WAS BUILT: no wire + // change, no hub change, no new event type. The `Feature` text carries the explanation because + // that is the field the hub puts in the operator's e-mail (the Reason does not travel). + s = storeGrantRepairedVerdict(targetID, critical) + repairLogger(repair).Error("store-grant: GRANT WAS MISSING AND HAS BEEN SELF-REPAIRED — investigate the loss (R-190)", + "target", targetID, "privilege", storeGrantRequiredPriv, + "action", "felhom-backup-target-apply grant "+targetID, "confirmed_by", "re-read") + return s +} + +// storeGrantRepairedVerdict is the post-repair verdict — the RECORD half of R-190, split out so the +// tests exercise the real thing rather than a copy of it (yesterday's hollow-test lesson). +// +// It reports DEGRADED although the tier now works, and that is the whole point: "ok" would be true of +// this instant and would throw away the only evidence that a permission vanished. The hub raises its +// ok→degraded edge (an operator e-mail) and logs the degraded→ok recovery on the next cycle, so one +// loss produces exactly one alert pair. Nothing new was built for this — no wire change, no hub +// change, no new event type. +// +// The explanation lives in FEATURE because that is the field the hub interpolates into the operator's +// e-mail (`monitor/host_capability.go` emitTransition builds its message from the capability names +// and features; Reason does not travel). Putting it in Reason alone would be a record nobody reads. +func storeGrantRepairedVerdict(targetID string, critical bool) capability.Status { + return capability.Status{ + Name: "pve:store-grant:" + targetID, + Critical: critical, + Status: capability.StatusDegraded, + Feature: "backup tier " + targetID + ": the agent's storage grant was MISSING and has been " + + "AUTOMATICALLY RESTORED — the tier works now, but a permission that vanished on its own needs investigating (R-190)", + Reason: "grant absent at probe time; `felhom-backup-target-apply grant " + targetID + + "` re-applied it and a re-read confirms " + storeGrantRequiredPriv + " is present again", + } +} + +// repairLogger returns the repairer's logger, or the default — the record must survive a nil. +func repairLogger(r *storeGrantRepairer) *slog.Logger { + if r != nil && r.log != nil { + return r.log + } + return slog.Default() } // storeGrantVerdict is the DECISION, split out from the API call so the tests exercise the real @@ -598,9 +741,17 @@ func runDaemon(cfg config.Config, logger *slog.Logger, logRing *applog.Ring) int // reaper fail-safes (locks stay uncleared) — visible on the hub report, no operator page. // R-185: the store-grant probes compose around the sudo prober the same way the pool read does // (an API read does not belong inside the sudo-policy probe — the v0.62.0 A1 precedent). + // R-190: the store-grant probe also REPAIRS a missing grant, through the root wrapper that + // already exists and is already sudoers-permitted for any storage id — and reports the loss. + // The runner is the DIRECT one for the same reason the sudo prober uses it: the wrapper is + // invoked through the privileged path, which prepends sudo itself. + grantRepairer := &storeGrantRepairer{ + run: (&proxmox.ExecRunner{Mode: proxmox.RunnerMode(cfg.Privileged.Mode)}).Run, + log: logger, + } probeAll := func(ctx context.Context) []capability.Status { out := append(capProber.Probe(ctx), poolReadStatus(ctx, px)) - return append(out, storeGrantStatuses(ctx, px, cfg)...) + return append(out, storeGrantStatuses(ctx, px, cfg, grantRepairer)...) } // (The startup self-check log runs AFTER the pbsdr manager is wired below, so its snapshot // already carries the gated view — v0.86.0.) diff --git a/cmd/felhom-agent/storegrant_test.go b/cmd/felhom-agent/storegrant_test.go index 3cc2fff..1998d75 100644 --- a/cmd/felhom-agent/storegrant_test.go +++ b/cmd/felhom-agent/storegrant_test.go @@ -2,9 +2,13 @@ package main import ( "context" + "errors" "go/ast" + "io" + "log/slog" "strings" "testing" + "time" "gitea.dooplex.hu/admin/felhom-agent/internal/capability" ) @@ -132,7 +136,7 @@ func TestStoreGrant_TheFallbackTargetIsNotCritical(t *testing.T) { // A probe that cannot ask must never answer "ok" — unknown reported as healthy is worse than no // probe, because it looks like coverage. func TestStoreGrant_UnreachablePVEIsDegradedNotOK(t *testing.T) { - s := storeGrantStatus(context.Background(), nil, "felhom-backup", true) + s := storeGrantStatus(context.Background(), nil, "felhom-backup", true, nil) if s.Status != capability.StatusDegraded { t.Fatalf("an unaskable probe must be DEGRADED, never ok; got %q", s.Status) } @@ -164,3 +168,188 @@ func TestMainWiresTheStoreGrantProbe(t *testing.T) { "which is precisely the silence R-185 is about") } } + +// ── R-190 — the grant repairs itself, and the repair is VISIBLE ────────────────────────────── +// +// R-190 is a storage grant that demonstrably worked at 04:44 on 2026-08-03 and was gone by 09:24, +// with a host reinstall, logged `pveum` activity and cluster-log entries all ruled out. The cause is +// open; the resilience is not conditional on it. +// +// The half that matters is the RECORD. R-190's own words: the probe sees the state, nothing sees the +// transition. A self-repair that leaves only "ok" behind destroys the only evidence a loss happened, +// so a recurring loss becomes undetectable forever — strictly worse than the fault it fixes. + +// fakeRepairRunner records wrapper invocations and can be made to fail. +type fakeRepairRunner struct { + calls [][]string + fail bool +} + +func (f *fakeRepairRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) { + f.calls = append(f.calls, append([]string{name}, args...)) + if f.fail { + return nil, []byte("pveum: refused"), errors.New("exit status 2") + } + return nil, nil, nil +} + +func newRepairer(f *fakeRepairRunner) *storeGrantRepairer { + return &storeGrantRepairer{run: f.Run, log: slog.New(slog.NewTextHandler(io.Discard, nil))} +} + +// ── SCENARIO F — the repair is BOUNDED ─────────────────────────────────────────────────────── +// +// COMPANION RED-PROOF (observed 2026-08-04): make mayAttempt always return true (drop the +// storeGrantRepairMinInterval check) → +// +// --- FAIL: TestGrantRepair_IsBounded +// storegrant_test.go: a repair must not run on every cycle; 5 cycles produced 5 attempt(s) +// +// which is a re-grant every report cycle, forever, against a fault an ACL cannot fix. Restored. +func TestGrantRepair_IsBounded(t *testing.T) { + f := &fakeRepairRunner{} + r := newRepairer(f) + // Jittered, so the series never lands exactly on the interval boundary — a perfectly regular + // series is how a threshold test passes its own mutation, which has happened here before. + base := time.Date(2026, 8, 4, 9, 17, 43, 0, time.UTC) + offsets := []time.Duration{0, 13*time.Minute + 7*time.Second, 27*time.Minute + 51*time.Second, + 41*time.Minute + 19*time.Second, 55*time.Minute + 3*time.Second} + attempts := 0 + for _, off := range offsets { + if r.mayAttempt("felhom-backup", base.Add(off)) { + attempts++ + } + } + if attempts != 1 { + t.Fatalf("a repair must not run on every cycle; %d cycles produced %d attempt(s) within %s", + len(offsets), attempts, storeGrantRepairMinInterval) + } + // ...and once the interval has genuinely passed, it may try again — a bound is not a ban. + if !r.mayAttempt("felhom-backup", base.Add(storeGrantRepairMinInterval+2*time.Minute+11*time.Second)) { + t.Fatal("after the interval a repair must be allowed again — otherwise one failure disables the repair forever") + } + // A DIFFERENT tier is not throttled by this one's attempt. + if !r.mayAttempt("felhom-pbs", base.Add(time.Minute)) { + t.Fatal("the bound must be per tier — one tier's attempt must not suppress another's") + } +} + +// A nil repairer (or one with no runner) never attempts, and never panics. +func TestGrantRepair_NilIsSafe(t *testing.T) { + var r *storeGrantRepairer + if r.mayAttempt("felhom-backup", time.Now()) { + t.Fatal("a nil repairer must never claim an attempt") + } + if (&storeGrantRepairer{}).mayAttempt("felhom-backup", time.Now()) { + t.Fatal("a repairer with no runner must never claim an attempt") + } +} + +// The repair calls the EXISTING wrapper verb, with the storage id — no new privileged surface. +func TestGrantRepair_CallsTheExistingWrapperVerb(t *testing.T) { + f := &fakeRepairRunner{} + r := newRepairer(f) + if err := r.repair(context.Background(), "felhom-backup"); err != nil { + t.Fatalf("repair should succeed with a healthy runner: %v", err) + } + if len(f.calls) != 1 { + t.Fatalf("exactly one wrapper invocation expected; got %d", len(f.calls)) + } + got := f.calls[0] + want := []string{"/usr/local/sbin/felhom-backup-target-apply", "grant", "felhom-backup"} + if len(got) != len(want) { + t.Fatalf("wrapper argv = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("wrapper argv = %v, want %v — the sudoers vector is `grant *`; anything else is a policy change", got, want) + } + } +} + +// A repair that FAILS must surface the failure, not swallow it (Scenario E's precondition). +func TestGrantRepair_FailureIsReturned(t *testing.T) { + f := &fakeRepairRunner{fail: true} + if err := newRepairer(f).repair(context.Background(), "felhom-backup"); err == nil { + t.Fatal("a failed wrapper run must return its error — a repair that cannot run must never read as done") + } +} + +// ── SCENARIO D (the half that matters) — the REPAIR MUST BE VISIBLE ────────────────────────── +// +// A repair that leaves only "ok" behind is worse than the fault: the tier works, and the fact that a +// permission vanished is gone with it. R-190 exists because nothing saw the transition. +// +// The channel is the hub's EXISTING ok→degraded→ok edge (§8.5) — nothing new was built. That only +// works if the agent deliberately reports ONE degraded cycle after repairing, and if the explanation +// rides the field the hub actually puts in the operator's e-mail. The hub's message is built from the +// capability NAME and FEATURE (`internal/monitor/host_capability.go` emitTransition) — **not** from +// Reason — so the Feature must carry it. +// +// COMPANION RED-PROOF (observed 2026-08-04): after a successful repair, report ok instead — +// +// s.Status = capability.StatusOK; s.Feature unchanged +// +// → --- FAIL: TestGrantRepair_ARepairedGrantIsReportedAsATransition +// +// storegrant_test.go: a self-repair must still report DEGRADED for one cycle so the hub raises +// its edge; got "ok" — the loss would be invisible +// +// i.e. exactly the silence R-190 is about. Restored. +func TestGrantRepair_ARepairedGrantIsReportedAsATransition(t *testing.T) { + // THE PRODUCTION verdict, not a copy of it. An earlier draft of this test built the Status + // itself and asserted its own construction — it would have passed while production reported ok, + // which is precisely the silence being guarded against. + if pre := probeWith(permUngranted, "felhom-backup", true); pre.Status != capability.StatusDegraded { + t.Fatalf("precondition: a missing grant is degraded; got %q", pre.Status) + } + s := storeGrantRepairedVerdict("felhom-backup", true) + + if s.Status != capability.StatusDegraded { + t.Fatalf("a self-repair must still report DEGRADED for one cycle so the hub raises its edge; "+ + "got %q — the loss would be invisible", s.Status) + } + // The hub e-mails the FEATURE text. If the explanation is not there, the operator is told a + // capability was degraded and never learns it repaired itself or that anything vanished. + for _, want := range []string{"MISSING", "RESTORED", "felhom-backup", "R-190"} { + if !strings.Contains(s.Feature, want) { + t.Fatalf("the Feature text is what the hub puts in the operator's e-mail; it must contain %q. Got: %s", want, s.Feature) + } + } + if !s.Critical { + t.Fatal("the transition must be CRITICAL or the hub does not alert on it at all") + } +} + +// ── SCENARIO H — the seam ──────────────────────────────────────────────────────────────────── +// +// The wrapper's `grant` verb is itself a "built but never wired" example: it exists, is +// sudoers-permitted for any id, and had only ever been called at storage CREATION. The repair must +// not become the seventh instance. AST, not grep — a commented-out call still contains the string. +func TestMainWiresTheGrantRepair(t *testing.T) { + f := parseMainForWiring(t) + + var built, passed bool + ast.Inspect(f, func(n ast.Node) bool { + switch node := n.(type) { + case *ast.CompositeLit: + if id, ok := node.Type.(*ast.Ident); ok && id.Name == "storeGrantRepairer" { + built = true + } + case *ast.CallExpr: + if id, ok := node.Fun.(*ast.Ident); ok && id.Name == "storeGrantStatuses" && len(node.Args) == 4 { + if a, ok := node.Args[3].(*ast.Ident); ok && a.Name == "grantRepairer" { + passed = true + } + } + } + return true + }) + if !built { + t.Error("main.go never constructs a storeGrantRepairer — nothing would ever repair a lost grant") + } + if !passed { + t.Error("storeGrantStatuses is not passed the repairer — the probe would detect the loss and " + + "leave it, which is v0.123.0's behaviour and not R-190's mitigation") + } +}