From 8324ed0dc26b52e8f09d55788dd054d5bd54c71e Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sun, 14 Jun 2026 08:59:53 +0200 Subject: [PATCH] Triage: fix-spec for live-drive findings F1-F20 (re-diagnosed at file:line, sequenced batches) --- LIVE-DRIVE-FIXSPEC-2026-06-14.md | 446 +++++++++++++++++++++++++++++++ 1 file changed, 446 insertions(+) create mode 100644 LIVE-DRIVE-FIXSPEC-2026-06-14.md diff --git a/LIVE-DRIVE-FIXSPEC-2026-06-14.md b/LIVE-DRIVE-FIXSPEC-2026-06-14.md new file mode 100644 index 0000000..b25cb35 --- /dev/null +++ b/LIVE-DRIVE-FIXSPEC-2026-06-14.md @@ -0,0 +1,446 @@ +# LIVE-DRIVE-FIXSPEC — triage of the 2026-06-14 live-drive findings + +- **Date:** 2026-06-14 +- **Branch:** `audit/2026-06-14-live-drive` (companion to `LIVE-DRIVE-FINDINGS-2026-06-14.md`) +- **Scope:** re-diagnose F1–F20 against **current source** at file:line, spec a fix for each worth fixing, then a sequenced plan. **Diagnose + spec only — no code changes, no deploys, no `main` commits.** +- **Method:** every finding's stated cause was treated as a claim and re-rooted in code. Two report causes were found **wrong** (F20-BUG1 location; F17 volume-capture sub-claim) and one is likely a measurement artifact (F3). Runtime-only facts reference the findings report's captured evidence. + +## Progress log + +- **t0** — Confirmed branch + repos (`felhom-controller`, `felhom-agent`, `app-catalog-felhom.eu`, `felhom.eu`). Launched parallel code investigations. +- **t1** — F17 + F20(agent) re-diagnosed: F17 DB-replay claim TRUE but volume-capture sub-claim mis-attributed; **F20-BUG1 relocated** (agent is correct; bug is in the controller's agentapi client). +- **t2** — F9 + F2 re-diagnosed: provisioning never binds user-data drives; "misreporting" is a missing guest-attached field; F2 is the expected symptom. +- **t3** — F1/F11/F13 + F5 catalog + controller-misc (F3/F4/F6/F7/F8/F15 + BUG1 controller mapping) re-diagnosed. F1 confirmed to defeat the deploy memory guard (not display-only). F5 catalog survey found additional at-risk healthchecks. +- **t4** — Wrote per-finding specs + sequenced batches. (this doc) + +--- + +## RE-DIAGNOSIS SUMMARY (verdict counts) + +| Verdict | Count | Findings | +|---|---|---| +| **real** (fix as specced) | 11 | F1, F5, F9, F11, F17, F20-BUG1, F20-BUG2, F20-BUG3, F6, F7, F8 | +| **mis-attributed** (report cause wrong → re-rooted) | 3 | **F20-BUG1** (controller not agent), **F17 sub-claim** (units DO capture volumes), **F3** (not in code) | +| **already-fine / no independent action** | 9 | F10, F12, F14, F16, F18, F19 (PASS), **F13** (consequence of F9), **F2** (expected symptom of F9), **F15** (standard Docker semantics) | +| **needs-runtime-confirmation** | 2 | **F3** (byte-level recheck), **F17 capture-side** (are volume tars actually written by `backup/run`?) | +| **trivial / doc-only** | 1 | F4 | + +> Findings whose **report cause was WRONG against the code** (the most important output): **F20-BUG1** (report blamed the agent's format handler; the agent is correct — the bug is the controller's `agentapi.FormatDisk` swallowing the agent's 502). **F17** (report said "recovery units don't capture volume data"; they do — `restoreDockerVolumes` + `VolumeDumps` exist; the real gap is the `.sql` dump is never replayed). **F3** (report said the JSON API double-encodes; no transcoding exists in the code path — likely a terminal/`curl` display artifact). + +--- + +## CRITICAL / HIGH — full specs + +### F17 — Per-app restore re-creates config + restores Docker volumes but never replays the `.sql` DB dump +``` +Report claim: "per-app restore never imports the DB dump; recovery units don't capture volume data" +Re-diagnosis: real (DB-replay gap) + mis-attributed (volume-capture sub-claim is FALSE) +True cause: POST /backup/restore → web/handlers.go:785 RestoreFromRecoveryUnit → + backup/restore_unit.go:125-131 does StopStack + restoreDockerVolumes(:128) + + RecreateStackFromUnit(:131). RecreateStackFromUnit (cmd/controller/main.go:953) + copies docker-compose.yml/.felhom.yml + RedeployFromEnv (compose up -d). NO .sql + replay anywhere in internal/backup/*. The captured romm-mariadb.sql is enumerated + into the unit manifest (recovery_unit.go:105) but never piped into the DB. + The reusable importer EXISTS but only in the .fab path: appexport/restore.go — + importDBDump(:971), findDBServiceInCompose(:816), waitForDB(:928), findDumpFile(:795), + restoreDatabase(:722) — all UNEXPORTED, package appexport. + Correction to the report: RestoreFromRecoveryUnit DOES restore Docker named-volume + tars (restore.go:84-137) and capture DOES populate VolumeDumps (recovery_unit.go:44). + So "units don't capture volumes" is wrong; the romm row was lost because (a) the .sql + is never replayed AND (b) romm's MariaDB data dir is under the appdata bind namespace + (live dir, not a named-volume tar), so the volume-restore path doesn't cover it either. +Severity: CRITICAL (data-loss on the headline disaster-recovery feature; matches report) +Repo / layer: controller — internal/backup (+ reuse internal/appexport DB-import logic) +Fix approach: 1) Extract the four free funcs (importDBDump/findDBServiceInCompose/waitForDB/ + findDumpFile) into a neutral pkg (e.g. internal/dbrestore) and have BOTH appexport + and backup call them (no duplication). 2) In RestoreFromRecoveryUnit, after + RecreateStackFromUnit + DB service healthy, for each /db-dumps/*.sql replay it + via the shared importer (bring DB svc up, waitForDB, importDBDump, then continue the + stack). 3) Make volume-restore failure NOT silently non-fatal: restore.go:64 currently + logs WARN + continues and RestoreApp returns nil regardless — surface a partial-restore + error/flag to the caller so the UI can't claim success on a silent data-restore failure. +Effort: L (cross-package extraction + restore-flow change + tests) +Risk: unattended-safe to WRITE/test (no live deploy); behaviour change is destructive-restore, + so validate against a scratch app before shipping. +Depends on / blocks: independent. Co-edits restore_unit.go/restore.go (see F-collision note: none in B1). +Proof of fix: unit test: capture marker row → drop → RestoreFromRecoveryUnit → row present. + Re-run the live romm marker repro from the findings report. +Open question: For an app whose DB lives in a named volume AND has a .sql dump, replaying both could + double-apply — decide precedence (prefer .sql replay for logical consistency, or skip + .sql when the DB's named volume tar was restored). Needs a one-line policy decision. +``` + +### F9 — felhom-usb HDD never bound into the guest; provisioning omits user-data binds; disk APIs lack a "guest-attached" signal +``` +Report claim: "HDD not passed to guest; HDD apps undeployable; disk APIs misrepresent it as available" +Re-diagnosis: real (two layers) — provisioning gap is primary; "misreporting" is really a MISSING field +True cause: LAYER A (provisioning): agent reconcile/bringup.go runBringUp (:158-294) writes only + mp0 (docker-data, :236-250) + spec.Mounts as fresh PVE volume specs (:348-356) + mp9 + (bootstrap). It NEVER adds a host-path bind for an external user-data drive. That bind + lives only in a separate, explicit enroll step: localapi/disks.go handleDiskGuestAttach + (:238-287) → guestbind.go AttachBind (:51-71, `pct set -mpN /felhom-data`) + → requires guest reboot (:77-84). It was never re-run after 9201 was re-provisioned. + A self-heal watchdog exists (disks.go:535-559) but only reconciles drives previously + ENROLLED (intent recorded) — a never-enrolled drive is invisible to it. + LAYER B (reporting): /disks state/reachable (storage/observe.go:290-299) and role + (storage/role.go:105-126) faithfully describe HOST presence/topology; /host-metrics + (host_metrics.go:28-49) emits the raw PVE content string. There is NO field anywhere + that asserts "bound into guest N", so host-presence reads as availability. The role + (user-data) vs content (backup) "disagreement" is two orthogonal attributes, not a bug. +Severity: CRITICAL (blocks 13/55 apps; silent OS-disk placement; unblocks F13 + data-migration) +Repo / layer: agent — internal/reconcile (provision) + internal/localapi (enroll, reporting); + + golden/bootstrap; + small controller/agent reporting field +Fix approach: A) Make user-data drive attachment part of (re)provisioning OR auto-re-enroll on + bring-up: record the drive's durable-id intent at first enroll (already done) and have + runBringUp re-assert known user-data binds for the guest after recreating it (reuse + AttachBind + the intent store the watchdog already reads), so destroy+recreate restores + the mp. B) Add a `guest_attached bool` (per-guest) to the DiskInfo/storage-target the + agent returns, set by checking the guest LXC config for an mp whose source is the + drive's mount_path; surface it in the controller so /api/disks and the UI distinguish + "present on host" from "usable by your server". This also fixes the F2 disagreement + presentation. (Operationally for THIS demo: run guest-attach + reboot for felhom-usb.) +Effort: L (provisioning change + new reporting field across agent+controller) +Risk: SUPERVISED — agent/provisioning/golden; touches pct/LXC config on a live host. +Depends on / blocks: blocks F13 (3-2-1 needs a real 2nd drive), F11 (real HDD class), data-migration. +Proof of fix: re-provision a scratch guest with an enrolled user-data drive → mp re-appears in + .conf and the drive is writable in-guest; /api/disks shows guest_attached=true; + an HDD app deploys onto it. +Open question: Should bring-up auto-re-enroll ALL previously-enrolled drives unattended, or require an + operator confirm (a bind is a data-path change)? Recommend auto-re-assert for drives + with a recorded intent + durable-id match; flag mismatches. Needs operator sign-off. +``` + +### F20-BUG1 — Controller's `agentapi.FormatDisk` swallows the agent's error and returns `ok:true` (zero-value) +``` +Report claim: "agent format handler maps mkfs failure to ok:true (swallowed error)" +Re-diagnosis: real, but MIS-ATTRIBUTED — the AGENT is correct; the bug is in the CONTROLLER client +True cause: Agent handleDiskFormat returns ok:false / HTTP 502 on mkfs failure (localapi/disks.go + :437-440, :476-480 → writeErr → {"ok":false} 502) — correct. The controller's + agentapi/client.go FormatDisk (:372-398) calls postWithStatus, which by design does + NOT surface an ok:false business error (comment :402). On the agent's 502, env.Data is + JSON `null` → json.Unmarshal leaves a zero-value FormatResult; the 403/NeedsConfirmation + guards (:389,:393) don't match a 502, so it falls through to `return out, nil` (:397). + web/agent_disk_handlers.go:191-207 then sees err==nil and writes + {"data":{...zeros...},"ok":true} HTTP 200 — exactly the observed response. +Severity: HIGH (a destructive op silently reports success on failure) +Repo / layer: controller — internal/agentapi/client.go (FormatDisk / postWithStatus) +Fix approach: Have postWithStatus also return the envelope's error string + ok flag; in FormatDisk, + after the NeedsConfirmation/403 handling, add: if status >= 300 (or !env.OK) and not a + confirmation case → return out, fmt.Errorf("agentapi: format HTTP %d: %s", status, msg). + The existing web handler `if err != nil → 502` branch then surfaces it correctly. +Effort: S +Risk: unattended-safe (controller-only error propagation) +Depends on / blocks: co-edits client.go FormatDisk with F20-BUG3 (async rework) — see collision note. +Proof of fix: unit test: stub agent 502 → FormatDisk returns non-nil err; handler returns ok:false. +Open question: none. +``` + +### F20-BUG3 — Synchronous format; controller's 15s client timeout cancels the request context → SIGKILLs in-flight mkfs → corrupt disk +``` +Report claim: "large-disk format times out → cancelled context kills mkfs → corrupt disk" +Re-diagnosis: real (confirmed end-to-end) +True cause: agentapi/client.go:87 sets http.Client{Timeout: 15s} for ALL agent calls incl. + /disks/format. Agent runs mkfs under the REQUEST context: handleDiskFormat passes + r.Context() (disks.go:437,:476) → hostops.go:346 → proxmox/privileged.go:57/59 + exec.CommandContext(ctx,...). When the 15s client timeout fires, Go cancels the request + → r.Context() done → CommandContext sends SIGKILL to mkfs mid-write → corrupt fs + (matches agent log "mkfs.ext4 ... signal: killed"). +Severity: HIGH (destructive: corrupts a large drive) +Repo / layer: agent — internal/localapi (format handler) + internal/storage; controller — agentapi +Fix approach: NOT "raise the timeout". Make format a detached job + status poll, modelled on the + agent's existing patterns: handleGuestReboot (disks.go:298-326) already runs a long op + in a goroutine off s.baseCtx and returns 202; the backup job (StartBackup/BackupStatus, + client.go:106-207, phases idle|running|...|done) is the status-poll template. Run mkfs + under s.baseCtx (NOT r.Context()) so a dropped HTTP request can't kill it; add + /disks/format → {job_id} + /disks/format/status; controller polls. The async worker + MUST re-resolve the durable-id at execution time (preserve AGENT-001 anti-retarget, + wipe_reresolve.go). +Effort: L +Risk: SUPERVISED — agent destructive path; design + live validation on a scratch device. +Depends on / blocks: co-edits client.go FormatDisk with F20-BUG1. +Proof of fix: format a large scratch device; drop the HTTP client mid-op → mkfs continues to + completion; status endpoint reports done; device mounts clean. +Open question: Status persistence across an agent restart (like BackupRecord)? Recommend yes, reuse + the backup record pattern. Confirm with operator whether a format must survive agent + restart. +``` + +### F5 — uptime-kuma catalog healthcheck points at a nonexistent file → permanent unhealthy → Traefik withholds route → 404 +``` +Report claim: "broken catalog healthcheck → unhealthy → Traefik 404; design: unhealthy = total outage" +Re-diagnosis: real (data fix) + a broader catalog risk + an unaddressed design question +True cause: app-catalog-felhom.eu/templates/uptime-kuma/docker-compose.yml:24-29 overrides + healthcheck with `["CMD","node","/app/extra/healthcheck.mjs"]`; image louislam/ + uptime-kuma:2 ships its healthcheck as a COMPILED Go binary at /app/extra/healthcheck + (no .mjs) and already defines a working built-in HEALTHCHECK. The override fails → + permanent unhealthy. Survey found MANY other apps using wget/curl healthchecks on + non-Alpine bases (claper, calcom, rallly, wanderer, wishlist, plant-it, papra, gokapi, + termix; kimai, komga, crafty-controller) where the tool may be absent — same failure + class, unconfirmed. Several migration-heavy apps use start_period:30s (calcom, outline, + docmost, rallly, tandoor, nextcloud, wger) → transient 404 right after deploy. +Severity: HIGH (one confirmed total-outage; broader latent set) +Repo / layer: app-catalog-felhom.eu (data) + DESIGN question spanning Traefik/controller dashboard +Fix approach: (data) Delete the uptime-kuma compose healthcheck block (lines 24-29) → inherit the + image's built-in; or set test:["CMD","extra/healthcheck"]. Then audit the wget/curl + healthchecks: for each, `docker run --rm which wget||which curl`; replace + absent-tool checks with inline `node -e`/`python3` socket checks (mealie's pattern is + the precedent already in-catalog). Bump start_period to 60-120s for migration apps. +Effort: S (uptime-kuma alone) / M (full catalog audit) +Risk: unattended-safe (catalog data; auto-syncs to controllers within 15m). +Depends on / blocks: independent. +Proof of fix: redeploy uptime-kuma → healthy within start_period → status. → 200. +Open question: DESIGN (needs operator decision, do NOT silently pick): an unhealthy container makes + Traefik withhold the route → hard 404, indistinguishable from "not deployed". Options: + (a) leave as-is (fail-safe: don't route to unhealthy); (b) publish the route even when + unhealthy so the app is reachable while degraded; (c) keep gating but surface + "route unpublished because unhealthy" distinctly in the dashboard so it's not a silent + 404. Recommend (c). Decision required. +``` + +### F1 — Memory metric reads host `/proc/meminfo` (16GB), not the LXC 2GB cgroup cap — and it defeats the deploy memory guard +``` +Report claim: "system memory reports host RAM not the 2GB guest cap; unsafe headroom" +Re-diagnosis: real — AND confirmed it is NOT display-only: it disables the deploy memory hard-block +True cause: system/info_linux.go readMemInfo (:80-113) parses /proc/meminfo MemTotal/MemAvailable + directly. No cgroup awareness anywhere (repo-wide: zero matches for cgroup/memory.max/ + limit_in_bytes/Sysinfo/lxcfs). The container has no lxcfs and unbounded cgroup + memory.max → reads host 16GB. Consumed by the deploy gate: stacks/deploy.go:162 + system.GetMemoryMB() → hard block at :173 (usedMB+newReqMB > usableMB) and overcommit + warning at :188. With totalMB=15771 the hard block effectively never fires → the + controller will deploy apps the 2GB LXC cannot run. Memory bar UI uses the same value. +Severity: HIGH (defeats the deploy OOM guard; not merely cosmetic) +Repo / layer: controller — internal/system/info_linux.go +Fix approach: Add a cgroup-limit read used as min(cgroupLimit, meminfoTotal): cgroup v2 + /sys/fs/cgroup/memory.max ("max" → fall back), v1 /sys/fs/cgroup/memory/ + memory.limit_in_bytes (ignore the ~unbounded sentinel). For used/available use v2 + memory.current / v1 memory.usage_in_bytes. Wire into readMemInfo so TotalMemMB/ + AvailMemMB/UsedMemMB/MemPercent reflect the cap; all accessors (GetMemoryMB, + GetTotalMemoryMB, GetInfo) become correct with no signature change. +Effort: S +Risk: unattended-safe (read-only metric change in the controller). +Depends on / blocks: independent. +Proof of fix: unit test with a fake cgroup file → reports the limit; on the 2GB guest /api/system/info + shows ~2048; deploy hard-block triggers when committed mem approaches 2GB. +Open question: none. +``` + +--- + +## MEDIUM — specs + +### F11 — `needs_hdd` app deploys onto the OS rootfs with no drive-class guard +``` +Report claim: "HDD-required app silently lands on the 32G OS disk" +Re-diagnosis: real +True cause: Path validation is existence-only: stacks/deploy.go:278-283 (os.Stat of the value). + isValidDrivePath (web/handler_export.go:356-364) only checks string-equality against a + registered StoragePath. StoragePath has no drive-class attribute; the deploy dropdown + (web/handlers.go:322-333 from GetSchedulableStoragePaths) has no class filter. NeedsHDD + (stacks/metadata.go:58) is a UI badge only, never cross-checked against the chosen + path's physical drive. sys_drive is the SSD fallback namespace + (config.go:265 /mnt/sys_drive; appbackup/paths.go:24-29 + backup.go:105-107). +Severity: MEDIUM (data-placement footgun; on a real customer it fills the OS disk) +Repo / layer: controller — internal/stacks (deploy) + internal/web (dropdown) + internal/settings +Fix approach: Classify each StoragePath by backing device (compare Stat_t.Dev against the rootfs + device; system.SamePhysicalDevice already exists for Tier-2, and agentapi exposes + durable-id/device info). In DeployStack after the path check (deploy.go:283): if + meta.Resources.NeedsHDD and the chosen path resolves to the OS/rootfs device → return + a deployWarning (the function already has that channel, :160/:191) or refuse. Annotate + the dropdown likewise. +Effort: M +Risk: unattended-safe (controller-only). +Depends on / blocks: best AFTER F9 (so a real HDD class exists to steer toward); works standalone as a + warning even now. +Proof of fix: deploy a needs_hdd app with only sys_drive available → warning/refusal surfaced. +Open question: warn-and-allow vs hard-refuse when no real HDD exists? Recommend warn-and-allow (demo/ + single-SSD nodes are legitimate). Minor product call. +``` + +### F20-BUG2 — durable_id scheme mismatch: `/disks` emits `uuid:…`, the wipe gate expects `byid:`/`byuuid:…` +``` +Report claim: "durable_id scheme mismatch across endpoints; customer copying the disk-list id is refused" +Re-diagnosis: real +True cause: List path: storage/durableid.go:54-63 emits `uuid:`+fs-uuid for usb/local-dir + (→ DiskInfo.DurableID in /disks). Gate path: storage/durable_device.go:34-51 + DeviceDurableID returns `byid:`+wwn (:43) or `byuuid:`+uuid (:47); ResolveDurableDevice + (:55-74, the AGENT-001 anti-retarget resolver) only accepts byid:/byuuid: and refuses a + bare/uuid: scheme. Gate compares the customer's ConfirmDurableID against DeviceDurableID + (localapi/disks.go:450,454-457) → binding_mismatch. Three prefixes (uuid: vs byid: vs + byuuid:) from two functions. +Severity: MEDIUM (usability: the documented confirm flow can't succeed with the advertised id) +Repo / layer: agent — internal/storage (durableid.go + durable_device.go); reflected in controller +Fix approach: Make the disk-LIST advertise the SAME durable-id the gate will accept (the byid:/byuuid: + scheme from DeviceDurableID), or include both an `id` (display) and a `wipe_durable_id` + (gate-accepted) field so the UI/customer always confirm with the gate's scheme. Do NOT + relax the gate to accept uuid: (that weakens AGENT-001 anti-retarget). Single source of + truth: have handleDisks call DeviceDurableID for the wipe id. +Effort: M +Risk: SUPERVISED — touches the wipe binding semantics; validate against AGENT-001 invariants. +Depends on / blocks: same files as F20-BUG3 region (agent storage/localapi) — sequence with Batch 3. +Proof of fix: read /disks id → POST /disks/format confirmed with it → gate authorizes (no + binding_mismatch) on a scratch device. +Open question: expose one canonical id or a display+wipe pair? Recommend the pair (UI clarity). +``` + +### F8 — Infra secrets in `controller.yaml` are plaintext at `0644` +``` +Report claim: "cf_api_token / cf_tunnel_token / hub api_key plaintext (not enc: like app secrets)" +Re-diagnosis: real, but largely by-design for runtime config; the concrete weakness is file perms +True cause: config/config.go fields (CFTunnelToken/CFAPIToken :72-75, HubConfig.APIKey :175, + GitConfig.Token :96, WebConfig.SessionSecret :88) are plain yaml; loadAndParse + (:206-224) does ReadFile→ExpandEnv→Unmarshal, no decryption. App.yaml secrets ARE + enc:-wrapped (deploy.go:683-692). Config is written 0644 on apply (router.go:1007). +Severity: LOW-MEDIUM (these are runtime-required creds the guest legitimately holds; the file + mode is the real gap) +Repo / layer: controller — internal/config / config apply +Fix approach: Write/chmod controller.yaml to 0600. (Encrypting at rest with a key co-located on the + same rootfs adds little; if wanted later, derive the key from an env-injected secret, + not a file on the guest.) +Effort: S +Risk: unattended-safe. +Depends on / blocks: independent. +Proof of fix: stat controller.yaml → 0600. +Open question: Is at-rest encryption of infra creds a requirement, or is 0600 sufficient given the + threat model (guest-rootfs compromise already = game over)? Recommend 0600 now. +``` + +--- + +## LOW / TRIVIAL — brief specs + +### F6 — Deploy POST returns `"deployed"` before compose finishes (async by design; message misleads) +``` +Re-diagnosis: real (by design). router.go:381 returns 200 after validation; DeployStack launches + compose in a goroutine (deploy.go:339) — the documented anti-stale-button pattern; UI + polls GET /api/stacks/{name} every 3s. Only the WORDING is wrong. +Fix: change message to "Telepítés elindítva" / return 202 Accepted. Do NOT make it sync. +Severity: LOW. Effort S. unattended-safe. controller internal/api. +``` + +### F7 — Dashboard state lags Docker health by ~10s +``` +Re-diagnosis: real (cache cadence). status-refresh ticker is 30s (cmd/controller/main.go:256); + stack-scan 2min (:259); the dashboard list serves the in-memory map. Health is read + correctly (manager.go:437), just polled slowly. (Deploy page already polls per-stack 3s.) +Fix: lower status-refresh to ~10s, and/or fire an extra RefreshStatus() shortly after + deploy/start completes (runComposeDeploy already calls it once at :401). +Severity: LOW. Effort S. unattended-safe. controller. +``` + +### F4 — `GET /api/stacks/rescan` → "stack not found: rescan" +``` +Re-diagnosis: real but trivial. A rescan route EXISTS but POST-only (router.go:112 rescanStacks); + GET falls through to GET /stacks/{name} (:116) → getStack("rescan") → "not found" (:314). + Reachable via POST /api/stacks/rescan or POST /api/sync. +Fix: add a default 405 for /stacks/rescan on non-POST so it doesn't masquerade as a stack + lookup; fix the runbook to use POST. (Doc-first.) +Severity: TRIVIAL. Effort S. unattended-safe. controller. +``` + +### F3 — JSON API shows double-encoded Hungarian (`SzemĂ©lyes`) +``` +Re-diagnosis: MIS-ATTRIBUTED / not-in-code → needs-runtime-confirmation. Source .felhom.yml is clean + single-UTF-8 (verified hexdump); yaml.Unmarshal (metadata.go:145) and + json.NewEncoder (router.go:1079) are UTF-8-native; git-sync copies bytes verbatim + (sync.go:379-411). No transcoding exists. The rendered HTML being correct (same source + string) is strong evidence the bytes aren't actually doubled — the mojibake is most + likely a terminal/curl display artifact in the live measurement, OR a stale cache. +Fix: none in code. Re-measure: `curl .../api/stacks | xxd | grep -i saj` and compare bytes to + source; only if doubled, re-sync the catalog cache on the guest. +Severity: LOW. needs-runtime-confirmation. +``` + +### F2 — `hdd_configured:false` disagrees with `/api/disks` listing the HDD +``` +Re-diagnosis: already-fine (expected symptom of F9, not a bug). hdd_configured comes from the + controller's own config plane: system/info_linux.go:32-35 (true iff hddPath != ""), + fed by cfg.Paths.HDDPath (router.go:711). /api/disks is the agent's HOST view. They + legitimately differ because the HDD is host-present but NOT guest-bound and NOT in + controller.yaml. +Fix: no independent fix; the F9 "guest_attached" reporting field resolves the user-facing + confusion. Optionally make /api/system/info reflect any registered guest storage path + (it currently ignores GetStoragePaths fallback that monitor/report use). +Severity: LOW (folds into F9). +``` + +### F15 — Controller not auto-restarted after a manual `docker stop`/`kill` +``` +Re-diagnosis: already-fine / standard semantics. Golden bakes `docker run --restart unless-stopped` + (felhom-agent/configs/build-golden.sh:179) — restarts on crash/daemon-restart but NOT + after a manual stop (by design). The systemd unit is Type=oneshot/RemainAfterExit + (:189-204) — runs the bootstrap once, not a supervisor. Genuine crashes ARE covered. +Fix: none required. If "survives manual stop" becomes a requirement: add a periodic + agent/hub desired-state reconcile that re-asserts the container is running (preferred + over fighting unless-stopped with systemd Restart=always). +Severity: LOW (acceptable as-is). +``` + +### F13 — 3-2-1 weakened: primary recovery units share the app's disk +``` +Re-diagnosis: already-fine as code; pure consequence of F9. Backup path helpers + (appbackup/paths.go:31-70) root the primary unit at the app's own nsRoot by design; + the OFF-drive copy is Tier-2 (internal/backup/tier2.go) which already targets a + different physical disk (system.SamePhysicalDevice) once one exists. +Fix: no path-helper change. Resolved by F9 (attach a real 2nd drive → Tier-2 has an off-disk + target). Until then Tier-2 correctly reports "needs 2nd drive". +Severity: MEDIUM consequence, but no independent action (→ F9). +``` + +### PASS findings — confirmed working, no action +``` +F10 (crash-loop detection), F12 (lifecycle ops), F14 (CTRL-T2-1 crash-window), F16 (import/export + +CTRL-001 traversal defense), F18 (removal + protected-stack guards), F19 (monitoring/settings/sync/ +hub reporting). Confirmed real PASSes against code + the report's evidence. Residual sub-issues that +DID get specs: F6 (optimistic POST wording), F11 (OS-disk footgun), F7 (state lag). No other residuals. +``` + +--- + +## SEQUENCED FIX PLAN + +### Batch 1 — quick contained wins → ship as controller **v0.61.0** (all S, unattended-safe) +- **F1** (cgroup memory read — `system/info_linux.go`) — restores the deploy OOM guard; highest value-per-effort. +- **F20-BUG1** (surface agent format error — `agentapi/client.go: FormatDisk`) — stop silent destructive-op "success". +- **F6** (deploy message wording / 202 — `api/router.go`). +- **F7** (status-refresh cadence 30s→10s — `cmd/controller/main.go`). +- **F8** (controller.yaml → 0600 — config apply). +- **F4** (405 for non-POST `/stacks/rescan` — `api/router.go`). +- **Ship alongside (separate repo, auto-syncs): F5 uptime-kuma** healthcheck fix in `app-catalog-felhom.eu` — delete the broken override. +- *Grouping rationale:* all controller-internal, no agent/provisioning, no destructive-path behaviour change, independent files. Each is independently testable and low-risk. +- *File-collision warnings:* **F4 and F6 both edit `internal/api/router.go`** (different cases — coordinate one PR). **F20-BUG1 edits `agentapi/client.go: FormatDisk`, which Batch-3 F20-BUG3 will re-architect** — land BUG1 first (small error-surface change), and have BUG3 build on it (BUG3's async path must keep the error-surfacing). + +### Batch 2 — controller, needs-spec (M/L), no live host risk +- **F17** (replay `.sql` on per-app restore; extract shared `dbrestore` pkg from `appexport`; make volume-restore failure non-silent). The CRITICAL data-recovery fix. +- **F11** (drive-class guard in `DeployStack` + dropdown). +- *Sequencing:* F17 first (it's the critical correctness fix). F11 is best AFTER F9 lands (real HDD class to steer toward) but can ship a warning now. +- *File-collision warnings:* **F17 co-edits `internal/backup/restore_unit.go` + `restore.go` + extracts from `internal/appexport/restore.go`** — keep this as one PR; nothing else in Batch 1/2 touches `internal/backup` or `internal/appexport`. F11 touches `internal/stacks/deploy.go` + `internal/web/handlers.go` + `internal/settings` — disjoint from F17. +- *Runtime-confirm before/with F17:* verify whether `POST /backup/run` actually writes volume tars to `/volume-dumps/` (the live drive saw none for romm). If capture is also missing for some apps, the F17 fix must ensure capture, not just replay. + +### Batch 3 — foundational, SUPERVISED (agent / provisioning / golden / destructive) +- **F9** (provisioning auto-re-enroll user-data binds + `guest_attached` reporting field) — agent `reconcile` + `localapi` + golden/bootstrap + small controller field. Unblocks data-migration, F13 (3-2-1), and the real-HDD half of F11/F2. +- **F20-BUG3** (async detached mkfs + `/disks/format/status`; run under `s.baseCtx`; preserve AGENT-001 re-resolve) — agent destructive path + controller `agentapi` polling. +- **F20-BUG2** (durable_id scheme consistency: `/disks` advertises the gate-accepted id) — agent `storage`. +- *Why last + supervised:* all touch the host agent / LXC config / destructive wipe path and need live validation on scratch guests/devices, not unattended pushes. They also carry the highest blast radius (provisioning, disk-wipe). +- *File-collision warnings:* **F20-BUG2 and F20-BUG3 both touch `felhom-agent/internal/storage` + `internal/localapi/disks.go`** (durable-id derivation + the format handler) — do them in one coordinated agent PR. **F20-BUG3 re-edits controller `agentapi/client.go: FormatDisk`** already changed in Batch 1 (BUG1) — rebase BUG3 on the BUG1 error-surfacing. **F9 touches `reconcile/bringup.go` + `localapi/disks.go` + `guestbind.go`** — `disks.go` is also touched by F20-BUG2/3, so sequence F9 and the F20 agent work to avoid stepping on `disks.go` (recommend: F9 first, then F20-BUG2/BUG3 rebased). + +--- + +## WRAP-UP + +**Re-diagnosis verdict counts:** real = **11** (F1, F5, F9, F11, F17, F20-BUG1, F20-BUG2, F20-BUG3, F6, F7, F8); mis-attributed = **3** (F20-BUG1 relocated to controller, F17 volume-capture sub-claim, F3 not-in-code); already-fine/no-independent-action = **9** (F10, F12, F14, F16, F18, F19, F13, F2, F15); needs-runtime-confirmation = **2** (F3 byte recheck, F17 capture-side volume-tar question); trivial/doc = **1** (F4). + +**Batch 1 (ship first, v0.61.0):** F1 (cgroup memory → restores deploy OOM guard), F20-BUG1 (stop silent format "success"), F6 (deploy message wording), F7 (status cadence), F8 (controller.yaml 0600), F4 (405 on rescan) + F5 uptime-kuma healthcheck in the catalog repo. + +**Report-causes found WRONG against the code (the key output):** +- **F20-BUG1** — report blamed the *agent's* format handler; the agent correctly returns 502. The bug is the *controller's* `agentapi.FormatDisk` (`client.go:393-397`) swallowing the agent's error and returning a zero-value `ok:true`. +- **F17** — report said "recovery units don't capture volume data"; they do (`restoreDockerVolumes` + `VolumeDumps`). The real gap is the captured `.sql` dump is **never replayed** on per-app restore (and the DB-import logic already exists, unused, in `appexport`). +- **F3** — report said the JSON API double-encodes Hungarian; no transcoding exists in the code path (source is clean UTF-8, encoder is UTF-8-native, sync copies bytes). Most likely a terminal/`curl` display artifact — re-measure with `xxd`. +- (Refinement, not contradiction) **F9** — the disk APIs don't "misreport"; they faithfully describe *host* presence. The real defect is the *absence* of a guest-attached signal, plus provisioning never binding user-data drives. + +**Open design questions needing the operator's decision before coding:** +1. **F5 (Traefik/unhealthy):** publish routes for unhealthy containers, keep gating, or keep gating but surface "route unpublished because unhealthy" distinctly? (Recommend the last.) +2. **F9 (re-enroll):** may bring-up auto-re-assert previously-enrolled user-data binds unattended, or must an operator confirm each data-path bind? (Recommend auto for durable-id-matched intents.) +3. **F17 (double-apply):** when an app has BOTH a named-volume tar and a `.sql` dump, which wins? (Recommend `.sql` replay for logical consistency.) +4. **F11 (no-HDD nodes):** warn-and-allow vs hard-refuse a needs_hdd app on a single-SSD node? (Recommend warn-and-allow.) +5. **F20-BUG3:** must an in-progress format survive an agent restart (persisted job record)? (Recommend yes, reuse the backup-record pattern.) +6. **F8:** is at-rest encryption of infra creds required, or is `0600` sufficient under the current threat model? (Recommend 0600 now.)