diff --git a/CHANGELOG.md b/CHANGELOG.md index 617f9df..9e54287 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,54 @@ +## v0.90.1 — R-39 hotfix: PBS reconcile must not pass `--server` to `pvesm set` (2026-07-18) + +**Config-only fix (wrapper + red-proof); the Go binary is unchanged.** Ship the wrapper with the +next agent deploy — `configs/felhom-pbs-apply` is a shipped artifact, not a build input. +Green: `go build ./... && go vet ./... && go test ./...` all pass. + +- **The defect.** The `reconcile` verb built its argv as + `args=(--server "$server" --fingerprint "$fp")`. PVE treats a PBS storage's `server` as a + **create-only** parameter and rejects the ENTIRE `pvesm set` call — + `can't change value of fixed parameter 'server'` — **even when the value passed is byte-identical + to the stored one**. So `reconcile` could never succeed against an existing entry; it exited 255 + every time. + +- **Why that was severe rather than cosmetic.** The agent consumes the hub's **one-time** PBS token + secret BEFORE invoking the wrapper. A wrapper failure therefore *burned* the credential: each hub + "Re-issue PBS credentials" minted a fresh secret, the agent consumed it, the wrapper rejected the + apply, and the storage entry stayed pinned to the **revoked** one. Observable end state: the PBS DR + tier authenticating **401 Unauthorized** indefinitely while the agent reported + `pbsdr: converged state=applied`. Live-diagnosed on the N100 demo host during the 2026-07-18 + rehearsal wrap (`felhom.eu/documentation/tests/VALIDATION-n100-rehearsal-2026-07-18.md`, finding + F2 / ROADMAP **R-39**). + +- **The fix.** Drop `--server` from the reconcile argv. The server address is immutable by + construction — relocating a PBS endpoint requires a fresh `create` — so there was never anything + for `reconcile` to reconcile there. `--fingerprint` (and `--password` when a secret is fed) remain, + which is the mutable identity the verb exists to push. Proven on the live box before committing: + `pvesm set felhom-pbs --server --fingerprint ` → rejected; + the same call without `--server` → **rc 0**. + +- **Red-proof** `TestReconcileNeverPassesServerToPvesmSet` (`internal/pbsdr/manager_test.go`): + isolates the `reconcile)` block from the shipped wrapper and asserts no `--server` reaches + `pvesm set`, plus that `--fingerprint` is still pushed (so the verb can't be hollowed out). + Verified RED against the unfixed wrapper and GREEN after the fix. Two traps the proof handles + explicitly: the pattern is **line-ending tolerant** (`\r?\n`) because this repo is cloned on + Windows and an `\n`-only pattern would match nothing and pass **vacuously**; and comment lines are + **stripped before matching**, because the WHY note above the fix necessarily quotes the very flag + the test forbids. + +- **NOT fixed here (deliberately, and each still open):** + 1. **R-39's primary half** — the agent re-applies on a change of the *descriptor hash* + (`manager.go` ~L235), but a hub credential re-issue leaves the descriptor byte-identical + (same `token_id`, same `fingerprint`; only the side-table secret rotates) and bumps only the + generation. So a converged agent still ignores a fresh secret. This wrapper fix means the apply + now *succeeds* once the agent is made to re-apply; it does not make it re-apply. + 2. **The verify-loop read** — `pbs: cannot read token secret … permission denied`: the non-root + agent reads `/etc/pve/priv/storage/.pw` directly, a path it can only ever *write* through + the root wrapper (`/etc/pve/priv` is `0700 root:www-data`, and sudoers exposes only + `create|reconcile|grant` — there is no read verb). The loop is therefore permanently blind to + the failure it exists to catch. + Both ride the spec'd R-39 agent train. + ## v0.90.0 — agent train: guest RAM resize (R-24) + fast-tick-until-convergence (R-28) (2026-07-17) **MinAgent coupling:** felhom-controller v0.143.0 gates its guest-memory-resize UI on this agent diff --git a/REPORT.md b/REPORT.md index 82ceb61..2d7e0eb 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,98 +1,102 @@ -# REPORT — felhom-agent v0.90.0 (agent train: guest RAM resize R-24 + fast-tick R-28) +# REPORT — felhom-agent v0.90.1 (R-39 hotfix: PBS reconcile must not pass `--server`) -**Date:** 2026-07-17 · **Baseline:** v0.89.0 (`9127f54`) → **v0.90.0** (`ac112c9`, pushed) · **MinAgent coupling:** felhom-controller v0.143.0 gates its guest-memory-resize UI on this agent. +**Date:** 2026-07-18 · **Baseline:** v0.90.0 → **v0.90.1** · **Scope:** config-only. +`configs/felhom-pbs-apply` (one argv line + its WHY) and one red-proof test. **The Go binary is +unchanged** — this ships with the next agent deploy as a config artifact. +**Green:** `go build ./... && go vet ./... && go test ./...` all pass. -## 1. What shipped +## What was wrong -Two ruled features on one train (Viktor's one-train/one-floor ruling), plus a diagnosis passenger. +`reconcile` built its argv as `args=(--server "$server" --fingerprint "$fp")`. PVE treats a PBS +storage's `server` as a **create-only** parameter and rejects the whole `pvesm set` call — +`can't change value of fixed parameter 'server'` — **even when the value is byte-identical to the +stored one**. `reconcile` could therefore never succeed against an existing entry. -- **R-24 guest RAM resize (controller-direct)** — new self-scoped local-API surface `GET`/`POST /guest/memory` - (`internal/localapi/guestmemory.go`). The agent is the security boundary: bounds recomputed FRESH per - request from a live read; applies via PVE `SetConfig` (live cgroup apply, no reboot); verify-after-apply. -- **R-28 fast-tick-until-first-convergence** — `internal/fasttick/`, wired over four cached sources; pulses - the shared out-of-band trigger every 30 s while unapplied, self-disarms on convergence. Seams added: - `reconcile.Engine.LastResult()`, `wgtunnel.Manager.TunnelConvergence()`. -- **Guests-0/0** — diagnosed live, hypothesis REFUTED (no code change). - -## 2. Phase-0 probe (the gate — PASSED) - -Prerequisite: prove `PUT /nodes/{node}/lxc/{vmid}/config memory=X` hot-applies under the AGENT token. Run on -the nested demo box `demo-vm-felhom-4846bc` (hostname `felhom-appliance`, guest LXC 9201) — reached via the -hub-vaulted G1 break-glass root credential (the box's root PW was unknown; retrieved operator-side from the -hub SQLite `host_recovery` table, then shredded). Transcript: +That is severe rather than cosmetic because **the agent consumes the hub's one-time PBS token secret +before invoking the wrapper**. Every failed reconcile burned a credential: ``` -node=felhom-appliance vmid=9201 token_user=felhom-agent@pve -BEFORE config.memory=2890 MB | status.maxmem=3030384640 B (2890 MB) | status=running | host-uptime=8226s -BEFORE guest MemTotal: 2959360 kB | guest /proc/uptime=8227.76 ---- PUT memory=3146 (agent token) --- PUT data field: None (empty == synchronous apply, no UPID) -AFTER config.memory=3146 MB | status.maxmem=3298820096 B (3146 MB) | status=running | host-uptime=8229s -AFTER guest MemTotal: 3221504 kB | guest /proc/uptime=8231.54 -VERDICT maxmem_grew=True guest_running=True guest_not_rebooted=True -REVERT memory=2890 → maxmem 3030384640 B (2890 MB), running; guest MemTotal 2959360 kB +hub "Re-issue PBS credentials" → fresh one-time secret minted +agent → secret CONSUMED (single-use, now spent) +wrapper reconcile → exit 255, "fixed parameter 'server'" +storage entry → still pinned to the REVOKED secret → 401 forever ``` -Proven: live cgroup apply (maxmem moves, guest running), **no reboot** (guest /proc/uptime keeps climbing), -synchronous (no UPID), agent token has the privilege (no 403), and **/proc/meminfo ripples via lxcfs** — the -mechanism that makes the controller's deploy-page memory math follow for free. The gate PASSED → the resize -leg was implemented. +## How it was found and proven -## 3. Files created / modified +Live-diagnosed on the N100 demo host during the 2026-07-18 rehearsal wrap +(`felhom.eu/documentation/tests/VALIDATION-n100-rehearsal-2026-07-18.md` finding F2, ROADMAP +**R-39**). The box had been reporting `pbsdr: converged state=applied` while `pvesm status` returned +`401 Unauthorized` / `inactive`. -- `internal/localapi/guestmemory.go` (new) + `guestmemory_test.go` (new); `internal/localapi/server.go` - (Options.Memory + fields + routes); `cmd/felhom-agent/main.go` (wire Memory=px; construct + launch fasttick). -- `internal/fasttick/fasttick.go` + `fasttick_test.go` (new). -- `internal/reconcile/engine.go` (`LastResult`/`recordResult`) + `lastresult_test.go` (new). -- `internal/wgtunnel/manager.go` (`TunnelConvergence` + cached snapshot refresh in `Apply`). -- `CHANGELOG.md`, `REPORT.md` (this), `CONTEXT.md`, `REUSE.md`. +Proven directly on the live entry before any code was written: -## 4. Tests + red-proofs (all restored green) +| Probe | Result | +|---|---| +| `pvesm set felhom-pbs --server --fingerprint ` | **rejected** — `can't change value of fixed parameter 'server'` | +| `pvesm set felhom-pbs --fingerprint ` | **rc 0 — accepted** | -- localapi memory: GET fields, grow, shrink-above-floor, the three refusals (each asserting **SetConfig - count == 0**), cross-guest 403, fresh-bounds-per-request, verify-not-reflected 502, nil-config 503. -- fasttick: pulse-while-unconverged, silent-when-converged, the ruled disarm-on-convergence, full-channel - non-blocking drop, first-reason. reconcile: `LastResult` effect + pre-first-run ok=false. -- **Red-proofs (run-fail-restore):** (i) floor guard removed → below_usage_floor 412 test fails; (ii) max - guard removed → above_max test fails; (iii) fast-tick ignores convergence (always pulse) → the silent + - disarm tests fail. All confirmed red, then restored. Full suite: `go build ./... && go vet ./... && - go test ./...` — all 28 packages pass. +The encryption key `K` (`.enc`) and the `.pw` store were verified byte-untouched after the +rejected call: PVE rejects atomically, so the set-only law held throughout. -## 5. Guests-0/0 diagnosis — hypothesis REFUTED, no fix +## The fix -Live read-only checks on the nested box: pool `felhom` `members: [9201]`; the agent token's ListLXC sees -`[9201]` (VM.Audit comes from the `/pool/felhom` grant — so pool membership IS the visibility mechanism); -hub reports 1/1. The agent log shows the observed 0/0 was the **pre-provision window**: `16:11 stale-lock … -pool=felhom listed=0` + `desired … guests=0`, then `16:16 lanresolver … vmid=9201` (guest created). No guest -existed yet → empty list, no error (the silent shape) — correct, not a bug. The existing `PoolAddVMID` -re-assert (`bringup.go:498`) already covers the known restore-over-existing edge (campaign-2 R2). **Item 2's -fast-tick is precisely the mitigation** — it shortens the pre-provision reporting window from ≤15 min to ~30 s. +Drop `--server` from the reconcile argv. The server address is immutable by construction — +relocating a PBS endpoint requires a fresh `create` — so there was never anything for `reconcile` to +reconcile there. `--fingerprint` (and `--password` when a secret is fed on stdin) remain, which is +the mutable identity the verb exists to push. -## 6. Deploy verification (BOTH hosts) +## Red-proof -Built on 180 (`go build -ldflags -X main.version=0.90.0`), deployed to both Proxmox hosts (backup + install + -restart): +`TestReconcileNeverPassesServerToPvesmSet` (`internal/pbsdr/manager_test.go`) isolates the +`reconcile)` block from the shipped wrapper and asserts no `--server` reaches `pvesm set`, plus that +`--fingerprint` is still pushed so the verb cannot be hollowed out. **Verified RED against the +unfixed wrapper and GREEN after the fix.** -- **felhom-pve** (the N100): `felhom-agent 0.90.0` active; `capabilities self-check ok=63 total=63 degraded=0`; - **`fast-tick armed: 30s out-of-band cadence while desired-state is unapplied`**; poke listener + local-api up. -- **demo-vm-felhom-4846bc** (nested, 192.168.0.135): `felhom-agent 0.90.0` active; `capabilities self-check - ok=63 degraded=0`; `fast-tick armed: 30s …`; local-api on 192.168.0.135:8443. (Converged box → fast-tick - quiet after the startup line, as designed.) +Two traps the proof handles explicitly, both of which would have made it pass vacuously — and both +of which actually fired during authoring, which is the argument for running a proof red first: +- **Line endings.** This repo is cloned on Windows; the working copy carries CRLF, so an `\n`-only + pattern matches nothing and the guard passes silently. The pattern is `\r?\n` throughout. The + first run failed with "could not locate the reconcile) block" — the vacuous-pass failure mode, + caught only because the test was run against the broken wrapper first. +- **Comments.** The WHY note above the fix necessarily quotes `--server`, the very flag the test + forbids, so the suite went red *after* the fix was applied. Comment lines are now stripped before + matching. -## 7. NOT yet live-validated / operator GO +## Deliberately NOT fixed here — each still open -- **Fast-tick real-onboarding proof:** the fast-tick's value shows on a REAL fresh box's pre-tunnel window - (the rehearsal S5 bind sequence should now be visibly instant). Not manufacturable non-destructively here; - both agents show the armed startup line + the unit tests prove the pulse/disarm. -- **Floor LAST (operator GO):** both demo agents verifiably run 0.90.0 (the safety gate) and the controller - coupling is PROVEN LIVE via the X-Felhom-Agent-Version header (see the controller REPORT's live refusals). - Publishing 0.90.0 to Gitea + the hub Day-0 manifest **vouch + MinAgent-floor raise to 0.90.0** are set via - the operator-password-gated Day-0 manifest UI (CC cannot log in) — the operator (Viktor) GO step, matching - the v0.89 train's "publish/vouch = operator follow-up". +1. **R-39's primary half.** The agent re-applies on a change of the **descriptor hash** + (`internal/pbsdr/manager.go` ~L235: + `if mk := m.loadMarker(); mk != nil && mk.Hash == h && (cf == nil || cf.Hash != h) { return }`). + A hub credential re-issue leaves the descriptor **byte-identical** — same `token_id`, same + `fingerprint`; only the side-table secret rotates — and bumps only `desired_generation`. So a + converged agent short-circuits and never consumes the fresh secret. This wrapper fix makes the + apply *succeed* once the agent is made to re-apply; **it does not make it re-apply.** +2. **The verify-loop read.** `pbs: cannot read token secret … permission denied` — the non-root + agent reads `/etc/pve/priv/storage/.pw` **directly**, a path it can only ever *write* through + the root wrapper. `/etc/pve/priv` is `0700 root:www-data` and sudoers exposes only + `create|reconcile|grant` — **there is no read verb**. The loop is permanently blind to exactly + the failure it exists to catch. -## 8. Observations +Both ride the spec'd R-39 agent train. Recorded, not improvised. -- The nested demo host is tiny (4 GB): its resize bounds are degenerate (`max_mb 1866 < min_mb 2048 < - current 2890`) — no resize *succeeds* through the bounded endpoint there (all refused correctly). The - successful apply is Phase-0-proven at the agent layer; the controller live-validation exercised the - refusals (above_max, below_min) end-to-end. -- REUSE.md gained the fasttick Source pattern + the localapi MemoryOps seam. +## Live state of the demo box at hand-off + +The wrapper is **hotfixed on the N100** (`/usr/local/sbin/felhom-pbs-apply`, backup kept at +`.bak-20260718-preR39`, `bash -n` clean) so the repo and the box agree. + +**The box is not yet healed, and cannot be by CC.** During diagnosis the agent was made to re-apply +(marker moved aside — never deleted, the R-22 precedent), which proved the mechanism but consumed +the one pending secret against the still-unfixed wrapper. The agent parked correctly in +`consumed-failed.json` with `NOT retrying silently` — **no burn loop**, the fail-safe worked. + +Healing needs one password-gated operator action CC cannot perform: + +> **Viktor: click "Re-issue PBS credentials" for `demo-felhom` on the hub.** + +The agent will then pick it up unaided — `marker.json` is absent, so the L235 short-circuit does not +apply — consume the fresh secret, and the patched wrapper will now apply it. Verification to run +afterwards: `pvesm status` shows `felhom-pbs` **active**, then one on-demand PBS backup of guest 9201 +confirmed in the PBS-side listing (the tier's first real backup on the reborn box). + +Nothing was destroyed: `.pw`, `.enc` (K), and the `storage.cfg` entry are all intact and verified. diff --git a/configs/felhom-pbs-apply b/configs/felhom-pbs-apply index b5b1ae2..e89a96f 100644 --- a/configs/felhom-pbs-apply +++ b/configs/felhom-pbs-apply @@ -98,7 +98,16 @@ reconcile) val_server "$server"; val_ns "$ns"; val_tok "$tok"; val_fp "$fp"; val_sdir "$sdir" entry_exists || die "entry $id does not exist (create is the fresh path)" SECRET=$(read_secret) - args=(--server "$server" --fingerprint "$fp") + # R-39 (2026-07-18): NEVER pass --server to `pvesm set`. PVE treats `server` as a CREATE-ONLY + # parameter and rejects the ENTIRE call — "can't change value of fixed parameter 'server'" — + # even when the value is byte-identical to the stored one. That made every reconcile exit 255, + # so each hub-re-issued one-time secret was consumed-then-burned and the tier stayed pinned to + # a revoked credential (401 forever). Proven live on the N100 demo host: `pvesm set + # --server --fingerprint ` -> rejected; the same call without --server -> rc 0. + # The server address is immutable by construction (relocating a PBS endpoint needs a fresh + # create), so there is nothing here to reconcile. Guarded by + # TestReconcileNeverPassesServerToPvesmSet. + args=(--fingerprint "$fp") [[ -n "$SECRET" ]] && args+=(--password "$SECRET") pvesm set "$id" "${args[@]}" >&2 place_copies "$sdir" diff --git a/internal/pbsdr/manager_test.go b/internal/pbsdr/manager_test.go index 0f4c1be..58d4344 100644 --- a/internal/pbsdr/manager_test.go +++ b/internal/pbsdr/manager_test.go @@ -348,6 +348,54 @@ func TestSetOnlyLaw(t *testing.T) { } } +// TestReconcileNeverPassesServerToPvesmSet is the R-39 red-proof (live-diagnosed 2026-07-18 on the +// N100 demo host). PVE treats a PBS storage's `server` as a CREATE-ONLY parameter: `pvesm set` +// rejects the WHOLE call with "can't change value of fixed parameter 'server'" even when the value +// passed is byte-identical to the stored one. The wrapper's reconcile verb used to pass it anyway, +// so EVERY re-apply exited 255 — and because the agent consumes the hub's one-time secret BEFORE +// invoking the wrapper, each hub re-issue burned a fresh credential while leaving the storage entry +// pinned to the revoked one. The observable end state was a PBS tier that authenticated 401 forever +// while the agent reported `converged state=applied`. +// +// Red-proof: revert the wrapper's reconcile args to include --server and this test fails. +func TestReconcileNeverPassesServerToPvesmSet(t *testing.T) { + wrapper, err := os.ReadFile(filepath.Join("..", "..", "configs", "felhom-pbs-apply")) + if err != nil { + t.Fatalf("read wrapper: %v", err) + } + + // Isolate the reconcile verb's block: from `reconcile)` to its terminating `;;`. + // Line-ending tolerant on purpose — this repo is cloned on Windows, where the working copy + // carries CRLF and an \n-only pattern silently matches nothing (which would make this guard + // pass vacuously, the exact failure mode a red-proof exists to prevent). + block := regexp.MustCompile(`(?s)\r?\nreconcile\)\r?\n(.*?)\r?\n\s*;;`).FindSubmatch(wrapper) + if block == nil { + t.Fatal("could not locate the reconcile) block in configs/felhom-pbs-apply") + } + // Strip comment lines before matching: the WHY note above the fix necessarily quotes the very + // flag this test forbids, and a naive grep would flag the explanation as the defect. + var code []string + for _, line := range strings.Split(string(block[1]), "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "#") { + continue + } + code = append(code, line) + } + body := strings.Join(code, "\n") + + // The args array that is handed to `pvesm set` must not carry --server in any form. + if regexp.MustCompile(`--server`).MatchString(body) { + t.Error("reconcile passes --server to `pvesm set` — PVE rejects the whole call on this " + + "create-only parameter, which burns every re-issued one-time secret (R-39)") + } + + // Guard the intent rather than the spelling: --fingerprint is the mutable identity field the + // reconcile exists to push, so its disappearance would make the verb pointless. + if !regexp.MustCompile(`--fingerprint`).MatchString(body) { + t.Error("reconcile no longer pushes --fingerprint — the verb has lost its purpose") + } +} + func TestConsumedButFailed_LoudAndRecoversOnlyViaFreshSecret(t *testing.T) { r := &fakeRunner{failVerb: "create", failErr: errors.New("pvesm add exploded")} st := &fakeStorage{found: false}