diff --git a/CHANGELOG.md b/CHANGELOG.md index 819951a..6110656 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,61 @@ +## v0.207.0 — a password stops living in the page source, and two refusals learn to say what to do (2026-08-08, R-249/R-252/R-253) — MinAgent 0.127.0 + +Three items the fifth walk exposed by passing. None of them touches the recovery path it proved; all +three are about what the product *says*. + +### 1. The retrieval passphrase leaves the page body (R-249) + +`settings_security.html` rendered the passphrase into a `display:none` span behind a „Megjelenít" +button. **That toggle stops a browser DRAWING the value and nothing else** — the plaintext was in the +response body of every render, so a `curl` of the page returned it. It was found by doing exactly +that: it landed in a session transcript on 2026-08-07 while driving the documented rebuild path. + +**The product already had this rule and this page did not follow it.** `escrow_handlers.go` states it +for the recovery code — *"reveal (claim XHR only — R is NEVER templated server-side into HTML)"*. The +passphrase now follows the same shape: the page carries only `HasRetrievalPassword`, and the value +comes from **`POST /settings/retrieval-password/reveal`**, behind the same RequireAuth + CsrfProtect +every other POST sits behind, `Cache-Control: no-store`, **and logged as an act** — reading it off the +markup left no trace anywhere, where the hub's equivalent break-glass reveal has always emitted an +event. + +**POST for a read, deliberately:** a GET would be re-fetchable from history, pre-fetchable, cacheable, +and — since CsrfProtect only covers unsafe methods — uncovered by CSRF. + +**The test asserts the raw response body, not a rendered view**, because that is precisely why this +survived: every test that asked what the customer *sees* passed while the bytes carried the secret. + +**Census (§7.1), reported not fixed:** the render-then-hide pattern appears **twice more** — +`deploy.html` (an auto-generated app secret in a `type="password"` input's `value=`; unavoidable on +the pre-deploy form, which must post it, but not on an already-deployed app's page) and +**`app_info.html`, which puts a per-install generated app password inside a `hidden` span** — the same +shape with a real secret. Filed as **R-254**. + +### 2. „nincs elérhető adatmeghajtó" now names the reason and the route (R-252) + +A rebuilt box's drives survive; their **registration** does not. Every restore then refused with a +sentence that named no next step and read like data loss. The restore page now states the precondition +**before** the customer presses anything, says the backups and the drives are both still there, and +links to Tárhely → Meghajtók. The refusal string says the same. + +The page asks the question through the backup manager's own `HasRestoreDestination()`, which reads the +**same** `GetSchedulableStoragePaths()` the resolver reads — a second copy of that predicate is exactly +how a page ends up promising what the handler refuses, which is the next item. + +### 3. The page no longer promises a reinstall the restore cannot do (R-253) + +The restore list said **„Nincs telepítve — a visszaállítás előbb újratelepíti."** Three lines later the +restore refused *because* the app was not installed. Two shipped sentences, in the customer's own +language, contradicting each other at the last step of a recovery. + +**The promise was the wrong half, and this is why:** reconstitution writes to the app's own data path +(`GetStackHDDPath`), which exists only once the customer has chosen a drive at deploy time. An +automatic reinstall would mean the product picking that drive for them — the one decision this whole +recovery path exists to leave with the customer. So the copy now says to install it first and routes +to `/stacks//deploy`; the refusal was reworded to match. + +**A healthy box renders exactly as before** — both notices are conditional, and a test fails if either +becomes unconditional. + ## v0.206.0 — the box does not mint a key over a sealed package, and abandoning ends the question (2026-08-07, R-241) — MinAgent 0.127.0 **R-241 was ruled a MINTING defect, not a screen-predicate defect** (`SPIKE-r241-recovery-offer-2026-08-07.md`), diff --git a/CONTEXT.md b/CONTEXT.md index 3b694b0..659bc21 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,38 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-08-07 (v0.206.0 — R-241: the box does not mint a key over a sealed package) +Last updated: 2026-08-08 (v0.207.0 — R-249/R-252/R-253: a secret leaves the page source, two refusals learn to say what to do) + +> **2026-08-08 — v0.207.0 (R-249, R-252, R-253). Three things the fifth walk exposed BY PASSING.** +> The walk closed R-201 (both halves) on 2026-08-07; none of the below touches the recovery path it +> proved. +> +> **R-249 — a secret was living in the page source.** `settings_security.html` rendered the retrieval +> passphrase into a `display:none` span behind a „Megjelenít" button. That toggle stops a browser +> DRAWING it and nothing else: the plaintext was in the response body of every render. Found by doing +> exactly that — it landed in a session transcript while driving the documented rebuild path. +> **THE RULE, which the codebase already stated for R and this page did not follow:** a secret is +> revealed by an XHR, never templated server-side into HTML (`escrow_handlers.go`). The page now +> carries only `HasRetrievalPassword`; the value comes from `POST /settings/retrieval-password/reveal` +> — CSRF-covered, `no-store`, and **logged as an act**, which reading it off the markup never was. +> **The test asserts the RAW RESPONSE BODY** — every test that asked what the customer *sees* passed +> while the bytes carried the secret, and that is why it survived. +> **The census found two more instances** (`app_info.html`, a real per-install app password in a +> `hidden` span; `deploy.html`, a generated secret in a `value=`) — **filed as R-254, not fixed.** +> +> **R-252 / R-253 — the two obstacles, and the rule they share.** A rebuilt box keeps its drives but +> loses their REGISTRATION, so every restore refused with a sentence naming no next step; and the +> restore list promised „a visszaállítás előbb újratelepíti" three lines above a refusal that fired +> *because* the app was not installed. **The promise was the wrong half:** reconstitution writes to +> the app's own `GetStackHDDPath`, which exists only once the CUSTOMER has chosen a drive at deploy +> time — an automatic reinstall would mean the product making that choice for them, which is the one +> decision this recovery path exists to leave with them. Both now name a reason and route to the step +> that clears it, and both notices are conditional (a healthy box is byte-identical, pinned by a test +> that fails if either becomes unconditional). +> +> **The page and the resolver ask ONE question:** `HasRestoreDestination()` reads the same +> `GetSchedulableStoragePaths()` the scratch resolver reads. A second copy of that predicate is +> exactly how a page ends up promising what the handler refuses — which is R-253 itself. > **2026-08-07 — v0.206.0 (R-241). THE RULING, and it reversed the fix: this was a MINTING defect, > not a screen-predicate defect.** The recovery screen was telling the truth — there genuinely was diff --git a/REUSE.md b/REUSE.md index 65ef023..9d92a08 100644 --- a/REUSE.md +++ b/REUSE.md @@ -127,6 +127,7 @@ | `EncryptFile` / `DecryptFile` / `IsEncryptedFAB` | controller/internal/appexport/crypto.go | password-based file crypto | .fab export bundles | scrypt-derived AES+HMAC keys | | `maskRepoURL` | controller/internal/sync/sync.go | `(url) string` | Logging git URLs | Strips embedded credentials | | `metrics.RedactLine` | controller/internal/metrics/redact.go | `(s string) string` | ANY log line shipped off-box (issue context, log tails) | Masks password/passwd/secret/token/api-key/authorization/bearer values + 64-hex; apply BEFORE the line leaves the box — controller-side redaction is authoritative | +| `settingsRetrievalPasswordRevealHandler` | controller/internal/web/handlers.go | `POST /settings/retrieval-password/reveal` | **THE PATTERN for showing a secret in the UI** — an XHR that returns only the value | **Never template a secret into a page and hide it with CSS.** `display:none` / `hidden` / `type="password"` stop a browser DRAWING the value; the plaintext is still in the response body, so a `curl` of the page returns it, and it reaches caches, history and any screen-share of the source. R-249 shipped exactly that for two months and was found by it landing in a transcript. The page carries a **boolean** (`HasRetrievalPassword`); the value comes from a POST (CSRF-covered, uncacheable) and the reveal is **logged as an act**. `escrow_handlers.go` states the same rule for R. **Test on the RESPONSE BODY** — a test asserting what the customer *sees* cannot see this class at all. **Known live violations: `app_info.html` (per-install app password in a `hidden` span) and `deploy.html` (auto-generated secret in a `value=`) — R-254.** | ### Storage registry + mount detection diff --git a/controller/README.md b/controller/README.md index 56f63c5..398bd97 100644 --- a/controller/README.md +++ b/controller/README.md @@ -160,6 +160,15 @@ backups, monitoring and notifications. All Proxmox/disk operations are delegated snapshots sat in the repository. Installed-ness is a property OF a row (it changes what restoring implies), never a filter on it; an unreadable store renders as UNKNOWN and keeps the action; the `felhom-offbox` and `_shares` marker tags are never offered as apps. + **Its two preconditions now name a reason AND a route (v0.207.0, R-252/R-253):** a rebuilt box keeps + its drives but loses their REGISTRATION, so the page renders a notice — *„Előbb csatold vissza az + adatmeghajtót"*, linking to `/storage` — whenever `HasRestoreDestination()` is false, asked through + the backup manager's own predicate so page and resolver read the same `GetSchedulableStoragePaths()`. + And the not-installed row no longer promises *„a visszaállítás előbb újratelepíti"*: reconstitution + writes to the app's own `GetStackHDDPath`, which exists only after the customer picks a drive at + deploy time, so the copy says to install it first and links to `/stacks//deploy`. **Both notices + are conditional** — a healthy box renders exactly as before, pinned by a test that fails if either + becomes unconditional. **A run that skipped a selected app is `incomplete` (v0.205.0, R-234):** the off-site verdict now counts `missingUnprotected` beside `mandatoryGaps` — an app the customer selected that is DEPLOYED but has no recovery unit is not protected, so the run is not `ok`. A selected-but-UNDEPLOYED app is @@ -2537,6 +2546,8 @@ During setup wizard drive scan, both current and historical backups are discover Generates `recovery-info.txt` on the system data partition with customer ID, Hub URL, retrieval password, and recovery instructions in Hungarian. Updated on startup and after config changes. Also displayed on the Settings page in a "Vészhelyzeti információk" section. +**The retrieval passphrase is NOT rendered into that page (v0.207.0, R-249).** `securityPageData` passes only `HasRetrievalPassword` (a boolean), and the value is fetched by an explicit act: **`POST /settings/retrieval-password/reveal`** → `{"ok":true,"data":{"password":"…"}}`, behind RequireAuth + CsrfProtect like every other POST, `Cache-Control: no-store`, and logged (`retrieval passphrase revealed via the security page from ` — the value is never logged). Until v0.207.0 the page rendered the plaintext into a `display:none` span, so any fetch of the page returned it; the toggle was cosmetic. This follows the rule `escrow_handlers.go` already states for the recovery code: a secret is revealed by an XHR, never templated server-side into HTML. + ### 11. Disaster Recovery When a system drive fails and is replaced, the recovery flow uses the setup wizard: diff --git a/controller/internal/backup/offbox_reconstitute.go b/controller/internal/backup/offbox_reconstitute.go index 36ce0fa..6278eec 100644 --- a/controller/internal/backup/offbox_reconstitute.go +++ b/controller/internal/backup/offbox_reconstitute.go @@ -197,7 +197,14 @@ func (m *Manager) ReconstituteFromOffsite(ctx context.Context, stack string) (Of hdd := strings.TrimSpace(m.stackProvider.GetStackHDDPath(stack)) if hdd == "" { - return res, fmt.Errorf("a(z) %s nincs telepítve — előbb állítsd helyre az alkalmazást, utána az adatokat", stack) + // R-253: the same sentence the restore page now shows, so the page and the refusal cannot + // drift apart again. It is a REFUSAL, not a failure — the data is untouched and the customer + // has one step to take. The restore deliberately does NOT deploy the app itself: the + // destination is the app's own HDD path, which is a drive the CUSTOMER chooses at deploy + // time, and picking it for them is the decision this whole recovery path exists to leave + // with them. + return res, fmt.Errorf("a(z) %s nincs telepítve, ezért nincs hová visszaállítani az adatait — "+ + "telepítsd újra az alkalmazást (Alkalmazások), utána ez a visszaállítás működni fog", stack) } liveNs := m.namespaceRoot(hdd) diff --git a/controller/internal/backup/offbox_restore.go b/controller/internal/backup/offbox_restore.go index 4ce3f48..a232e37 100644 --- a/controller/internal/backup/offbox_restore.go +++ b/controller/internal/backup/offbox_restore.go @@ -171,7 +171,36 @@ func (m *Manager) offboxRestoreScratchDir(stack string) (scratch, nsRoot string, } } } - return "", "", fmt.Errorf("nincs elérhető adatmeghajtó a visszaállításhoz") + // R-252: name the reason AND the way to act on it. This refusal is what a rebuilt box hits — the + // drives are physically fine and still mounted, it is their REGISTRATION that the destroyed guest + // took with it — and until v0.207.0 it said only that a drive was missing, which reads like data + // loss and offers nothing to do. + return "", "", fmt.Errorf("nincs regisztrált adatmeghajtó, ezért nincs hová visszaállítani — " + + "a meghajtók megvannak, csak újra kell csatolni őket a Tárhely → Meghajtók oldalon, utána " + + "ez a visszaállítás működni fog") +} + +// HasRestoreDestination reports whether an offsite restore has anywhere on this box to write. +// +// R-252: the restore PAGE asks this question through the same helper the resolver answers it with, +// so the notice cannot appear on a box that would restore fine (Scenario E) nor stay hidden on one +// that would refuse. A second copy of the predicate is exactly how a page ends up promising what the +// handler then refuses — which is the neighbouring defect, R-253. +// +// It mirrors the resolver's BOX-level branches (2) and (3) — the schedulable storage paths. Branch +// (1), the app's own HDD path, is deliberately not consulted: an installed app's HDD path IS a +// registered storage path, so the two cannot disagree in practice, and where they could, erring +// toward showing the notice is erring toward telling the customer something true. +func (m *Manager) HasRestoreDestination() bool { + if m.settings == nil { + return false + } + for _, sp := range m.settings.GetSchedulableStoragePaths() { + if strings.TrimSpace(sp.Path) != "" { + return true + } + } + return false } // RestoreOffboxScratch restores an app's latest offsite snapshot to an on-data-drive scratch dir diff --git a/controller/internal/backup/restore_destination_test.go b/controller/internal/backup/restore_destination_test.go new file mode 100644 index 0000000..2473e2d --- /dev/null +++ b/controller/internal/backup/restore_destination_test.go @@ -0,0 +1,46 @@ +package backup + +import ( + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-252 — THE PAGE'S QUESTION AND THE RESOLVER'S ANSWER ARE THE SAME QUESTION. +// +// The restore page renders its "re-attach your drive" notice from HasRestoreDestination(); every +// restore refuses from offboxRestoreScratchDir(). If those two ever disagree the page either warns a +// box that would restore fine, or stays silent on one that cannot — and the second is R-252 exactly +// as the fifth walk met it. So this asserts the CONSEQUENCE (does the resolver refuse?) against the +// predicate the page renders from, on one fixture. +// +// RED-PROOF: make HasRestoreDestination return true unconditionally and the first block fails — the +// predicate claims a destination while the resolver refuses in the very next assertion. +func TestHasRestoreDestination_AgreesWithTheResolver(t *testing.T) { + m, sett := bareManager(t) + + // No registered storage path: the resolver refuses, and the predicate must say so. + if m.HasRestoreDestination() { + t.Error("HasRestoreDestination() is true with no registered storage path — the restore page " + + "would stay silent while every restore refuses") + } + if _, _, err := m.offboxRestoreScratchDir("calibre-web"); err == nil { + t.Fatal("the resolver found a destination with no registered storage path — fixture is wrong, " + + "so the agreement below would prove nothing") + } else if !strings.Contains(err.Error(), "Tárhely") { + t.Errorf("the refusal names no route: %v", err) + } + + // Re-attach a drive, exactly as the customer does from Tárhely → Meghajtók. + if err := sett.AddStoragePath(settings.StoragePath{Path: "/mnt/felhom-drives/adatok", Label: "Adatok", Schedulable: true}); err != nil { + t.Fatalf("register storage path: %v", err) + } + if !m.HasRestoreDestination() { + t.Error("HasRestoreDestination() is still false after the drive was re-attached — the notice " + + "would stay on screen after the customer fixed the thing it asked them to fix") + } + if _, _, err := m.offboxRestoreScratchDir("calibre-web"); err != nil { + t.Errorf("the resolver still refuses after a drive was registered: %v", err) + } +} diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index 91b1c1b..54a2230 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -1030,6 +1030,11 @@ func (s *Server) backupsRestoreHandler(w http.ResponseWriter, r *http.Request) { rows, state := s.offsiteRestoreRows(r.Context()) data["OffsiteRestoreRows"] = rows data["OffsiteStoreState"] = string(state) + // R-252: a rebuilt box's drives survive but their REGISTRATION does not, and every restore + // then refuses with a message that names no next step. Asked through the backup manager's own + // predicate so the page and the resolver cannot disagree. FALSE on a healthy box, where the + // template renders exactly as before (Scenario E). + data["NoRestoreDestination"] = !s.backupMgr.HasRestoreDestination() } s.executeTemplate(w, r, "backups_restore", data) } @@ -1576,8 +1581,23 @@ func (s *Server) notificationsPageData() map[string]interface{} { func (s *Server) securityPageData() map[string]interface{} { data := s.settingsBaseData("settings-security", "Biztonság és hozzáférés") - // Recovery info for emergency section - data["RetrievalPassword"] = s.settings.GetRetrievalPassword() + // Recovery info for emergency section. + // + // ⚠ R-249 — THE VALUE DOES NOT GO IN THE PAGE, AND THAT IS THE WHOLE FIX. + // + // Until v0.207.0 this line put the retrieval passphrase into the template data and + // `settings_security.html` rendered it into a `display:none` span behind a „Megjelenít" button. + // That toggle stops the browser DRAWING it and nothing else: the plaintext was in the response + // body of every render, so a `curl` of the page returned it — which is how it was found, by + // landing in a session transcript during the 2026-08-07 walk. It was therefore also in browser + // caches, in history, in any saved page and in any screen-share of the page source. + // + // The product already had this exact rule and this page did not follow it — `escrow_handlers.go` + // states it for the recovery code: *"reveal (claim XHR only — R is NEVER templated server-side + // into HTML)"*. The passphrase now follows the same shape: the page carries only whether one + // EXISTS, and the value comes from POST /settings/retrieval-password/reveal, which is an + // explicit authenticated act and is logged as one. + data["HasRetrievalPassword"] = strings.TrimSpace(s.settings.GetRetrievalPassword()) != "" data["HubURL"] = s.cfg.Hub.URL data["SupportEmail"] = "support@felhom.eu" data["SupportURL"] = "https://felhom.eu/kapcsolat" @@ -1611,6 +1631,37 @@ func (s *Server) securityPageData() map[string]interface{} { return data } +// settingsRetrievalPasswordRevealHandler — POST /settings/retrieval-password/reveal (v0.207.0, R-249). +// +// The ONLY path by which the retrieval passphrase reaches a browser. It is behind RequireAuth and +// CsrfProtect like every other POST on this mux, so reaching it takes a live session AND a token +// bound to it — where the old rendering took nothing but the ability to read a page the customer +// merely opened. +// +// WHY A POST FOR A READ, deliberately and not by accident: a GET would be re-fetchable from history, +// pre-fetchable by a browser, and loggable in any proxy's access log with the response cached. The +// act of revealing a secret is a state change in every sense that matters here, and CsrfProtect only +// covers unsafe methods — a GET would have no CSRF cover at all. +// +// `no-store` matters as much as the method: without it a back-navigation can re-present the response +// body from the disk cache, which is the same defect one layer down. +func (s *Server) settingsRetrievalPasswordRevealHandler(w http.ResponseWriter, r *http.Request) { + pw := strings.TrimSpace(s.settings.GetRetrievalPassword()) + if pw == "" { + // Not an error: a box that never stored one has nothing to reveal, and saying so is not a + // leak. The page does not offer the button in that case (HasRetrievalPassword gates it). + w.Header().Set("Cache-Control", "no-store") + escrowJSON(w, http.StatusNotFound, nil, "Ezen a gépen nincs tárolt visszaállítási jelszó.") + return + } + // The reveal is an event, and it was not one before: the same act on the hub's break-glass + // credential writes `recovery_credential_revealed`, while reading this value off the page markup + // left no trace anywhere. The VALUE is never logged — only that it was asked for, and by whom. + s.logger.Printf("[INFO] [web] retrieval passphrase revealed via the security page from %s (value never logged)", clientIP(r)) + w.Header().Set("Cache-Control", "no-store") + escrowJSON(w, http.StatusOK, map[string]any{"password": pw}, "") +} + func (s *Server) settingsHandler(w http.ResponseWriter, r *http.Request) { s.executeTemplate(w, r, "settings_system", s.systemPageData()) } diff --git a/controller/internal/web/offsite_restore_list_test.go b/controller/internal/web/offsite_restore_list_test.go index 24b5404..74e9447 100644 --- a/controller/internal/web/offsite_restore_list_test.go +++ b/controller/internal/web/offsite_restore_list_test.go @@ -146,9 +146,17 @@ func TestOffsiteRestoreList_RenderedForRebuiltBox(t *testing.T) { if !strings.Contains(html, `href="/backups/restore/app?name=calibre-web"`) { t.Errorf("a rebuilt box must be offered its stored app; the page did not render the entry") } - if !strings.Contains(html, "Nincs telepítve — a visszaállítás előbb újratelepíti.") { + // v0.207.0 (R-253): the INTENT of this assertion is unchanged — the not-installed consequence + // must be stated rather than hidden — but the sentence it pinned was a promise the restore could + // not keep ("a visszaállítás előbb újratelepíti"), and the reconstitute path refused on exactly + // this state. The copy now states the consequence AND routes to the step that clears it, so this + // asserts both halves rather than the old wording. + if !strings.Contains(html, "Nincs telepítve") || !strings.Contains(html, "telepítsd újra") { t.Errorf("the not-installed consequence must be stated, not hidden") } + if !strings.Contains(html, `href="/stacks/calibre-web/deploy"`) { + t.Errorf("the consequence is stated but the customer is not routed to the step that clears it") + } if strings.Contains(html, "Nincs távoli mentésre jelölt alkalmazás") { t.Errorf("the old toggle-keyed dead end is still rendered") } diff --git a/controller/internal/web/restore_preconditions_test.go b/controller/internal/web/restore_preconditions_test.go new file mode 100644 index 0000000..ed55663 --- /dev/null +++ b/controller/internal/web/restore_preconditions_test.go @@ -0,0 +1,93 @@ +package web + +import ( + "strings" + "testing" +) + +// R-252 / R-253 — A REFUSAL ON THE RECOVERY PATH NAMES A REASON A PERSON CAN ACT ON, AND THE WAY TO +// ACT ON IT. +// +// Both of these were met on the fifth walk (2026-08-07) at the LAST step of a successful recovery — +// the customer had entered their recovery code, seen their data listed, and pressed through. Neither +// needed a shell to clear, which is why the walk's journey half passed; both required knowing +// something the product never said, which is why they are filed. +// +// These assert the RENDERED page, not the state that feeds it: the defect in both cases was copy +// that disagreed with behaviour, and only the rendered bytes can show that. + +// restoreData is the healthy fixture — app in the store, installed here, drives registered. Every +// test below overlays exactly one field, so what it proves is attributable. +func restoreData() map[string]interface{} { + d := splitTestData() + d["NoRestoreDestination"] = false + d["OffsiteRestoreRows"] = []OffsiteRestoreRow{ + {App: "calibre-web", DisplayName: "Calibre-Web", InStore: true, Installed: true, Enabled: true}, + } + return d +} + +// ── SCENARIO C — a refusal for a missing drive says what to do ────────────────────────────────── + +// RED-PROOF: restore the old copy — delete the `{{if .NoRestoreDestination}}` block from +// backups_restore.html — and this fails on both assertions: the customer is left with a page that +// looks normal and a button that will refuse with a message naming no next step. +func TestRestorePage_NoRegisteredDrive_NamesTheReasonAndTheRoute(t *testing.T) { + d := restoreData() + d["NoRestoreDestination"] = true + html := renderBackupPage(t, "backups_restore", d) + + if !strings.Contains(html, "csatold vissza az adatmeghajtót") { + t.Error("R-252: the restore page says nothing about the drives being unregistered — the " + + "customer presses through and gets a refusal that names no next step") + } + if !strings.Contains(html, `href="/storage"`) { + t.Error("R-252: the notice does not ROUTE to the place that fixes it — a reason without a " + + "route is what made this an obstacle rather than a message") + } + // It must say the data is safe. „nincs elérhető adatmeghajtó" reads like data loss; it is not. + if !strings.Contains(html, "megvannak") { + t.Error("R-252: the notice does not say the backups and the drives are both still there") + } +} + +// ── SCENARIO E — a healthy box is unchanged ───────────────────────────────────────────────────── + +// RED-PROOF: make the notice unconditional (drop the `{{if .NoRestoreDestination}}` guard) and this +// fails — a box with every precondition satisfied is warned about a problem it does not have, which +// is the R-215 shape (a screen about a situation the customer is not in). +func TestRestorePage_HealthyBox_HasNoPreconditionNotice(t *testing.T) { + html := renderBackupPage(t, "backups_restore", restoreData()) + + if strings.Contains(html, "csatold vissza az adatmeghajtót") { + t.Error("the drive notice rendered on a box whose drives ARE registered") + } + if strings.Contains(html, "előbb") && strings.Contains(html, "telepítsd újra") { + t.Error("the not-installed copy rendered for an app that IS installed") + } +} + +// ── SCENARIO D — the page and the handler agree about reinstalling ────────────────────────────── + +// RED-PROOF: restore the old string „Nincs telepítve — a visszaállítás előbb újratelepíti." and this +// fails on the first assertion — the page promises a reinstall the reconstitute path refuses to do, +// which is the contradiction R-253 filed. +func TestRestorePage_NotInstalled_DoesNotPromiseAReinstall(t *testing.T) { + d := restoreData() + d["OffsiteRestoreRows"] = []OffsiteRestoreRow{ + {App: "calibre-web", DisplayName: "Calibre-Web", InStore: true, Installed: false, Enabled: true}, + } + html := renderBackupPage(t, "backups_restore", d) + + if strings.Contains(html, "a visszaállítás előbb újratelepíti") { + t.Error("R-253: the page still promises that the restore reinstalls the app — the " + + "reconstitute path refuses precisely because it is not installed, and cannot deploy it " + + "itself (the destination is the app's own HDD path, a drive the CUSTOMER chooses)") + } + if !strings.Contains(html, "telepítsd újra") { + t.Error("R-253: the page no longer tells the customer to install the app first") + } + if !strings.Contains(html, `href="/stacks/calibre-web/deploy"`) { + t.Error("R-253: the copy names the step but does not route to it") + } +} diff --git a/controller/internal/web/retrieval_password_exposure_test.go b/controller/internal/web/retrieval_password_exposure_test.go new file mode 100644 index 0000000..0577023 --- /dev/null +++ b/controller/internal/web/retrieval_password_exposure_test.go @@ -0,0 +1,147 @@ +package web + +import ( + "bytes" + "io" + "log" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" +) + +// R-249 — THE PASSPHRASE MUST NOT BE IN THE RESPONSE BODY OF A PAGE THE CUSTOMER MERELY OPENS. +// +// WHY THESE TESTS ASSERT ON THE BODY AND NOT ON A RENDERED VIEW, which is the whole reason the +// defect survived: the old markup put the value inside ``. Every test +// that asked "what does the customer SEE" passed, because a browser drew asterisks. The value was +// in the bytes the whole time, and a `curl` of the page returned it — which is how it was found, by +// landing in a session transcript on 2026-08-07. +// +// So: render the real page and search the raw HTML. A test that cannot see a display:none span +// cannot see this defect at all. + +const testRetrievalPassphrase = "edeni-oshalom-disztok-harul-Zsolna-TESTONLY" + +// securityHarness builds a Server complete enough for securityPageData, which also reads the stack +// list for the geo per-app override selector. +func securityHarness(t *testing.T) *Server { + t.Helper() + dir := t.TempDir() + lg := log.New(io.Discard, "", 0) + cfg := config.Default() + cfg.Customer.ID = "c1" + cfg.Customer.Domain = "example.hu" + cfg.Paths.StacksDir = filepath.Join(dir, "stacks") + cfg.Paths.DataDir = filepath.Join(dir, "data") + cfg.Web.SessionSecret = "test-session-secret-abcdef" + sett, err := settings.Load(filepath.Join(dir, "settings.json"), lg) + if err != nil { + t.Fatalf("settings: %v", err) + } + mgr, err := stacks.NewManager(cfg, lg) + if err != nil { + t.Fatalf("stacks: %v", err) + } + return &Server{cfg: cfg, settings: sett, stackMgr: mgr, logger: lg, version: "test"} +} + +// renderSecurityPage drives the REAL page-data builder and the REAL template, and returns the bytes +// a browser would receive. +func renderSecurityPage(t *testing.T, passphrase string) string { + t.Helper() + s := securityHarness(t) + if passphrase != "" { + if err := s.settings.SetRetrievalPassword(passphrase); err != nil { + t.Fatalf("seed retrieval password: %v", err) + } + } + s.loadTemplates() + var buf bytes.Buffer + if err := s.tmpl.ExecuteTemplate(&buf, "settings_security", s.securityPageData()); err != nil { + t.Fatalf("render settings_security: %v", err) + } + return buf.String() +} + +// ── SCENARIO A — the password is not in the page ──────────────────────────────────────────────── + +// RED-PROOF (run and confirmed failing before the fix was restored): put the value back in the page +// data and the template — +// +// handlers.go: data["RetrievalPassword"] = s.settings.GetRetrievalPassword() +// template: {{.RetrievalPassword}} inside the display:none span +// +// — and this test fails on the first assertion, printing the plaintext's presence in the body. That +// is the defect, reproduced. The mutation was applied and observed failing; see the session report. +func TestSecurityPage_DoesNotContainTheRetrievalPassphrase(t *testing.T) { + html := renderSecurityPage(t, testRetrievalPassphrase) + + if strings.Contains(html, testRetrievalPassphrase) { + t.Error("R-249: the retrieval passphrase is in the response body of the security page — " + + "a fetch of this page returns the plaintext, and the reveal toggle only stops a " + + "browser DRAWING it (this is the defect, and it is invisible to any test that asserts " + + "on what is displayed)") + } + // The card must still be there — the fix is to remove the VALUE, not the feature (Scenario B). + if !strings.Contains(html, "Visszaállítási jelszó") { + t.Error("the recovery-info card vanished — the fix must not remove the customer's access, " + + "only the value from the markup") + } + if !strings.Contains(html, `id="retrieval-pw-btn"`) { + t.Error("no reveal control rendered, so the customer has no way to obtain the passphrase at all") + } +} + +// The card is gated on EXISTENCE, and existence is not the value. A box with no stored passphrase +// must not render the card — the old gate was `{{if .RetrievalPassword}}`, which read the secret to +// decide whether to show the secret. +func TestSecurityPage_NoCardWhenNoPassphraseStored(t *testing.T) { + html := renderSecurityPage(t, "") + if strings.Contains(html, "Visszaállítási jelszó") { + t.Error("the recovery-info card rendered on a box with no stored retrieval passphrase") + } +} + +// ── SCENARIO B — the customer can still get it ────────────────────────────────────────────────── + +// RED-PROOF: delete the `/settings/retrieval-password/reveal` case from server.go (or make the +// handler return 404 unconditionally) and this fails — the customer is shown unable to obtain the +// passphrase at all, which is the wrong fix for Scenario A. +func TestRevealEndpoint_ReturnsThePassphraseToAnAuthenticatedCaller(t *testing.T) { + s := securityHarness(t) + if err := s.settings.SetRetrievalPassword(testRetrievalPassphrase); err != nil { + t.Fatalf("seed: %v", err) + } + rr := httptest.NewRecorder() + s.settingsRetrievalPasswordRevealHandler(rr, httptest.NewRequest("POST", "/settings/retrieval-password/reveal", nil)) + + if rr.Code != 200 { + t.Fatalf("reveal returned %d, want 200 — the customer cannot get their own passphrase", rr.Code) + } + if !strings.Contains(rr.Body.String(), testRetrievalPassphrase) { + t.Error("the reveal endpoint did not return the passphrase — Scenario A's fix must not " + + "protect the secret by removing the customer's access to it") + } + // A cached reveal is the same defect one layer down: a back-navigation would re-present the body. + if got := rr.Header().Get("Cache-Control"); !strings.Contains(got, "no-store") { + t.Errorf("reveal response Cache-Control = %q, want no-store", got) + } +} + +// A box with nothing stored answers cleanly rather than leaking the distinction as a 500. +func TestRevealEndpoint_404sWhenNothingStored(t *testing.T) { + s := securityHarness(t) + rr := httptest.NewRecorder() + s.settingsRetrievalPasswordRevealHandler(rr, httptest.NewRequest("POST", "/settings/retrieval-password/reveal", nil)) + if rr.Code != 404 { + t.Errorf("reveal on a box with no passphrase returned %d, want 404", rr.Code) + } + if strings.Contains(rr.Body.String(), testRetrievalPassphrase) { + t.Error("the empty-case response carried a passphrase") + } +} diff --git a/controller/internal/web/server.go b/controller/internal/web/server.go index 82c292e..c77dd19 100644 --- a/controller/internal/web/server.go +++ b/controller/internal/web/server.go @@ -493,6 +493,10 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { s.settingsSecurityPageHandler(w, r) case path == "/settings/password" && r.Method == http.MethodPost: s.settingsPasswordHandler(w, r) + // R-249: the retrieval passphrase is fetched by an explicit authenticated act, never templated + // into the security page. POST (not GET) so it is CSRF-covered and uncacheable — see the handler. + case path == "/settings/retrieval-password/reveal" && r.Method == http.MethodPost: + s.settingsRetrievalPasswordRevealHandler(w, r) case path == "/settings/notifications" && r.Method == http.MethodPost: s.settingsNotificationsHandler(w, r) case path == "/settings/notifications/test" && r.Method == http.MethodPost: diff --git a/controller/internal/web/templates/backups_restore.html b/controller/internal/web/templates/backups_restore.html index e00c9b9..2f33aa9 100644 --- a/controller/internal/web/templates/backups_restore.html +++ b/controller/internal/web/templates/backups_restore.html @@ -69,6 +69,16 @@ + + {{if .NoRestoreDestination}} +

