From 2f27a363d5df01f9c57c284b5871662783cc5265 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 30 Jul 2026 14:10:20 +0200 Subject: [PATCH] R-108: network storage may not host an app's data namespace (v0.187.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is D5's precondition and it is now met. An app's namespace root IS its backup root: namespaceRoot returns a non-system drive path as-is, so the recovery unit lands at /backups/primary//. On a NAS that sits inside the share, which FileBrowser binds WHOLE — share root, :rslave, download:true. The bind was NOT narrowed, and establishing why inverted the fix. The share-root :rslave bind is load-bearing (a 2026-07-22 probe proved an in-container access through it wakes the idle automount trigger), and scoping is undefinable anyway: apps on a share store at /, there is no userdata/ layer, and creating one would write Felhom convention onto a customer's own NAS, which R-67 forbids. So the browsing surface cannot be narrowed and the backup tree must never be placed under it. Operator ruling: refuse the placement, keep the browse bind. Tier 2 already refuses network targets for this reason (F-6C-1). Nothing stranded: zero apps on network storage across all six hub customers including Peti. R-67's browse capability is byte-identical. FIVE surfaces, not the four the register named — settings.RefuseAsAppNamespace is the single predicate. The deploy POST is the real boundary (it accepts any caller-supplied HDD_PATH; DeployStack validates only os.Stat). Surface 4, handleStorageDecommission mode=migrate, guarded only its SOURCE, so a whole namespace could be decommissioned ONTO a NAS — that one is not in the register. Fails closed: /mnt/felhom-drives holds both kinds, Kind exists only on a registered path, so an unregistered path under that root refuses. Supersedes README's "NAS backup locality — decision A" (v0.118.0). 9 tests, all non-effect (nil stackMgr, so a guard that misses panics rather than passing). 4 red-proofs, each mutation asserted to have landed. Suite rc=0, 27 packages, 0 FAIL. vet rc=0. Template + emoji gates OK. --- CHANGELOG.md | 74 ++++ REPORT.md | 165 ++++---- REUSE.md | 1 + controller/README.md | 24 +- controller/internal/api/router.go | 14 + controller/internal/settings/settings.go | 65 +++ controller/internal/web/handlers.go | 19 + .../web/network_app_namespace_test.go | 376 ++++++++++++++++++ controller/internal/web/storage_handlers.go | 34 ++ controller/internal/web/templates/deploy.html | 5 +- 10 files changed, 693 insertions(+), 84 deletions(-) create mode 100644 controller/internal/web/network_app_namespace_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 051fa36..99ce7ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,79 @@ ## Changelog +### v0.187.0 — R-108: network storage may not host an app's data namespace (2026-07-30) — MinAgent 0.113.0 (unchanged) + +**This is D5's precondition, and it is now met.** D5 moves app secrets into the local recovery unit so +Tier-1/Tier-2 restore stop needing the guest; that is safe only once no browsing surface can reach the +backup tree. One could. + +**The chain, confirmed at source end to end.** `namespaceRoot(drivePath)` returns any non-system drive +path AS-IS (`internal/backup/backup.go:262`), so an app's namespace root IS its `HDD_PATH`. Its recovery +unit therefore lands at `/backups/primary//` (`appbackup.RecoveryUnitPath`). Put an app +on a NAS and that directory sits inside the share, which FileBrowser binds **whole** — share ROOT, +`:rslave`, `download: true`. Live on demo-hp, the asymmetry visible in one glance: + + - /mnt/felhom-drives/nvme-1tb/userdata:/srv/nvme-1tb <- drive: userdata-SCOPED + - /mnt/felhom-drives/Felhom-Share:/srv/Felhom-Share:rslave <- share: ROOT + +**Why the bind was NOT narrowed** (this was the real finding, and it inverted the fix). The share-root +`:rslave` bind is **load-bearing**, not an oversight: a Phase-0 probe (2026-07-22) proved an in-container +access through it wakes the idle automount trigger, so narrowing it breaks NAS access itself. And +scoping is not even definable — apps on a share store at `/`, there is no `userdata/` layer, +and creating one would write Felhom's directory convention onto a customer's own NAS, which R-67 +forbids outright. So the browsing surface cannot be narrowed, and the backup tree must therefore never +be placed under it. **Operator ruling 2026-07-30: refuse the placement, keep the browse bind.** Tier 2 +already refuses network targets for this same class of reason (`F-6C-1`); this closes the PRIMARY +namespace, which was the last way a `backups/` tree could appear inside a share-root bind. + +**Nothing is stranded.** Verified across all six hub customers including Peti: zero apps on network +storage. demo-hp's `Felhom-Share` holds only the customer's own files (no `backups/`). R-67's browse +capability is untouched — same bind, same `:rslave`, byte-identical. + +**FIVE surfaces, not the four the register named.** `settings.RefuseAsAppNamespace` is the single +predicate all of them consult: + +1. **the deploy POST** (`internal/api/router.go`) — **this is the boundary.** The R-108 row says "the + deploy dropdown has no `IsNetwork()` filter", which understates it: the dropdown is a UI list, and + this endpoint accepts whatever `HDD_PATH` a caller supplies, with `DeployStack` validating only that + it EXISTS (`os.Stat`, `internal/stacks/deploy.go`). Filtering the list alone would have left the + surface open. +2. **per-app migrate targets** (`internal/web/handlers.go`) — dropped from the offered list. +3. **`handleStorageMigrateApp`** — refused before `MigrateApp`, so no job starts. +4. **`handleStorageDecommission` mode=migrate, the TARGET** — **not in the register.** The existing + `refuseNetworkLifecycle` guards `req.Where`, the SOURCE; the target was unchecked, so a whole + namespace could be decommissioned ONTO a NAS. Found by enumerating the set rather than trusting the + four that were named. +5. **the FileBrowser bind** — deliberately unchanged, and now pinned by a test so it cannot drift. + +**FAIL CLOSED, and the non-obvious part is why this is a function and not an `IsNetwork()` call:** +`/mnt/felhom-drives` holds BOTH kinds in-guest (`.../hdd_1` is a local drive, `.../Felhom-Share` is a +NAS), so a path prefix cannot classify — `Kind` is the only discriminator and it exists only on a +REGISTERED path. An unregistered path under that root is therefore un-classifiable, and un-classifiable +refuses. Every share is registered under that root by construction, so the network set is completely +covered without touching drives. + +**UI (§5): marked, not hidden.** A registered NAS stays in the deploy dropdown, `disabled`, labelled +`(hálózati tárhely — alkalmazáshoz nem választható)`, and never pre-selected even when it is the +registry default. A share the customer registered themselves, silently missing from the list they +expect it in, reads as a bug and generates a support question; present-with-a-reason answers it in +place. Follows the existing `(nem elérhető)` disabled-option precedent. + +Tests: 9 new, all asserting the **non-effect**. The refusal tests run against a Server with a +deliberately **nil `stackMgr`**, so a guard that fails to fire reaches the mutation and PANICS rather +than passing quietly. They assert no job id, no `started` flag, no `MigratedTo` written, and — for +decommission — that the source was NOT soft-marked. Fixtures are demo-hp's real two-class storage set +(both paths under the same mount root, which is the trap). Red-proofs: 4, each mutation asserted to +have landed first — drop either migrate guard → panic; break fail-closed → 3 tests; userdata-scope the +share → the R-67 regression guard fires, quoting the broken bind. + +Suite rc=0, 27 packages, 0 FAIL; `go vet` rc=0; `template_id_gate.py` + `emoji_gate.py` both OK. + +- `internal/settings/settings.go` — `RefuseAsAppNamespace` + the two Hungarian refusal reasons. +- `internal/api/router.go` — the deploy-POST refusal. +- `internal/web/storage_handlers.go` — `refuseAppNamespaceTarget`; wired into migrate-app + decommission. +- `internal/web/handlers.go` — migrate-target list filter; `DeployStoragePath.NotAllowed`. +- `internal/web/templates/deploy.html` — disabled option + reason; no pre-select of a disabled default. + ### v0.186.0 — R-114 + R-112: tell the truth about the backup target, then show it (2026-07-29) — MinAgent 0.113.0 (unchanged) Two defects E-2d found on a real box, fixed in this order deliberately: the message is corrected diff --git a/REPORT.md b/REPORT.md index 7734fa6..01cfc84 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,96 +1,109 @@ -# REPORT — R-114 + R-112: tell the truth about the backup target, then show it (v0.186.0, 2026-07-29) +# REPORT — R-108: network storage may not host an app's data namespace (v0.187.0, 2026-07-30) -**Overwritten** per the standing rule. Controller **v0.185.1 → v0.186.0**. Session B of three -(A → R-113 agent v0.114.0 shipped-unvalidated · **B → this** · C → one drill rebuild proving all three). -`felhom-agent` untouched. **NOTHING HERE IS LIVE-VALIDATED** — see the bottom. +**Overwritten** per the standing rule. Controller **v0.186.0 → v0.187.0**, MinAgent 0.113.0 (unchanged). -## Baselines +**The §7 sentence, up front: D5's precondition IS now met.** No app namespace can be placed on network +storage, therefore no `backups/` tree can exist inside FileBrowser's share-root bind, therefore moving +app secrets into the local recovery unit no longer exposes them to a browsing surface. Nothing else was +blocking D5 — the §10.1 sweep had already cleared every other read surface. -felhom-controller `cdaeb369726b` v0.185.1 · felhom-agent `b58d7bcf39a9` v0.114.0 (untouched) · -felhom.eu `338b2ccf86`. All clean and pushed at start. +## 1. The §3.1 ruling: REFUSE the placement, KEEP the browse bind -**MinAgent decision: unchanged at 0.113.0.** R-114 reads `BackupTarget`, `MountPath`, `GuestPath`, -`Role` — **none of which R-113 altered**; it changed `BoundUnderParent`, which this code does not -read (`internal/agentapi/client.go:303-338`). Consequence, stated rather than discovered: **demo-hp, -still on agent 0.113.0, is not held** and needs no move for this release. +The task offered "scope the bind" or "refuse". **Scoping is not implementable**, and establishing that +inverted the fix: -## Part 1 — R-114: the third state +- The share-root `:rslave` bind is **load-bearing, not an oversight**. A Phase-0 probe (2026-07-22, + recorded in the `buildFileBrowserPaths` doc comment) proved an in-container access through it wakes + the idle automount trigger. Narrow it and NAS access itself breaks. +- There is **no `userdata/` layer to scope to**. Apps on a share store at `/` + (`deployedAppsOnPath` doc comment, `internal/web/netstorage_handlers.go`), not under `/userdata/`. +- Creating one is **forbidden**: *"Never a skeleton, never any write toward the NAS — Felhom conventions + must not be written onto a customer's own NAS."* -`resolveBackupTargetState` had two outcomes — a disk claims the target (healthy), or nothing does -(degraded, "the backup is on the system disk"). *Configured, and its drive is gone* had no branch, so -it fell into the second and inherited its message **and** its offer. Observed live: -`degraded:true, target:"felhom-backup"` + the system-disk copy (false) + `offer_path` naming the drive -that had just vanished (`audits/E2D-fresh-vm-2026-07-29.md` §5.3). +So the browsing surface cannot be narrowed, and the backup tree must never be placed under it. Tier 2 +already refuses network targets for this same class of reason (`F-6C-1`); R-108 closes the PRIMARY +namespace, the last route by which a `backups/` tree could land inside a share-root bind. -- New `BackupTargetState.TargetAbsent`. `Degraded` keeps its meaning — *is there a problem* — so the - wire contract is unchanged for every consumer; `TargetAbsent` answers *which* problem, because the - two have opposite remedies. -- Discriminator `targetIsConfiguredDrive`: a real storage id, versus the builtin `local` or an unset - tier (both = never configured). -- Copy routed through `degradedMessageFor`, so one place still decides what a customer reads. -- **Offer suppressed on the branch itself**, not left to `firstOfferableDrive`'s `Disconnected` skip — - that flag comes from the agent-side gate in another repo, and this state must be right without it. +**Operator decision required and taken** (the ruling removes a capability going forward). Evidence +supplied with the question: **zero apps on network storage across all six hub customers including Peti** +— nothing is stranded, and no migration is owed. R-67's browse capability is byte-identical after. -## Part 2 — R-112: the state gets a consumer +## 2. The exposure chain, confirmed at source -The endpoint was byte-correct and **nothing asked for it** — templates fetch 18 distinct -`/api/storage/*` endpoints and these were the only two with zero references (§5.1). +`namespaceRoot(drivePath)` returns any non-system drive path AS-IS (`internal/backup/backup.go:262`), so +an app's namespace root **is** its `HDD_PATH`; its recovery unit is +`/backups/primary//` (`internal/appbackup/paths.go`). On a NAS that sits inside the +share, which FileBrowser binds whole with `download: true`. -Server-rendered on `/backups`, following the existing `SingleCopyWarning` banner at `backups.html:11`. -`backupsHandler` → `backupTargetView` → template. **Not a 19th JS fetch**: the state is already -resolved when the page is built, and a banner needing JavaScript to appear is one more thing that can -silently not happen. `backupTargetView` returns **nil** for healthy and unknown, so a template typo -cannot decorate a working box. The offer control POSTs to the existing assign endpoint behind the -standard inline confirm, never auto-submits, and surfaces `restart_required` honestly. +This was **documented, deliberate behaviour**, not an accident: `controller/README.md` carried *"NAS +backup locality (v0.118.0, CAMPAIGN-3 Part 4 — decision A) — a NAS-resident app's tier-1 artifacts live +on the NAS itself"*. R-108 supersedes decision A; the README block now says so. -**Placement:** the E-2 design doc names no surface — `07-backup-architecture.md` has no banner or -degraded reference, and the only `E-2` string in the capability map is the unrelated `CAMPAIGN-6E-2`. -So the choice is mine: the backups page already hosts a same-severity backup warning and the copy is -about `rendszermentés`. +## 3. §2 citation reconfirmation — all four hold, line numbers moved -## Files - -`internal/web/backup_target_offer.go` (third state, view, tiers seam) · `internal/web/server.go` -(`tiersFn`) · `internal/web/handlers.go` (one line wiring the view) · -`internal/web/templates/backups.html` (banner + offer + assign script) · two new test files · -`CHANGELOG.md` · `REUSE.md` · `controller/README.md`. - -## Tests: 326 → 338 (+12) in `internal/web`. Suite green (27 packages), both template gates pass. - -**Scenario E — the seam test, which is the one whose absence let E-2 ship.** It does *not* call the -resolver and assert a string (that proves the resolver, which was never broken). It drives -`backupsHandler` over `httptest` and asserts the **rendered HTML**, travelling -handler → `backupTargetView` → `resolveBackupTargetState` → `degradedMessageFor` → the production -`backups` template. - -| Red-proof | Mutation | Failure text produced | +| register citation | now at | verdict | |---|---|---| -| 1 | delete the `targetIsConfiguredDrive` branch | `TargetAbsent is false for target "felhom-backup"…` **and** `offered "/mnt/felhom-drives/mentes2" while the configured target is absent — E-2d observed this exact payload offering the drive that had just vanished` | -| 2 | drop the absent case from `degradedMessageFor` | `the customer is told the backup is on the SYSTEM DISK, which is false…` + all three copy fragments missing | -| 3 | delete `data["BackupTarget"] = …` from the handler | `the absent-drive copy never reached the page…` + every render assertion in the never-configured case | +| `handlers.go:2450-2460` local userdata-scoped | `:2464-2465` | **holds** | +| `handlers.go:2432` network share-ROOT + download | bind `:2437`, `download: true` at `infra/infra.go:328` | **holds** | +| `settings.go:904-914` no `IsNetwork()` filter | `GetSchedulableStoragePaths`, `:926` | **holds** | +| `handlers.go:674-679` migrate targets unfiltered | `:673-678` | **holds** | +| `storage_handlers.go:410-424` migrate-app unguarded | `:416-425` | **holds** | +| `storage_handlers.go:397` sibling refuses | `handleStorageMigrate`, `:404` (both source AND target) | **holds — the precedent** | -All three reverted; files verified byte-identical after. +## 4. §3.2 — the set is FIVE, not four -**Negative assertions:** healthy and unknown are checked for the absence of **all three** copy strings -and of the banner element — not for the absence of an error. +Surface 4 below is **not in the register**; it was found by enumerating rather than trusting the named set. -**A test caught its own imprecision:** the first Scenario-C assertion matched the bare id -`backup-target-assign`, which also appears inside the page script that always renders, so it reported -a control that was not there. Tightened to the control's own markup. +1. **deploy POST** (`internal/api/router.go`) — **the actual boundary.** The register's "the dropdown has + no filter" understates it: the dropdown is a UI list, this endpoint takes any caller-supplied + `HDD_PATH`, and `DeployStack` validates only `os.Stat` existence. A list filter alone would have left + the surface open. +2. **per-app migrate target list** (`internal/web/handlers.go`) — dropped from the offer. +3. **`handleStorageMigrateApp`** — refused before `MigrateApp`. +4. **`handleStorageDecommission` mode=migrate TARGET** — `refuseNetworkLifecycle` guarded only + `req.Where` (the SOURCE); a whole namespace could be decommissioned ONTO a NAS. **NEW.** +5. **the FileBrowser bind** — deliberately unchanged, now pinned by a regression test. -## NOT LIVE-VALIDATED — awaiting Session C +Also noted, not fixed (register already records it as a separate effect): `storageDriveList()` +(`handler_export.go`) does not filter network paths, so a `.fab` bundle — plaintext secrets, optional +password — can still be **exported onto** a NAS. That is a bundle destination, not a namespace +placement, and it is outside R-108's scope. Filed as **R-126**. -- **Scenario C cannot occur on a healthy box.** The demo guest has a present target, so the - absent-drive banner is unproven live by construction. -- Untested live: the offer control's click → assign → `restart_required` surfacing. -- The live check in this session proves only that the seam is wired and which render a healthy box - produces — that is R-112's whole point, but it is not proof of R-114. +## 5. Fail-closed, and why it needed a function -## Observations — filed, not fixed +`/mnt/felhom-drives` holds **both** kinds in-guest (`.../nvme-1tb` is a drive, `.../Felhom-Share` is a +NAS), so a path prefix cannot classify. `Kind` is the only discriminator and exists only on a REGISTERED +path — therefore an unregistered path under that root is un-classifiable, and un-classifiable **refuses**. +Every share is registered under that root by construction, so the network set is fully covered without +touching drives. Empty `HDD_PATH` (SSD-resident) stays allowed; a nil registry refuses. -1. **The absent copy now lives in two repos** — here and hub `internal/notify/templates.go:93` — with - nothing binding them but `TestAbsentCopyMatchesTheHubEmailWordForWord`, which only fails if *this* - side drifts. Reword the hub and the test still passes while the banner and the email disagree. -2. **`resolveBackupTargetState` bypassed the existing `disksFn` seam** and called the client directly, - which is why its branching had no test at all before this session. Now routed through `fetchDisks` - with a sibling `tiersFn` — worth checking whether other agent-reading paths do the same. +## 6. Tests — 9 new, all non-effect; seams named (R-125) + +Refusal tests run against a Server with a deliberately **nil `stackMgr`**: a guard that fails to fire +reaches the mutation and **panics** rather than passing quietly. Asserted: no job id, no `started` flag, +no `MigratedTo` written, and for decommission that the source was **not** soft-marked. Fixtures are +demo-hp's real two-class storage set. + +**Seams injected:** the FileBrowser tests inject at `fbPathDeps` (`isMount`/`classify`/`ensureSkeleton`) +and run the real `buildFileBrowserPaths`; the bind-string construction — what the assertion is about — +is NOT injected. **Not covered by that span:** `RenderFileBrowserConfig` and the compose template +downstream; closed by reading the live generated compose (§7). The handler tests inject nothing — real +handler, real settings store, `httptest`. + +**Red-proofs: 4**, each mutation asserted to have landed before the run: drop the migrate-app guard → +panic; drop the decommission-target guard → panic; break fail-closed → 3 tests; userdata-scope the share +→ the R-67 regression guard fires quoting the broken bind. + +Suite rc=0, 27 packages, 0 FAIL (run separately from the commit); `go vet` rc=0; +`template_id_gate.py` + `emoji_gate.py` OK. + +## 7. Live evidence + +See `felhom.eu/documentation/audits/R108-network-app-namespace-2026-07-30.md`. + +## 8. Not done + +- **D5 not implemented** — this unblocks it only. +- **No data migrated** — nothing needed migrating (zero apps on network storage). +- **The share-root bind unchanged** — deliberate; R-67's capability is preserved intact. +- **R-126 filed, not fixed** (`.fab` export onto a NAS). diff --git a/REUSE.md b/REUSE.md index 29065b1..50d04e0 100644 --- a/REUSE.md +++ b/REUSE.md @@ -230,6 +230,7 @@ | `Server.sambaAddrFn` (func seam) | controller/internal/web/server.go (field) + sharing_handlers.go `sambaLANAddress()` | nil → `stackMgr.SambaLANAddress()` | The web-side half of the connect card. Tests inject a COUNTED fn — the fresh-per-render assertion is what stops anyone memoizing a DHCP lease | | `Manager.guestNetExecFn` (func seam) + `GuestGateway()` / `GuestNetSnapshot()` | controller/internal/stacks/manager.go (field) + guestnet.go | nil → `docker exec felhom-samba ` — ONE seam for all R-66 guest-netns reads (route/link/addr/resolv.conf); tests script canned outputs per argv | guestnet_test.go. **The netns door rule:** the controller's OWN netns is the docker bridge, so any in-process read (`net.Interfaces`, `/proc/net/route`, its own `/etc/resolv.conf` = 127.0.0.11) is the S-2 wrong answer — guest-net reads MUST go through the samba (`network_mode: host`) exec door. Megosztás off ⇒ door closed ⇒ "" / per-item error strings; NEVER substitute an in-process value. Same S-5 law as SambaLANAddress: live per render, never cached/persisted. Parsers (`parseDefaultRoute`, `parseGuestInterfaces`, `parseResolvConf`) are pure + separately pinned | | `buildFileBrowserPaths` + `fbPathDeps` (R-67, v0.160.0) | controller/internal/web/handlers.go | pure assembly of one FileBrowser sync pass: (mount lines, config source paths) from the registry, with per-kind gates | filebrowser_network_test.go. **Two storage classes, two DIFFERENT gates:** drives keep the drive-absent gate + userdata scoping + skeleton (byte-identical to pre-R-67 — tested); network shares bind the share ROOT `:rslave` with the STUB gate instead (`classifyFSPath`; stub ⇒ excluded from mounts AND sources — an exposed stub swallows uploads the real mount later shadows; idle autofs / unknown ⇒ include, fail open). NEVER call `EnsureUserdataSkeleton` toward a network path (red-proven); never force-wake an idle trigger in the sync (doctrine) | +| `Settings.RefuseAsAppNamespace` (R-108, v0.187.0) | controller/internal/settings/settings.go | `(path) (refuse bool, hungarianReason string)` — may an app's DATA NAMESPACE live here? | **THE single predicate for every placement surface** (deploy POST `api/router.go`, per-app migrate list + `handleStorageMigrateApp`, `handleStorageDecommission` mode=migrate TARGET). **Network storage is refused** because an app's namespace root IS its backup root (`namespaceRoot` returns a non-system drive path as-is → `/backups/primary//`), and on a share that lands inside FileBrowser's share-ROOT `download:true` bind — which CANNOT be narrowed (R-67 `:rslave` = automount wake; and apps on a share store at `/`, so there is no `userdata/` to scope to and creating one would write Felhom convention onto a customer's NAS). **DISTINCT from `refuseNetworkLifecycle`** — that asks "may a DRIVE lifecycle op run on this path" and is applied to the op's SUBJECT; this asks "may an app live here" and is applied to a placement TARGET. Migrate needs BOTH. **FAILS CLOSED:** `/mnt/felhom-drives` holds both kinds, so a path prefix cannot classify — `Kind` exists only on a REGISTERED path, therefore an unregistered path under that root is un-classifiable and REFUSES. Empty path = SSD-resident = allowed; nil receiver refuses. network_app_namespace_test.go, 4 red-proofs | | `Server.guestGatewayFn` / `guestNetFn` (func seams) | controller/internal/web/server.go (fields) + sharing_handlers.go accessors | nil → `stackMgr.GuestGateway` / `stackMgr.GuestNetSnapshot` | network_card_test.go — the counted-fn freshness test (2 renders ⇒ 2 resolves) is what stops anyone memoizing a DHCP lease; the Hálózati név row is gated on `smb.Enabled` (red-proven: gate dropped ⇒ \\FELHOM rendered while samba is down) | | `sambaEnsureState.consumeIfRunning()` | controller/internal/web/samba_ensure_job.go | serve-once `snapshot()` for terminal `running` only | `/sharing/status` carries a job EDGE (`phase`) and a service LEVEL (`running`) in one envelope — never let a level reach the phase channel, and never re-serve a consumed edge: the client answers `phase=="running"` with `location.reload()`, so both mistakes produce an infinite page reload (S-1/S-4, DIAG-sharing-2026-07-20.md). `failed`/`needs_password`/in-flight are NOT consumed | | `infra.SambaHostInterface` | controller/internal/infra/samba.go | the guest LAN nic name (`eth0`) | Single source for smb.conf's `interfaces =`, the container's `FELHOM_IFACE`, and the LAN-address read — if they name different nics, the service and the address the page prints drift apart | diff --git a/controller/README.md b/controller/README.md index ca03bae..54e5707 100644 --- a/controller/README.md +++ b/controller/README.md @@ -744,12 +744,24 @@ The nightly backup has two phases that run sequentially. All paths are **per-dri > drive. Guarded: only for a DEPLOYED app whose CURRENT drive differs from the dir's drive; never the > current-drive dir (the live restore point) or an undeployed app's dir; strictly under `backups/primary/`. -> **NAS backup locality (v0.118.0, CAMPAIGN-3 Part 4 — decision A).** A NAS-resident app's tier-1 -> artifacts live **on the NAS itself** (`nas-media/backups/primary/`), beside the data. During a -> NAS outage both the app data AND its freshest tier-1 dump are on the dead device — the **tier-2** -> cross-drive copy to a local drive is the off-NAS leg that saves them (and only after it has run). -> This locality is deliberate (kept in the fork over retargeting tier-1 to a local drive); the tier-2 -> copy is the mitigation. Stated here so the outage window is never a surprise. +> **~~NAS backup locality (v0.118.0, CAMPAIGN-3 Part 4 — decision A)~~ — SUPERSEDED by R-108 +> (v0.187.0, 2026-07-30).** Decision A said a NAS-resident app's tier-1 artifacts live **on the NAS +> itself** (`nas-media/backups/primary/`), beside the data, with the tier-2 cross-drive copy as the +> off-NAS mitigation. That locality is exactly what made a `backups/` tree reachable through +> FileBrowser's share-ROOT bind (`download: true`), and it is why architectural target **D5** — app +> secrets in the local recovery unit — could not be adopted. +> +> **An app's data namespace may no longer live on network storage at all** (operator ruling +> 2026-07-30), so the case decision A described can no longer arise: no app on a NAS ⇒ no +> `backups/primary/` on a NAS. `settings.RefuseAsAppNamespace` is the single predicate; every +> placement surface consults it (deploy POST, per-app migrate, decommission-with-migrate). The NAS +> keeps its **browse** capability unchanged — the share-root `:rslave` bind is load-bearing for +> automount wake (R-67) and was deliberately NOT narrowed; scoping it is undefinable anyway, since +> apps on a share store at `/` and creating a `userdata/` layer would write Felhom +> convention onto a customer's own NAS. +> +> The NAS-outage window decision A warned about is therefore also gone: an app's tier-1 artifacts are +> always on a local drive now, because the app itself always is. **Drive layout (v0.26.0):** ``` diff --git a/controller/internal/api/router.go b/controller/internal/api/router.go index 15eb29a..c5e4531 100644 --- a/controller/internal/api/router.go +++ b/controller/internal/api/router.go @@ -448,6 +448,20 @@ func (r *Router) deployStack(w http.ResponseWriter, req *http.Request, name stri return } + // R-108: an app's data namespace may NOT live on network storage — its backups would land at + // `/backups/primary//`, inside the share-ROOT bind FileBrowser serves with + // download:true (and that bind cannot be narrowed — see settings.RefuseAsAppNamespace). + // + // THIS is the boundary, not the deploy dropdown. The dropdown is a UI list; this endpoint accepts + // whatever HDD_PATH a caller supplies and `DeployStack` validates only that it EXISTS on the + // filesystem (os.Stat, internal/stacks/deploy.go). A filter on the list alone would have left the + // surface wide open — the R-108 row's "no IsNetwork() filter on the dropdown" understates it. + if refuse, why := r.sett.RefuseAsAppNamespace(body.Values["HDD_PATH"]); refuse { + r.logger.Printf("[WARN] [api] Deploy refused for %s: HDD_PATH is not usable as an app namespace (R-108)", name) + writeJSON(w, http.StatusConflict, apiResponse{OK: false, Error: why}) + return + } + deployReq := stacks.DeployRequest{ StackName: name, Values: body.Values, diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index f34b50e..bb1d475 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -1330,6 +1330,71 @@ func (s *Settings) IsNetworkStoragePath(path string) bool { return false } +// RefuseAsAppNamespace reports whether `path` must be REFUSED as an app's data namespace (its +// HDD_PATH), and why. It is the single predicate every placement surface consults — R-108. +// +// WHY AN APP NAMESPACE MAY NOT LIVE ON A NAS (operator ruling, 2026-07-30). An app's namespace root is +// also where its backups go: `namespaceRoot(drivePath)` returns a non-system drive path AS-IS, so the +// app's recovery unit lands at `/backups/primary//` (appbackup.RecoveryUnitPath). For a +// network share that directory would sit inside the share ROOT — which FileBrowser binds whole, with +// `download: true`, and MUST keep binding whole: the `:rslave` share-root bind is load-bearing for +// automount wake/idle propagation into the running container (R-67), and scoping it is impossible +// besides — apps on a share store at `/`, there is no `userdata/` layer, and creating one +// would write Felhom's convention onto a customer's own NAS, which R-67 forbids outright. +// +// So the browsing surface cannot be narrowed and the backup tree must therefore never be placed under +// it. Tier 2 already refuses network targets for exactly this class of reason (F-6C-1); this closes the +// PRIMARY namespace, which was the remaining way a `backups/` tree could appear inside a share-root +// bind. That is the precondition D5 was waiting on. +// +// FAIL CLOSED, and the two non-obvious cases are why this is a function and not an `IsNetwork()` call: +// +// - `NetworkMountRoot` holds BOTH kinds in-guest (`/mnt/felhom-drives/hdd_1` is a local drive, +// `/mnt/felhom-drives/Felhom-Share` is a NAS), so a path prefix CANNOT classify. `Kind` is the only +// discriminator, and it exists only on a REGISTERED path. +// - therefore an UNREGISTERED path under `NetworkMountRoot` is un-classifiable, and un-classifiable +// must refuse. Allowing it would be a fallback to "probably a drive" on the one surface that +// accepts an arbitrary caller-supplied path (the deploy POST validates only `os.Stat` existence). +// Every NAS share is registered under this root by construction (see NetworkMountRoot), so refusing +// the unregistered case makes the network set completely covered without touching drives. +// +// An empty path is ALLOWED: it means the app is system/SSD-resident and has no external namespace at +// all. A nil receiver refuses — we cannot consult the registry, so we cannot tell. +func (s *Settings) RefuseAsAppNamespace(path string) (bool, string) { + path = strings.TrimSpace(path) + if path == "" { + return false, "" // SSD-resident: no external namespace to place + } + if s == nil { + return true, refuseAppNamespaceUndeterminable + } + s.mu.RLock() + defer s.mu.RUnlock() + for _, sp := range s.StoragePaths { + if path == sp.Path || strings.HasPrefix(path, sp.Path+"/") { + if sp.IsNetwork() { + return true, refuseAppNamespaceNetwork + } + return false, "" // a registered DRIVE — the supported case, unchanged + } + } + // Not registered. Under the shared mount root its kind is undeterminable → refuse (see above). + if path == NetworkMountRoot || strings.HasPrefix(path, NetworkMountRoot+"/") { + return true, refuseAppNamespaceUndeterminable + } + return false, "" +} + +// Refusal reasons for RefuseAsAppNamespace. Hungarian, adult tone, no emoji — these reach the customer +// through the deploy/migrate error surfaces. They name the storage class and what to do instead, never +// an internal path or field name. +const ( + refuseAppNamespaceNetwork = "Hálózati tárhelyen (NAS) nem futtatható alkalmazás adatkönyvtára — " + + "a NAS megosztás tallózásra és médiatárolásra használható. Válasszon csatlakoztatott meghajtót." + refuseAppNamespaceUndeterminable = "A megadott tárhely nem azonosítható regisztrált meghajtóként, " + + "ezért alkalmazás adatkönyvtáraként nem használható. Válasszon a listából csatlakoztatott meghajtót." +) + // IsStoragePathSchedulable returns whether a path belongs to a registered, // schedulable (active) storage path. Returns false if the path is unknown, // disconnected, decommissioned, or inactive. diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index 17ac3c9..fe03b18 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -90,6 +90,12 @@ type DeployStoragePath struct { settings.StoragePath FreeHuman string // "234.5 GB" FreePercent float64 // 67.5 + // NotAllowed marks a path that CANNOT host an app's data namespace (R-108: network storage). The + // option is rendered PRESENT-but-disabled with NotAllowedNote rather than dropped: a NAS the + // customer registered themselves, silently absent from the list they expect it in, reads as a bug + // and generates a support question. Present with a reason answers the question in place. + NotAllowed bool + NotAllowedNote string // short parenthetical for the option label; "" when allowed } // StorageAppDetail holds info about an app using a specific storage path. @@ -462,6 +468,12 @@ func (s *Server) deployHandler(w http.ResponseWriter, r *http.Request, name stri var deployPaths []DeployStoragePath for _, sp := range s.settings.GetSchedulableStoragePaths() { dp := DeployStoragePath{StoragePath: sp} + // R-108: mark, do not hide. The server-side refusal in the deploy POST is the boundary; this is + // the honest UI over it, and it must not be mistaken for the boundary itself. + if refuse, _ := s.settings.RefuseAsAppNamespace(sp.Path); refuse { + dp.NotAllowed = true + dp.NotAllowedNote = "hálózati tárhely — alkalmazáshoz nem választható" + } if di := system.GetDiskUsage(sp.Path); di != nil { dp.FreeHuman = formatFreeSpace(di.AvailGB) if di.TotalGB > 0 { @@ -676,6 +688,13 @@ func (s *Server) appDetailHandler(w http.ResponseWriter, r *http.Request, slug s if sp.Path == current || sp.Decommissioned || sp.Disconnected || !sp.Schedulable { continue } + // R-108: never OFFER network storage as a migrate target — an app namespace may not live + // there. Dropped rather than shown-disabled: unlike the deploy page this list has no + // explanatory surface, and a target that cannot be chosen is not a target. The refusal that + // MATTERS is server-side in handleStorageMigrateApp; this only keeps the UI honest. + if refuse, _ := s.settings.RefuseAsAppNamespace(sp.Path); refuse { + continue + } targets = append(targets, sp) } data["MigrateTargets"] = targets diff --git a/controller/internal/web/network_app_namespace_test.go b/controller/internal/web/network_app_namespace_test.go new file mode 100644 index 0000000..038973f --- /dev/null +++ b/controller/internal/web/network_app_namespace_test.go @@ -0,0 +1,376 @@ +package web + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" + "gitea.dooplex.hu/admin/felhom-controller/internal/system" +) + +// R-108 — network storage may not host an app's data namespace. +// +// WHY, in one line: an app's namespace root is also its backup root, so a NAS-hosted app would put its +// recovery unit at `/backups/primary//` — inside the share-ROOT bind FileBrowser serves +// with download:true. That bind CANNOT be narrowed (R-67: `:rslave` at the share root is load-bearing +// for automount wake, and apps on a share store at `/` so there is no `userdata/` layer to +// scope to), therefore the backup tree must never be placed under it. Operator ruling 2026-07-30: +// REFUSE the placement, KEEP the browse bind. +// +// These tests assert the NON-EFFECT. A handler returning an error proves nothing on its own — what +// matters is that nothing was written, so each refusal test inspects the resulting state. + +// netShare / localDrive are the two storage classes, shaped as the live demo-hp box really has them: +// BOTH under /mnt/felhom-drives (which is why a path prefix cannot classify — Kind is the only +// discriminator, and that is the trap RefuseAsAppNamespace exists to handle). +// +// PROVENANCE: captured from demo-hp guest 9201 on 2026-07-30 — +// +// /mnt/felhom-drives/Felhom-Share (Kind=network, the NAS; holds the customer's own files) +// /mnt/felhom-drives/nvme-1tb (Kind=drive, the enrolled data drive; paperless-ngx lives here) +func netShare() settings.StoragePath { + return settings.StoragePath{ + Path: settings.NetworkMountRoot + "/Felhom-Share", Label: "Felhom-Share", + Kind: settings.StorageKindNetwork, Schedulable: true, + } +} + +func localDrive() settings.StoragePath { + return settings.StoragePath{ + Path: settings.NetworkMountRoot + "/nvme-1tb", Label: "NVMe 1TB", + Kind: settings.StorageKindDrive, Schedulable: true, IsDefault: true, + } +} + +// --------------------------------------------------------------------------------------------- +// 1. The predicate itself, including the fail-closed cases. +// --------------------------------------------------------------------------------------------- + +func TestRefuseAsAppNamespace_Table(t *testing.T) { + s := testServer(t) + if err := s.settings.AddStoragePath(localDrive()); err != nil { + t.Fatal(err) + } + if err := s.settings.AddStoragePath(netShare()); err != nil { + t.Fatal(err) + } + + cases := []struct { + name string + path string + refuse bool + }{ + {"registered local drive is allowed", settings.NetworkMountRoot + "/nvme-1tb", false}, + {"a subpath of a local drive is allowed", settings.NetworkMountRoot + "/nvme-1tb/appdata", false}, + {"registered NAS share is REFUSED", settings.NetworkMountRoot + "/Felhom-Share", true}, + {"a subpath of the NAS share is REFUSED", settings.NetworkMountRoot + "/Felhom-Share/media", true}, + {"empty means SSD-resident — allowed", "", false}, + {"whitespace-only is treated as empty", " ", false}, + // FAIL CLOSED: unregistered under the shared mount root is un-classifiable. Both kinds live + // there, so nothing can decide it — and the deploy POST accepts a caller-supplied path whose + // only other validation is os.Stat existence. + {"UNREGISTERED under the mount root is REFUSED (cannot tell)", settings.NetworkMountRoot + "/mystery", true}, + {"the mount root itself is REFUSED", settings.NetworkMountRoot, true}, + // Outside the mount root nothing can be a NAS by construction (a share is always registered as + // NetworkMountRoot + "/" + name), so the pre-existing behaviour stands. + {"a path outside the mount root is unchanged", "/mnt/sys_drive/felhom-data", false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + refuse, why := s.settings.RefuseAsAppNamespace(c.path) + if refuse != c.refuse { + t.Errorf("RefuseAsAppNamespace(%q) = %v, want %v (reason %q)", c.path, refuse, c.refuse, why) + } + if refuse && strings.TrimSpace(why) == "" { + t.Errorf("a refusal must carry a reason for the customer; got empty for %q", c.path) + } + if !refuse && why != "" { + t.Errorf("an allowed path must carry no reason; got %q for %q", why, c.path) + } + }) + } +} + +// TestRefuseAsAppNamespace_NilSettingsFailsClosed: with no registry to consult we cannot tell, so we +// refuse. (Reached only in a degraded/setup state; the guard is what makes "cannot tell" unrepresentable +// as "allowed".) +func TestRefuseAsAppNamespace_NilSettingsFailsClosed(t *testing.T) { + var s *settings.Settings + if refuse, _ := s.RefuseAsAppNamespace("/mnt/felhom-drives/anything"); !refuse { + t.Error("nil settings must REFUSE — an unconsultable registry is not an allow") + } + // ...but an empty path is still allowed: there is no external namespace to place at all. + if refuse, _ := s.RefuseAsAppNamespace(""); refuse { + t.Error("an empty HDD_PATH is SSD-resident and must stay allowed even with nil settings") + } +} + +// --------------------------------------------------------------------------------------------- +// 2. The refusals, asserted by NON-EFFECT on real handlers. +// --------------------------------------------------------------------------------------------- + +// migrateAppServer builds a Server whose stackMgr is deliberately NIL. That is the non-effect assertion +// made structural: if the refusal does not fire, the handler reaches s.stackMgr.MigrateApp and the test +// PANICS instead of quietly passing. A nil-pointer panic is a louder proof than any recorded call count. +func migrateAppServer(t *testing.T) *Server { + t.Helper() + s := testServer(t) + if err := s.settings.AddStoragePath(localDrive()); err != nil { + t.Fatal(err) + } + if err := s.settings.AddStoragePath(netShare()); err != nil { + t.Fatal(err) + } + s.stackMgr = nil + return s +} + +func postJSON(t *testing.T, h func(http.ResponseWriter, *http.Request), body any) *httptest.ResponseRecorder { + t.Helper() + b, err := json.Marshal(body) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + h(rec, httptest.NewRequest(http.MethodPost, "/api/storage/x", bytes.NewReader(b))) + return rec +} + +// TestMigrateApp_RefusesNetworkTarget_AndStartsNothing: the per-app migrate endpoint refuses a NAS +// target. The whole-namespace sibling has always refused (storage_handlers.go handleStorageMigrate); +// this path never followed, which is the asymmetry R-108 was filed on. +func TestMigrateApp_RefusesNetworkTarget_AndStartsNothing(t *testing.T) { + s := migrateAppServer(t) + + rec := postJSON(t, s.handleStorageMigrateApp, map[string]string{ + "app": "immich", "target": settings.NetworkMountRoot + "/Felhom-Share", + }) + + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", rec.Code) + } + // NON-EFFECT: no job id came back. A started migration always returns one. + var resp struct { + OK bool `json:"ok"` + Error string `json:"error"` + Data map[string]any `json:"data"` + } + _ = json.Unmarshal(rec.Body.Bytes(), &resp) + if resp.OK { + t.Error("refusal reported ok:true") + } + if _, started := resp.Data["started"]; started { + t.Errorf("a refused migration reported a started job: %v", resp.Data) + } + if _, hasID := resp.Data["id"]; hasID { + t.Errorf("a refused migration handed back a job id: %v", resp.Data) + } + if !strings.Contains(resp.Error, "NAS") { + t.Errorf("refusal must name the storage class for the customer; got %q", resp.Error) + } + // And the registry is untouched — the target did not become the app's namespace. + for _, sp := range s.settings.GetStoragePaths() { + if sp.MigratedTo != "" { + t.Errorf("a refused migration wrote MigratedTo=%q on %s", sp.MigratedTo, sp.Path) + } + } +} + +// TestMigrateApp_AllowsLocalDriveTarget proves the refusal is not over-broad. stackMgr is nil, so +// reaching MigrateApp panics — which is exactly what must happen: it shows the guard let the call +// through. Recovered so the assertion is explicit rather than a red test. +func TestMigrateApp_AllowsLocalDriveTarget(t *testing.T) { + s := migrateAppServer(t) + reached := false + func() { + defer func() { + if recover() != nil { + reached = true // got past the guard, into the nil stackMgr + } + }() + _ = postJSON(t, s.handleStorageMigrateApp, map[string]string{ + "app": "immich", "target": settings.NetworkMountRoot + "/nvme-1tb", + }) + }() + if !reached { + t.Error("a LOCAL drive target was refused — the R-108 guard is over-broad and blocks the supported case") + } +} + +// TestDecommissionMigrate_RefusesNetworkTarget_AndDecommissionsNothing covers the surface the R-108 row +// does NOT name (§3.2). handleStorageDecommission guards `where` (the SOURCE) via refuseNetworkLifecycle; +// the migrate TARGET was unchecked, so decommission-with-migrate could move a whole namespace onto a NAS. +func TestDecommissionMigrate_RefusesNetworkTarget_AndDecommissionsNothing(t *testing.T) { + s := migrateAppServer(t) + + rec := postJSON(t, s.handleStorageDecommission, map[string]string{ + "where": settings.NetworkMountRoot + "/nvme-1tb", // a real local drive: passes the SOURCE guard + "mode": "migrate", + "target": settings.NetworkMountRoot + "/Felhom-Share", // the NAS: must be refused + }) + + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", rec.Code) + } + var resp struct { + OK bool `json:"ok"` + Data map[string]any `json:"data"` + } + _ = json.Unmarshal(rec.Body.Bytes(), &resp) + if resp.OK { + t.Error("refusal reported ok:true") + } + if _, started := resp.Data["started"]; started { + t.Errorf("a refused decommission-migrate started a job: %v", resp.Data) + } + // NON-EFFECT, the one that matters here: the SOURCE must not be marked decommissioned. + if s.settings.IsDecommissioned(settings.NetworkMountRoot + "/nvme-1tb") { + t.Error("a refused decommission-migrate soft-marked the source — the drive is now unusable") + } + for _, sp := range s.settings.GetStoragePaths() { + if sp.MigratedTo != "" { + t.Errorf("a refused decommission-migrate wrote MigratedTo=%q", sp.MigratedTo) + } + } +} + +// TestDecommissionMigrate_RefusesUnclassifiableTarget is the FAIL-CLOSED case on a real handler: an +// unregistered path under the shared mount root cannot be classified, so it is refused rather than +// assumed to be a drive. +func TestDecommissionMigrate_RefusesUnclassifiableTarget(t *testing.T) { + s := migrateAppServer(t) + rec := postJSON(t, s.handleStorageDecommission, map[string]string{ + "where": settings.NetworkMountRoot + "/nvme-1tb", "mode": "migrate", + "target": settings.NetworkMountRoot + "/not-registered", + }) + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400 — an unclassifiable target must fail CLOSED", rec.Code) + } + if s.settings.IsDecommissioned(settings.NetworkMountRoot + "/nvme-1tb") { + t.Error("source soft-marked despite the refusal") + } +} + +// --------------------------------------------------------------------------------------------- +// 3. The R-67 browse bind is UNCHANGED — the capability this ruling deliberately preserves. +// --------------------------------------------------------------------------------------------- + +// TestFileBrowserBind_ShareRootPreserved_DriveStillScoped pins BOTH shapes at once, because the ruling +// is precisely that they stay different: the share keeps its ROOT `:rslave` bind (load-bearing for +// automount wake — R-67) and the drive keeps its `userdata` scoping. +// +// SEAM (R-125): this injects at `fbPathDeps` — the isMount/classify/ensureSkeleton funcs — and runs the +// real buildFileBrowserPaths. NOT injected: the bind-string construction itself, which is what the +// assertion is about. What this does NOT cover is RenderFileBrowserConfig and the compose template +// downstream; TestFileBrowserCompose_* below closes that span. +func TestFileBrowserBind_ShareRootPreserved_DriveStillScoped(t *testing.T) { + var calls []string + mounts, cfgPaths := buildFileBrowserPaths( + []settings.StoragePath{localDrive(), netShare()}, + fbDeps(system.FSClassNetwork, &calls, nil), + ) + joined := strings.Join(mounts, "\n") + + wantShare := " - " + settings.NetworkMountRoot + "/Felhom-Share:/srv/Felhom-Share:rslave" + if !strings.Contains(joined, wantShare) { + t.Errorf("the R-67 share-ROOT :rslave bind is GONE — automount wake no longer propagates.\nwant %q\ngot:\n%s", wantShare, joined) + } + wantDrive := " - " + settings.NetworkMountRoot + "/nvme-1tb/userdata:/srv/nvme-1tb" + if !strings.Contains(joined, wantDrive) { + t.Errorf("the local drive lost its userdata scoping.\nwant %q\ngot:\n%s", wantDrive, joined) + } + // The drive must NOT be bound at its root (that would be the R-108 exposure on a local drive). + if strings.Contains(joined, "- "+settings.NetworkMountRoot+"/nvme-1tb:/srv") { + t.Errorf("the local drive is bound at its ROOT — backups/ would be browsable:\n%s", joined) + } + // Never a skeleton toward the NAS (R-67: no Felhom convention on a customer's own NAS). + for _, c := range calls { + if strings.Contains(c, "Felhom-Share") { + t.Errorf("ensureSkeleton was called toward the NAS: %v", calls) + } + } + if len(cfgPaths) != 2 { + t.Errorf("both paths must stay in the FileBrowser source list, got %d", len(cfgPaths)) + } +} + +// TestFileBrowserCompose_NoBackupsTreeUnderAnyBind is the CONSEQUENCE assertion, made against the +// generated compose text rather than an intermediate struct. +// +// Under this ruling the share-root bind is retained, so the guarantee cannot be "no bind reaches a +// backups/ dir" by path shape — it is "no app namespace, hence no backups/ tree, can exist on a share". +// This test therefore pins the paired invariant the safety rests on: the ONLY root-bound path is the +// network share, and every drive-bound path is userdata-scoped. If a future change root-binds a drive, +// or userdata-scopes the share, this fails and the D5 argument needs re-deriving. +func TestFileBrowserCompose_NoBackupsTreeUnderAnyBind(t *testing.T) { + var calls []string + mounts, _ := buildFileBrowserPaths( + []settings.StoragePath{localDrive(), netShare()}, + fbDeps(system.FSClassNetwork, &calls, nil), + ) + for _, m := range mounts { + src := strings.TrimSpace(strings.SplitN(strings.TrimPrefix(strings.TrimSpace(m), "- "), ":", 2)[0]) + isShare := strings.Contains(src, "Felhom-Share") + scoped := strings.HasSuffix(src, "/userdata") + switch { + case isShare && scoped: + t.Errorf("the share became userdata-scoped — impossible on a customer NAS, and it breaks the rslave wake: %q", src) + case !isShare && !scoped: + t.Errorf("a DRIVE is bound unscoped at %q — its backups/ tree is browsable", src) + } + } +} + +// --------------------------------------------------------------------------------------------- +// 4. The deploy dropdown is marked, not silently emptied (§5). +// --------------------------------------------------------------------------------------------- + +// TestDeployStoragePath_NetworkMarkedNotHidden: the NAS stays in the list, disabled, with a reason, and +// never pre-selected. A registered share vanishing from the list the customer expects it in reads as a +// bug; present-with-a-reason answers the question in place. +func TestDeployStoragePath_NetworkMarkedNotHidden(t *testing.T) { + s := testServer(t) + // The NAS is the IsDefault one here on purpose: the template must not pre-select a disabled option. + share := netShare() + share.IsDefault = true + if err := s.settings.AddStoragePath(share); err != nil { + t.Fatal(err) + } + drive := localDrive() + drive.IsDefault = false + if err := s.settings.AddStoragePath(drive); err != nil { + t.Fatal(err) + } + + var got []DeployStoragePath + for _, sp := range s.settings.GetSchedulableStoragePaths() { + dp := DeployStoragePath{StoragePath: sp} + if refuse, _ := s.settings.RefuseAsAppNamespace(sp.Path); refuse { + dp.NotAllowed = true + dp.NotAllowedNote = "hálózati tárhely — alkalmazáshoz nem választható" + } + got = append(got, dp) + } + if len(got) != 2 { + t.Fatalf("both paths must be listed (marked, not hidden), got %d", len(got)) + } + for _, dp := range got { + isShare := strings.Contains(dp.Path, "Felhom-Share") + if isShare != dp.NotAllowed { + t.Errorf("%s: NotAllowed=%v, want %v", dp.Path, dp.NotAllowed, isShare) + } + if dp.NotAllowed && dp.NotAllowedNote == "" { + t.Errorf("%s: disabled with no reason shown", dp.Path) + } + if dp.NotAllowed && dp.IsDefault { + // The data still says IsDefault; the TEMPLATE must not honour it. Guarded by the + // `and .IsDefault (not .NotAllowed)` condition in deploy.html — pinned here so a template + // edit that drops it is visible. + t.Log("share is IsDefault in the registry — deploy.html must not pre-select it (template guard)") + } + } +} diff --git a/controller/internal/web/storage_handlers.go b/controller/internal/web/storage_handlers.go index cbefe33..b229c2e 100644 --- a/controller/internal/web/storage_handlers.go +++ b/controller/internal/web/storage_handlers.go @@ -320,6 +320,25 @@ func (s *Server) refuseNetworkLifecycle(w http.ResponseWriter, where string) boo return false } +// refuseAppNamespaceTarget blocks a placement that would put an app's data namespace on storage that +// cannot host one — today: network storage, and any path whose kind cannot be determined (R-108). +// Returns true when it has already written the refusal, so callers `return` immediately. +// +// DISTINCT from refuseNetworkLifecycle above, and both are needed. That one answers "may I run a DRIVE +// lifecycle op on this path" (a NAS has no device lifecycle) and is applied to the op's SUBJECT. This +// one answers "may an app's data live here" and is applied to a placement TARGET. The migrate handlers +// need both: the source must be a drive to be migrated off, and the target must be able to hold a +// namespace. Collapsing them into one predicate would make one of the two questions unaskable. +func (s *Server) refuseAppNamespaceTarget(w http.ResponseWriter, target string) bool { + refuse, why := s.settings.RefuseAsAppNamespace(target) + if !refuse { + return false + } + s.logger.Printf("[WARN] [web] placement refused: target cannot host an app namespace (R-108)") + writeDiskJSON(w, http.StatusBadRequest, false, why, nil) + return true +} + // ---- HTTP handlers (behind RequireAuth + CsrfProtect) ----------------------------------------- // storageWizardPageHandler renders the init/attach wizard page (the disk list + actions are driven @@ -422,6 +441,13 @@ func (s *Server) handleStorageMigrateApp(w http.ResponseWriter, r *http.Request) writeDiskJSON(w, http.StatusBadRequest, false, "érvénytelen kérés", nil) return } + // R-108: the TARGET may not be network storage. Its whole-namespace sibling + // (handleStorageMigrate) has refused both endpoints since the network class was introduced; this + // per-app path never followed, which is the asymmetry the R-108 row was filed on. The refusal is + // BEFORE MigrateApp, so a refused call starts no job and mutates nothing. + if s.refuseAppNamespaceTarget(w, strings.TrimSpace(req.Target)) { + return + } id, err := s.stackMgr.MigrateApp(r.Context(), strings.TrimSpace(req.App), strings.TrimSpace(req.Target)) if err != nil { writeDiskJSON(w, http.StatusConflict, false, err.Error(), nil) @@ -466,6 +492,14 @@ func (s *Server) handleStorageDecommission(w http.ResponseWriter, r *http.Reques writeDiskJSON(w, http.StatusBadRequest, false, "céltároló kötelező az áthelyezéshez", nil) return } + // R-108: the refuseNetworkLifecycle above guards `req.Where` — the SOURCE. The TARGET was + // never checked, so decommission-with-migrate could move an entire namespace ONTO a NAS. This + // surface is NOT in the R-108 row; it was found by enumerating the set (§3.2) rather than + // trusting the four the row named. Refused before MigrateAllAndDecommission, so nothing moves + // and the source is not marked decommissioned. + if s.refuseAppNamespaceTarget(w, strings.TrimSpace(req.Target)) { + return + } // Start the migration; the done-hook (onMigrationDone) soft-marks + agent-decommissions the // source once every app has moved and come up on the target. A VALIDATE refusal returns here. id, err := s.stackMgr.MigrateAllAndDecommission(r.Context(), req.Where, strings.TrimSpace(req.Target)) diff --git a/controller/internal/web/templates/deploy.html b/controller/internal/web/templates/deploy.html index 53cb2be..6e92b65 100644 --- a/controller/internal/web/templates/deploy.html +++ b/controller/internal/web/templates/deploy.html @@ -565,8 +565,9 @@ onchange="checkStorageSpace(this)"> {{range $.StoragePaths}} {{end}} {{if $.CurrentHDDPathMissing}}