From db01ae0f2a577592d80f153804d4c85f5b326198 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sun, 15 Nov 2026 08:20:49 +0100 Subject: [PATCH] =?UTF-8?q?agent=20(unreleased):=20a=20cut=20write=20of=20?= =?UTF-8?q?agent.json=20cannot=20leave=20a=20blind=20box=20=E2=80=94=20a?= =?UTF-8?q?=20journal=20of=20the=20old=20bytes,=20a=20repair=20at=20start?= =?UTF-8?q?=20(R-1380)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- CHANGELOG.md | 11 ++ REUSE.md | 1 + cmd/felhom-agent/config_cutwrite_test.go | 141 +++++++++++++++++ cmd/felhom-agent/main.go | 24 +++ internal/config/cutwrite.go | 188 +++++++++++++++++++++++ internal/config/cutwrite_test.go | 140 +++++++++++++++++ internal/localapi/backup_target.go | 20 +-- internal/localapi/cutwrite_main_test.go | 21 +++ internal/pbsdr/cutwrite_main_test.go | 21 +++ internal/pbsdr/manager.go | 22 +-- internal/pvetrust/cutwrite_main_test.go | 21 +++ internal/pvetrust/pvetrust.go | 38 +---- 12 files changed, 582 insertions(+), 66 deletions(-) create mode 100644 cmd/felhom-agent/config_cutwrite_test.go create mode 100644 internal/config/cutwrite.go create mode 100644 internal/config/cutwrite_test.go create mode 100644 internal/localapi/cutwrite_main_test.go create mode 100644 internal/pbsdr/cutwrite_main_test.go create mode 100644 internal/pvetrust/cutwrite_main_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ef1de0a..0309ecc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ +## Unreleased (2026-11-15) — a cut write of `agent.json` cannot leave a blind box (R-1380; found before the release) + +- **`config.ReplaceInPlace` + `config.RecoverCutWrite`** (`internal/config/cutwrite.go`). `agent.json` is the agent's + file in a root-owned folder, so the trust move, the PBS seed and the backup-target move write it in place + (truncate, write). A power cut in between left an empty or half file and an agent that does not start. Now the old + bytes go into a journal in the state folder first (`agent.json.write-in-flight`, durable), the config is written, + synced and read back, and the journal is removed; the daemon's start repairs a config that does not parse from the + journal BEFORE it reads it. A selftest never repairs. No journal folder → no write. Tests: a cut staged at each of + four steps, the wiring in `main`, and a source pin that the three writers use the helper and open no file by hand. + Red-proofs: felhom.eu `documentation/audits/release-2b-3-2026-11-15/part0/`. Not proven: a real power cut. + ## Unreleased (2026-11-14 night) — release 3 („the hub tells you”) and the clock-step fix, the agent's parts, built and NOT released Commit only. Each part can be released alone; none needs release 2b. No `configs/` change in this section. diff --git a/REUSE.md b/REUSE.md index eaf7a99..4ea0526 100644 --- a/REUSE.md +++ b/REUSE.md @@ -102,6 +102,7 @@ | `Leg.runKernelCleanup` / `kernelCleanupSkip` / `Leg.KernelCleanupDryRun` + the wrapper's `Kernel.cleanup` | internal/osupdate/kernel.go; configs/felhom-os-apply (mode `kernel-cleanup`) | `kernelCleanupSkip(appliance, trigger, host, pve, kernel, blk, okStep) string` ("" = call it) | R-1368 (`11` §5.11.1): old kernel images are removed by the WRAPPER's rule; the agent decides only WHEN (the night leg, after a healthy host step, never in a night with a kernel step) | The plan names NO package (C1). Never `apt-get autoremove`. Whether a box may do it is the root-owned ring-0 mark, never the hub's ring. The hub report carries NO kernel view and no kernel version (the hub would read it as a kernel step). `--selftest=kernel-cleanup-dry-run` changes nothing. | | `Leg.ProfileRefresh` + the wrapper's `Apply.profile_refresh` | internal/osupdate/profilerefresh.go; configs/felhom-os-apply (mode `profile-refresh`) | `ProfileRefresh(ctx, why, gate, tries, wait) string` (the outcome) | R-1388 (`11` §5.12): load every AppArmor profile from a fresh compile once per kernel the box starts on; restart only a FAILED service whose own program's cached profile was stale | The plan names nothing (F1). A stale copy is told by the CACHED FILE changing at the rewrite — never by the kernel's loaded hash (it differs between a cache load and a fresh load for multi-profile files; `lxc-containers` never compiles the same twice). Called at agent start and after a bundle install, under the heavy-op gate. | | `pvetrust.Manager` (`Ensure`, `OnCallError`, `Status`) + `proxmox.Client.SetTLS` / `proxmox.TrustProblem` + `Leg.PVECACopy` + the wrapper's `pve_ca_copy` | internal/pvetrust/pvetrust.go; internal/proxmox/{client,trust}.go; internal/osupdate/pvecacopy.go; configs/felhom-os-apply (mode `pve-ca-copy`) | `Ensure(ctx, why) (changed bool)`; `TrustProblem(err) string` | R-1380 (felhom.eu `03` §12a): trust in Proxmox by a copy of the box's own root CA; the move from a pinned certificate; the cause of a trust failure | THE ORDER: the copy, a real call with the copy ALONE, then ONE config write that clears the fingerprint LAST (a set fingerprint wins over `ca_file` in `TLSConfig.build`). Never clear the fingerprint anywhere else. The wrapper's plan names nothing (T1). `agent.json` is written by the agent in place, with a parked recovery copy — the same way as the backup-target move. | +| `config.ReplaceInPlace` / `config.RecoverCutWrite` (+ `config.FileMu`) | internal/config/cutwrite.go; internal/config/filelock.go | `ReplaceInPlace(path, prev, next) error`; `RecoverCutWrite(path) (note string, err error)` | R-1380 (felhom.eu `03` §12a item 7): the only way to write `agent.json` — the folder is root-owned, so no rename; the old bytes go to a journal in the state folder first, and the daemon repairs a cut write at start, before it reads the config | A new writer takes `FileMu`, calls `ReplaceInPlace`, opens no file by hand, and is added to the two source pins in `cmd/felhom-agent` (`…TakeTheSharedLock`, `…UseTheJournalledWrite`). A package whose tests write the config sets `config.CutWriteDir` in `TestMain` | | `Client.WaitTask` | internal/proxmox/task.go | `WaitTask(ctx, upid, opts) (TaskStatus, error)` | asserting EVERY mutating op | POST 200 ≠ success; authz can fail at task exec; `AllowWarnings` opt-in | | `Client.Pool` | internal/proxmox/query.go | `Pool(ctx, name) (PoolInfo, error)` | felhom-pool membership (the ownership registry, A1) | Needs `Pool.Audit` at `/pool/` (host-install v1.9.0+); `Pool.Allocate` does NOT satisfy the read; members can be storages (type `storage`, vmid 0) — filter them | | `Client` mutate wrappers (`RestoreLXC/Vzdump/DestroyLXC/Snapshot/Rollback/SetConfig/ResizeLXC/Start/Stop`) | internal/proxmox/mutate.go | return `(upid, error)` | all API mutations | Async → always pair with WaitTask; route via gate/queue, not ad-hoc | diff --git a/cmd/felhom-agent/config_cutwrite_test.go b/cmd/felhom-agent/config_cutwrite_test.go new file mode 100644 index 0000000..b49c7eb --- /dev/null +++ b/cmd/felhom-agent/config_cutwrite_test.go @@ -0,0 +1,141 @@ +package main + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/config" +) + +// A box whose agent.json was cut in the middle of a write must start (R-1380): main repairs the file from the +// journal BEFORE it reads it. The staging is the state a power cut leaves: a durable journal of the old bytes and a +// half-written config. +func TestMainRecoversACutConfigWrite(t *testing.T) { + dir := t.TempDir() + prev := config.CutWriteDir + config.CutWriteDir = filepath.Join(dir, "state") + t.Cleanup(func() { config.CutWriteDir = prev }) + if err := os.MkdirAll(config.CutWriteDir, 0o700); err != nil { + t.Fatal(err) + } + cfgPath := filepath.Join(dir, "agent.json") + old := `{"proxmox":{"node":"pve","tls":{"fingerprint":"AA:BB"}}}` + if err := os.WriteFile(filepath.Join(config.CutWriteDir, "agent.json.write-in-flight"), []byte(old), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(cfgPath, []byte(`{"proxmox":{"node":"pv`), 0o600); err != nil { + t.Fatal(err) + } + if _, err := config.Load(cfgPath); err == nil { + t.Fatal("the staging is wrong: the cut config loads") + } + note, err := recoverCutConfigWrite(cfgPath) + if err != nil || note == "" { + t.Fatalf("note=%q err=%v", note, err) + } + cfg, err := config.Load(cfgPath) + if err != nil { + t.Fatalf("the agent does not start after the repair: %v", err) + } + if cfg.Proxmox.TLS.Fingerprint != "AA:BB" { + t.Fatalf("the repaired config lost the fingerprint: %+v", cfg.Proxmox.TLS) + } +} + +// main must call the repair before config.Load, or the repair never runs on the box that needs it (a seam that is +// built and not wired). Read from the source: the call's position in main's body. +func TestMainCallsTheConfigRepairBeforeItLoadsTheConfig(t *testing.T) { + src, err := os.ReadFile("main.go") + if err != nil { + t.Fatal(err) + } + f, err := parser.ParseFile(token.NewFileSet(), "main.go", src, 0) + if err != nil { + t.Fatal(err) + } + var repair, load token.Pos + for _, d := range f.Decls { + fn, ok := d.(*ast.FuncDecl) + if !ok || fn.Name.Name != "main" || fn.Body == nil { + continue + } + ast.Inspect(fn.Body, func(n ast.Node) bool { + c, ok := n.(*ast.CallExpr) + if !ok { + return true + } + if id, ok := c.Fun.(*ast.Ident); ok && id.Name == "recoverCutConfigWrite" && repair == 0 { + repair = c.Pos() + } + if sel, ok := c.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "Load" { + if pkg, ok := sel.X.(*ast.Ident); ok && pkg.Name == "config" && load == 0 { + load = c.Pos() + } + } + return true + }) + } + if repair == 0 || load == 0 { + t.Fatalf("main: repair call found=%v, config.Load found=%v", repair != 0, load != 0) + } + if repair > load { + t.Fatal("main reads the config BEFORE it repairs a cut write — a cut config stops the agent at start") + } +} + +// Every writer of agent.json goes through config.ReplaceInPlace and none truncates the file itself: a writer that +// opens the config with O_TRUNC by hand is the unprotected write again. +func TestAgentJSONWritersUseTheJournalledWrite(t *testing.T) { + for _, w := range []struct{ file, fn string }{ + {"../../internal/pvetrust/pvetrust.go", "writeConfig"}, + {"../../internal/pbsdr/manager.go", "seedEscrowStorageID"}, + {"../../internal/localapi/backup_target.go", "setConfiguredBackupTarget"}, + } { + src, err := os.ReadFile(w.file) + if err != nil { + t.Fatal(err) + } + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, w.file, src, 0) + if err != nil { + t.Fatal(err) + } + found := false + for _, d := range f.Decls { + fn, ok := d.(*ast.FuncDecl) + if !ok || fn.Name.Name != w.fn || fn.Body == nil { + continue + } + found = true + body := string(src[fset.Position(fn.Body.Pos()).Offset:fset.Position(fn.Body.End()).Offset]) + calls := 0 + ast.Inspect(fn.Body, func(n ast.Node) bool { + if c, ok := n.(*ast.CallExpr); ok { + if sel, ok := c.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "ReplaceInPlace" { + if pkg, ok := sel.X.(*ast.Ident); ok && pkg.Name == "config" { + calls++ + } + } + if sel, ok := c.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "OpenFile" { + t.Errorf("%s %s opens a file by hand — the config is written only by config.ReplaceInPlace", w.file, w.fn) + } + } + return true + }) + if calls != 1 { + t.Errorf("%s %s: %d call(s) to config.ReplaceInPlace, want 1", w.file, w.fn, calls) + } + if strings.Contains(body, "O_TRUNC") { + t.Errorf("%s %s truncates a file itself", w.file, w.fn) + } + } + if !found { + t.Errorf("%s: function %s not found — the pin reads nothing", w.file, w.fn) + } + } +} diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 18dea85..95de14f 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -217,6 +217,15 @@ func main() { return } + // Only the daemon repairs: a selftest can run beside a live agent that is in the middle of a write. + var cutWriteNote string + var cutWriteErr error + if selftest.mode == "" { + cutWriteNote, cutWriteErr = recoverCutConfigWrite(cfgPath) + if cutWriteErr != nil { + fmt.Fprintln(os.Stderr, "config repair:", cutWriteErr) + } + } cfg, err := config.Load(cfgPath) if err != nil { // A missing default config file is fine if env provides the values; only a @@ -230,6 +239,11 @@ func main() { // logRing is the always-DEBUG capture ring (v0.83.0 observability): served by the // local API's GET /debug/logs and the heartbeat log-pull. Selftests ignore it. logger, logRing := applog.New(cfg.LogLevel) + if cutWriteErr != nil { + logger.Error("config: the last write of the config was cut and could not be repaired", "err", cutWriteErr) + } else if cutWriteNote != "" { + logger.Warn("config: a write of the config was cut by a stop — repaired at start", "result", cutWriteNote) + } switch selftest.mode { case "": @@ -4246,3 +4260,13 @@ func agentStateDir(cfg config.Config) string { func wireBundlePolicyAfterCommit(m *selfupdate.Manager, e osupdate.ConfigUpdateExecutor, lg *slog.Logger, ver string) { m.SetOnCommitted(func(ctx context.Context) { e.PolicyPassAfterUpdate(ctx, lg, ver) }) } + +// recoverCutConfigWrite repairs agent.json BEFORE it is read, when the last write of it was cut by a power loss +// (config.RecoverCutWrite; R-1380). It runs before the logger exists, so main logs what it returns. +// Pinned by TestMainRecoversACutConfigWrite. +func recoverCutConfigWrite(cfgPath string) (string, error) { + if cfgPath == "" { + return "", nil + } + return config.RecoverCutWrite(cfgPath) +} diff --git a/internal/config/cutwrite.go b/internal/config/cutwrite.go new file mode 100644 index 0000000..c7ccb9a --- /dev/null +++ b/internal/config/cutwrite.go @@ -0,0 +1,188 @@ +package config + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "path/filepath" +) + +// A cut write of agent.json must never leave a blind box (R-1380, 2026-11-15). +// +// agent.json is the agent's own file inside a ROOT-owned folder, so the agent cannot write a new file and rename it +// over the old one: every writer truncates the file and writes it again. A power cut between the truncate and the +// sync leaves an empty or half file, the agent does not start, and nothing can repair a box whose agent does not +// start from the hub. So the three writers (the trust move, the PBS seed, the backup-target move) go through +// ReplaceInPlace: the OLD bytes are first made durable in a journal file in the agent's own state folder (a new file +// and a rename, which the agent CAN do there), then the config is written, synced and read back, then the journal is +// removed. RecoverCutWrite runs at every start, before the config is read: a journal beside a config that does not +// parse means the write was cut, and the old bytes go back. A journal beside a config that parses means the cut fell +// before the truncate or after the sync — the file is whole, old or new, and only the journal is removed. +// +// Pinned by TestReplaceInPlace_* and TestRecoverCutWrite_* (this package) and by TestMainRecoversACutConfigWrite +// (cmd/felhom-agent). + +// CutWriteDir is where the journal lives. It is NOT read from the config: the recovery runs before the config can be +// read. /var/lib/felhom-agent is the unit's StateDirectory. A test points it at a temp folder. +var CutWriteDir = "/var/lib/felhom-agent" + +// cutWriteJournal is the journal's file name: the bytes agent.json held before the write that is in flight. +const cutWriteJournal = "agent.json.write-in-flight" + +// cutPoint lets a test stop a write where a power cut would (the names are the steps of ReplaceInPlace). +var cutPoint = func(step string) error { return nil } + +// ReplaceInPlace writes next over the config at path, whose present content is prev. The caller holds FileMu. +// With no journal folder there is no write: an unprotected write is the defect this exists to remove. +func ReplaceInPlace(path string, prev, next []byte) error { + if !json.Valid(prev) || !json.Valid(next) { + return fmt.Errorf("config: refusing to write %s: the old or the new content is not JSON", path) + } + st, err := os.Stat(path) + if err != nil { + return err + } + if err := os.MkdirAll(CutWriteDir, 0o700); err != nil { + return fmt.Errorf("config: the journal folder: %w", err) + } + journal := filepath.Join(CutWriteDir, cutWriteJournal) + if err := writeDurable(journal, prev); err != nil { + return fmt.Errorf("config: the journal of the old content: %w", err) + } + if err := cutPoint("journal-written"); err != nil { + return err + } + f, err := os.OpenFile(path, os.O_WRONLY|os.O_TRUNC, st.Mode().Perm()) + if err != nil { + _ = removeDurable(journal) + return err + } + if err := cutPoint("truncated"); err != nil { + f.Close() + return err + } + half := len(next) / 2 + _, werr := f.Write(next[:half]) + if werr == nil { + if err := cutPoint("half-written"); err != nil { + f.Close() + return err + } + _, werr = f.Write(next[half:]) + } + if werr == nil { + werr = f.Sync() + } + if cerr := f.Close(); werr == nil { + werr = cerr + } + if werr == nil { + var back []byte + if back, werr = os.ReadFile(path); werr == nil && !bytes.Equal(back, next) { + werr = fmt.Errorf("the file did not read back as written") + } + } + if werr != nil { + // The write failed with the process alive: put the old bytes back now. If that fails too, the journal stays + // and the next start repairs the file. + if rerr := restoreInPlace(path, prev); rerr != nil { + return fmt.Errorf("config: writing %s failed (%v) and the old content could not be put back (%v) — the next start repairs it from the journal", path, werr, rerr) + } + _ = removeDurable(journal) + return fmt.Errorf("config: writing %s failed, the old content was put back: %w", path, werr) + } + if err := cutPoint("synced"); err != nil { + return err + } + return removeDurable(journal) +} + +// RecoverCutWrite repairs a config whose last write was cut. It returns what it did, for the start's log line: +// "" (no journal), "journal removed — the config is whole", or "the old config was put back". +func RecoverCutWrite(path string) (string, error) { + journal := filepath.Join(CutWriteDir, cutWriteJournal) + prev, err := os.ReadFile(journal) + if os.IsNotExist(err) { + return "", nil + } + if err != nil { + return "", fmt.Errorf("config: reading the write journal: %w", err) + } + cur, cerr := os.ReadFile(path) + if cerr == nil && isJSONObject(cur) { + return "journal removed — the config is whole", removeDurable(journal) + } + if !isJSONObject(prev) { + // A journal that is itself broken cannot have been followed by a truncate (it is made durable first), so + // this is not a cut write. Leave both for a person. + return "", fmt.Errorf("config: %s does not parse and the write journal does not parse either", path) + } + if err := restoreInPlace(path, prev); err != nil { + return "", fmt.Errorf("config: putting the old content back into %s: %w", path, err) + } + return "the old config was put back", removeDurable(journal) +} + +func isJSONObject(b []byte) bool { + var doc map[string]json.RawMessage + return json.Unmarshal(b, &doc) == nil && doc != nil +} + +// restoreInPlace writes content into the EXISTING file (no create: the agent cannot make a file in the config's +// folder), so the file keeps its owner and mode. +func restoreInPlace(path string, content []byte) error { + f, err := os.OpenFile(path, os.O_WRONLY|os.O_TRUNC, 0) + if err != nil { + return err + } + if _, err := f.Write(content); err != nil { + f.Close() + return err + } + if err := f.Sync(); err != nil { + f.Close() + return err + } + return f.Close() +} + +// writeDurable makes path hold content, whole, across a power cut: a new file, synced, renamed, the folder synced. +func writeDurable(path string, content []byte) error { + tmp := path + ".tmp" + f, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) + if err != nil { + return err + } + if _, err := f.Write(content); err != nil { + f.Close() + return err + } + if err := f.Sync(); err != nil { + f.Close() + return err + } + if err := f.Close(); err != nil { + return err + } + if err := os.Rename(tmp, path); err != nil { + return err + } + return syncDir(filepath.Dir(path)) +} + +func removeDurable(path string) error { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return err + } + return syncDir(filepath.Dir(path)) +} + +func syncDir(dir string) error { + d, err := os.Open(dir) + if err != nil { + return err + } + defer d.Close() + return d.Sync() +} diff --git a/internal/config/cutwrite_test.go b/internal/config/cutwrite_test.go new file mode 100644 index 0000000..55ea980 --- /dev/null +++ b/internal/config/cutwrite_test.go @@ -0,0 +1,140 @@ +package config + +import ( + "errors" + "os" + "path/filepath" + "testing" +) + +const ( + oldCfg = `{"proxmox":{"tls":{"fingerprint":"AA:BB"}},"keep":"me"}` + newCfg = `{"proxmox":{"tls":{"ca_file":"/etc/felhom-agent/pve-root-ca.pem","fingerprint":""}},"keep":"me"}` +) + +// cutBox is a config file and a journal folder in a temp dir, with the write cut at one named step. +func cutBox(t *testing.T) (cfg string) { + t.Helper() + dir := t.TempDir() + cfg = filepath.Join(dir, "etc", "agent.json") + if err := os.MkdirAll(filepath.Dir(cfg), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(cfg, []byte(oldCfg), 0o600); err != nil { + t.Fatal(err) + } + prevDir, prevCut := CutWriteDir, cutPoint + CutWriteDir = filepath.Join(dir, "state") + t.Cleanup(func() { CutWriteDir, cutPoint = prevDir, prevCut }) + return cfg +} + +func cutAt(step string) { + cutPoint = func(s string) error { + if s == step { + return errors.New("power cut at " + s) + } + return nil + } +} + +func read(t *testing.T, p string) string { + t.Helper() + b, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +// The consequence that matters: after a power cut at ANY step of the write and the repair at the next start, the +// config is the whole old file or the whole new file — never a broken one — and Load reads it. +func TestRecoverCutWrite_AfterACutAtEveryStepTheConfigIsWholeAndLoads(t *testing.T) { + want := map[string]string{ + "journal-written": oldCfg, // nothing was touched yet + "truncated": oldCfg, // the file is EMPTY at the cut + "half-written": oldCfg, // the file is HALF at the cut + "synced": newCfg, // the new file is whole; only the journal is left + } + for step, wantAfter := range want { + t.Run(step, func(t *testing.T) { + cfg := cutBox(t) + cutAt(step) + if err := ReplaceInPlace(cfg, []byte(oldCfg), []byte(newCfg)); err == nil { + t.Fatal("the staged cut did not stop the write") + } + atCut := read(t, cfg) + if (step == "truncated" || step == "half-written") && isJSONObject([]byte(atCut)) { + t.Fatalf("the staging is wrong: at %s the file must be broken, it is %q", step, atCut) + } + cutPoint = func(string) error { return nil } // the next start: no cut + note, err := RecoverCutWrite(cfg) + if err != nil { + t.Fatalf("repair: %v", err) + } + if note == "" { + t.Fatal("the repair saw no journal — a cut write would stay broken") + } + if got := read(t, cfg); got != wantAfter { + t.Fatalf("after a cut at %s and the repair the config is %q, want %q", step, got, wantAfter) + } + if _, err := Load(cfg); err != nil { + t.Fatalf("the agent does not start with the repaired config: %v", err) + } + if _, err := os.Stat(filepath.Join(CutWriteDir, cutWriteJournal)); !os.IsNotExist(err) { + t.Fatalf("the journal stayed after the repair (%v) — an old journal would undo a later change", err) + } + }) + } +} + +// A write that is not cut leaves the new file and NO journal: a journal that stayed would put an old config back +// at some later start. +func TestReplaceInPlace_AWholeWriteLeavesTheNewFileAndNoJournal(t *testing.T) { + cfg := cutBox(t) + if err := ReplaceInPlace(cfg, []byte(oldCfg), []byte(newCfg)); err != nil { + t.Fatal(err) + } + if got := read(t, cfg); got != newCfg { + t.Fatalf("config = %q", got) + } + if _, err := os.Stat(filepath.Join(CutWriteDir, cutWriteJournal)); !os.IsNotExist(err) { + t.Fatalf("the journal stayed after a whole write: %v", err) + } + if note, err := RecoverCutWrite(cfg); err != nil || note != "" { + t.Fatalf("a start after a whole write must do nothing: note=%q err=%v", note, err) + } + if got := read(t, cfg); got != newCfg { + t.Fatalf("the start changed a whole config to %q", got) + } +} + +// No journal folder, no write: the old file stays as it was. +func TestReplaceInPlace_WithoutAJournalNothingIsWritten(t *testing.T) { + cfg := cutBox(t) + // the journal cannot be made: a folder sits where its new file must be written + if err := os.MkdirAll(filepath.Join(CutWriteDir, cutWriteJournal+".tmp"), 0o700); err != nil { + t.Fatal(err) + } + if err := ReplaceInPlace(cfg, []byte(oldCfg), []byte(newCfg)); err == nil { + t.Fatal("the write went on with no journal") + } + if got := read(t, cfg); got != oldCfg { + t.Fatalf("the config changed with no journal: %q", got) + } +} + +// A config that is broken with NO journal is not a cut write: the repair does not touch it, and says nothing. +func TestRecoverCutWrite_ABrokenConfigWithNoJournalIsLeftAlone(t *testing.T) { + cfg := cutBox(t) + if err := os.WriteFile(cfg, []byte(`{"half":`), 0o600); err != nil { + t.Fatal(err) + } + note, err := RecoverCutWrite(cfg) + if err != nil || note != "" { + t.Fatalf("note=%q err=%v", note, err) + } + if got := read(t, cfg); got != `{"half":` { + t.Fatalf("the repair wrote a config it had no journal for: %q", got) + } +} diff --git a/internal/localapi/backup_target.go b/internal/localapi/backup_target.go index edc4d48..5dc2c4b 100644 --- a/internal/localapi/backup_target.go +++ b/internal/localapi/backup_target.go @@ -146,8 +146,8 @@ func (s *Server) validateBackupTargetMount(ctx context.Context, where string) er // begin with. A typed round-trip would silently drop any key this build does not know about. // // Written IN PLACE (O_TRUNC), not tmp+rename: /etc/felhom-agent is root-owned while agent.json is -// agent-owned 0600, so the non-root agent cannot rename into that directory. A recovery copy is -// parked first, so a torn write is recoverable. +// agent-owned 0600, so the non-root agent cannot rename into that directory. config.ReplaceInPlace +// journals the old bytes first; the next start repairs a cut write (TestRecoverCutWrite_*). func (s *Server) setConfiguredBackupTarget(id string) error { config.FileMu.Lock() // one writer of agent.json at a time in this process (config.FileMu) defer config.FileMu.Unlock() @@ -182,24 +182,14 @@ func (s *Server) setConfiguredBackupTarget(id string) error { if err != nil { return err } - st, err := os.Stat(path) - if err != nil { - return err - } if s.stateDir != "" { if err := os.MkdirAll(s.stateDir, 0o700); err == nil { _ = os.WriteFile(filepath.Join(s.stateDir, "agent.json.pre-backup-target"), raw, 0o600) } } - f, err := os.OpenFile(path, os.O_WRONLY|os.O_TRUNC, st.Mode().Perm()) - if err != nil { - return err - } - if _, err := f.Write(out); err != nil { - f.Close() - return err - } - return f.Close() + // config.ReplaceInPlace: the old bytes go into a journal first, so a power cut in the middle of the write is + // repaired at the next start (R-1380). + return config.ReplaceInPlace(path, raw, out) } // wrapperReason surfaces the wrapper's own REFUSED line when it produced one — it explains WHY in diff --git a/internal/localapi/cutwrite_main_test.go b/internal/localapi/cutwrite_main_test.go new file mode 100644 index 0000000..0cd6ca2 --- /dev/null +++ b/internal/localapi/cutwrite_main_test.go @@ -0,0 +1,21 @@ +package localapi + +import ( + "os" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/config" +) + +// TestMain points the config write journal (config.CutWriteDir, R-1380) at a temp folder: the real one is the +// agent's state folder on a box, and a test must not need it. +func TestMain(m *testing.M) { + dir, err := os.MkdirTemp("", "felhom-cutwrite-") + if err != nil { + panic(err) + } + config.CutWriteDir = dir + code := m.Run() + _ = os.RemoveAll(dir) + os.Exit(code) +} diff --git a/internal/pbsdr/cutwrite_main_test.go b/internal/pbsdr/cutwrite_main_test.go new file mode 100644 index 0000000..35af614 --- /dev/null +++ b/internal/pbsdr/cutwrite_main_test.go @@ -0,0 +1,21 @@ +package pbsdr + +import ( + "os" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/config" +) + +// TestMain points the config write journal (config.CutWriteDir, R-1380) at a temp folder: the real one is the +// agent's state folder on a box, and a test must not need it. +func TestMain(m *testing.M) { + dir, err := os.MkdirTemp("", "felhom-cutwrite-") + if err != nil { + panic(err) + } + config.CutWriteDir = dir + code := m.Run() + _ = os.RemoveAll(dir) + os.Exit(code) +} diff --git a/internal/pbsdr/manager.go b/internal/pbsdr/manager.go index 8b91004..f3ddb79 100644 --- a/internal/pbsdr/manager.go +++ b/internal/pbsdr/manager.go @@ -484,7 +484,7 @@ func (m *Manager) consumedFail(h, msg string, block *hub.WirePBSDR) { // seedEscrowStorageID sets escrow.pbs_storage_id in agent.json when empty/absent — the bare // `--selftest=escrow-create` one-liner must find its storage with no flags. A different existing // value is NEVER clobbered (warn-and-keep). Unknown config keys are preserved verbatim -// (map[string]RawMessage read-modify-write, atomic rename). +// (map[string]RawMessage read-modify-write, in place behind a journal — config.ReplaceInPlace). func (m *Manager) seedEscrowStorageID(storageID string) error { config.FileMu.Lock() // one writer of agent.json at a time in this process (config.FileMu) defer config.FileMu.Unlock() @@ -530,30 +530,18 @@ func (m *Manager) seedEscrowStorageID(storageID string) error { if err != nil { return err } - st, err := os.Stat(m.configPath) - if err != nil { - return err - } // The config DIRECTORY (/etc/felhom-agent) is root-owned while the FILE is agent-owned — // tmp+rename is impossible for the non-root agent (rename mutates the directory; live - // finding on the demo). So: park a recovery copy in the agent state dir, then rewrite the - // file IN PLACE (O_TRUNC). The agent is the file's only writer and the content is small; - // a torn write is recoverable from the parked copy. + // finding on the demo). So: park a copy for a person in the agent state dir, then rewrite the + // file IN PLACE through config.ReplaceInPlace, which journals the old bytes first — the next + // start repairs a cut write (R-1380, TestRecoverCutWrite_*). if err := os.MkdirAll(m.stateDir, 0o700); err != nil { return err } if err := os.WriteFile(filepath.Join(m.stateDir, "agent.json.pre-seed"), raw, 0o600); err != nil { return fmt.Errorf("parking the pre-seed copy: %w", err) } - f, err := os.OpenFile(m.configPath, os.O_WRONLY|os.O_TRUNC, st.Mode().Perm()) - if err != nil { - return err - } - if _, err := f.Write(out); err != nil { - f.Close() - return err - } - if err := f.Close(); err != nil { + if err := config.ReplaceInPlace(m.configPath, raw, out); err != nil { return err } m.logger.Info("pbsdr: seeded escrow.pbs_storage_id (the ceremony one-liner needs no flags)", "storage_id", storageID) diff --git a/internal/pvetrust/cutwrite_main_test.go b/internal/pvetrust/cutwrite_main_test.go new file mode 100644 index 0000000..de493c1 --- /dev/null +++ b/internal/pvetrust/cutwrite_main_test.go @@ -0,0 +1,21 @@ +package pvetrust + +import ( + "os" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/config" +) + +// TestMain points the config write journal (config.CutWriteDir, R-1380) at a temp folder: the real one is the +// agent's state folder on a box, and a test must not need it. +func TestMain(m *testing.M) { + dir, err := os.MkdirTemp("", "felhom-cutwrite-") + if err != nil { + panic(err) + } + config.CutWriteDir = dir + code := m.Run() + _ = os.RemoveAll(dir) + os.Exit(code) +} diff --git a/internal/pvetrust/pvetrust.go b/internal/pvetrust/pvetrust.go index 382628f..d59a391 100644 --- a/internal/pvetrust/pvetrust.go +++ b/internal/pvetrust/pvetrust.go @@ -388,10 +388,6 @@ func (m *Manager) writeConfig(caPath string) error { return err } out = append(out, '\n') - st, err := os.Stat(m.ConfigPath) - if err != nil { - return err - } if m.StateDir != "" { if err := os.MkdirAll(m.StateDir, 0o700); err != nil { return err @@ -400,37 +396,11 @@ func (m *Manager) writeConfig(caPath string) error { return fmt.Errorf("parking the recovery copy: %w", err) } } - // In place (O_TRUNC), not tmp+rename: the folder is root-owned and the file is the agent's (the same reason as the - // backup-target move and the PBS seed). One small write, then fsync. - f, err := os.OpenFile(m.ConfigPath, os.O_WRONLY|os.O_TRUNC, st.Mode().Perm()) - if err != nil { + // In place, not tmp+rename: the folder is root-owned and the file is the agent's. config.ReplaceInPlace keeps the + // old bytes in a journal first, so a power cut in the middle is repaired at the next start (R-1380); it also reads + // the file back and puts the old bytes back when the write fails with the agent alive. + if err := config.ReplaceInPlace(m.ConfigPath, raw, out); err != nil { return err } - if _, err := f.Write(out); err != nil { - f.Close() - _ = os.WriteFile(m.ConfigPath, raw, st.Mode().Perm()) // put the old bytes back: the fingerprint must not be lost - return err - } - if err := f.Sync(); err != nil { - f.Close() - return err - } - if err := f.Close(); err != nil { - return err - } - // read back: the file must say exactly what was meant - var check struct { - Proxmox struct { - TLS struct { - CAFile string `json:"ca_file"` - Fingerprint string `json:"fingerprint"` - } `json:"tls"` - } `json:"proxmox"` - } - back, err := os.ReadFile(m.ConfigPath) - if err != nil || json.Unmarshal(back, &check) != nil || check.Proxmox.TLS.CAFile != caPath || check.Proxmox.TLS.Fingerprint != "" { - _ = os.WriteFile(m.ConfigPath, raw, st.Mode().Perm()) - return fmt.Errorf("the config did not read back as written (%v) — the old config was put back", err) - } return nil }