Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 admin
|
a2e914f683 | ||
|
admin
|
0404f60e6a | ||
|
admin
|
3f5f61b716 |
@@ -29,6 +29,41 @@ root=$(git rev-parse --show-toplevel 2>/dev/null) || {
|
||||
}
|
||||
cd "$root" || exit 1
|
||||
|
||||
# ── WORKSPACE-ROOT ASSERTION (2026-08-05, R-204 rider) ───────────────────────────────────────────
|
||||
# Refuse a push from a clone outside the felhom workspace.
|
||||
#
|
||||
# WHY THIS IS A HOOK AND NOT A LINE IN A DOCUMENT: the workspace root is ALREADY written down, in
|
||||
# documentation/runbooks/workspace-CLAUDE.md and in the workspace-root CLAUDE.md ("stay inside it"),
|
||||
# and work drifted into a home directory anyway. A rule that has failed once as a reminder is not
|
||||
# fixed by writing it down again — it has to be asserted where it can bite.
|
||||
#
|
||||
# A PUSH IS THE RIGHT TRIGGER, deliberately: throwaway clones under /tmp for probes and red-proofs
|
||||
# never push, so nothing legitimate breaks. Reads and builds elsewhere stay unaffected.
|
||||
#
|
||||
# Symlinks are resolved on BOTH sides before comparison, so a symlinked path neither falsely passes
|
||||
# nor falsely fails. If the workspace root does not exist on this machine the check is SKIPPED, not
|
||||
# failed — this hook must not brick a legitimate clone on a different host.
|
||||
#
|
||||
# The only bypass is the documented `git push --no-verify`, whose use is already reportable.
|
||||
FELHOM_WORKSPACE_ROOT=/mnt/5_hdd/felhom.eu
|
||||
if [ -d "$FELHOM_WORKSPACE_ROOT" ]; then
|
||||
ws_real=$(cd "$FELHOM_WORKSPACE_ROOT" 2>/dev/null && pwd -P) || ws_real=""
|
||||
root_real=$(pwd -P) || root_real=""
|
||||
if [ -n "$ws_real" ] && [ -n "$root_real" ]; then
|
||||
case "$root_real/" in
|
||||
"$ws_real"/*) : ;; # inside the workspace — proceed
|
||||
*)
|
||||
echo "pre-push: PUSH REFUSED - this clone is OUTSIDE the felhom workspace." >&2
|
||||
echo " clone: $root_real" >&2
|
||||
echo " expected: under $ws_real (repos live in $ws_real/git/<repo>)" >&2
|
||||
echo " Work in the workspace clone, or bypass with 'git push --no-verify'" >&2
|
||||
echo " and state that you did in the session report." >&2
|
||||
exit 1
|
||||
;;
|
||||
esac
|
||||
fi
|
||||
fi
|
||||
|
||||
if ! command -v python3 >/dev/null 2>&1; then
|
||||
echo "pre-push: FAIL - python3 not found, so the gates CANNOT run. This is a failure, never a" >&2
|
||||
echo " pass by default. Install python3, or push with --no-verify and say so." >&2
|
||||
|
||||
@@ -1,3 +1,40 @@
|
||||
## v0.126.0 — a fetch failure is not a wrong recovery code (2026-08-06, R-224)
|
||||
|
||||
**A hub the agent could not reach was reported to the customer as a bad recovery code.** Measured live
|
||||
on 2026-08-05 (CAMPAIGN-11 F3): with the hub REJECTed at the appliance's firewall and a **correct,
|
||||
current** recovery code, the customer was told their code did not open their package — **in 0.0556 s**,
|
||||
against ~1.0 s for a genuine unseal. No unseal was attempted. F4 produced the same message in 0.0299 s
|
||||
with this agent stopped.
|
||||
|
||||
**The discriminator existed here the whole time and this boundary threw it away.** `recover.go` fails
|
||||
at four distinguishable points; the local-api handler had cases for two of them and a `default` that
|
||||
answered *"the recovery code did not open the sealed bundle, or the bundle could not be fetched"* —
|
||||
one sentence for two situations, only one of which is the customer's doing.
|
||||
|
||||
**The fix is a value, not a log line.** `escrow.ErrBundleFetch` joins the fetch leg's error, and the
|
||||
handler routes it to **502** with its own words: *"the sealed recovery bundle could not be fetched from
|
||||
the hub — the recovery code was NOT used and nothing was written."* 502 rather than 4xx because the
|
||||
request was not bad; an upstream dependency failed. The `default` now carries **only** the fail-closed
|
||||
wrong-code case and says so without the "or".
|
||||
|
||||
Four situations, four statuses — **502** fetch failed · **400** the bundle was fetched and refused the
|
||||
code · **404** the hub holds no bundle · **409** the bundle predates the repository-password field.
|
||||
The controller classifies on the STATUS and must never parse these sentences.
|
||||
|
||||
⚠ **A GREEN TEST NAMED THIS DEFECT AND DID NOT PREVENT IT.**
|
||||
`TestRecoverOffsiteRepoPassword_FetchErrorIsDistinct` has said since v0.125.0 that *"the operator must
|
||||
not be sent to re-read their recovery code because the hub was unreachable"* — and it passed
|
||||
throughout, because it asserted this package's error **string** one layer below where the merge
|
||||
happened, and a string is something no caller can branch on. It now asserts the sentinel, and its
|
||||
consequence-level twin asserts the STATUS at the boundary the customer's message is derived from.
|
||||
**Prefer the test that asserts the consequence over the one that asserts the mechanism.**
|
||||
|
||||
Tests: `recover_test.go` (fetch classifies as `ErrBundleFetch`; a wrong code does **not**; an absent
|
||||
blob keeps its own identity) and `localapi/escrow_recover_class_test.go` (each situation's status, and
|
||||
a standalone assertion that fetch-failure and wrong-code never share one). **Red-proofs:** removing the
|
||||
`%w` join fails the sentinel test; deleting the handler case makes both answer `400` with the
|
||||
wrong-code sentence — the exact pre-fix code, and the exact defect CAMPAIGN-11 measured.
|
||||
|
||||
## v0.125.0 — the agent opens the sealed bundle and returns one field (2026-08-04, R-199 links 7–8)
|
||||
|
||||
**Link 7 had one production caller and it was a `--selftest`.** `UnwrapIdentityBundle` has existed
|
||||
|
||||
+34
@@ -3,6 +3,40 @@
|
||||
> Snapshot of the current state + open threads. Authoritative history lives in `CHANGELOG.md` (top
|
||||
> entry = current); the end-of-task detail lives in `REPORT.md`.
|
||||
|
||||
## R-199 (v0.125.0) — links 6–8 of the recovery chain, assembled and walked
|
||||
|
||||
`POST /escrow/recover-offsite-password` (pinned local API, `withGuest`): the controller supplies the
|
||||
customer's recovery code, the agent fetches THIS host's own sealed blob from the hub
|
||||
(`hub.Client.FetchIdentityEscrow` → `GET /hosts/{id}/escrow`, hub >= v0.94.0, self-scoped by the
|
||||
per-host key), unseals it via `escrow.OffsiteKeyRecoverer`, and returns **only** the offsite restic
|
||||
repository password plus its sha256.
|
||||
|
||||
**Rules that must not erode:**
|
||||
- **Only that field.** Not the tunnel token, not the PBS token, not the WG key — the controller is a
|
||||
trust tier down and needs none of them. Narrowing cost nothing and is not recoverable later.
|
||||
- **The unseal stays in the agent.** `age` is an agent runtime dependency (`/usr/bin/age` — hardcoded,
|
||||
no config override; 1.2.1 on demo-felhom) and is deliberately absent from the controller image.
|
||||
- **R:** in memory for one call, cleared on the success path AND every failure path, never on disk,
|
||||
never in argv, never logged at any level including inside an error, never echoed. Verified live: 0
|
||||
log lines, 0 files, 0 leftover `felhom-idesc-*` dirs, with a positive control proving the search worked.
|
||||
- **Three distinct outcomes**, not one generic failure: no blob (404), a bundle that opens but predates
|
||||
the field (409 — pre-fork-4, cannot be retro-fitted), a code that does not open it (400 — fail-closed
|
||||
at age's KDF, nothing written).
|
||||
- **The wiring is pinned by an AST walk** (`cmd/felhom-agent/escrow_recover_wiring_test.go`):
|
||||
`main` → `runDaemon` → `buildLocalAPIServer`, an `escrow.OffsiteKeyRecoverer` constructed there, the
|
||||
`Options.EscrowRecovery` field present, and the fetcher calling the DAEMON's own `hubClient` (the
|
||||
self-scoping that makes cross-host retrieval impossible is a property of WHICH key is used).
|
||||
Links 6 and 7 were two of this project's six built-but-never-wired instances.
|
||||
|
||||
**Proven live on demo-felhom 2026-08-04:** recovered sha256 == on-disk sha256 == the hub's stored hash.
|
||||
A wrong code five minutes earlier failed closed. **The chain stops at link 8** — nothing installs a
|
||||
recovered password, reopens a repository, or restores a file.
|
||||
|
||||
**§8.6, fixed while here:** `runSelftestIdentityConsume`'s success line used to recite
|
||||
"tunnel_token + pbs_token", which became a misstatement when v0.77.0 sealed the repository password
|
||||
into the same bundle — anyone reading it would conclude the password was not there. It now names what
|
||||
THIS bundle carried and what it did not.
|
||||
|
||||
## Current
|
||||
|
||||
- **2026-08-03 — v0.123.0 (R-185): a tier the box cannot READ now says so.** The agent's token had
|
||||
|
||||
@@ -1,205 +1,53 @@
|
||||
# REPORT — R-185: a tier the box cannot READ must say so
|
||||
# REPORT — felhom-agent v0.126.0: a fetch failure is not a wrong recovery code (R-224)
|
||||
|
||||
**Date:** 2026-08-03 · **Repos:** `felhom-agent` **v0.122.0 → v0.123.0** (`fe14bc6`) · `felhom.eu`
|
||||
installer **1.23.0 → 1.24.0** (`688470c`, tag `installer-v1.24.0`, manifest bump `311dc06`) ·
|
||||
**no hub change and no hub bump** — the hub already alerts on a degraded critical capability, which is
|
||||
why that mechanism was chosen.
|
||||
**Scope: this repo's half of R-224.** The controller half ships as felhom-controller v0.202.0.
|
||||
|
||||
---
|
||||
## Why the agent changed at all
|
||||
|
||||
## 1. Baselines, re-read on arrival
|
||||
The task that commissioned this work scoped `felhom-agent` as **untouched**. It could not be. Its
|
||||
Scenario A (a hub outage must not blame the customer's code) and Scenario C (a genuine mistype must be
|
||||
told to re-check the ten words) are **mutually unsatisfiable** while this agent answers both with one
|
||||
HTTP 400 and one sentence. No value available to the controller separates them. The task's own §5
|
||||
anticipates this — *"if the step is not recoverable from the value, make it so, and say what that
|
||||
cost"* — and §4.3 says the source outranks the register's recorded shape. **The cost is this version,
|
||||
a publish, and a `MinAgent` coupling on the controller side.**
|
||||
|
||||
| Repo | `main` @ commit | Version | Matched §1? |
|
||||
|---|---|---|---|
|
||||
| `felhom-agent` | `0b28eae7bb14` | `v0.122.0` | **yes** |
|
||||
| `felhom.eu` | `7a5694341d59` | installer `1.23.0`, both `--ref=installer-v1.23.0` (lines 327, 372) | **yes** |
|
||||
## What changed
|
||||
|
||||
Highest register ID in use **R-189**; R-190+ confirmed free by grep, and none was needed.
|
||||
|
||||
## 2. Part 0 — the measurements, before anything was designed against them
|
||||
|
||||
**The row's three-way observation, reproduced unchanged:**
|
||||
|
||||
| leg | result |
|
||||
| File | Change |
|
||||
|---|---|
|
||||
| root, `pvesh … /storage/felhom-backup/content` | **3 archives** — 6.1 / 6.2 / 6.3 GB, dated 08-01, 08-02, 08-03 |
|
||||
| the **agent's token**, same endpoint | `{"data":[]}` |
|
||||
| the agent's token, `/storage/local/content` | **8 entries** — the token works where it is granted |
|
||||
| `internal/escrow/recover.go` | new `ErrBundleFetch` sentinel; the fetch leg joins it with `%w: %w` so the cause survives for the operator log |
|
||||
| `internal/localapi/escrow_recover.go` | new `case errors.Is(err, escrow.ErrBundleFetch)` → **502** with its own words; the `default` now carries only the wrong-code case and drops the "or" |
|
||||
| `internal/escrow/recover_test.go` | three new tests; the pre-existing `FetchErrorIsDistinct` re-pointed from a string to the sentinel, with the reason it failed to protect |
|
||||
| `internal/localapi/escrow_recover_class_test.go` | new — the consequence-level test: four situations, four statuses |
|
||||
|
||||
So the token is the variable, not the storage. Two further checks removed the obvious alternative
|
||||
explanation: guest **9201 IS in the `felhom` pool** (so `VM.Backup` is not the discriminator), and
|
||||
`pveum acl list` showed ACL rows for `/storage/{local,local-lvm,felhom-pbs}` and **none** for
|
||||
`/storage/felhom-backup`.
|
||||
**Four statuses:** `502` fetch failed (the code was **not used**) · `400` fetched and refused ·
|
||||
`404` no bundle · `409` bundle predates the field.
|
||||
|
||||
**The permission query, asked by the token itself — and the obvious reading is wrong:**
|
||||
## The finding this turned up
|
||||
|
||||
```
|
||||
/storage/felhom-pbs → {"Datastore.Allocate":1,"Datastore.AllocateSpace":1}
|
||||
/storage/felhom-backup → {"Sys.Audit":1,"SDN.Use":1,"Datastore.Audit":1}
|
||||
```
|
||||
**A green test named the defect and did not prevent it.** `TestRecoverOffsiteRepoPassword_FetchErrorIsDistinct`
|
||||
has asserted since v0.125.0 that *"the operator must not be sent to re-read their recovery code because
|
||||
the hub was unreachable"*. It passed throughout, because it checked this package's error **string** one
|
||||
layer below the local-api `default` that did the merging — and a string is not something a caller can
|
||||
branch on. **Mechanism asserted, consequence unpinned**; the project's own rule names this exact case.
|
||||
It is also a comment-vs-code entry: `recover.go`'s header said the errors were *"DISTINCT on purpose"*
|
||||
and named **three** situations while a fourth was silently folded into one of them.
|
||||
|
||||
The ungranted path answers **neither empty nor 403**. It answers with the privileges **inherited**
|
||||
from the box-wide `/` grant. A probe asking *"did the path come back?"* — or *"does it hold
|
||||
`Datastore.Audit`?"* — would have reported the blinded storage **healthy**. This is exactly what §3
|
||||
required to be measured rather than assumed, and it changed the design: the probe tests
|
||||
`Datastore.AllocateSpace` specifically, and a red-proof pins that choice.
|
||||
## Green gate
|
||||
|
||||
## 3. The probe
|
||||
`go build ./...` clean · `go vet ./...` clean · `go test ./...` → **29 packages ok** ·
|
||||
`python3 scripts/agent_gates.py --fast` → all gates OK.
|
||||
|
||||
`Client.Permissions` reads `/access/permissions?path=/storage/<target>` **as the agent's own token**
|
||||
(asking as root answers a different question and always says yes). `storeGrantStatuses` emits one
|
||||
`capability.Status` per configured tier.
|
||||
**Red-proofs, each demonstrated failing then restored:**
|
||||
|
||||
**Deviation from §5/§8.1, stated because a recommendation not followed gets a line:** the spec asked
|
||||
for the sudo `Prober` to be minimally generalised. This repo already has the better-established
|
||||
pattern for exactly this — `poolReadStatus`, composed **around** the prober, with the comment *"an API
|
||||
read does not belong inside the sudo-policy probe"* (v0.62.0, audit A1). The probe follows that
|
||||
precedent instead. `capability.Status` is untouched either way, which is the constraint that mattered.
|
||||
|
||||
**Decisions:**
|
||||
|
||||
- **The probed set comes from the box's own `BackupTiers()`**, never a fixed list — a hardcoded probe
|
||||
list is the defect reproduced inside the fix.
|
||||
- **Critical** (§8.3): the hub alerts only on critical, so a non-critical entry would ride the report
|
||||
and alert nobody — the same silence with extra steps. **Except** the `local` fallback target, which
|
||||
host-install's own comment calls the DEGRADED configuration: still probed, still reported, but it
|
||||
does not page. Turning an ordinary documented setup into an alert is how a signal becomes something
|
||||
an operator archives unread.
|
||||
- **It never consults content**, so it cannot alarm on a newborn tier by construction — a stronger
|
||||
guarantee than gating on emptiness would be.
|
||||
- **It never reports ok when it could not ask.** Unreachable PVE is degraded: a self-check that fails
|
||||
open converts *"I do not know"* into *"fine"*.
|
||||
|
||||
## 4. The installer — the root cause was not where the row or the task expected
|
||||
|
||||
Both assumed `PVE_STORAGES` (the fixed grant list) was the culprit. **It is not.**
|
||||
`configure_backup_target` has two arms:
|
||||
|
||||
- **Case A** creates the storage and calls `felhom-backup-target-apply grant` in the same breath — a
|
||||
box that builds its own target has always been correct.
|
||||
- **The Scenario-F arm** — *"the target already exists, leave it exactly as it is"* — **returned
|
||||
without granting**.
|
||||
|
||||
So a box whose `felhom-backup` pre-dated the install (created by the vzdump-target-move runbook, or
|
||||
surviving a reinstall — which is both demo boxes) pointed `local_backup_target` at a storage its own
|
||||
token could not read. The reuse arm now ensures the ACL through the same guarded wrapper.
|
||||
|
||||
**Scenario F is unviolated:** the storage DEFINITION is still untouched. Granting the role the agent is
|
||||
supposed to have on the target this same script is about to write into `agent.json` is finishing the
|
||||
job, not retargeting the box; `pveum acl modify` is idempotent, so a box that already has it is
|
||||
unchanged and a box whose token was rotated gets it back.
|
||||
|
||||
**`$BACKUP_TARGET_ID` is deliberately still NOT in `PVE_STORAGES`,** and the comment now says why: that
|
||||
list is granted in step 4/5, *before* `configure_backup_target` runs in step 6, and `--acl-storages`
|
||||
entries are preflight-checked for existence. Adding it there would grant on a storage that may not yet
|
||||
exist and would split ownership of the decision across two places.
|
||||
|
||||
**A gate now asserts it:** every arm of `configure_backup_target` that resolves the target must also
|
||||
grant on it — the check that would have caught this.
|
||||
|
||||
## 5. Live validation, in order
|
||||
|
||||
| # | evidence |
|
||||
| Mutation | Result |
|
||||
|---|---|
|
||||
| 1 | Part 0's measurements above, taken **before** any change |
|
||||
| 2 | **The signal that has never existed**, on the still-blind box: `capability DEGRADED … capability=pve:store-grant:felhom-backup … reason="the agent token lacks Datastore.AllocateSpace on /storage/felhom-backup (grant FelhomAgentStore there) — this tier's archives are INVISIBLE to the agent and it is never restore-tested" critical=true`, with `ok=69 total=70 degraded=1`. The hub: `Host capability: demo-felhom-8363b5 ok → degraded (agent_capability_degraded)` and **`Operator email sent`** |
|
||||
| 3 | Grant applied (user **and** token — a privsep token's rights are the intersection); the token then lists **3 archives** where it listed none, and the permission answer becomes `{"Datastore.AllocateSpace":1,"Datastore.Allocate":1}` |
|
||||
| 4 | `capabilities self-check ok=70 total=70 degraded=0`; the hub: `degraded → ok (agent_capability_recovered)` |
|
||||
| 5 | **The host tier is a due-check candidate for the first time on that box**: `tier=felhom-backup due=true archive="…2026_08_02-04_42_14.tar.zst" proven=""` — and the settle rule applies to it exactly as to the others, selecting the **08-02** archive because the 08-03 one has not settled 24 h |
|
||||
| 6 | The served installer over HTTPS: `SCRIPT_VERSION="1.24.0"`, and the served bytes carry the fix itself, not merely the version |
|
||||
| remove the `%w: %w` join (pre-R-224 wrap) | `FetchFailureIsClassifiedAsFetch` **FAILS** |
|
||||
| delete the `ErrBundleFetch` handler case | fetch answers `400 "the recovery code did not open the sealed bundle"` — **the exact defect**, and both status tests **FAIL** |
|
||||
|
||||
## 6. The other machines
|
||||
## Not changed
|
||||
|
||||
- **demo-hp CARRIES THE SAME DRIFT — and was fixed.** `local_backup_target=felhom-backup`, ACL rows for
|
||||
`local`, `local-lvm`, `felhom-pbs` only. §8.6 assumed a single affected box; the same one-line,
|
||||
additive, path-scoped, idempotent grant applies to the other, and leaving a known-blind backup tier
|
||||
on a Tier-0 box after finding it would be this row happening twice. Granted (user + token); its
|
||||
token now lists **4 archives**. It still runs agent `0.120.0`, so it has no probe yet — that arrives
|
||||
when you vouch.
|
||||
- **The tester's box was NOT touched** (Tier 2, protected). **What is known without connecting to it:**
|
||||
it very likely carries the same drift — the mechanism is the Scenario-F reuse arm, which fires on
|
||||
any box whose target pre-dated its install, and its target was moved by the very runbook that
|
||||
creates that condition. It is due for reinstall, and installer 1.24.0 fixes it on the way in.
|
||||
|
||||
## 7. Tests and red-proofs
|
||||
|
||||
Green gate: `go build ./... && go vet ./... && go test ./...` — rc=0, plus `agent_gates.py` and
|
||||
`repo_gates.py` all OK. Test runs and commits were always separate commands.
|
||||
|
||||
| # | Test | Asserts | Mutation | Observed |
|
||||
|---|---|---|---|---|
|
||||
| A | `TestStoreGrant_ForbiddenStorageIsDegradedAndNamed` | degraded, critical, naming storage **and** role | probe removed from `probeAll` | **FAIL** — `main.go never calls storeGrantStatuses` (via the seam test); with the wrong-privilege mutation: `must be DEGRADED, not "ok"` |
|
||||
| A′ | `TestStoreGrant_InheritedPrivilegesAreNotAGrant` | the measured trap: inherited ≠ granted | probe `Datastore.Audit` instead | **FAIL** — `checking for the wrong privilege reports a blinded storage healthy; got "ok"` |
|
||||
| B | `TestStoreGrant_GrantedButEmptyIsHealthy` | a readable-but-empty tier is healthy | — (it never reads content, so emptiness cannot reach it) | pass |
|
||||
| B′ | `TestStoreGrant_TheFallbackTargetIsNotCritical` | `local` is reported but does not page | gating removed (`return true`) | **FAIL** — `must not page the operator about an ordinary, documented configuration` |
|
||||
| C | `TestStoreGrant_ForbiddenAndNewbornAreDistinguishable` | different status **and** different capability id | — | pass |
|
||||
| — | `TestStoreGrant_UnreachablePVEIsDegradedNotOK` | unknown ≠ ok | — | pass |
|
||||
| F | `hostinstall_gates.py` backup-target assertion | every resolving arm also grants | reuse arm reverted | **FAIL** — `resolves the backup target in 2 place(s) but grants in only 1` |
|
||||
| H | `TestMainWiresTheStoreGrantProbe` | **AST** of `main.go` | call commented out | **FAIL** — a `strings.Contains` check would have passed |
|
||||
|
||||
**A hollow test caught and fixed before it shipped:** the first draft of `storegrant_test.go`
|
||||
re-implemented the verdict branch inside the test. It passed, and would have kept passing while
|
||||
production diverged. The decision was extracted into `storeGrantVerdict` and the tests now call it.
|
||||
|
||||
**Scenario B's red-proof, honestly:** the spec asked for "degrade on an empty content listing" as the
|
||||
mutation. That is not a mutation of this code — the probe never looks at content, which is a stronger
|
||||
guarantee than gating on emptiness. The gating red-proof above (`storeGrantCritical`) is the one that
|
||||
exercises the guard that does exist, and it fails as required.
|
||||
|
||||
## 8. Files, commits, tag
|
||||
|
||||
`internal/proxmox/query.go` (`Permissions`), `cmd/felhom-agent/main.go` (`storeGrantStatuses`,
|
||||
`storeGrantVerdict`, `storeGrantCritical`, `storeGrantRequiredPriv`, wiring),
|
||||
`cmd/felhom-agent/storegrant_test.go`, `CHANGELOG.md`, `CONTEXT.md`, `REUSE.md`, `REPORT.md`.
|
||||
`felhom.eu`: `scripts/felhom-host-install.sh`, `scripts/hostinstall_gates.py`, `scripts/CHANGELOG.md`,
|
||||
`manifests/webpage.yaml`, `CONTEXT.md`, `STATUS.md`, `documentation/architecture/00-capability-map.md`,
|
||||
`documentation/backlog/OPEN-ITEMS.md`, `documentation/runbooks/RUNBOOK-vzdump-target-move-2026-07-29.md`.
|
||||
|
||||
**Commits** — `felhom-agent`: `fe14bc6` (v0.123.0). `felhom.eu`: `688470c` (installer 1.24.0), `311dc06`
|
||||
(manifest refs), `e3187c8` (docs). **Installer tag:** `installer-v1.24.0`.
|
||||
|
||||
## 9. Deployment
|
||||
|
||||
Agent released through `release-agent.sh` — tag `v0.123.0`, sha256
|
||||
`74910135ac4feb1b7f0ad4dbd1541d965cbc0fe70d4f47b62ebf7e4bfb962453`, round-trip verified. The
|
||||
**published bytes** were downloaded and deployed: the running binary's sha matches the published one.
|
||||
`felhom-agent --version` → **0.123.0**, `systemctl is-active` → active, prior kept as `.bak-0.122.0`.
|
||||
**NOT VOUCHED** — that stays the operator's act.
|
||||
|
||||
## 10. Registers
|
||||
|
||||
- **R-185 → CLOSED** (shipped + proven live on both demo boxes), with the corrected root cause
|
||||
recorded on the row.
|
||||
- No new IDs minted; `ROADMAP.md` contains no R-185 row, so there was nothing to collapse.
|
||||
- **The capability map's whole-guest row was OPTIMISTIC and now says so:** every live restore-test it
|
||||
cited is on the OFFSITE tier, and the HOST tier was not merely unproven but *unprovable* on both
|
||||
demo boxes. It now records that, the closure, and that it will carry a host-tier live proof when one
|
||||
runs.
|
||||
- The vzdump-target-move runbook's item 5 **predicted this** and is annotated, not rewritten: it
|
||||
expected a 403 on backup, and the reason it did not surface that way is that `vzdump` writes through
|
||||
a root path, so backups kept landing while the agent's *read* stayed blind.
|
||||
- `CONTEXT.md`: agent-side entry, plus `felhom.eu` **S-21** (empty ≠ forbidden; the measured trap) and
|
||||
**S-22** (the Scenario-F arm must finish the job).
|
||||
|
||||
## 11. Teardown
|
||||
|
||||
**Nothing was provisioned.** No scratch storage, no fixture grant, no probe tag, no scratch package
|
||||
version. The two ACL grants are the intended durable change; the only other mutation was the
|
||||
installer label, which is reversible by moving the tag.
|
||||
|
||||
## 12. Observations — noticed, recorded, NOT acted on
|
||||
|
||||
- **Both demo boxes are now due for a host-tier restore-test**, which has never run on either. The
|
||||
scheduler will pick it up within 6 h unattended (a ~6 GB local restore — fast, and cheaper than the
|
||||
offsite ones). Expected, not a defect, and the first host-tier proof this fleet will have.
|
||||
- **`--acl-storages` semantics are unchanged and the automatic grant does not consult it.** If an
|
||||
operator passes `--acl-storages` deliberately excluding the backup target, the target is still
|
||||
granted by the resolution path. That is the correct precedence — a box cannot function with an
|
||||
unreadable backup target — but it is a place where an override is not absolute, and it is written
|
||||
here rather than left to be discovered.
|
||||
- **`storeGrantRequiredPriv` is a single privilege**, chosen from measurement. If PVE ever changes
|
||||
which privilege gates content listing, the probe would report healthy while the tier is blind. The
|
||||
test asserts the constant's value so a change forces a re-measurement, but nothing detects a change
|
||||
on PVE's side.
|
||||
- **Ten pre-existing `gofmt`-unclean files** remain in the agent repo (unchanged from yesterday's
|
||||
observation); every file touched here is clean.
|
||||
No Proxmox surface, no privileged path, no report/hub contract, no config schema. The route's
|
||||
success path, its scoping and its R-handling discipline (`R = ""` on both paths, never logged, never
|
||||
persisted) are untouched.
|
||||
|
||||
@@ -19,10 +19,27 @@ import (
|
||||
// keeps that property: it takes R as an argument, passes it straight through, and holds no copy.
|
||||
// Callers must clear their own reference (the `R = ""` discipline in cmd/felhom-agent).
|
||||
//
|
||||
// The errors below are DISTINCT on purpose. "no blob", "wrong code" and "the blob predates the field"
|
||||
// are three different situations for the operator and only one of them is a fault.
|
||||
// The errors below are DISTINCT on purpose. "could not fetch", "no blob", "wrong code" and "the blob
|
||||
// predates the field" are FOUR different situations for the operator and only one of them is a fault.
|
||||
//
|
||||
// ⚠ THERE WERE THREE, AND THE FOURTH WAS THE DEFECT (R-224, 2026-08-06). This comment said "three"
|
||||
// and named "no blob", "wrong code" and "predates the field" — while a FAILED FETCH was wrapped as an
|
||||
// anonymous error and fell through the caller's `default` branch into the wrong-code message. So a
|
||||
// hub that could not be reached was reported to the customer as a bad recovery code.
|
||||
//
|
||||
// Measured live on 2026-08-05 (CAMPAIGN-11 F3): with the hub REJECTed at the appliance's firewall and
|
||||
// a CORRECT current recovery code, the customer was told the code did not open their package — in
|
||||
// 0.0556 s, when a real unseal costs ~1 s of scrypt. The agent's own log carried the truth the whole
|
||||
// time (`escrow: fetching the sealed bundle: hub: transport error: … no route to host`) and the HTTP
|
||||
// boundary threw it away.
|
||||
//
|
||||
// The discriminator therefore has to be a VALUE, not a log line — that is what ErrBundleFetch is.
|
||||
|
||||
var (
|
||||
// ErrBundleFetch — the sealed bundle could not be FETCHED (the hub refused, was unreachable, or
|
||||
// the transport failed). **The recovery code was never used**, so nothing about it is known and
|
||||
// nothing may be said about it. Wraps the underlying cause for the operator log; carries no secret.
|
||||
ErrBundleFetch = errors.New("escrow: the sealed bundle could not be fetched")
|
||||
// ErrNoEscrowBlob — the hub holds no sealed bundle for this host. Not a fault: no ceremony has run.
|
||||
ErrNoEscrowBlob = errors.New("escrow: the hub holds no sealed identity bundle for this host (no ceremony has run)")
|
||||
// ErrNoResticPassword — the bundle opened, but carries no repository password. Real and expected
|
||||
@@ -58,7 +75,11 @@ func (r OffsiteKeyRecoverer) RecoverOffsiteRepoPassword(ctx context.Context, rec
|
||||
}
|
||||
blob, present, err := r.Fetch(ctx)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("escrow: fetching the sealed bundle: %w", err) // carries no secret
|
||||
// R-224: joined with ErrBundleFetch so the caller can classify by VALUE. The cause stays
|
||||
// wrapped for the operator log; neither carries a secret. Before this, the fetch failure was
|
||||
// an anonymous error and the local-api handler's `default` branch reported it to the customer
|
||||
// as a wrong recovery code.
|
||||
return "", fmt.Errorf("%w: %w", ErrBundleFetch, err)
|
||||
}
|
||||
if !present || len(blob) == 0 {
|
||||
return "", ErrNoEscrowBlob
|
||||
|
||||
@@ -175,15 +175,93 @@ func TestRecoverOffsiteRepoPassword_RLeavesNoTrace(t *testing.T) {
|
||||
|
||||
// A fetch failure surfaces as a fetch failure, not as a wrong-code error — the operator must not be
|
||||
// sent to re-read their recovery code because the hub was unreachable.
|
||||
//
|
||||
// ⚠ THIS TEST WAS GREEN THROUGHOUT THE DEFECT IT DESCRIBES (R-224, 2026-08-06). Its sentence is
|
||||
// exactly right and it did not prevent anything, for two reasons worth keeping:
|
||||
//
|
||||
// 1. **It asserted the MECHANISM, one layer below the consequence.** It checked this package's error
|
||||
// STRING. The merge happened one layer up, in the local-api handler's `default` branch, which
|
||||
// answered a fetch failure with "the recovery code did not open the sealed bundle". The customer
|
||||
// never sees this string; they see that one. The project's own rule — prefer the test that asserts
|
||||
// the CONSEQUENCE (does the customer get blamed?) over the one that asserts the MECHANISM (is the
|
||||
// error distinct here?) — names this case precisely.
|
||||
// 2. **It asserted on TEXT.** `strings.Contains(err.Error(), …)` cannot be consumed by a caller, so
|
||||
// it pinned something no production code could branch on. The distinction it checked was real and
|
||||
// unusable.
|
||||
//
|
||||
// It now asserts the SENTINEL, which is what the handler branches on, and its consequence-level twin
|
||||
// lives in `internal/localapi/escrow_recover_class_test.go` where the status is asserted.
|
||||
func TestRecoverOffsiteRepoPassword_FetchErrorIsDistinct(t *testing.T) {
|
||||
rec := OffsiteKeyRecoverer{Fetch: func(context.Context) ([]byte, bool, error) {
|
||||
return nil, false, errors.New("hub: connection refused")
|
||||
}}
|
||||
_, err := rec.RecoverOffsiteRepoPassword(context.Background(), testR)
|
||||
if err == nil || !strings.Contains(err.Error(), "fetching the sealed bundle") {
|
||||
t.Fatalf("a fetch failure must say so, got %v", err)
|
||||
if err == nil || !errors.Is(err, ErrBundleFetch) {
|
||||
t.Fatalf("a fetch failure must classify as ErrBundleFetch, got %v", err)
|
||||
}
|
||||
if errors.Is(err, ErrNoEscrowBlob) || errors.Is(err, ErrNoResticPassword) {
|
||||
t.Fatal("a transport failure must not masquerade as a content verdict")
|
||||
}
|
||||
}
|
||||
|
||||
// ── R-224 — A FAILED FETCH IS NOT A WRONG CODE ──────────────────────────────────────────────────
|
||||
//
|
||||
// CAMPAIGN-11 F3 measured the consequence of these two being indistinguishable: with the hub
|
||||
// firewalled off and a CORRECT current recovery code, the customer was told the code did not open
|
||||
// their package, in 0.0556 s — no unseal was attempted at all.
|
||||
//
|
||||
// The pair below is the whole point. Asserting only the first would pass with a `return ErrBundleFetch`
|
||||
// stuck on every error path, which is the same defect pointing the other way.
|
||||
func TestRecoverOffsiteRepoPassword_FetchFailureIsClassifiedAsFetch(t *testing.T) {
|
||||
boom := errors.New("hub: transport error: dial tcp 37.191.56.193:443: connect: no route to host")
|
||||
r := OffsiteKeyRecoverer{Fetch: func(context.Context) ([]byte, bool, error) { return nil, false, boom }}
|
||||
|
||||
_, err := r.RecoverOffsiteRepoPassword(context.Background(), testR)
|
||||
if err == nil {
|
||||
t.Fatal("a failing fetch must return an error")
|
||||
}
|
||||
// RED-PROOF: drop the `%w: %w` join in RecoverOffsiteRepoPassword (return the bare wrapped cause,
|
||||
// as it was before R-224) → this FAILS, and the local-api handler falls back to the wrong-code
|
||||
// message exactly as it did on 2026-08-05.
|
||||
if !errors.Is(err, ErrBundleFetch) {
|
||||
t.Fatalf("a failed fetch must classify as ErrBundleFetch, got %v", err)
|
||||
}
|
||||
// The underlying cause survives for the operator log.
|
||||
if !errors.Is(err, boom) {
|
||||
t.Fatalf("the fetch cause must stay wrapped for the operator, got %v", err)
|
||||
}
|
||||
// And it must NOT be mistaken for either of the bundle-content situations.
|
||||
if errors.Is(err, ErrNoEscrowBlob) || errors.Is(err, ErrNoResticPassword) {
|
||||
t.Fatalf("a transport failure is neither of the bundle-content errors: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// The other half: a genuinely wrong code must NOT classify as a fetch failure, or the fix trades one
|
||||
// misattribution for its mirror image and the customer is told the hub is down when they mistyped.
|
||||
func TestRecoverOffsiteRepoPassword_WrongCodeIsNotAFetchFailure(t *testing.T) {
|
||||
ensureAge(t)
|
||||
blob := sealBundle(t, IdentityBundle{ResticRepoPassword: "0123456789abcdef"}, testR)
|
||||
r := OffsiteKeyRecoverer{Fetch: fetcherFor(blob)}
|
||||
|
||||
_, err := r.RecoverOffsiteRepoPassword(context.Background(),
|
||||
"wrong horse battery staple sedative anaconda wobbly kingdom placard yodel")
|
||||
if err == nil {
|
||||
t.Fatal("a wrong recovery code must fail closed")
|
||||
}
|
||||
if errors.Is(err, ErrBundleFetch) {
|
||||
t.Fatalf("a wrong code must NOT classify as a fetch failure, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// A clean "the hub holds nothing" keeps its own identity too — it is not a fetch failure, and the
|
||||
// customer must not be told the hub was unreachable when it answered perfectly well.
|
||||
func TestRecoverOffsiteRepoPassword_AbsentBlobIsNotAFetchFailure(t *testing.T) {
|
||||
r := OffsiteKeyRecoverer{Fetch: func(context.Context) ([]byte, bool, error) { return nil, false, nil }}
|
||||
_, err := r.RecoverOffsiteRepoPassword(context.Background(), testR)
|
||||
if !errors.Is(err, ErrNoEscrowBlob) {
|
||||
t.Fatalf("an absent blob must stay ErrNoEscrowBlob, got %v", err)
|
||||
}
|
||||
if errors.Is(err, ErrBundleFetch) {
|
||||
t.Fatalf("an absent blob is not a fetch FAILURE, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -75,6 +75,26 @@ func (s *Server) handleRecoverOffsitePassword(w http.ResponseWriter, r *http.Req
|
||||
if err != nil {
|
||||
// Each situation gets its own status and its own words. None of them names a secret.
|
||||
switch {
|
||||
// ── R-224 (2026-08-06) — THE FETCH FAILURE IS NOT A WRONG CODE. ────────────────────────
|
||||
//
|
||||
// This case did not exist, and its absence is the defect. A failed fetch fell through to the
|
||||
// `default` below and was answered with "the recovery code did not open the sealed bundle" —
|
||||
// so a hub that could not be reached was reported to the customer as a bad recovery code, on
|
||||
// the one screen whose whole purpose is to be believed about their backups.
|
||||
//
|
||||
// Measured live 2026-08-05 (CAMPAIGN-11 F3 and F4): a CORRECT current code returned that
|
||||
// message in 0.0556 s with the hub firewalled off, and in 0.0299 s with this agent stopped —
|
||||
// against ~1.0 s for a genuine unseal. No unseal was attempted in either case.
|
||||
//
|
||||
// 502 rather than 400: 4xx says "your request was bad", and the request was not bad — an
|
||||
// upstream dependency failed. The status is the machine-readable half; the controller
|
||||
// classifies on it and must never parse this sentence.
|
||||
//
|
||||
// ⚠ THE CODE WAS NOT USED. Nothing may be said about it — not that it was wrong, and not
|
||||
// that it was right.
|
||||
case errors.Is(err, escrow.ErrBundleFetch):
|
||||
s.logger.Warn("local-api: offsite key recovery: the sealed bundle could not be FETCHED — the recovery code was never used", "vmid", vmid, "err", err)
|
||||
writeErr(w, http.StatusBadGateway, "the sealed recovery bundle could not be fetched from the hub — the recovery code was NOT used and nothing was written")
|
||||
case errors.Is(err, escrow.ErrNoEscrowBlob):
|
||||
s.logger.Warn("local-api: offsite key recovery: the hub holds no sealed bundle for this host", "vmid", vmid)
|
||||
writeErr(w, http.StatusNotFound, "the hub holds no sealed recovery bundle for this host — no escrow ceremony has run")
|
||||
@@ -82,9 +102,11 @@ func (s *Server) handleRecoverOffsitePassword(w http.ResponseWriter, r *http.Req
|
||||
s.logger.Warn("local-api: offsite key recovery: the bundle opened but predates the repository-password field", "vmid", vmid)
|
||||
writeErr(w, http.StatusConflict, "the recovery code opened the bundle, but it carries NO offsite repository password (sealed before that field existed; it cannot be retro-fitted)")
|
||||
default:
|
||||
// Includes the fail-closed wrong-code case. The agent log records the STEP, never the code.
|
||||
s.logger.Warn("local-api: offsite key recovery FAILED (wrong recovery code, or the blob could not be fetched)", "vmid", vmid, "err", err)
|
||||
writeErr(w, http.StatusBadRequest, "the recovery code did not open the sealed bundle, or the bundle could not be fetched — nothing was written")
|
||||
// The fail-closed WRONG-CODE case, and only it: the bundle was fetched and `age -d`
|
||||
// refused it. Every other situation above has its own status. The agent log records the
|
||||
// STEP, never the code.
|
||||
s.logger.Warn("local-api: offsite key recovery: the fetched bundle did not open with the supplied recovery code", "vmid", vmid, "err", err)
|
||||
writeErr(w, http.StatusBadRequest, "the recovery code did not open the sealed bundle — nothing was written")
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
package localapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/escrow"
|
||||
)
|
||||
|
||||
// R-224 — THE STATUS IS THE DISCRIMINATOR, and this test asserts the CONSEQUENCE (what the HTTP
|
||||
// boundary answers) rather than the mechanism (that the sentinel exists).
|
||||
//
|
||||
// The controller one trust tier down classifies on the STATUS and must never parse the sentence. So
|
||||
// the contract this pins is: four distinguishable situations, four distinct statuses, and the
|
||||
// wrong-code message reachable ONLY from a real refusal.
|
||||
//
|
||||
// Before R-224 the first and last rows both answered 400 with the same sentence — which is how
|
||||
// CAMPAIGN-11 F3 told a customer holding a CORRECT code that it did not open their package.
|
||||
|
||||
type fakeRecoverer struct{ err error }
|
||||
|
||||
func (f fakeRecoverer) RecoverOffsiteRepoPassword(context.Context, string) (string, error) {
|
||||
if f.err != nil {
|
||||
return "", f.err
|
||||
}
|
||||
return "0123456789abcdef0123456789abcdef", nil
|
||||
}
|
||||
|
||||
func TestRecoverOffsitePassword_EachSituationGetsItsOwnStatus(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
err error
|
||||
wantStatus int
|
||||
// mustNotSay guards the specific misattribution each status exists to prevent.
|
||||
mustNotSay []string
|
||||
}{
|
||||
{
|
||||
name: "fetch failed — the code was NEVER used",
|
||||
err: errors.Join(escrow.ErrBundleFetch, errors.New("hub: transport error: no route to host")),
|
||||
wantStatus: 502,
|
||||
mustNotSay: []string{"did not open"},
|
||||
},
|
||||
{
|
||||
name: "wrong code — the bundle WAS fetched and refused it",
|
||||
err: errors.New("escrow: the recovery code did not unwrap the identity escrow"),
|
||||
wantStatus: 400,
|
||||
mustNotSay: []string{"could not be fetched"},
|
||||
},
|
||||
{
|
||||
name: "the hub holds no bundle",
|
||||
err: escrow.ErrNoEscrowBlob,
|
||||
wantStatus: 404,
|
||||
mustNotSay: []string{"did not open"},
|
||||
},
|
||||
{
|
||||
name: "the bundle predates the repository-password field",
|
||||
err: escrow.ErrNoResticPassword,
|
||||
wantStatus: 409,
|
||||
mustNotSay: []string{"could not be fetched"},
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
srv := newTestServerS(t, &fakeGuests{}, &fakeBackups{}, &fakeStore{}, nil)
|
||||
srv.escrowRecovery = fakeRecoverer{err: tc.err}
|
||||
w := do(t, srv.Handler(), "POST", "/escrow/recover-offsite-password", "A",
|
||||
`{"vmid":8200,"recovery_code":"correct horse battery staple sedative anaconda wobbly kingdom placard yodel"}`)
|
||||
if w.Code != tc.wantStatus {
|
||||
t.Fatalf("status: got %d, want %d — body=%s", w.Code, tc.wantStatus, w.Body.String())
|
||||
}
|
||||
for _, phrase := range tc.mustNotSay {
|
||||
if strings.Contains(w.Body.String(), phrase) {
|
||||
t.Fatalf("the %d answer must not say %q — body=%s", tc.wantStatus, phrase, w.Body.String())
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// The pair that matters most, stated as its own assertion so a regression cannot hide inside a table:
|
||||
// a fetch failure and a wrong code must never answer with the SAME status. Collapsing them is the
|
||||
// whole of R-224.
|
||||
func TestRecoverOffsitePassword_FetchFailureAndWrongCodeDiffer(t *testing.T) {
|
||||
status := func(err error) int {
|
||||
srv := newTestServerS(t, &fakeGuests{}, &fakeBackups{}, &fakeStore{}, nil)
|
||||
srv.escrowRecovery = fakeRecoverer{err: err}
|
||||
return do(t, srv.Handler(), "POST", "/escrow/recover-offsite-password", "A",
|
||||
`{"vmid":8200,"recovery_code":"correct horse battery staple sedative anaconda wobbly kingdom placard yodel"}`).Code
|
||||
}
|
||||
fetch := status(errors.Join(escrow.ErrBundleFetch, errors.New("no route to host")))
|
||||
wrong := status(errors.New("escrow: the recovery code did not unwrap the identity escrow"))
|
||||
// RED-PROOF: delete the ErrBundleFetch case from handleRecoverOffsitePassword → both become 400
|
||||
// → this FAILS. That is the exact pre-R-224 code, and the exact defect CAMPAIGN-11 measured.
|
||||
if fetch == wrong {
|
||||
t.Fatalf("a failed fetch and a wrong code must not share a status (both %d)", fetch)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user