From 0c20c91e855933fee928abf62cf6686b33bdfc95 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sun, 26 Jul 2026 06:59:46 +0200 Subject: [PATCH] SPIKE: catalog-derived userdata skeleton + import surfaces (2026-07-26) Verdict GO, with one mandatory constraint. Phase 0 HOLDS: the customer-facing path set is fully derivable from data the controller already parses (ParseComposeClassifiableBinds), and ValidateBackupSpec's refusal set already covers the path-safety class data_paths: needs -- so the annotation-only design introduces no new filesystem-write primitive. No catalog folder is reachable only via env indirection; zero templates use long `type: bind` syntax. Found off-brief: the derivation is ALREADY LIVE at deploy time (stacks/manager.go:183 ensureUserdataMounts, "the deploy belt"), proven by a clean two-box control -- media/podcasts exists on demo-felhom where audiobookshelf is deployed, not on demo-hp, and is in no skeleton. P0 GO: derived set 14 dirs vs hardcoded 14 (+media/podcasts, -documents); all three hand-verified anchors match. P1 GO: 4 sources incl. nested + accented names all index ready. P2 GO: deep-link template constructible in Go from (sourceName, relPath) alone; login redirect preserves the target. P3 GO for the feature, with a pre-existing setgid-chain break recorded. P4 GO: userdata/import is shareable, guard unchanged, no live share created. P5: before-state inventory captured read-only on both boxes. P6 GO-conditional: the naive derivation produced 20 distinct outputs from 20 identical runs -- a guaranteed FileBrowser force-recreate loop -- fixed by one sort. P7: 12 sources safe server-side. No production code, no version bump, no live mutation. --- .../SPIKE-catalog-data-paths-2026-07-26.md | 866 ++++++++++++++++++ 1 file changed, 866 insertions(+) create mode 100644 documentation/audits/SPIKE-catalog-data-paths-2026-07-26.md diff --git a/documentation/audits/SPIKE-catalog-data-paths-2026-07-26.md b/documentation/audits/SPIKE-catalog-data-paths-2026-07-26.md new file mode 100644 index 0000000..9513f35 --- /dev/null +++ b/documentation/audits/SPIKE-catalog-data-paths-2026-07-26.md @@ -0,0 +1,866 @@ +# SPIKE — catalog-derived userdata skeleton + import surfaces (2026-07-26) + +**Class:** spike. Output is evidence, not shipped code. No production code was written, no version +bumped, no live box mutated. All probes ran in `/tmp` (scratchpad) or a throwaway copy of the +controller tree; the `felhom-controller` working tree is untouched and clean. + +**Verdict up front: GO, with one mandatory constraint and one pre-existing defect surfaced.** +Phase 0 holds — the customer-facing path set is fully derivable from data the controller already +parses. P6 found the naive derivation is *totally* non-deterministic (20 distinct outputs from 20 +identical runs) and would force-recreate FileBrowser on every sync pass; the fix is one `sort`. +P3 surfaced a pre-existing setgid-chain break that the drop-zone design must account for. + +--- + +## 1. Confirmed baselines + +| Repo | `main` @ commit | Notes | +|---|---|---| +| felhom-controller | `3b672ba74cb7a36e9e6c2203fe5a1fdbd71ae866` | tree clean, `HEAD == origin/main`, v0.171.0 | +| felhom-agent | `dfd5d731` | not touched (no host surface in this arc) | +| felhom.eu | `2d78c283` | this doc + the ROADMAP entry | +| app-catalog-felhom.eu | `3067a946529615e4b39fcbc94fc6b14bb21a1c25` | 53 templates; cloned read-only to scratchpad | + +FileBrowser image: `gtstef/filebrowser:1.3.3-stable` +(digest `sha256:eb3733681db8757412632c61a99ad656f0d94ed6781bb2ea114b4d70babab78c`), pulled fresh. +Every container probe used this exact tag. The startup banner confirms +`Initializing FileBrowser Quantum (v1.3.3-stable)` — Quantum, not upstream filebrowser. + +**Exit-code discipline.** Several commands here are pipelines, where `$?` is the *last* stage's +status. One reading was corrupted by exactly that (`docker image inspect … | head; echo rc=$?` +printed `rc=0` while docker had failed with `No such image`); it was caught and re-run with +`${PIPESTATUS[0]}`. `docker pull` reported `PULL_RC=0` with a clean digest line. The P4 test run's +`TEST_RC=0` was read via `PIPESTATUS`, and its stderr was empty. `go run` probes printed no stderr. + +--- + +## 2. Phase 0 — the design finding, confirmed at source + +**Claim: the customer-facing path set is already derivable from data the controller parses today, so +`data_paths:` in `.felhom.yml` can be a pure ANNOTATION over paths that must already exist as +userdata-rooted compose binds — never a place where new paths are declared.** + +### VERDICT: HOLDS. + +Source citations: + +1. **The parser already yields exactly the needed shape.** + `controller/internal/stacks/classify_binds.go:38` `ParseComposeClassifiableBinds(composePath) + []appbackup.ComposeBind`, returning (`internal/appbackup/classify.go:55`): + + ```go + type ComposeBind struct { + Root BindRoot // "userdata" (${USERDATA_PATH}) | "hdd" (${HDD_PATH}) + RelPath string // path.Clean'd, forward-slash, relative; "" for a bare-root bind + ReadOnly bool // true iff the mode field carries a `ro` token + } + ``` + It is pure given the file bytes — no env resolution, no filesystem access beyond the read. + +2. **`ValidateBackupSpec`'s refusal set is exactly the path-safety class `data_paths:` needs** + (`internal/appbackup/classify.go:97`): unknown/empty class, empty path, backslash, absolute, + non-`path.Clean`'d, leading `..`, duplicate `(root, path)`, and — the load-bearing one — + *matches no compose bind*. That last rule is what makes annotation-only enforceable rather than + merely intended: a `data_paths:` entry naming a path that is not already a compose bind is a + whole-block reject. **Do not write a second validator.** + +3. **Consequence: no new filesystem-write primitive is introduced from catalog data.** Every path a + derived skeleton would create is a path the controller *already* creates today — see the deploy + belt in §3. + +### Completeness check (does any customer folder exist only via env indirection the parser can't see?) + +Every host-side token in every `volumes:` short-syntax line across all 53 templates, grouped: + +``` + 3 ${USERDATA_PATH}/media + 2 ${USERDATA_PATH}/downloads + 1 ${USERDATA_PATH}/roms + 1 ${USERDATA_PATH}/media/tv + 1 ${USERDATA_PATH}/media/podcasts + 1 ${USERDATA_PATH}/media/photos + 1 ${USERDATA_PATH}/media/music + 1 ${USERDATA_PATH}/media/movies + 1 ${USERDATA_PATH}/media/comics + 1 ${USERDATA_PATH}/media/books + 1 ${USERDATA_PATH}/media/audiobooks + 1 ${USERDATA_PATH}/import/paperless + 1 ${USERDATA_PATH}/import/calibre + 1 ${HDD_PATH}/appdata/romm/resources + 1 ${HDD_PATH}/appdata/paperless/media + 1 ${HDD_PATH}/appdata/paperless/export + 1 ${HDD_PATH}/appdata/nextcloud + 1 ${HDD_PATH}/appdata/immich + 1 /var/run/docker.sock + … (all remaining ~90 tokens are NAMED DOCKER VOLUMES, e.g. `immich_postgres_data`) +``` + +Every non-`${VAR}` host token is either a named docker volume (no leading `/`, not a host path) or +`/var/run/docker.sock`. **There is no customer folder reachable only through env indirection.** + +Note for the impl task: `${HDD_PATH}/appdata/paperless/export` is an *export* dir that lives under +`appdata/`, i.e. outside the customer-browsable tree. If the role vocabulary grows an `export` role, +this path is NOT a candidate — it is not userdata-rooted and `appdata/` is share-deny-listed. + +--- + +## 3. A finding that reshapes the design: the derivation is ALREADY LIVE, at deploy time + +Not on the brief, found while reading for P5. `controller/internal/stacks/manager.go:183`: + +```go +// ensureUserdataMounts is the deploy belt: pre-create every ${USERDATA_PATH}/... bind source the +// stack declares with the userdata convention, so Docker never auto-creates one as guest-root. +func (m *Manager) ensureUserdataMounts(stackDir string, env []string) { + … + for _, src := range ParseComposeUserdataMounts(composePath, userdataPath) { + if err := appbackup.EnsureUserdataDir(src); err != nil { … } + } +} +``` + +So the controller **already derives userdata dirs from compose binds and applies the 2775/GID-1000 +convention** — lazily, per app, at deploy. `UserdataSkeleton()` is only the *eager, all-apps-upfront* +half of the same idea. + +`ParseComposeUserdataMounts` (`internal/stacks/delete.go:517`) is structurally identical to +`ParseComposeClassifiableBinds` (same `volumes:` scanner, same `SplitN(":",3)`, same quote-trim); it +differs only in resolving to absolute paths and discarding `:ro`. + +**This was confirmed by a clean natural experiment across the two demo boxes** — see P5. `media/podcasts` +is NOT in `UserdataSkeleton()`. It exists on demo-felhom (where audiobookshelf is deployed) and does +NOT exist on demo-hp (where it is not). Nothing but the deploy belt can have created it. + +Consequence for the spec: the mechanism is proven in production, and design fork #2 ("all catalog +apps or only deployed ones?") already has a shipped precedent — the belt does *deployed-only*. + +--- + +## P0 — derivation output set (offline, read-only) + +**Question:** what does `ParseComposeClassifiableBinds` yield across all 53 templates, and how does +that compare to the hardcoded skeleton? + +**Command:** throwaway Go program (`spikep0/main.go`) inside a **copy** of the controller module +(`internal/…` packages are import-restricted, so an out-of-module `/tmp` program cannot reach them — +the repo tree itself was never modified): + +``` +cp -a felhom-controller/controller /tmp/…/scratchpad/ctrl +cd /tmp/…/scratchpad/ctrl && go run ./spikep0 /tmp/…/scratchpad/catalog/templates +``` + +**Raw output:** + +``` +=== TABLE: app -> userdata RelPath -> ReadOnly -> class(origin) === +audiobookshelf media/audiobooks rw optional(explicit) +audiobookshelf media/podcasts rw excluded(explicit) +calibre-web import/calibre rw excluded(explicit) +calibre-web media/books rw mandatory(explicit) +emby media ro excluded(explicit) +immich media/photos ro optional(explicit) +jellyfin media ro excluded(explicit) +komga media/comics rw optional(explicit) +navidrome media/music ro excluded(explicit) +paperless-ngx import/paperless rw excluded(explicit) +plex media ro excluded(explicit) +radarr media/movies rw excluded(explicit) +radarr downloads rw excluded(explicit) +romm roms rw optional(explicit) +sonarr media/tv rw excluded(explicit) +sonarr downloads rw excluded(explicit) + +=== apps with NO userdata bind (41) === +actualbudget adventurelog bentopdf bookstack calcom claper code-server crafty-controller docmost +ghost gitea glance gokapi grafana gramps-web home-assistant homebox homepage kimai mealie n8n +nextcloud onlyoffice opengist outline papra plant-it privatebin rallly recipe-importer seerr +sparkyfitness tandoor termix uptime-kuma vaultwarden vikunja wanderer wger wishlist zipline + +=== apps whose compose contains 'type: bind' long syntax (0) === +[] + +=== (a) DERIVED SET — 14 distinct dirs (leaf binds expanded to ancestor chain) === + downloads <- radarr,sonarr + import (implied parent) + import/calibre <- calibre-web + import/paperless <- paperless-ngx + media <- emby,jellyfin,plex + media/audiobooks <- audiobookshelf + media/books <- calibre-web + media/comics <- komga + media/movies <- radarr + media/music <- navidrome + media/photos <- immich + media/podcasts <- audiobookshelf + media/tv <- sonarr + roms <- romm + +=== (a) DIFF vs hardcoded UserdataSkeleton() (14 entries) === +ADDED by derivation (1): [media/podcasts] +NO LONGER IMPLIED (1): [documents] + +=== (b) import-only (1): [paperless-ngx] +=== (b) library-only (10): [audiobookshelf emby immich jellyfin komga navidrome plex radarr romm sonarr] +=== (b) BOTH (1): [calibre-web] + +=== (b) ANCHORS === + paperless-ngx [import/paperless[rw,excluded]] + calibre-web [import/calibre[rw,excluded] media/books[rw,mandatory]] + romm [roms[rw,optional]] +``` + +### (a) The diff table + +The derived set and the hardcoded skeleton are both 14 entries and differ by exactly one each way: + +| | entry | why | +|---|---|---| +| **ADDED by derivation** | `media/podcasts` | audiobookshelf's second bind. Already live on demo-felhom via the deploy belt (§3) — so this is not a new directory, it is the skeleton catching up to reality. | +| **NO LONGER IMPLIED** | `documents` | No catalog app binds it. It exists on **both** demo boxes. | + +`documents` is the entire removal risk in this arc, and it is a customer-visible folder that may hold +customer files. The impl task's **zero-removals** invariant covers it; it must be carried as an +explicit non-derived entry, not silently dropped. + +### (b) Roles — all three hand-verified anchors match exactly + +| anchor | expected | parser said | ✓ | +|---|---|---|---| +| paperless-ngx | `import/paperless` only | `import/paperless[rw,excluded]` | ✓ | +| calibre-web | `import/calibre` **and** `media/books` | both, and only those | ✓ | +| romm | `roms` only, no import path | `roms[rw,optional]` | ✓ | + +Only **2 of 53** apps have an `import/*` path at all (paperless-ngx, calibre-web). This is a much +smaller drop-zone surface than the feature framing implies — see design fork #2. + +### (c) Binds the parser misses + +**Zero.** No template uses long `type: bind` syntax (the parser's one documented blind spot): +the string `type: bind` does not occur in any of the 53 compose files. Combined with the Phase 0 +completeness scan, the parser sees every userdata-rooted host bind in the catalog. + +### GO / NO-GO: **GO.** + +--- + +## P1 — FileBrowser Quantum source semantics + +**Question:** with four sources — one nested inside another, plus accented/spaced/ampersand names — +does it start, do all four appear, does the nested one corrupt the index? + +**Command:** throwaway container on DooPlex (`~/fbspike-2026-07-26`), pinned image, production +entrypoint wrapper (`sh -c "umask 002; exec /home/filebrowser/filebrowser"`), config shaped like +`infra.RenderFileBrowserConfig` output, port 18080. Sources `/srv/a` (`hdd_1`), `/srv/a/import` +(`Beolvasás`, **nested inside `/srv/a`**), `/srv/b-import` (`Beolvasas`), `/srv/c` +(`Média & könyvtár`). + +**Raw output (startup log, trimmed to the relevant lines):** + +``` +[INFO ] Initializing FileBrowser Quantum (v1.3.3-stable) +[INFO ] Using Config file : /home/filebrowser/config.yaml +[INFO ] Auth Methods : [password] +[INFO ] Sources : [hdd_1: /srv/a Beolvasás: /srv/a/import Beolvasas: /srv/b-import Média & könyvtár: /srv/c] +[INFO ] initializing index: [Beolvasás] +[INFO ] initializing index: [Média & könyvtár] +[DEBUG] Created 1 scanners for [Beolvasás] (1 root + 0 children) +[INFO ] initializing index: [Beolvasas] +[DEBUG] Created 2 scanners for [Média & könyvtár] (1 root + 1 children) +[DEBUG] Created 2 scanners for [Beolvasas] (1 root + 1 children) +[INFO ] initializing index: [hdd_1] +[DEBUG] Created 3 scanners for [hdd_1] (1 root + 2 children) +[INFO ] Running at : http://0.0.0.0/ +``` + +Container status: `Up 6 seconds (healthy)`. **No warning or error relating to the nested source, the +accents, the space, or the ampersand.** The only WARN in the whole log is the benign +`database file could not be found` on first boot. + +Sidebar contents, read from the endpoints the UI itself calls: + +``` +GET /api/users?id=self -> "scopes": [ + {"name": "hdd_1", "scope": "/"}, + {"name": "Beolvasás", "scope": "/"}, + {"name": "Beolvasas", "scope": "/"}, + {"name": "Média & könyvtár", "scope": "/"} ] + +GET /api/settings/sources -> all four present, every one "status": "ready" + Beolvasas numDirs=1 numFiles=1 + Beolvasás numDirs=0 numFiles=1 + Média & könyvtár numDirs=1 numFiles=1 + hdd_1 numDirs=2 numFiles=2 +``` + +**Findings:** +- All four render. Accented, spaced and `&` names round-trip correctly through the config, the + index and the API. +- **The nested source is indexed TWICE** — `hdd_1` reports `numDirs=2 numFiles=2`, which includes + `/srv/a/import` and its file, *and* `Beolvasás` indexes the same file again. Two independent + scanner sets over the same bytes. No corruption, no error, but duplicated index cost and the same + file reachable under two sidebar entries. +- **Source names are the identity key.** `/api/settings/sources` returns a map keyed by name, and + the deep-link URL (P2) is built from the name. Names must therefore be unique — a per-drive + fan-out that produced two `Beolvasás` entries would collide. + +### GO / NO-GO: **GO for the separate-bind route** (`/srv/b-import` renders and browses cleanly). +Nested works too (informational, as the brief specified) but is still not recommended: double +indexing and the duplicate-path confusion buy nothing over a separate bind. + +--- + +## P2 — deep-link URL shape + +**Question:** is a per-app deep link constructible in Go from `(sourceName, relPath)` alone? + +No browser exists on DooPlex, so instead of reading an address bar I read **the router source +itself**, which is strictly more authoritative, and then verified the resulting URLs against the +server. The frontend bundle is served brotli/gzip-compressed (`curl` without `--compressed` yields +binary — an early read was garbage until that was noticed). + +**Raw output — the URL builder, from `index-CdQ4hx-V.js`:** + +```js +// builder: n = source name, t = path +function Ms(n,t,i,r=!1){ … let a=q2(t), s=`/files/${encodeURIComponent(n)}${a}`; … } + +// path encoder q2: per-segment encodeURIComponent +… .map(r=>encodeURIComponent(r)).join("/").replace("//","/") + +// parser (round-trip): source is the 3rd path segment +function H2(n){ let t,i=n; t=i.split("/")[2], i=g0(i,`/files/${t}`); return {source:t,path:i} } +``` + +### The template + +``` +{baseURL}files/{encodeURIComponent(sourceName)}/{each relPath segment, encodeURIComponent'd, joined by "/"} +``` + +It depends on nothing but the source name and the path — **no internal id, no index, no client-side +state.** Constructible server-side in Go. + +**Verification (with a valid session cookie):** + +``` +=== SPA route, WITH session === + /files/Beolvasas/paperless -> 200 + /files/Beolvas%C3%A1s -> 200 + /files/M%C3%A9dia%20%26%20k%C3%B6nyvt%C3%A1r/k%C3%B6nyvek -> 200 + +=== underlying API resource resolution, WITH session === + source=Beolvasas -> 200 + source=Beolvasás -> 200 + source=Média & könyvtár -> 200 + deep path b-import/paperless -> {"name":"paperless","size":4096,…,"type":"directory", + "files":[{"name":"deep-marker.txt",… +``` + +**Honest caveat on those 200s:** `/files/...` is a client-side SPA route, so the server returns the +same shell for *any* such path — the body is byte-identical whether the path is valid or not. The +`200` alone proves nothing. The real evidence is (a) the router source above, and (b) the +`/api/resources` calls, which do server-side resolution and returned the actual directory listing. + +**Cold, no session:** + +``` +=== COLD (no session) === + route /files/Beolvasas/paperless -> 200 (SPA shell; the gate is client-side) + route /files/Beolvas%C3%A1s -> 200 (SPA shell) + api resources (cold) -> 401 +``` + +**The login redirect DOES preserve the target path.** From the router guard and the login component: + +```js +// guard: unauthenticated -> /login carrying the FULL path +if(!(_e.isLoggedIn()||n.matched.some(l=>l.meta.optionalAuth))){ + se.setCurrentUser(null), i({path:"/login",query:{redirect:n.fullPath}}); return } + +// login submit: navigate to the preserved target, else the generic root +async submit(n){ … let t=U.route.query.redirect; + (t===""||t===void 0||t===null)&&(t="/files/"); … } +``` + +`n.fullPath` is the already-encoded path, so a cold deep link survives the login round-trip. + +### The encoding trap — real, and quantified + +The URL is embedded in HTML, so it needs percent-encoding *and* HTML-escaping. They compose safely, +**but Go's `url.PathEscape` is NOT equivalent to `encodeURIComponent`:** + +``` + "O'Brien's" PathEscape="O%27Brien%27s" QueryEscape="O%27Brien%27s" + "Média & könyvtár" PathEscape="M%C3%A9dia%20&%20k%C3%B6nyvt%C3%A1r" QueryEscape="M%C3%A9dia+%26+k%C3%B6nyvt%C3%A1r" + "Beolvasás" PathEscape="Beolvas%C3%A1s" QueryEscape="Beolvas%C3%A1s" + "a+b" PathEscape="a+b" QueryEscape="a%2Bb" +``` + +- `url.PathEscape` leaves `&` **unescaped**. In an `href` attribute Go's `html/template` then emits + `&`, which the browser decodes back to a literal `&`. A bare `&` is a legal path sub-delim and + the server accepted it, so this is correct — but it only works because the two escapings compose. +- `url.QueryEscape` is **wrong here**: it encodes space as `+`, which in a *path* segment means a + literal plus, not a space. +- **Recommendation: `url.PathEscape` per segment**, and let `html/template` do the attribute + escaping. Do not hand-roll, and do not use `QueryEscape`. +- Source names cannot contain `/` today (they come from `filepath.Base`), which keeps the + `split("/")[2]` round-trip safe. Worth stating as an invariant in the impl task. + +### GO / NO-GO: **GO.** The template is constructible from `(sourceName, relPath)` alone. + +--- + +## P3 — upload identity through a new bind + +**Question:** what uid/gid/mode do a file and a folder created through the UI land with, and does +setgid inherit? + +The first run was inconclusive by construction: the probe dir's group was 1000 and the container's +process gid is also 1000, so "inherited the parent's group" and "used the process gid" are +indistinguishable. Re-run with the parent group set to **100**, which separates them. + +**Command:** the exact endpoints the UI invokes (extracted from the bundle: +`POST /api/resources?path=&source=&override=` with the raw body, and the same with `&isDir=true`). + +**Raw output:** + +``` +=== PID1 umask (the wrapped entrypoint) === +Umask: 0002 +=== mimic production convention on the probe dir: 2775, group 1000 === +PARENT: 2775 1000:100 /home/kisfenyo/fbspike-2026-07-26/srv/b-import +=== upload FILE === upload_http=200 +=== create FOLDER === mkdir_http=200 +=== HOST stat (mode owner:group) === + 2775 1000:100 …/srv/b-import + 644 1000:100 …/srv/b-import/p3-uploaded.txt + 755 1000:100 …/srv/b-import/p3-uifolder +``` + +Container identity: `uid=1000(filebrowser) gid=1000(filebrowser)`. PID1 umask is `0002` — the +entrypoint wrapper works as documented at `infra.go:156`. + +**Two separate results, and they point opposite ways:** + +1. **Group inheritance WORKS.** Both the file and the folder landed group **100** — the parent's + group, not the process's gid 1000. Setgid on the parent does its job. For production this means + uploads land group 1000 as intended. ✓ + +2. **The MODE does not honour `umask 002`, and the setgid bit is not propagated.** File `644` (not + `664`), folder `755` (not `2775`). Since PID1's umask really is `0002`, FileBrowser must be + creating with an explicit `0644`/`0755` — the umask wrapper is **inert for these paths**. The + brief asked me to confirm the umask produces the result rather than assume it; it does not. + +### The consequence: a UI-created folder breaks the setgid chain one level down + +``` +=== nested folder created through the UI, inside the UI-created folder === + 755 1000:100 …/p3-uifolder <- parent group 100, NO setgid + 755 1000:1000 …/p3-uifolder/nested <- group 1000 = the PROCESS gid. Chain broken. + +=== a gid-1000 process writing into the UI-made folder === + write OK + 644 1000:1000 …/p3-uifolder/from-app.txt <- group 1000, NOT the inherited 100 + +=== control: the same write into the CONVENTION dir (2775 setgid) === + 644 1000:100 …/from-app-root.txt <- group 100 inherited. Convention intact. +``` + +The control is what makes this conclusive: identical write, identical process, opposite outcome — +the only difference is whether the target dir carries setgid. + +**Is this breaking today?** No — it is **latent**. Every catalog app that touches userdata and +declares an identity declares uid/gid **1000**, the same uid FileBrowser runs as, so owner +permissions cover everything: + +``` +audiobookshelf calibre-web PUID=1000 PGID=1000 +emby UID=1000 GID=1000 immich +jellyfin komga +navidrome paperless-ngx USERMAP_UID=1000 USERMAP_GID=1000 +plex radarr PUID=1000 PGID=1000 +romm sonarr PUID=1000 PGID=1000 +``` + +For the paperless drop-zone specifically the feature works: paperless runs uid 1000, files land uid +1000, and **deletion needs write on the containing directory**, which `import/paperless` has (2775). +The bite comes the day a content app runs as a *different non-root* uid with gid 1000 — then a +customer-created subfolder inside the drop zone becomes unwritable to it, silently. + +**And it is already visible in production** — see P5: `import/calibre` on demo-felhom is live at +`755 1000:1000` instead of `2775 root:1000`. + +### GO / NO-GO: **GO for the stated feature** (paperless can consume and delete), **with the +setgid-chain break recorded as a pre-existing defect the impl task must decide about.** It is not +introduced by this arc and should not be fixed inside it — see Observations. + +--- + +## P4 — share-guard acceptance (throwaway test, no live mutation) + +**Question:** does the real `sharingResolvePath` accept `userdata/import` and still refuse +everything it must? + +**Hard boundary respected:** Megosztás was NOT enabled on demo-felhom or demo-hp, and no share was +created anywhere. This probe is a Go test against a `t.TempDir()` fake root, run in the **copied** +controller tree (the guard is an unexported method on `*web.Server`, so the test must be in-package; +putting it in the copy kept the real repo clean). + +**Command:** `go test ./internal/web/ -run 'TestP4' -v` + +**Raw output:** + +``` +=== RUN TestP4_DenyRootsDoNotCoverUserdataImport + spike_p4_test.go:46: SharingDeniedRoots("/mnt/hdd_1") = [/mnt/hdd_1/appdata /mnt/hdd_1/backups + /mnt/hdd_1/felhom-data /mnt/hdd_1/felhom-data/appdata /mnt/hdd_1/felhom-data/backups] +--- PASS: TestP4_DenyRootsDoNotCoverUserdataImport (0.00s) +=== RUN TestP4_ImportAcceptedRestRefused +--- PASS: TestP4_ImportAcceptedRestRefused (0.00s) +=== RUN TestP4_SymlinkInsideImportEscapes +--- PASS: TestP4_SymlinkInsideImportEscapes (0.00s) +PASS +ok gitea.dooplex.hu/admin/felhom-controller/internal/web 0.009s +TEST_RC=0 +``` + +The deny-list is confirmed as exactly the five entries the brief quoted — `userdata/import` is **not** +among them. Asserted and passing: + +| case | expected | result | +|---|---|---| +| `/userdata/import` | ACCEPTED | ✓ | +| `/userdata/import/paperless` | ACCEPTED | ✓ | +| `/userdata/media/books` | ACCEPTED | ✓ | +| `/appdata/paperless` | refused | ✓ | +| `` itself | refused | ✓ | +| relative path | refused | ✓ | +| `/userdata/../../etc` | refused | ✓ | +| symlink inside `userdata/import` → outside `` | refused | ✓ | + +Both directions live in the same test, so it cannot pass on an always-accept or an always-deny guard. + +### GO / NO-GO: **GO.** `userdata/import` is shareable with no guard change. + +--- + +## P5 — live on-disk inventory (READ-ONLY, both boxes) + +**Question:** the before-state the impl task's zero-removals invariant will be proven against. + +Read-only: `find … -type d -printf`. No `mkdir`, no `chmod`, no `chown`, no service touched. +ASCII-only patterns throughout (the accented-grep trap). demo-hp has no baked key — access was the +G1 break-glass root password from the hub `host_recovery` vault; **the hub.db copy and the plaintext +were both `shred -u`'d immediately after use.** + +### demo-felhom, guest 9201 — `/mnt/felhom-drives/hdd_1/userdata` + +``` +2775 1000:1000 /mnt/felhom-drives/hdd_1/userdata/import/paperless +2775 1000:1000 /mnt/felhom-drives/hdd_1/userdata/media/books +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/documents +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/downloads +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/import +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/media +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/media/audiobooks +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/media/comics +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/media/movies +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/media/music +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/media/photos +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/media/podcasts +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/media/tv +2775 root:1000 /mnt/felhom-drives/hdd_1/userdata/roms +755 1000:1000 /mnt/felhom-drives/hdd_1/userdata/import/calibre +### /mnt/sys_drive/felhom-data/userdata (no output — does not exist) +``` + +Deployed there: `bookstack bookstack-db calibre-web cloudflared docmost docmost-postgres +docmost-redis felhom-controller felhom-samba filebrowser immich-machine-learning immich-postgres +immich-redis immich-server traefik` + +### demo-hp, guest 9201 — `/mnt/felhom-drives/nvme-1tb/userdata` + +``` +2775 1000:1000 /mnt/felhom-drives/nvme-1tb/userdata/import/paperless +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/documents +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/downloads +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/import +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/import/calibre +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/media +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/media/audiobooks +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/media/books +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/media/comics +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/media/movies +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/media/music +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/media/photos +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/media/tv +2775 root:1000 /mnt/felhom-drives/nvme-1tb/userdata/roms +### /mnt/felhom-drives/Felhom-Share/userdata (no output — the NAS, correctly never written to) +### /mnt/sys_drive/felhom-data/userdata (no output — does not exist) +``` + +Deployed there: `cloudflared felhom-controller filebrowser paperless-postgres paperless-redis +paperless-webserver traefik` + +### What the two boxes prove together + +| observation | demo-felhom | demo-hp | reading | +|---|---|---|---| +| dir count under `userdata/` | 15 | 14 | — | +| `media/podcasts` | **present**, 2775 root:1000 | **absent** | Not in `UserdataSkeleton()`. Present exactly where audiobookshelf is deployed → **created by the deploy belt (§3)**. This is the clean control. | +| `documents` | present | present | In the skeleton, implied by no app. The one removal risk. | +| `import/calibre` | **755 1000:1000** | 2775 root:1000 | calibre-web is deployed on demo-felhom and not on demo-hp. | +| `import/paperless` | 2775 1000:1000 | 2775 1000:1000 | owner 1000 = touched by the app; mode intact. | +| `Felhom-Share` (NAS) | n/a | no `userdata/` | R-67 holds: Felhom conventions are never written onto a customer NAS. | + +**The `import/calibre` anomaly.** On demo-felhom it is `755 1000:1000` where every sibling is +`2775 root:1000`. The sharpest control is *within the same app*: calibre-web's other bind, +`media/books`, is `2775 1000:1000` on the same box, same deploy, same belt. Both binds are plain +short syntax and both parsers pick up both (I checked `ParseComposeUserdataMounts` against +calibre-web's compose specifically — there is no parser asymmetry). The difference between them is +that `/cwa-book-ingest` is the app's *managed ingest directory* while `/calibre-library` is not, +which points at the app rewriting the mode of its own drop zone after the belt sets it. I did not +confirm that causally, and I am not asserting it — but it matches P3's shape exactly, and it means +**a drop-zone directory's mode is not stable against the consuming app.** The impl task should verify +before designing anything that depends on `import/*` staying 2775. + +--- + +## P6 — config churn / force-recreate storm risk + +**Question 1: does the catalog sync currently trigger `SyncFileBrowserMounts`?** + +**No.** `cmd/controller/main.go:215`: + +```go +syncer := catalogsync.New(cfg, logger, stackMgr.ScanStacks, func(updated []string) { + stackMgr.InjectMissingFields(updated) +}) +``` + +The post-sync callbacks are `ScanStacks` and `InjectMissingFields` — **neither touches FileBrowser.** +Moreover the rescan is itself gated (`internal/sync/sync.go:215`): + +```go +// Step 3: Trigger rescan if anything changed +if len(newApps) > 0 || len(updated) > 0 { … s.rescanFn() … } +``` + +So today the 15-minute cycle (`sync.go:95`, default `15 * time.Minute`) is a no-op for FileBrowser +even when the catalog *does* change. The ~14 `SyncFileBrowserMounts` call sites are all +storage/network-share lifecycle events, plus startup: + +``` +internal/web/storage_handlers.go:292,306,550,731,889 +internal/web/server.go:234 (Reset), 236 +internal/web/netstorage_job.go:287 internal/web/netstorage_handlers.go:330 +internal/web/handlers.go:2056,2102 +internal/web/intermediary.go:289,307,425,504,548 +``` + +**Reading:** the 15-minute restart storm only materialises **if the impl task wires the catalog cycle +into the FileBrowser sync.** That is a design choice still open, not an existing hazard. + +**Question 2: is a derived source list deterministic?** + +`fbNeedsRecreate` (`handlers.go:2374`) force-recreates on **any** byte difference: + +```go +func fbNeedsRecreate(oldConfig, newConfig, oldCompose, newCompose []byte) bool { + return !bytes.Equal(oldConfig, newConfig) || !bytes.Equal(oldCompose, newCompose) +} +``` + +Probe: generate the derived config **20 times from identical input**, hash each, count distinct +outputs. Three variants — the way an implementer would most likely write it (accumulate into a map, +range over the map), the same with one `sort.Strings`, and today's fixed-slice renderer as a control. + +**Raw output:** + +``` +derive-naive (map) 1/20 identical to the first generation; 20 distinct output(s) +derive-sorted 20/20 identical to the first generation; 1 distinct output(s) +today (fixed slice) 20/20 identical to the first generation; 1 distinct output(s) +``` + +### The determinism number: **1 / 20 for the naive derivation — 20 distinct outputs from 20 identical runs.** +### **20 / 20 with `sort.Strings`. 20 / 20 for today's code (control — the regression baseline is sound).** + +Not "sometimes differs": **every single generation differed.** With 14 entries, Go's randomised map +iteration essentially never repeats an order. Wired into `fbNeedsRecreate`, that is a guaranteed +force-recreate of the customer's file browser on **every** sync pass — a v0.151-class reload loop, +fleet-wide, and it would fire on all ~14 storage-event call sites even without the catalog cycle. + +**The fix is one line.** The spec must state it as a hard requirement with a red-proof, not leave it +to the implementer's care: *the derived source/dir list is sorted before rendering, and a test +asserts N identical generations.* This is the "no silent non-determinism" instance of a standing +project gotcha. + +### GO / NO-GO: **GO, conditional.** The risk is real and total, and it is fully mitigated by an +explicit sort + a determinism test. The spec is not blocked; it gains a mandatory clause. + +--- + +## P7 — sidebar scale + +**Question:** at 8 and 12 sources, is the sidebar still usable? Where does it degrade? + +**Command:** same pinned image, regenerated configs, 8 then 12 sources with realistic Hungarian +names including an apostrophe stress case (`O'Brien's`). + +**Raw output:** + +``` +--- N=8 status: Up 8 seconds (healthy) +--- warnings/errors in log: +2026/07/26 04:49:27 [WARN ] database file could not be found. … (benign first-boot) +--- sources ready: + count= 8 + {'Beolvasas':'ready','Beolvasás':'ready','Filmek':'ready','Fényképek':'ready', + 'Média & könyvtár':'ready','Sorozatok':'ready','Zene':'ready','hdd_1':'ready'} + +--- N=12 status: Up 8 seconds (healthy) +--- warnings/errors in log: +2026/07/26 04:49:37 [WARN ] database file could not be found. … (benign first-boot) +--- sources ready: + count= 12 + {'Beolvasas':'ready','Beolvasás':'ready','Dokumentumok':'ready','Filmek':'ready', + 'Fényképek':'ready','Játékok':'ready','Könyvek':'ready','Média & könyvtár':'ready', + "O'Brien's":'ready','Sorozatok':'ready','Zene':'ready','hdd_1':'ready'} +``` + +All 12 index and reach `ready`; startup stays ~8 s to healthy; no new warnings at either count. +`O'Brien's` resolves (`/api/resources` → 200), confirming the apostrophe path from P2 end to end. + +**Honest limit of this probe:** "usable" in the *visual* sense (scroll length, truncation, whether 12 +entries crowd the sidebar) cannot be measured without a browser, and I did not measure it. What is +established is that the **server side does not degrade** from 4 → 12 sources: no failures, no +warnings, no startup-time cliff, every source ready. Visual usability at 12 remains a manual +click-through question for the operator. + +### Bound for the spec: **12 sources is safe server-side.** Per-drive × per-role fan-out should be +budgeted against that ceiling, and a per-drive × per-role scheme on a 2-drive box with 3 roles +(6 + 2 drive roots = 8) sits comfortably inside it. + +--- + +## Design forks — evidence + recommendation, awaiting operator ruling + +### Fork 1 — per-drive vs canonical import root + +Sources are per registered storage path, so two drives = two `import/` trees, two sidebar entries, +two candidate shares. The per-app deep link is unambiguous either way (it derives from that app's own +`HDD_PATH`); the *share* and the *sidebar* are not. + +**Evidence:** P1 — source names are the identity key and must be unique, so a per-drive scheme must +qualify the name (`Beolvasás — hdd_1`), which is exactly the clutter fork 1 is about. P7 — 12 sources +is safe server-side, so per-drive is not blocked by scale. P5 — both demo boxes have exactly one +drive today, so the ambiguity is not yet observable in the field. + +**Recommendation:** **per-drive**, with the drive name in the source label. It is the only option that +never lies: a canonical root would have to pick one drive, and an app on the other drive would deep-link +into a folder that is not the one its compose bind points at. Accept the extra sidebar entries; P7 +shows they cost nothing server-side. + +### Fork 2 — create dirs for all catalog apps, or only deployed ones? + +**Evidence:** P0(a) — the whole catalog implies **14** dirs, i.e. the all-apps set is barely larger +than today's hardcoded 14 (diff: +`media/podcasts`, −`documents`). Only **2 of 53** apps have an +`import/*` path at all. §3 — the deploy belt already does **deployed-only** and is proven in +production. P5 — the two boxes differ by exactly one dir, and that difference is the belt's work. + +**Recommendation:** **all catalog apps for the skeleton, deployed-only for anything the customer is +told about.** The numbers make the "litters the drop-zone with empty folders" objection nearly moot — +the all-apps import set is two folders — while all-apps keeps the skeleton idempotent and +storage-path-scoped exactly as it is today. Surface only deployed apps' folders in the UI; that is +where emptiness would actually confuse someone. + +### Fork 3 — role vocabulary, and how an unknown role fails + +`import` / `library` / `export`, and whether an unknown role fails OPEN (not surfaced) or is a +whole-block reject. + +**Evidence:** two existing precedents, and they point opposite ways *for good reasons*. +`Lifecycle` (`metadata.go:31`) fails **OPEN** — "An UNKNOWN value degrades to available with one WARN +— a typo in a catalog push must never brick a template." `Backup` (`metadata.go:53`) is a +**whole-block reject** — an invalid class sets the block back to nil and logs one ERROR, degrading to +legacy. The distinguishing principle: lifecycle governs *presentation*, backup governs *data +handling*. + +**Recommendation:** **fail OPEN to "not surfaced", following the R-57 lifecycle precedent.** `data_paths:` +is annotation over paths that already exist and are already created by the belt; an unknown role +costs a missing UI affordance, never a lost or mishandled file. Reserve whole-block reject for the +path-validity rules, which `ValidateBackupSpec`'s refusal set already provides — so a *malformed path* +rejects the block while an *unknown role* just doesn't render. Note the asymmetry explicitly in the +spec so it reads as a decision rather than an inconsistency. + +### Fork 4 — the backup-class collision + +**Evidence, from P0 — the exact classes the derived paths carry:** + +| path | class | origin | +|---|---|---| +| `import/paperless` | **excluded** | explicit | +| `import/calibre` | **excluded** | explicit | +| `media/books` | mandatory | explicit | +| `media/audiobooks`, `media/comics`, `media/photos`, `roms` | optional | explicit | +| `media`, `media/movies`, `media/music`, `media/tv`, `downloads`, `media/podcasts` | excluded | explicit | + +**Both** import paths are `class: excluded` — never shipped offsite, opt-in only for a manual `.fab`. +Every class in the catalog is `explicit`, so the copy can be data-driven with no fallback branch. + +**Recommendation:** drive the UI copy from the class, not by hand. Any surface that says "put your +files here" for an `excluded` path must also say the folder is temporary and unbacked — for +`import/paperless` the catalog's own words are "consume inbox — deleted after ingest by contract". +Because every path carries an explicit class, a two-way mapping (`excluded` → temporary-and-unbacked +notice; otherwise → normal) covers the entire catalog with no default case. + +--- + +## Observations — noticed, deliberately not acted on + +1. **FileBrowser Quantum 1.3.3 ignores `umask 002` for created files and folders** (P3): files land + `0644`, folders `0755`, and the setgid bit is not propagated even though PID1's umask really is + `0002`. The comment at `infra.go:156` explains why the wrapper exists and is correct about the + image not honouring `-e UMASK`; what it does not say is that the wrapper does not achieve the + intended result for these paths either. Latent today (every userdata-touching app runs uid 1000), + real the day one does not. **Deserves its own item — not this arc's to fix.** + +2. **`import/calibre` is live at `755 1000:1000` on demo-felhom** (P5) where every sibling is + `2775 root:1000`, with `media/books` on the same box, same app, same deploy at `2775` as the + control. Consistent with the consuming app rewriting its own ingest dir's mode. Not confirmed + causally; flagged for the impl task to verify before depending on `import/*` staying 2775. + +3. **The nested FileBrowser source is indexed twice** (P1): `hdd_1` scans `/srv/a/import` as a child + *and* `Beolvasás` scans it as a root. Harmless at this size, wasteful at library scale. An argument + for the separate-bind route beyond the one the brief already made. + +4. **`documents` is in the skeleton and implied by no app** (P0/P5). It exists on both boxes and may + hold customer files. It is the only entry the derivation would drop, and the whole reason the + zero-removals invariant needs to be stated rather than assumed. + +5. **FileBrowser's login is not what the API shape suggests** — `POST /api/auth/login?username=` + with the password in an **`X-Password` header** (URL-encoded), not a JSON body. A JSON body is + accepted by the parser and silently yields an empty password, so every attempt fails with a + *bcrypt mismatch*, which reads exactly like a wrong password. Cost real time here. Also: + `auth.adminUsername` / `auth.adminPassword` exist as config keys, and the `set`/`set user` CLI + subcommands in 1.3.3 contradict their own `--help`. Worth a line in the impl task if anything ever + needs to drive FileBrowser programmatically. + +6. **`find / -xdev` will not find `userdata`** on a guest — the storage roots are separate mounts, so + `-xdev` stops at the boundary and returns nothing, which reads identically to "the tree is gone". + Same false-negative class as the accented-grep trap. + +--- + +## Cleanup + +- Probe containers `fbspike` / `fbmany` removed. +- The hub.db copy and the demo-hp break-glass plaintext were `shred -u`'d. +- Nothing was created, chmod'ed or chowned under any live `userdata/` tree; Megosztás was not enabled + and no share was created on either box; no live `config.yaml` or `docker-compose.yml` was edited. +- Probe sources live in the session scratchpad and `~/fbspike-2026-07-26`; nothing was committed to + `felhom-controller`, whose tree is clean at `3b672ba7`.