+ Előbb csatold vissza az adatmeghajtót. A mentéseid megvannak, és a meghajtók is + megvannak — újratelepítés után viszont a gép még nem ismeri őket, ezért most nincs hová + visszaállítani. Ez két kattintás: Tárhely → + Meghajtók, „Meglévő meghajtó csatolása". Utána gyere vissza ide. +

+ {{end}} {{if eq .OffsiteStoreState "unreadable"}}

Nem tudjuk elolvasni a távoli tárolót, ezért nem tudjuk, mi van benne. Ez nem azt jelenti, hogy üres — próbáld újra később, vagy jelezd az üzemeltetőnek.

{{else if eq .OffsiteStoreState "no-target"}} @@ -79,8 +89,16 @@ {{range .OffsiteRestoreRows}} {{template "app_list_row" dict "Slug" .Slug "Name" .DisplayName}} {{if .Restorable}} + {{if and .InStore (not .Installed)}} - Nincs telepítve — a visszaállítás előbb újratelepíti. + Nincs telepítve — előbb telepítsd újra, utána hozhatod vissza az adatait. {{end}} {{if .StoreUnknown}} Nem tudjuk, van-e mentése — a tárolót nem sikerült elolvasni. diff --git a/controller/internal/web/templates/settings_security.html b/controller/internal/web/templates/settings_security.html index e2d65ff..4e482c5 100644 --- a/controller/internal/web/templates/settings_security.html +++ b/controller/internal/web/templates/settings_security.html @@ -405,7 +405,7 @@ function openDialog(opts){ -{{if .RetrievalPassword}} +{{if .HasRetrievalPassword}}

Vészhelyzeti információk

@@ -422,14 +422,13 @@ function openDialog(opts){

Visszaállítási jelszó + - •••••••••••••••• - - - + •••••••••••••••• + +
@@ -442,6 +441,42 @@ function openDialog(opts){
+ {{end}} {{template "layout_end" .}} {{end}}