From f31a76f7880301d4befc57bc47d6536baedf95af Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Fri, 3 Jul 2026 15:48:24 +0200 Subject: [PATCH] =?UTF-8?q?v0.63.0:=20B3+B2=20fresh-install=20fixes=20?= =?UTF-8?q?=E2=80=94=20TokenStore=20reload-on-miss=20+=20guesthook=20snipp?= =?UTF-8?q?ets=20dir?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B3: Lookup re-reads the append-only store once on a miss (cross-process coherence with the one-shot provisioner; size short-circuit bounds the cost; behind the TokenAuthority seam). B2: fenced mkdir -p /var/lib/vz/snippets before the snippet install + the one narrow sudoers grant. Both red-proofed; drill findings DRILL-day0-cleanroom-2026-07-03 B3/B2. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6 --- CHANGELOG.md | 29 +++++++ REUSE.md | 4 +- cmd/felhom-agent/main.go | 2 +- configs/felhom-agent.sudoers | 5 +- internal/guesthook/install.go | 7 ++ internal/guesthook/install_test.go | 53 +++++++++++- internal/localapi/tokenstore.go | 39 +++++++++ internal/localapi/tokenstore_test.go | 123 +++++++++++++++++++++++++++ 8 files changed, 254 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb4cabc..592b9e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,32 @@ +## v0.63.0 — B3 + B2: fresh-install fixes — token reload-on-miss + guesthook snippets dir (2026-07-03) + +The two agent-side gaps the Day-0 clean-room drill surfaced +(`felhom.eu/documentation/audits/DRILL-day0-cleanroom-2026-07-03.md` findings B3/B2). Both are +fresh-install path fixes; no behavior change on a warm box. + +- **B3 — `TokenStore.Lookup` reload-on-miss** (localapi/tokenstore.go): provisioning is a SEPARATE + one-shot process (`--selftest=provision`) that Mints the new guest's token into the shared + append-only JSONL, while the long-lived daemon serves Lookup from an index built once at open — + so the daemon 401'd every token minted after it started (the drill's + `POST /controller/swap: HTTP 401`, until a manual `systemctl restart felhom-agent`). Lookup now + re-reads the file ONCE on a miss (`reloadLocked()`, factored from `load()`; full re-read is + idempotent under `apply`'s last-write-wins) and re-checks. An append-only size short-circuit + bounds the cost: an unknown token on an unchanged file is one `stat`, no re-read — never a reload + loop. Fix is entirely behind the `TokenAuthority` seam (no server change). Fail-closed on an + unreadable store; missing file loads as empty. Tests (tokenstore_test.go): cross-process-mint + coherence (red-proofed: pre-fix shape returns (0,false)), exactly-once reload bound + + size short-circuit, cross-process re-mint rotation coherence, deleted-file no-crash + (linux-only; windows can't unlink an open handle). +- **B2 — `guesthook.InstallSnippet` ensures the snippets dir** (guesthook/install.go): a fresh PVE + has no `/var/lib/vz/snippets` and `install` (without `-D`) won't create parents — the pre-start + self-heal hook silently failed to install on every freshly-bootstrapped box (warn-only in the + back-half). A fenced `mkdir -p /var/lib/vz/snippets` now precedes the install; **sudoers gains + exactly that one grant** (FELHOM_GUESTHOOK — configs/felhom-agent.sudoers must ship WITH this + binary, as always). Test: mkdir-precedes-install argv assertion (red-proofed: pre-fix has no + mkdir op). +- Guide follow-through (felhom.eu, separate commit): D.1b's "restart the agent first" step drops + once B3 is live-verified. + ## v0.62.0 — A1: pool-membership ownership check for the stale-lock reaper (2026-07-03) Implements audit finding A1 (`AUDIT-blast-radius-hostroot-localapi-2026-07-02` §A) per the spike diff --git a/REUSE.md b/REUSE.md index 5d6f302..7f4595e 100644 --- a/REUSE.md +++ b/REUSE.md @@ -15,7 +15,7 @@ | `SudoHostOps.run` | internal/storage/hostops.go | `run(ctx, name, args...) error` | allowlisted exec with stderr-wrapped error | Every arg pre-validated via validate.go before this is called | | `Prober.Probe` | internal/capability/probe.go | `Probe(ctx) []Status` | live sudo-policy capability check (`sudo -n -l --`) | Needs a DIRECT runner (never the sudo-prefixing one — double-sudo); never executes probed cmds | | `stageTemp` | internal/localapi/intermediary.go | `stageTemp(pattern, content) (path, err)` | random-named temp before a root `install` (audit B1) | Fixed /tmp names are a TOCTOU — sudoers globs expect `/tmp/felhom-*-*.ext` | -| `guesthook.InstallSnippet` / `Register` | internal/guesthook/install.go | `InstallSnippet(ctx, runner) error` | pre-start self-heal hook install (C1 net) | Same random-temp+install pattern; snippet delegates to the agent binary (no shell logic) | +| `guesthook.InstallSnippet` / `Register` | internal/guesthook/install.go | `InstallSnippet(ctx, runner) error` | pre-start self-heal hook install (C1 net) | Same random-temp+install pattern; snippet delegates to the agent binary (no shell logic). Issues `mkdir -p /var/lib/vz/snippets` FIRST (v0.63.0, B2 — fresh boxes lack the dir; sudoers grants exactly that argv) | ### Disk / format safety (role gates, durable IDs, format guards) @@ -56,7 +56,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 | +| `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 | | `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 | diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 54324e3..94e52b5 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -45,7 +45,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.62.0" +var version = "0.63.0" // runGuestHook is the PVE pre-start hook body (`felhom-agent guest-hook `). On the // pre-start phase it creates placeholder dirs for any absent bind-mount source so the guest always boots diff --git a/configs/felhom-agent.sudoers b/configs/felhom-agent.sudoers index 251a266..c87836e 100644 --- a/configs/felhom-agent.sudoers +++ b/configs/felhom-agent.sudoers @@ -70,8 +70,11 @@ Cmnd_Alias FELHOM_DNSMASQ = \ # guest at next boot (the B3 C1 fix). The agent fine-validates the vmid (numeric) + slot (mp[0-9]+) and # the snippet path is fixed — the wildcards are the coarse allowlist. The install SOURCE is a # random-named agent temp (os.CreateTemp, audit B1 — a fixed /tmp name was a local TOCTOU), hence the -# glob; the DESTINATION stays pinned. +# glob; the DESTINATION stays pinned. The `mkdir -p` creates the snippets dir on a FRESH box — +# `install` won't create parents, so without it the hook install failed silently on Day-0 boxes +# (B2, DRILL-day0-cleanroom-2026-07-03; fixed agent v0.63.0). Cmnd_Alias FELHOM_GUESTHOOK = \ + /usr/bin/mkdir -p /var/lib/vz/snippets, \ /usr/bin/install -m 0755 -- /tmp/felhom-guest-hook-*.sh /var/lib/vz/snippets/felhom-guest-hook.sh, \ /usr/sbin/pct set [0-9]* --hookscript local\:snippets/felhom-guest-hook.sh, \ /usr/sbin/pct set [0-9]* --delete mp[0-9]*, \ diff --git a/internal/guesthook/install.go b/internal/guesthook/install.go index 0fb819a..7ca2c61 100644 --- a/internal/guesthook/install.go +++ b/internal/guesthook/install.go @@ -53,6 +53,13 @@ func InstallSnippet(ctx context.Context, runner proxmox.Runner) error { if err := f.Close(); err != nil { return fmt.Errorf("guesthook: close temp snippet: %w", err) } + // Ensure the snippets dir exists FIRST (B2, DRILL-day0-cleanroom-2026-07-03): a fresh PVE has + // no /var/lib/vz/snippets, and `install` (without -D) won't create the parent — the whole + // hook install silently failed on a freshly-bootstrapped box. Fenced root op like the install + // itself; idempotent. + if _, stderr, err := runner.Run(ctx, "mkdir", "-p", SnippetDir); err != nil { + return fmt.Errorf("guesthook: ensure snippets dir %s: %w: %s", SnippetDir, err, string(stderr)) + } if _, stderr, err := runner.Run(ctx, "install", "-m", "0755", "--", tmp, SnippetPath); err != nil { return fmt.Errorf("guesthook: install snippet to %s: %w: %s", SnippetPath, err, string(stderr)) } diff --git a/internal/guesthook/install_test.go b/internal/guesthook/install_test.go index 590977e..d154132 100644 --- a/internal/guesthook/install_test.go +++ b/internal/guesthook/install_test.go @@ -41,16 +41,22 @@ func TestInstallSnippet_RandomTempName(t *testing.T) { if err := InstallSnippet(context.Background(), r); err != nil { t.Fatalf("InstallSnippet #2: %v", err) } - if len(r.calls) != 2 { - t.Fatalf("expected 2 install calls, got %d: %v", len(r.calls), r.calls) + var installs [][]string + for _, call := range r.calls { + if call[0] == "install" { + installs = append(installs, call) + } + } + if len(installs) != 2 { + t.Fatalf("expected 2 install calls, got %d: %v", len(installs), r.calls) } randomName := regexp.MustCompile(`felhom-guest-hook-[^/\\]+\.sh$`) fixedName := regexp.MustCompile(`felhom-guest-hook\.sh$`) var srcs []string - for i, call := range r.calls { + for i, call := range installs { // install -m 0755 -- - if call[0] != "install" || len(call) != 6 { + if len(call) != 6 { t.Fatalf("call %d: unexpected vector %v", i, call) } src, dest := call[4], call[5] @@ -81,3 +87,42 @@ func TestInstallSnippet_RandomTempName(t *testing.T) { } } } + +// B2 Scenario D (DRILL-day0-cleanroom-2026-07-03): on a fresh PVE, /var/lib/vz/snippets does not +// exist and `install` (no -D) cannot create it — the drill saw +// `install: cannot create regular file … No such file or directory` and the guest silently got no +// pre-start self-heal hook. InstallSnippet must therefore issue a `mkdir -p ` fenced op +// BEFORE the `install` op. Pre-fix wrong outcome: no mkdir call at all — only the doomed install. +func TestInstallSnippet_EnsuresSnippetsDirFirst(t *testing.T) { + r := &recordingRunner{} + if err := InstallSnippet(context.Background(), r); err != nil { + t.Fatalf("InstallSnippet: %v", err) + } + + mkdirIdx, installIdx := -1, -1 + for i, call := range r.calls { + switch call[0] { + case "mkdir": + if mkdirIdx == -1 { + mkdirIdx = i + want := []string{"mkdir", "-p", SnippetDir} + if len(call) != 3 || call[1] != want[1] || call[2] != want[2] { + t.Errorf("mkdir vector = %v, want %v (the sudoers fence matches exactly this argv)", call, want) + } + } + case "install": + if installIdx == -1 { + installIdx = i + } + } + } + if mkdirIdx == -1 { + t.Fatalf("no `mkdir -p %s` op issued — on a fresh box the snippet install fails ENOENT (B2); calls: %v", SnippetDir, r.calls) + } + if installIdx == -1 { + t.Fatalf("no install op issued; calls: %v", r.calls) + } + if mkdirIdx > installIdx { + t.Fatalf("mkdir (call %d) must PRECEDE install (call %d) — order: %v", mkdirIdx, installIdx, r.calls) + } +} diff --git a/internal/localapi/tokenstore.go b/internal/localapi/tokenstore.go index 3e2d87c..54199bc 100644 --- a/internal/localapi/tokenstore.go +++ b/internal/localapi/tokenstore.go @@ -36,6 +36,9 @@ type TokenStore struct { f *os.File byHash map[string]int // tokenHash(hex) -> vmid byVMID map[int]string // vmid -> current tokenHash(hex) + + loadedSize int64 // size of the file at the last (re)load — the append-only short-circuit + reloads int // count of reload-on-miss re-reads (test-visible bound, B3) } // tokenRecord is one durable line: VMID v gets token-hash h (last write per v wins). @@ -71,6 +74,19 @@ func OpenTokenStore(path string) (*TokenStore, error) { } func (s *TokenStore) load() error { + return s.reloadLocked() +} + +// reloadLocked rebuilds both indexes from the on-disk log. Caller holds the mutex (or is in +// single-threaded open). Because the file is append-only and apply is last-write-wins, a full +// re-read is idempotent — it can only converge the index to the file's current truth. A missing +// file loads as empty (the store file IS the authority). Used at open AND by Lookup's +// reload-on-miss (B3, DRILL-day0-cleanroom-2026-07-03): a token minted by ANOTHER process (the +// one-shot provisioner) after this daemon opened its store becomes visible without a restart. +func (s *TokenStore) reloadLocked() error { + clear(s.byHash) + clear(s.byVMID) + s.loadedSize = 0 b, err := os.ReadFile(s.path) if errors.Is(err, fs.ErrNotExist) { return nil @@ -78,6 +94,7 @@ func (s *TokenStore) load() error { if err != nil { return err } + s.loadedSize = int64(len(b)) for _, line := range bytes.Split(b, []byte("\n")) { line = bytes.TrimSpace(line) if len(line) == 0 { @@ -129,12 +146,20 @@ func (s *TokenStore) Mint(vmid int) (string, error) { return "", fmt.Errorf("localapi: token store sync: %w", err) } s.apply(tokenRecord{VMID: vmid, Hash: hash}) + s.loadedSize += int64(len(rec)) // keep the append-only size short-circuit accurate return token, nil } // Lookup resolves a presented bearer token to its guest VMID. It hashes the candidate and // 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. func (s *TokenStore) Lookup(token string) (int, bool) { if token == "" { return 0, false @@ -150,6 +175,20 @@ func (s *TokenStore) Lookup(token string) (int, bool) { return vmid, true } } + // 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 + } + s.reloads++ + if err := s.reloadLocked(); err != nil { + return 0, false // unreadable store: fail closed, never crash the auth path + } + if vmid, ok := s.byHash[want]; ok { + if subtle.ConstantTimeCompare([]byte(want), []byte(s.byVMID[vmid])) == 1 { + return vmid, true + } + } return 0, false } diff --git a/internal/localapi/tokenstore_test.go b/internal/localapi/tokenstore_test.go index e746427..8fd38a4 100644 --- a/internal/localapi/tokenstore_test.go +++ b/internal/localapi/tokenstore_test.go @@ -3,6 +3,7 @@ package localapi import ( "os" "path/filepath" + "runtime" "strings" "testing" ) @@ -110,6 +111,128 @@ func TestTokenStore_SurvivesReopen(t *testing.T) { } } +// B3 Scenario A (DRILL-day0-cleanroom-2026-07-03): the provisioner is a SEPARATE one-shot process +// that Mints into the SAME file the long-lived daemon serves Lookup from. A token minted after the +// daemon built its index must authorize WITHOUT a restart — the reload-on-miss re-reads the file. +// Pre-fix wrong outcome: (0,false) — the fresh-install 401 the drill hit on /controller/swap. +func TestTokenStore_ReloadOnMiss_CrossProcessMint(t *testing.T) { + path := filepath.Join(t.TempDir(), "tokens.log") + daemon, err := OpenTokenStore(path) // index built now (empty file) + if err != nil { + t.Fatalf("open daemon store: %v", err) + } + defer daemon.Close() + + minter, err := OpenTokenStore(path) // the --selftest=provision process + if err != nil { + t.Fatalf("open minter store: %v", err) + } + defer minter.Close() + tok, err := minter.Mint(120) + if err != nil { + t.Fatalf("cross-process mint: %v", err) + } + + vmid, ok := daemon.Lookup(tok) + if !ok || vmid != 120 { + t.Fatalf("daemon.Lookup(token minted after daemon start) = (%d,%v), want (120,true) — the B3 fresh-install 401", vmid, ok) + } +} + +// B3 Scenario B: a genuinely unknown token still returns (0,false), and the miss path re-reads the +// file AT MOST once per Lookup — and not at all when the append-only log has not grown (the size +// short-circuit). Guards against a reload loop / per-candidate re-read. +func TestTokenStore_ReloadOnMiss_BoundedReloads(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() + if _, err := minter.Mint(5); err != nil { // grow the file behind the daemon's back + t.Fatalf("mint: %v", err) + } + + base := daemon.reloads + if vmid, ok := daemon.Lookup("garbage-never-minted"); ok { + t.Fatalf("unknown token authorized: (%d,%v)", vmid, ok) + } + if got := daemon.reloads - base; got != 1 { + t.Fatalf("first miss after an external append: %d reloads, want exactly 1", got) + } + if vmid, ok := daemon.Lookup("garbage-never-minted"); ok { + t.Fatalf("unknown token authorized on retry: (%d,%v)", vmid, ok) + } + if got := daemon.reloads - base; got != 1 { + t.Fatalf("second miss on an UNCHANGED store re-read the file: %d reloads total, want still 1 (size short-circuit)", got) + } +} + +// B3 Scenario C: a cross-process re-mint (rotation) stays coherent through a reload — apply's +// last-write-wins holds after the full re-read: the new hash resolves, the rotated-out one 401s, +// and the indexes agree. +func TestTokenStore_ReloadOnMiss_RemintCoherence(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() + + tok1, _ := minter.Mint(120) + if vmid, ok := daemon.Lookup(tok1); !ok || vmid != 120 { // daemon absorbs tok1 via miss-reload + t.Fatalf("tok1 lookup: (%d,%v), want (120,true)", vmid, ok) + } + tok2, _ := minter.Mint(120) // rotation, appended externally + + if vmid, ok := daemon.Lookup(tok2); !ok || vmid != 120 { + t.Fatalf("rotated token lookup: (%d,%v), want (120,true)", vmid, ok) + } + if vmid, ok := daemon.Lookup(tok1); ok { + t.Fatalf("rotated-OUT token still authorizes vmid %d after reload — last-write-wins broken", vmid) + } + daemon.mu.Lock() + gotHash, gotVMID := daemon.byVMID[120], daemon.byHash[hashToken(tok2)] + oldGone := daemon.byHash[hashToken(tok1)] + daemon.mu.Unlock() + if gotHash != hashToken(tok2) || gotVMID != 120 || oldGone != 0 { + t.Fatalf("index incoherent after reload: byVMID[120]=%.8s byHash[tok2]=%d byHash[tok1]=%d", gotHash, gotVMID, oldGone) + } +} + +// §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) { + if runtime.GOOS == "windows" { + t.Skip("windows cannot unlink the store while its O_APPEND handle is open; production target is linux") + } + path := filepath.Join(t.TempDir(), "tokens.log") + s, err := OpenTokenStore(path) + if err != nil { + t.Fatalf("open: %v", err) + } + defer s.Close() + if _, err := s.Mint(1); err != nil { + t.Fatalf("mint: %v", err) + } + if err := os.Remove(path); err != nil { + t.Fatalf("remove store: %v", err) + } + if vmid, ok := s.Lookup("garbage-never-minted"); ok { + t.Fatalf("lookup on a deleted store authorized (%d,%v)", vmid, ok) + } +} + func TestTokenStore_Uniqueness(t *testing.T) { path := filepath.Join(t.TempDir(), "tokens.log") s, err := OpenTokenStore(path)