From 7ccbaff36bf03e3f1982e8ad4934d64a7d02e3b5 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sat, 13 Jun 2026 23:01:30 +0200 Subject: [PATCH] =?UTF-8?q?fix/m19:=20notes=20=E2=80=94=20LIVE=20stackname?= =?UTF-8?q?=20misattribution=20edge=20+=20fix=20plan=20(not=20deployed)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- FIX-M19-NOTES.md | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 FIX-M19-NOTES.md diff --git a/FIX-M19-NOTES.md b/FIX-M19-NOTES.md new file mode 100644 index 0000000..555549b --- /dev/null +++ b/FIX-M19-NOTES.md @@ -0,0 +1,56 @@ +# fix/m19-stackname-crossref — NOTES (pending review, NOT deployed) + +**Verdict:** LIVE @ `controller/internal/appbackup/dbdump.go:536-551`, used at `:115` (commit `6953899`). +**Class:** correctness edge — **low real-world incidence** with the current catalog. **Escape-hatch branch** +— the clean fix injects the deployed-stack list into `appbackup` (cross-package), not forced unattended. + +## The bug (verified mechanism) + +`deriveStackName(containerName)` pure-suffix-strips: it splits on `-` and, if the last segment is in +`{postgres,db,mariadb,mysql,database,redis,cache}`, returns the join of the remaining parts. It never +cross-references actual deployed stack names. `DiscoverDatabases` assigns `StackName: deriveStackName(name)` +directly (line 115). + +So a stack whose real name **ends** in one of those tokens is misattributed: +- a stack literally named `my-cache` → its DB container `my-cache` (or `my-cache-postgres`) derives to + `my` / `my-cache`, attributing the dump to the wrong (or a non-existent) stack. +- worse, `a-db` and `a` could collide. + +## Impact + +A DB dump is filed under the wrong stack name → that stack's backup/restore accounting is wrong, and the +restore-by-stack path could miss or cross-wire the dump. **Incidence is effectively zero in the current +felhom catalog** (stack slugs are `romm`, `nextcloud`, `paperless-ngx`, `immich`, `adventurelog`, +`actualbudget`, `mealie`, `vikunja`, … — none ends in a DB-role token; DB containers are `-postgres` +etc., which strip correctly). It becomes real only if a future app slug ends in a role token. + +## Fix plan (implementable) + +1. Thread the set of **known deployed stack names** into discovery: + `DiscoverDatabases(ctx, logger, debug, knownStacks []string)` and + `deriveStackName(containerName string, known map[string]bool)`. + Source the list from the caller in `backup.go` (it holds the `StackDataProvider` — expose/known stack + names via a lookup func to avoid importing `stacks` into `appbackup` and creating a cycle). +2. New `deriveStackName` logic: + - candidate := current suffix-strip result. + - if `known[candidate]` → use it (the suffix was a real DB-role suffix of a real stack). + - else if `known[containerName]` → the container name IS the stack (don't strip). + - else → longest `known` stack name that is a prefix of `containerName` (handles `_postgres`, + `-1`, compose-suffixed names); tie-break to the longest match. + - else → fall back to the current suffix-strip (preserve today's behaviour when the stack list is + unavailable/empty, so nothing regresses). +3. Keep a nil/empty-`known` fast path = today's behaviour (back-compat for other callers/tests). + +## Test plan (regression) + +- Table test for `deriveStackName` with `known = {romm, my-cache}`: + - `romm-postgres` → `romm` (suffix is a role; `romm` is known). + - `my-cache` → `my-cache` (known as-is; must NOT strip to `my`). ← fails on pre-fix code. + - `my-cache-postgres` → `my-cache` (strip role, result known). + - `unknown-db` with no matching known → falls back to `unknown` (today's behaviour). + +## Why not tonight + +Requires plumbing the deployed-stack list across the `backup`→`appbackup` boundary (cycle-avoidance via a +lookup func) and touching `DiscoverDatabases`'s signature + caller. Low-incidence, so not worth a risky +unattended change. The plan + tests above are complete for the supervised session.