diff --git a/CHANGELOG.md b/CHANGELOG.md index b2da8a0..dfdc2cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,24 @@ -## unreleased — comments and the retention record only, no binary change (burn-down 2026-10-05: R-291, R-348) +## unreleased (to be released as v0.147.0) — the recovery recipe spells the root namespace the way PBS does; a removed drive no longer shows the root disk's size; a rotated-out token stops at once; the dnsmasq check looks at the right package (burn-down round 2: R-124, R-118, R-269, R-317) (2026-10-05) + +MinAgent impact: none (the controller needs nothing new from this agent). Config bundle content unchanged from v0.146.1. + +- **R-124 (operator ruling 2026-10-05: fix it):** the DR recipe's `pbs.namespace` for a box in PBS's ROOT namespace is + now `""` — PBS's own spelling — beside `namespace_state: resolved`; it used to be the word `root`, which no namespace + is named, so `--ns root` failed in a recovery. `hub.PBSRootNamespace`; `TestR124_RootNamespaceOnTheWireIsPBSSpelling` + (red-proof: back to "root" → FAIL). Runbook: `felhom.eu runbooks/ep0-datastore-copy.md` step 2 says how to read it + (and to treat a recorded `root` from older agents as empty). The hub stores the recipe raw; its fixture follows. +- **R-118:** the local API's drive list reads a drive's capacity only while its DEVICE is present — with the device gone + the bare mountpoint is a directory on the root filesystem, whose size was reported as the drive's. + `TestDisks_UnionPath_AbsentDeviceReportsNoRootCapacity` (red-proof convicts). +- **R-269:** the token store re-reads its shared file whenever it has grown, BEFORE answering — so a token rotated out by + another process stops authorizing on its next use (it used to keep working until an unrelated miss). One `stat` per + call. `TestTokenStore_RotatedOutTokenRejectedFirst` (red-proof convicts). +- **R-317:** the LAN resolver decides whether to install `dnsmasq` by its service UNIT, not by `/usr/sbin/dnsmasq` (which + the `dnsmasq-base` package also ships). Same `apt-get install` command; no sudoers change. `TestEnsureDnsmasq_*` + (red-proof convicts). Red-proofs: `felhom.eu/documentation/audits/burndown2-2026-10-05/agent-red-proofs.txt`, + `r124-red-proof.txt`. + +Also in this release (no binary effect; from burn-down round 1): - **R-291:** `scripts/retention-policy.json` names where its 10 comes from — the R-267 newest-10 prune of generic packages, established 2026-08-10 (R-287) — instead of „observed, no located ruling"; the non-existent diff --git a/REUSE.md b/REUSE.md index 0dac0a7..ddcf963 100644 --- a/REUSE.md +++ b/REUSE.md @@ -67,7 +67,7 @@ | `IntentStore` (`Get/SetEnrolled/SetEjected/SetDecommissioned/OnAbsent`) | internal/storage/intent.go | `OpenIntentStore(path)` | drive intent (4-state self-heal) | Keyed by durable-id only; `OnAbsent` is the ONLY ejected→enrolled path; refuses empty ids | | `GuestBindStore` (`Record/Remove/Guests`) | internal/localapi/guestbindstore.go | `OpenGuestBindStore(path)` | per-guest enrolled binds (F9 re-assert) | Same tmp+rename 0600 pattern as IntentStore | | `FormatJobStore` + `startFormatDetached` + `RecoverFormatJob` | internal/localapi/formatjob.go | `startFormatDetached(device, durableID, fstype, blank) <-chan error` | detached, restart-surviving mkfs (F20-BUG3) | Runs off `s.baseCtx` (60-min bound) so a request deadline can't SIGKILL mkfs; recovery re-resolves by durable id; blank jobs re-check STILL-blank | -| `TokenStore.Mint` / `Lookup` | internal/localapi/tokenstore.go | `Mint(vmid) (plaintext, error)` | per-guest local-API tokens | Only the SHA-256 hash persists (fsync'd append log); constant-time compare on lookup; plaintext returned exactly once. Lookup RELOADS the file once on a miss (v0.63.0, B3): the one-shot provisioner mints into the same file the daemon indexes — cross-process coherence without a restart; append-only size check bounds the re-read | +| `TokenStore.Mint` / `Lookup` | internal/localapi/tokenstore.go | `Mint(vmid) (plaintext, error)` | per-guest local-API tokens | Only the SHA-256 hash persists (fsync'd append log); constant-time compare on lookup; plaintext returned exactly once. Lookup stats the file on EVERY call and reloads BEFORE answering when the append-only log grew (R-269; was reload-on-miss only, v0.63.0 B3, which let a token rotated out by another process keep authorizing as a map hit): the one-shot provisioner mints into the same file the daemon indexes — cross-process coherence both ways without a restart; unchanged size = no re-read. Pinned by `TestTokenStore_RotatedOutTokenRejectedFirst` | | `FileNonceStore.SeenOrRecord` | internal/authz/noncestore.go | `SeenOrRecord(nonce, exp) bool` | durable anti-replay | fsync'd before returning false; prune only after exp | | `Journal` (`Append/Latest/InFlight/AlreadyApplied`) | internal/reconcile/journal.go | `OpenJournal(path)` | op journal + idempotency + crash recovery | `Recover` consumes `InFlight()`; scratch entries special-cased | @@ -157,6 +157,7 @@ | `storage.HostOps` | internal/storage/hostops.go | `*SudoHostOps` (prod), `NoopHostOps` (degraded) | fakes in internal/storage/observe_test.go, watchdog_test.go | | `storage.HostReader` | internal/storage/hostread.go | `*ProcHostReader` | `fakeHostReader` internal/localapi/disks_test.go; internal/storage/role_test.go. v0.87.0: `BlockSlaves(name)` lists `/sys/block//slaves` (root-free) — backs the `SystemDisks` dm/md walk (`physicalDisksOf`/`walkSlaves`, role.go); per-branch conservatism: an unresolvable slave fails the WHOLE walk → all-system fail-safe. NEVER weaken the signature test `TestSystemDisks_WalkTopologies` (root-backing disk always in the system set). | | `localapi.DiskOps` / `StorageGate` / `GuestAttacher` / `GuestLister` | internal/localapi/disks.go | `*storage.SudoHostOps`; `storageGateAdapter` (cmd/felhom-agent/main.go); `*GuestBinder`; `*proxmox.Client` | `fakeDiskOps`/`fakeGate`/`fakeGuestAttacher`/`fakeGuestList` internal/localapi/disks_test.go | +| `lanresolver.hostRoot` + `dnsmasqUnitPaths` (data seam, R-317) | internal/lanresolver/lanresolver.go | prod `hostRoot = "/"`; probe = the `dnsmasq` package's systemd UNIT, never `/usr/sbin/dnsmasq` (owned by `dnsmasq-base`) | internal/lanresolver/ensure_dnsmasq_test.go — fixture root tree + recording `proxmox.Runner`; the REAL `os.Stat` probe and `EnsureDnsmasq` run. `TestEnsureDnsmasq_ProductionProbeIsTheUnit` pins the production wiring | | `localapi.GuestAPI` / `BackupService` / `BackupStore` / `TokenAuthority` | internal/localapi/server.go | `*proxmox.Client`, `*backup.BackupRunner`, `*backup.Store`, `*TokenStore` | `fakeGuests`/`fakeBackups`/`fakeStore` internal/localapi/server_test.go | | `backup.InFlight` | internal/backup/inflight.go | `TryAcquire(what) (release, busy, ok)` / `Busy()` | THE host-wide "one heavy guest operation at a time" gate — shared by the local-API backup path and the restore-test scheduler (R-85) | A **LINK** guard, not a lock one: the scratch VMID never touches the live guest's vzdump lock, but an offsite restore PULLS multi-GB over the tunnel a backup PUSHES one. Callers **DEFER, never cancel** — a deferred restore-test costs coverage, a cancelled backup costs the backup. A nil gate is ungated (pre-R-85 callers). | | `capability` store-grant probe (`storeGrantStatuses` / `storeGrantVerdict` / `Client.Permissions`) | cmd/felhom-agent/main.go, internal/proxmox/query.go | *"may the agent READ this backup tier?"*, one `capability.Status` per configured tier | R-185. **Never infer permission from an empty content listing** — `{"data":[]}` is what a FORBIDDEN tier and a NEWBORN tier both return, and that ambiguity hid an unreadable host tier on both demo boxes. Ask `/access/permissions` **as the agent's own token** (root always says yes). **The ungranted answer is not empty and not a 403** — it carries the privileges inherited from the box-wide `/` grant, so test for **`Datastore.AllocateSpace`** specifically; path-presence or `Datastore.Audit` reports a blinded storage healthy. Probed set comes from `BackupTiers()`, never a fixed list. Critical except the `local` fallback. Composes AROUND the sudo prober (the `poolReadStatus` precedent); `Status`'s wire shape is untouched so the hub alert is free. Unreachable PVE ⇒ degraded, never ok. | diff --git a/internal/hub/dr_recipe.go b/internal/hub/dr_recipe.go index 398365a..70b4fb3 100644 --- a/internal/hub/dr_recipe.go +++ b/internal/hub/dr_recipe.go @@ -54,11 +54,13 @@ const ( DRReasonNoPBSStorage = "no_pbs_storage_observed" ) -// PBSRootNamespace is how the recipe spells PBS's root namespace. The PBS API spells it as the EMPTY -// string (and `pct restore --ns root` would name a namespace that does not exist) — "root" is a display -// convention this wire has always used, kept here so the field's meaning did not change under R-106. -// Only a box with no `namespace` line in its pbs storage.cfg stanza ever emits it. -const PBSRootNamespace = "root" +// PBSRootNamespace is how the recipe spells PBS's root namespace: the EMPTY string, PBS's own spelling (R-124, +// agent v0.147.0). It used to be the display word "root", which no PBS namespace is named — an operator pasting it +// into `proxmox-backup-client … --ns root` during a real recovery got a failure. An empty namespace is ambiguous on +// its own, so READ IT WITH namespace_state: resolved + "" = the root namespace (pass no --ns, or --ns ""); unknown + +// "" = the agent could not tell. Only a box with no `namespace` line in its pbs storage.cfg stanza emits it. +// Pinned by TestDRRecipe_PBSNamespaceRootIsResolvedNotUnknown and TestR124_RootNamespaceOnTheWireIsPBSSpelling. +const PBSRootNamespace = "" // DRRecipeHostHalf is the agent-emitted half (guest/drive/storage/PBS scaffolding). Derived entirely // from facts the report already collects — no new privileged reads. @@ -123,8 +125,8 @@ type DRPBSCoord struct { RepoID string `json:"repo_id"` // the PVE pbs storage id (e.g. "felhom-pbs") — not a token // Namespace is the PBS namespace the restore targets, resolved from the pbs storage's storage.cfg // stanza — the same field `vzdump --storage ` makes PVE read, so the recipe cannot disagree - // with the backup that produced the snapshot. PBSRootNamespace when the box has no namespace - // configured; "" when NamespaceState is unknown. + // with the backup that produced the snapshot. PBSRootNamespace ("", PBS's spelling, R-124) when the box has no + // namespace configured; also "" when NamespaceState is unknown — consult NamespaceState. // // R-106: this used to come from the listed snapshot's own `ns`, which PBS does not echo per item once // the request is already namespace-scoped via `?ns=` (internal/pbs/client.go). The field was diff --git a/internal/hub/dr_recipe_test.go b/internal/hub/dr_recipe_test.go index c7640e4..74b1019 100644 --- a/internal/hub/dr_recipe_test.go +++ b/internal/hub/dr_recipe_test.go @@ -63,8 +63,8 @@ func TestBuildDRRecipeHostHalf(t *testing.T) { t.Error("felhom-flash (local-dir user-data drive) missing from drives") } // pbs: latest snapshot's coords + the pbs storage id as repo_id. - if h.PBS == nil || h.PBS.RepoID != "felhom-pbs" || h.PBS.Namespace != "root" || h.PBS.LatestSnapshotID != "9201" { - t.Errorf("pbs coord = %+v, want repo felhom-pbs/root/9201", h.PBS) + if h.PBS == nil || h.PBS.RepoID != "felhom-pbs" || h.PBS.Namespace != PBSRootNamespace || h.PBS.LatestSnapshotID != "9201" { + t.Errorf("pbs coord = %+v, want repo felhom-pbs, the root namespace (\"\", R-124), snapshot 9201", h.PBS) } } @@ -252,7 +252,7 @@ func TestDRRecipe_PBSNamespaceIsThePerCustomerOne(t *testing.T) { } // TestDRRecipe_PBSNamespaceRootIsResolvedNotUnknown: a box with a pbs storage and NO namespace line is -// genuinely in the root namespace. That is an answer, not a gap — it must read resolved/"root", so the +// genuinely in the root namespace. That is an answer, not a gap — it must read resolved/"" (PBS's spelling, R-124), so the // honest root case is never confused with "I could not tell". func TestDRRecipe_PBSNamespaceRootIsResolvedNotUnknown(t *testing.T) { h := BuildDRRecipeHostHalf(nil, @@ -453,3 +453,39 @@ func assertNoSecretKeys(t *testing.T, jsonBytes []byte) { } walk("", v) } + +// R-124: on the WIRE the root namespace is PBS's own spelling — an empty string, present (not omitted), beside +// namespace_state "resolved". The display word "root" names no PBS namespace, and `--ns root` fails in a recovery. +// RED-PROOF: set PBSRootNamespace back to "root" → this test fails. +func TestR124_RootNamespaceOnTheWireIsPBSSpelling(t *testing.T) { + h := BuildDRRecipeHostHalf(nil, + []StorageTarget{{Name: "felhom-pbs", Type: StorageTypePBS, Content: "backup", PBSNamespace: ""}}, + capturedDemoFelhomSnapshots(), + ConfiguredBackupTarget{StorageID: "felhom-pbs", Known: true}) + b, err := json.Marshal(h.PBS) + if err != nil { + t.Fatal(err) + } + var m map[string]any + if err := json.Unmarshal(b, &m); err != nil { + t.Fatal(err) + } + ns, present := m["namespace"] + if !present { + t.Fatalf("namespace key missing from %s — an omitted key reads as 'unknown', not 'root'", b) + } + if ns != "" { + t.Fatalf("root namespace on the wire = %q, want \"\" (PBS's spelling; no namespace is named %q)", ns, ns) + } + if m["namespace_state"] != DRStateResolved { + t.Fatalf("namespace_state = %v, want %q beside the empty root namespace", m["namespace_state"], DRStateResolved) + } + // A configured namespace still passes through unchanged. + h2 := BuildDRRecipeHostHalf(nil, + []StorageTarget{{Name: "felhom-pbs", Type: StorageTypePBS, Content: "backup", PBSNamespace: "demo-felhom"}}, + capturedDemoFelhomSnapshots(), + ConfiguredBackupTarget{StorageID: "felhom-pbs", Known: true}) + if h2.PBS.Namespace != "demo-felhom" { + t.Fatalf("configured namespace = %q, want demo-felhom", h2.PBS.Namespace) + } +} diff --git a/internal/lanresolver/ensure_dnsmasq_test.go b/internal/lanresolver/ensure_dnsmasq_test.go new file mode 100644 index 0000000..63d8c84 --- /dev/null +++ b/internal/lanresolver/ensure_dnsmasq_test.go @@ -0,0 +1,122 @@ +package lanresolver + +import ( + "context" + "io" + "log/slog" + "os" + "path/filepath" + "strings" + "sync" + "testing" +) + +// recRunner records every privileged command EnsureDnsmasq would run and succeeds — nothing reaches +// apt, systemctl or the root checker. +type recRunner struct { + mu sync.Mutex + calls []string +} + +func (r *recRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) { + r.mu.Lock() + defer r.mu.Unlock() + r.calls = append(r.calls, strings.Join(append([]string{name}, args...), " ")) + return nil, nil, nil +} + +func (r *recRunner) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) { + return r.Run(ctx, name, args...) +} + +func (r *recRunner) installed() bool { + for _, c := range r.calls { + if strings.HasPrefix(c, "apt-get install") && strings.HasSuffix(c, " dnsmasq") { + return true + } + } + return false +} + +// fixtureRoot builds a fake host root holding exactly the given relative files and points the REAL +// probe at it for the test's duration. +func fixtureRoot(t *testing.T, files ...string) { + t.Helper() + root := t.TempDir() + for _, f := range files { + p := filepath.Join(root, f) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, nil, 0o644); err != nil { + t.Fatal(err) + } + } + prev := hostRoot + hostRoot = root + t.Cleanup(func() { hostRoot = prev }) +} + +func ensure(t *testing.T) *recRunner { + t.Helper() + r := &recRunner{} + m := NewManager(r, "192.0.2.10", []string{"1.1.1.1"}, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err := m.EnsureDnsmasq(context.Background()); err != nil { + t.Fatalf("EnsureDnsmasq: %v", err) + } + return r +} + +// R-317: a host with `dnsmasq-base` (the /usr/sbin/dnsmasq binary) but WITHOUT the `dnsmasq` package +// (the service unit) must get the package installed — else the following `systemctl enable --now +// dnsmasq` hits a unit that does not exist and LAN name resolution silently never comes up. +// +// RED-PROOF: probe "usr/sbin/dnsmasq" instead of the unit paths in dnsmasqUnitInstalled → this fails +// with "install was skipped". +func TestEnsureDnsmasq_BinaryWithoutUnitInstalls(t *testing.T) { + fixtureRoot(t, "usr/sbin/dnsmasq") + r := ensure(t) + if !r.installed() { + t.Fatalf("install was skipped on a dnsmasq-base-only host (binary present, unit absent) — "+ + "the enable that follows targets a missing unit (R-317). calls: %q", r.calls) + } +} + +func TestEnsureDnsmasq_UnitPresentSkipsInstall(t *testing.T) { + for _, unit := range []string{"usr/lib/systemd/system/dnsmasq.service", "lib/systemd/system/dnsmasq.service"} { + t.Run(unit, func(t *testing.T) { + fixtureRoot(t, "usr/sbin/dnsmasq", unit) + if r := ensure(t); r.installed() { + t.Fatalf("apt-get install ran although the dnsmasq unit is present at %s: %q", unit, r.calls) + } + }) + } +} + +func TestEnsureDnsmasq_NothingPresentInstalls(t *testing.T) { + fixtureRoot(t) + if r := ensure(t); !r.installed() { + t.Fatalf("install skipped on a host with no dnsmasq at all: %q", r.calls) + } +} + +// Production wiring for the hostRoot seam: the shipped probe resolves against the real root and asks +// about the unit the `dnsmasq` package owns — never the dnsmasq-base binary. +func TestEnsureDnsmasq_ProductionProbeIsTheUnit(t *testing.T) { + if hostRoot != "/" { + t.Fatalf("hostRoot default = %q, want \"/\" — the production probe would look in the wrong tree", hostRoot) + } + var sawUsrLib bool + for _, p := range dnsmasqUnitPaths { + full := filepath.Join(hostRoot, p) + if strings.HasSuffix(full, "/sbin/dnsmasq") || strings.HasSuffix(full, "/bin/dnsmasq") { + t.Errorf("probe path %s is the dnsmasq-base binary, not the dnsmasq unit (R-317)", full) + } + if full == "/usr/lib/systemd/system/dnsmasq.service" { + sawUsrLib = true + } + } + if !sawUsrLib { + t.Errorf("probe paths %q miss /usr/lib/systemd/system/dnsmasq.service (dpkg -S: owned by dnsmasq)", dnsmasqUnitPaths) + } +} diff --git a/internal/lanresolver/lanresolver.go b/internal/lanresolver/lanresolver.go index 7e1103c..59ade55 100644 --- a/internal/lanresolver/lanresolver.go +++ b/internal/lanresolver/lanresolver.go @@ -101,11 +101,35 @@ func NewManager(runner proxmox.Runner, hostIP string, upstreams []string, logger } } +// hostRoot is the filesystem root the install probe resolves against: "/" in production; a test +// points it at a fixture tree so the REAL probe runs against files it controls. +var hostRoot = "/" + +// dnsmasqUnitPaths are where the `dnsmasq` package ships its systemd unit (Debian; /lib is the +// pre-usrmerge spelling). R-317: probe the UNIT, never /usr/sbin/dnsmasq — that binary belongs to +// `dnsmasq-base`, so a host carrying dnsmasq-base without dnsmasq used to skip the install and then +// `systemctl enable --now dnsmasq` failed against a unit that is not there (resolver never up). +// Pinned by TestEnsureDnsmasq_BinaryWithoutUnitInstalls. +var dnsmasqUnitPaths = []string{ + "usr/lib/systemd/system/dnsmasq.service", + "lib/systemd/system/dnsmasq.service", +} + +// dnsmasqUnitInstalled reports whether the dnsmasq service unit (the `dnsmasq` package) is present. +func dnsmasqUnitInstalled() bool { + for _, p := range dnsmasqUnitPaths { + if _, err := os.Stat(filepath.Join(hostRoot, p)); err == nil { + return true + } + } + return false +} + // EnsureDnsmasq makes dnsmasq present + enabled and writes the host base config. Idempotent: it // installs the package only when absent, and writes the base drop-in only when its content changes. func (m *Manager) EnsureDnsmasq(ctx context.Context) error { - if _, err := os.Stat("/usr/sbin/dnsmasq"); err != nil { // metadata read, no privilege needed - m.logger.Info("lanresolver: dnsmasq absent — installing") + if !dnsmasqUnitInstalled() { // metadata read, no privilege needed + m.logger.Info("lanresolver: dnsmasq service unit absent — installing") if out, errOut, ierr := m.runner.Run(ctx, "apt-get", "install", "-y", "-q", "dnsmasq"); ierr != nil { return fmt.Errorf("install dnsmasq: %s: %w", strings.TrimSpace(string(errOut))+string(out), ierr) } diff --git a/internal/localapi/disks.go b/internal/localapi/disks.go index 969521c..cd095c7 100644 --- a/internal/localapi/disks.go +++ b/internal/localapi/disks.go @@ -398,9 +398,16 @@ func (s *Server) handleDisks(w http.ResponseWriter, r *http.Request, vmid int) { di.Smart = &sm } } - if total, used, okc := statfsCapacity(d.MountPath); okc { - di.TotalBytes, di.UsedBytes = total, used - di.UsedFraction = float64(used) / float64(total) + // R-118: statfs ONLY while the drive's device is present. With the device gone the raw + // mountpoint reverts to a bare directory on the ROOT filesystem, and statfs would report + // pve-root's size as this drive's (measured: a 4 GB drive advertising 46 GiB). Same trap + // observe.go guards on the Observe path. Absent → capacity left zero (unknown), never root's. + // Pinned by TestDisks_UnionPath_AbsentDeviceReportsNoRootCapacity. + if s.devicePresent(d.MountPath) { + if total, used, okc := statfsCapacity(d.MountPath); okc { + di.TotalBytes, di.UsedBytes = total, used + di.UsedFraction = float64(used) / float64(total) + } } out = append(out, di) } diff --git a/internal/localapi/disks_device_presence_test.go b/internal/localapi/disks_device_presence_test.go index b482f1f..7b2f99b 100644 --- a/internal/localapi/disks_device_presence_test.go +++ b/internal/localapi/disks_device_presence_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "io" "log/slog" + "runtime" "strings" "testing" @@ -184,3 +185,39 @@ func TestDisks_DevicePresence_WireFieldIsFalseOnDeviceLoss(t *testing.T) { t.Fatalf("the drive never reached the wire: %s", body) } } + +// ── R-118 — an absent drive must not advertise the ROOT filesystem's capacity ─────────────────── + +// TestDisks_UnionPath_AbsentDeviceReportsNoRootCapacity drives the REAL statfsCapacity (no capacity +// seam): the registry drive's mount path is a real, bare temp directory — exactly what /mnt/ +// becomes once its device is gone (a plain directory on the host's filesystem). With the device absent +// the row must carry NO capacity; before R-118 the union path statfs'd that bare directory and reported +// the host filesystem's size and usage as the drive's (46 GiB at 9.2 % for a 4 GB drive, measured). +// The present half proves the test is not hollow: the same directory DOES yield capacity when the +// device is there, so a zero on the absent half is the guard's doing, not a statfs failure. +// +// RED-PROOF: drop the `if s.devicePresent(d.MountPath)` guard around statfsCapacity in disks.go → the +// absent subtest fails with "advertises ... bytes". +func TestDisks_UnionPath_AbsentDeviceReportsNoRootCapacity(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("statfsCapacity is linux-only; production target is linux") + } + bare := t.TempDir() + known := []storage.KnownTarget{ + {Name: "cel", Type: hub.StorageTypeUSB, MountPath: bare, DurableID: "uuid:4242", UUID: "4242"}, + } + t.Run("absent", func(t *testing.T) { + di := diskByMount(t, presenceServer(t, nil, known, true, false), bare) + if di.TotalBytes != 0 || di.UsedBytes != 0 || di.UsedFraction != 0 { + t.Errorf("absent drive advertises total=%d used=%d frac=%.3f — that is the filesystem UNDER "+ + "the bare mountpoint, not the drive (R-118)", di.TotalBytes, di.UsedBytes, di.UsedFraction) + } + }) + t.Run("present", func(t *testing.T) { + di := diskByMount(t, presenceServer(t, nil, known, true, true), bare) + if di.TotalBytes <= 0 { + t.Errorf("present drive reports no capacity (total=%d) — the guard over-corrected and the "+ + "size bar is gone for every healthy registry drive", di.TotalBytes) + } + }) +} diff --git a/internal/localapi/tokenstore.go b/internal/localapi/tokenstore.go index 54199bc..7b42f80 100644 --- a/internal/localapi/tokenstore.go +++ b/internal/localapi/tokenstore.go @@ -154,12 +154,15 @@ func (s *TokenStore) Mint(vmid int) (string, error) { // looks it up; the per-candidate comparison is constant-time to avoid a timing oracle on the // stored hash. ok is false for an unknown/empty token. // -// Reload-on-miss (B3): the store FILE is shared across processes — the one-shot provisioner -// (`--selftest=provision`) Mints into it while the long-lived daemon serves Lookup from an index -// built at open. On a miss, re-read the file ONCE and re-check, so a token minted after this -// process started authorizes without a daemon restart (the drill's fresh-install 401). The -// append-only log makes an unchanged file size proof of no new records, so a genuinely unknown -// token costs at most one stat once the index is current — never a reload loop. +// Reload-on-change (B3, R-269): the store FILE is shared across processes — the one-shot +// provisioner (`--selftest=provision`) Mints into it while the long-lived daemon serves Lookup from +// an index built at open. Every Lookup stats the file first and re-reads it when the append-only +// log has grown, BEFORE answering — so a token minted elsewhere authorizes without a restart AND a +// token rotated out elsewhere stops authorizing on its very next presentation. (Before R-269 the +// re-read ran only on a MISS, so a superseded token was a direct map hit and kept authorizing until +// some unrelated miss forced the reload.) An unchanged size is proof of no new records, so the +// steady state costs one stat per call and never a reload loop. Pinned by +// TestTokenStore_RotatedOutTokenRejectedFirst. func (s *TokenStore) Lookup(token string) (int, bool) { if token == "" { return 0, false @@ -167,23 +170,34 @@ func (s *TokenStore) Lookup(token string) (int, bool) { want := hashToken(token) s.mu.Lock() defer s.mu.Unlock() - // Direct map hit is the common path; the constant-time compare guards against a timing - // side-channel by re-checking the matched key (map lookup itself is not the secret-bearing - // comparison — the hash of a random 256-bit token is not feasibly guessable regardless). - if vmid, ok := s.byHash[want]; ok { - if subtle.ConstantTimeCompare([]byte(want), []byte(s.byVMID[vmid])) == 1 { - return vmid, true + st, statErr := os.Stat(s.path) + if statErr == nil && st.Size() != s.loadedSize { + // The log changed under us (another process minted/rotated): converge first, then answer. + s.reloads++ + if err := s.reloadLocked(); err != nil { + return 0, false // unreadable store: fail closed, never crash the auth path } + return s.matchLocked(want) } - // Miss: skip the re-read when the append-only log has not grown (nothing new to see). - // A stat error falls through to the reload, which handles a missing file as empty. - if st, err := os.Stat(s.path); err == nil && st.Size() == s.loadedSize { - return 0, false + if vmid, ok := s.matchLocked(want); ok { + return vmid, true } + if statErr == nil { + return 0, false // file unchanged since the last (re)load: genuinely unknown + } + // Stat failed (e.g. the file vanished): reload, which treats a missing file as empty. s.reloads++ if err := s.reloadLocked(); err != nil { - return 0, false // unreadable store: fail closed, never crash the auth path + return 0, false } + return s.matchLocked(want) +} + +// matchLocked answers from the in-memory index. Direct map hit is the common path; the +// constant-time compare re-checks the matched key against the guest's CURRENT hash (map lookup +// itself is not the secret-bearing comparison — the hash of a random 256-bit token is not +// feasibly guessable regardless). Caller holds the mutex. +func (s *TokenStore) matchLocked(want string) (int, bool) { if vmid, ok := s.byHash[want]; ok { if subtle.ConstantTimeCompare([]byte(want), []byte(s.byVMID[vmid])) == 1 { return vmid, true diff --git a/internal/localapi/tokenstore_test.go b/internal/localapi/tokenstore_test.go index 8fd38a4..4ac3598 100644 --- a/internal/localapi/tokenstore_test.go +++ b/internal/localapi/tokenstore_test.go @@ -210,6 +210,51 @@ func TestTokenStore_ReloadOnMiss_RemintCoherence(t *testing.T) { } } +// R-269: a token rotated out by ANOTHER process must stop authorizing on its very next +// presentation — with NO intervening lookup of the new token. This is the order the operator hits +// after rotating a leaked token: the leaked one is presented first. RemintCoherence above looks the +// NEW token up first, and that miss is what used to evict the old hash, so it passed while the leaked +// token kept returning HTTP 200 on hardware (2026-08-09) until something unrelated forced a reload. +// +// RED-PROOF: restore the reload-on-MISS-only Lookup (answer a map hit before stat-ing the file) and +// this fails with "rotated-out token still authorizes". +func TestTokenStore_RotatedOutTokenRejectedFirst(t *testing.T) { + path := filepath.Join(t.TempDir(), "tokens.log") + daemon, err := OpenTokenStore(path) + if err != nil { + t.Fatalf("open daemon store: %v", err) + } + defer daemon.Close() + minter, err := OpenTokenStore(path) + if err != nil { + t.Fatalf("open minter store: %v", err) + } + defer minter.Close() + + old, err := minter.Mint(130) + if err != nil { + t.Fatalf("mint old: %v", err) + } + if vmid, ok := daemon.Lookup(old); !ok || vmid != 130 { // the daemon has learned the old token + t.Fatalf("old token before rotation: (%d,%v), want (130,true)", vmid, ok) + } + fresh, err := minter.Mint(130) // rotation, written by another process + if err != nil { + t.Fatalf("mint fresh: %v", err) + } + + if vmid, ok := daemon.Lookup(old); ok { // the leaked token FIRST + t.Fatalf("rotated-out token still authorizes vmid %d on its first presentation after rotation — "+ + "Mint's 'any previous token for this guest is revoked' is false across processes (R-269)", vmid) + } + if vmid, ok := daemon.Lookup(fresh); !ok || vmid != 130 { + t.Fatalf("fresh token after rotation: (%d,%v), want (130,true)", vmid, ok) + } + if vmid, ok := daemon.Lookup(old); ok { + t.Fatalf("rotated-out token authorizes vmid %d after the fresh one was seen", vmid) + } +} + // §8 edge: the store file deleted between open and a miss — reload treats it as empty; Lookup // fails closed, no crash. func TestTokenStore_ReloadOnMiss_MissingFile(t *testing.T) { diff --git a/internal/pbs/live_reporter_test.go b/internal/pbs/live_reporter_test.go index 9e12873..fe7c6eb 100644 --- a/internal/pbs/live_reporter_test.go +++ b/internal/pbs/live_reporter_test.go @@ -60,8 +60,8 @@ func TestLiveReporter_CoordPresentWithoutPriorVerify(t *testing.T) { if h.PBS == nil { t.Fatal("pbs coord absent despite a reachable PBS — the gap this fixes") } - if h.PBS.RepoID != "felhom-pbs" || h.PBS.Namespace != "root" || h.PBS.LatestSnapshotID != "9201" { - t.Errorf("pbs coord = %+v, want felhom-pbs/root/9201", h.PBS) + if h.PBS.RepoID != "felhom-pbs" || h.PBS.Namespace != hub.PBSRootNamespace || h.PBS.LatestSnapshotID != "9201" { + t.Errorf("pbs coord = %+v, want felhom-pbs, the root namespace (\"\", R-124), 9201", h.PBS) } // COMPANION (pre-fix): the bare SnapshotStore (no live read) with an empty store omits pbs.