diff --git a/docs/roadmap/first-run-restore.md b/docs/roadmap/first-run-restore.md index 9f2d49d..416bc0c 100644 --- a/docs/roadmap/first-run-restore.md +++ b/docs/roadmap/first-run-restore.md @@ -77,6 +77,47 @@ That single fact drives two requirements: Once the repo opens, ordering is already correct in the CLI and should be preserved: **system config first** (it carries every *other* location's credentials, so one password unlocks the rest), then apps. +### 4.1 — Where does the typed password actually travel? (blocker for phase 2) + +Found while building phase 2, and it needs a decision before the restore branch +can be written, because it is a security trade-off rather than an implementation +detail. + +The WebUI cannot run restic. So a password the user types in the browser has to +reach the host somehow, and the two existing channels both leak it: + +| Channel | Problem | +|---|---| +| Task command string (what the Backup page already does for this exact field, via `config_update CFG_BACKUP_LOC__PASSWORD=…`) | lands in the task JSON under `frontend/data/tasks/`, which is **0644** so the manager can read it — i.e. world-readable — and is visible in `ps` while the task runs | +| A file in `frontend/data/tasks/` | the container writes as `dockerinstall`; the manager runs as `libreportal`. At 0640 the manager cannot read it (verified), and 0644 is world-readable again | + +Note the first row is **existing behaviour**, not something this feature would +introduce: editing a backup location's password on the Backup page already +sends it that way. So this is a product-wide finding that phase 2 happens to +surface, and the restore case is the sharpest version of it — that password is +the key to every backup the user has. + +Three ways out, roughly in order of effort: + +1. **A one-shot secret drop.** The ownership helper already knows how to make a + path readable across exactly this boundary (`_webui_bind_access` chowns + `MANAGER:cowner` 0640 so the container can read manager-owned files). The + reverse needs the same treatment: a root-helper-created directory owned + `cowner:MANAGER` 0730, into which the container drops a 0640 file the manager + reads once and unlinks. +2. **Never persist it.** Hold the password only in the task processor's memory + for the life of the restore; write it into the location config only after the + system-config restore lands (and reconcile with what the backup contained). +3. **Accept the existing channel** for consistency, and fix it product-wide + later — cheapest now, and no worse than what shipping code already does, but + it does mean a restore password sits world-readable in a task file until that + task is pruned. + +Recommendation: **(1)**, and apply it to the Backup page's password field at the +same time. It is a small, well-scoped addition to a helper that already exists +for the mirror-image case, and it fixes a live weakness rather than only +avoiding a new one. + ## 5. "Set up a backup server if you don't have one" Same components, other direction. After a **New install**, offer: *"Where should your backups go?"* — the same location fields, then `engineInit` and a first `backup system`. That closes a real gap: today backups exist but nothing prompts you to configure them, so the people most likely to need a restore are the least likely to have one. @@ -96,8 +137,8 @@ If a genuine single-file import is wanted, that is a **different feature**: a po | Phase | Deliverable | |---|---| -| **1** | Backup destination step for the New-install path (§5) — small, useful on its own, exercises the location fields inside the wizard | -| **2** | The two blocks, and the restore branch through discovery (steps 1–3). Read-only: nothing is written, so it can ship before reconciliation exists | +| **1** ✅ | Backup destination step for the New-install path (§5) — built | +| **2** ⛔ | The two blocks and the restore branch. **Blocked on §4.1** — the typed repository password has no safe channel to the host yet | | **3** | The reconciliation screen (§3) and the restore itself, driven by `restoreFirstRunBulk` | | **4** | Portable per-app export/import (§6), if wanted |