R-67: the NAS share appears in FileBrowser (v0.160.0)

Network shares bind their share ROOT :rslave into FileBrowser — no
skeleton, no userdata scoping, nothing written toward the NAS. Gate is
the stub classifier (stub ⇒ excluded from mounts AND sources — an
exposed stub swallows uploads the real mount later shadows); idle autofs
is healthy and included. Drives byte-identical. Add/remove trigger the
debounced sync. Phase-0 probe on demo-hp: GO (in-container rslave access
wakes the idle trigger). Red-proofs A + B run.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UuFPHmHNrCJj1VhY6QdDMU

controller/internal/web/filebrowser_network_test.go

@@ -0,0 +1,162 @@
+package web
+
+import (
+	"bytes"
+	"log"
+	"strings"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/appbackup"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/infra"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/system"
+)
+
+// R-67 — the NAS share appears in FileBrowser. Scenarios A–D drive buildFileBrowserPaths with
+// every edge seamed (no drive, no NAS, no docker anywhere near these tests).
+
+func fbTestDrive() settings.StoragePath {
+	return settings.StoragePath{Path: "/mnt/felhom-drives/hdd_1", Label: "Külső HDD", Schedulable: true}
+}
+
+func fbTestShare() settings.StoragePath {
+	return settings.StoragePath{
+		Path: "/mnt/felhom-drives/Felhom-Share", Label: "Felhom Share", Schedulable: true,
+		Kind: settings.StorageKindNetwork, Protocol: "smb", Server: "192.168.0.104", Export: "Felhom-Share",
+	}
+}
+
+// fbDeps returns deps where the drive is a live mountpoint, the classifier is scripted, and the
+// skeleton fake RECORDS every invocation — the A red-proof hangs off that recording.
+func fbDeps(classifyResult string, skeletonCalls *[]string, logBuf *bytes.Buffer) fbPathDeps {
+	var lg *log.Logger
+	if logBuf != nil {
+		lg = log.New(logBuf, "", 0)
+	}
+	return fbPathDeps{
+		isMount:  func(string) bool { return true },
+		classify: func(string) string { return classifyResult },
+		ensureSkeleton: func(path string) error {
+			*skeletonCalls = append(*skeletonCalls, path)
+			return nil
+		},
+		logger: lg,
+	}
+}
+
+// Scenario A: one drive + one network share (healthy verdict) → the drive's userdata line is
+// BYTE-IDENTICAL to a drives-only pass (the B′ invariant), the share binds its ROOT with :rslave,
+// the config carries both sources, and the skeleton ran for the drive ONLY — never toward the NAS.
+func TestFileBrowserNetworkShareIncluded(t *testing.T) {
+	drive, share := fbTestDrive(), fbTestShare()
+	var calls []string
+	mounts, cfgPaths := buildFileBrowserPaths([]settings.StoragePath{drive, share}, fbDeps(system.FSClassNetwork, &calls, nil))
+
+	var onlyCalls []string
+	onlyMounts, _ := buildFileBrowserPaths([]settings.StoragePath{drive}, fbDeps(system.FSClassNetwork, &onlyCalls, nil))
+
+	// B′: the drive line with the share present is byte-identical to the drives-only render.
+	if len(onlyMounts) != 1 || len(mounts) != 2 {
+		t.Fatalf("mounts: got %v / drives-only %v", mounts, onlyMounts)
+	}
+	if mounts[0] != onlyMounts[0] {
+		t.Errorf("drive line changed by the network branch:\n with: %q\n only: %q", mounts[0], onlyMounts[0])
+	}
+	wantDrive := "      - " + appbackup.UserdataDir(drive.Path) + ":/srv/hdd_1"
+	if mounts[0] != wantDrive {
+		t.Errorf("drive line = %q, want %q", mounts[0], wantDrive)
+	}
+	// The share binds its ROOT (not a userdata subtree) with :rslave — the propagation flag is
+	// load-bearing (idle-wake events must reach the running container).
+	wantShare := "      - /mnt/felhom-drives/Felhom-Share:/srv/Felhom-Share:rslave"
+	if mounts[1] != wantShare {
+		t.Errorf("share line = %q, want %q", mounts[1], wantShare)
+	}
+	// Skeleton: exactly the drive, NEVER the NAS. (Red-proof: routing network paths through the
+	// drive branch makes this fail with the share path recorded.)
+	if len(calls) != 1 || calls[0] != drive.Path {
+		t.Errorf("skeleton calls = %v, want exactly [%s] — a skeleton toward the NAS writes Felhom convention dirs onto the customer's own NAS", calls, drive.Path)
+	}
+	// Config has both sources, share named by its display label.
+	cfg := infra.RenderFileBrowserConfig(cfgPaths)
+	for _, m := range []string{`- path: "/srv/hdd_1"`, `- path: "/srv/Felhom-Share"`, `name: "Felhom Share"`} {
+		if !strings.Contains(cfg, m) {
+			t.Errorf("config missing %q:\n%s", m, cfg)
+		}
+	}
+	// And the full compose render carries both lines (the renderer passes propagation through).
+	compose := infra.RenderFileBrowserCompose("example.hu", mounts)
+	if !strings.Contains(compose, wantShare) || !strings.Contains(compose, wantDrive) {
+		t.Errorf("compose lost a mount line:\n%s", compose)
+	}
+}
+
+// Scenario B: stub verdict → the share is excluded from BOTH mounts and sources this pass, the
+// drive is untouched, and the warn is logged. The wrong case this gate kills: a customer uploads
+// into a local stub dir that the real mount later shadows — their files silently vanish from view.
+// Red-proof: dropping the stub gate fails the absent-line assertions.
+func TestFileBrowserNetworkStubExcluded(t *testing.T) {
+	drive, share := fbTestDrive(), fbTestShare()
+	var calls []string
+	var logBuf bytes.Buffer
+	mounts, cfgPaths := buildFileBrowserPaths([]settings.StoragePath{drive, share}, fbDeps(system.FSClassStub, &calls, &logBuf))
+
+	if len(mounts) != 1 || strings.Contains(mounts[0], "Felhom-Share") {
+		t.Errorf("stub share leaked into mounts: %v", mounts)
+	}
+	cfg := infra.RenderFileBrowserConfig(cfgPaths)
+	if strings.Contains(cfg, "Felhom-Share") {
+		t.Errorf("stub share leaked into the source list:\n%s", cfg)
+	}
+	if !strings.Contains(cfg, `- path: "/srv/hdd_1"`) {
+		t.Errorf("drive source lost while excluding the stub share:\n%s", cfg)
+	}
+	if !strings.Contains(logBuf.String(), "local stub") {
+		t.Errorf("stub exclusion not logged: %q", logBuf.String())
+	}
+}
+
+// Scenario C: the autofs-trigger verdict (idle automount) INCLUDES the share — idle is healthy;
+// the wrong case here is treating idle like a detached drive (the pre-R-67 gate skipped an idle
+// share forever). unknown also includes (fail open), as does a nil classifier.
+func TestFileBrowserNetworkIdleIncluded(t *testing.T) {
+	share := fbTestShare()
+	for _, verdict := range []string{system.FSClassAutofs, system.FSClassNetwork, system.FSClassUnknown} {
+		var calls []string
+		mounts, cfgPaths := buildFileBrowserPaths([]settings.StoragePath{share}, fbDeps(verdict, &calls, nil))
+		if len(mounts) != 1 || !strings.Contains(mounts[0], ":rslave") {
+			t.Errorf("verdict %q: share not mounted: %v", verdict, mounts)
+		}
+		if len(cfgPaths) != 1 {
+			t.Errorf("verdict %q: share not in sources", verdict)
+		}
+	}
+	// nil classifier (no seam wired) must fail OPEN — never hide the share.
+	var calls []string
+	d := fbDeps("", &calls, nil)
+	d.classify = nil
+	if mounts, _ := buildFileBrowserPaths([]settings.StoragePath{share}, d); len(mounts) != 1 {
+		t.Errorf("nil classifier hid the share: %v", mounts)
+	}
+}
+
+// Scenario D: removal — a registry without the share renders with no trace of it, and the F2
+// change detection sees the difference (forces the recreate that drops the live mount).
+func TestFileBrowserNetworkRemoval(t *testing.T) {
+	drive, share := fbTestDrive(), fbTestShare()
+	var calls []string
+	withMounts, withCfg := buildFileBrowserPaths([]settings.StoragePath{drive, share}, fbDeps(system.FSClassNetwork, &calls, nil))
+	withoutMounts, withoutCfg := buildFileBrowserPaths([]settings.StoragePath{drive}, fbDeps(system.FSClassNetwork, &calls, nil))
+
+	oldCompose := infra.RenderFileBrowserCompose("example.hu", withMounts)
+	newCompose := infra.RenderFileBrowserCompose("example.hu", withoutMounts)
+	oldCfg := infra.RenderFileBrowserConfig(withCfg)
+	newCfg := infra.RenderFileBrowserConfig(withoutCfg)
+
+	if strings.Contains(newCompose, "Felhom-Share") || strings.Contains(newCfg, "Felhom-Share") {
+		t.Error("removed share left a trace in the renders")
+	}
+	if !fbNeedsRecreate([]byte(oldCfg), []byte(newCfg), []byte(oldCompose), []byte(newCompose)) {
+		t.Error("F2 change detection missed the share removal — the live mount would linger")
+	}
+}

controller/internal/web/handlers.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"log"
 	"net/http"
 	"net/url"
 	"os"
@@ -2119,33 +2120,18 @@ func (s *Server) syncFileBrowserMounts(resetDBOnChange bool) {
 		return
 	}
 
-	// Build volume mount lines. SCOPE to the drive's `userdata/` subtree (v0.66.0): the customer
-	// browses ONLY userdata — app internals (appdata/) and the recovery units + Tier 2 copies
-	// (backups/) are NOT mounted into FileBrowser. userdata is owned group 1000 mode 2775 (setgid),
-	// and FileBrowser runs as uid 1000 → it can create folders + upload files (the old appdata mount
-	// was guest-root 0755 → permission-denied). Pre-create the full skeleton with the convention.
-	var storageMounts []string
-	for _, sp := range paths {
-		mountName := filepath.Base(sp.Path) // "/mnt/hdd_1" → "hdd_1"
-		// Drive-absent gate: an external drive path that isn't currently a live mountpoint is detached —
-		// don't create its userdata skeleton (would write onto the rootfs) and don't mount it into
-		// FileBrowser this pass. It returns on the next sync after reconnect. Matches planDriveGates'
-		// external-only rule (system paths, not under StableParentDir, are never skipped).
-		if skipFileBrowserPath(sp.Path, system.IsMountPoint) {
-			s.logger.Printf("[INFO] [web] FileBrowser: drive %s not mounted — skipping userdata skeleton", sp.Path)
-			continue
-		}
-		if err := appbackup.EnsureUserdataSkeleton(sp.Path); err != nil {
-			s.logger.Printf("[WARN] [web] FileBrowser: could not ensure userdata skeleton on %s: %v", sp.Path, err)
-		}
-		userdataSrc := appbackup.UserdataDir(sp.Path)
-		line := fmt.Sprintf("      - %s:/srv/%s", userdataSrc, mountName)
-		storageMounts = append(storageMounts, line)
-	}
+	// Build volume mount lines + the config source set (R-67: the two must agree — a source with
+	// no mount behind it renders a broken sidebar entry).
+	storageMounts, configPaths := buildFileBrowserPaths(paths, fbPathDeps{
+		isMount:        system.IsMountPoint,
+		classify:       s.classifyFSPath,
+		ensureSkeleton: appbackup.EnsureUserdataSkeleton,
+		logger:         s.logger,
+	})
 
-	// Generate and write config.yaml (sources + sidebar entries per drive)
+	// Generate and write config.yaml (sources + sidebar entries per drive/share)
 	configPath := stackDir + "/config.yaml"
-	fbConfig := generateFileBrowserConfig(paths)
+	fbConfig := generateFileBrowserConfig(configPaths)
 
 	// Capture the current on-disk content BEFORE any writes, so we can detect whether this sync
 	// actually changes anything (F2). The integrations' ReapplyConfigForTarget edits config.yaml
@@ -2215,6 +2201,78 @@ func (s *Server) syncFileBrowserMounts(resetDBOnChange bool) {
 	}
 }
 
+// fbPathDeps are the injectable edges of buildFileBrowserPaths — everything that would otherwise
+// touch the real mount table, the real filesystem or a real statfs, so the A/B/C/D scenarios run
+// without a drive, a NAS or docker.
+type fbPathDeps struct {
+	isMount        func(string) bool   // drive-absent gate probe (production: system.IsMountPoint)
+	classify       func(string) string // network stub gate (production: Server.classifyFSPath; nil → include, fail open)
+	ensureSkeleton func(string) error  // userdata skeleton (production: appbackup.EnsureUserdataSkeleton) — DRIVES ONLY
+	logger         *log.Logger
+}
+
+// buildFileBrowserPaths computes one FileBrowser sync pass's volume mount lines + the source-list
+// paths, with the per-kind gates applied. Two storage classes, two DIFFERENT gates:
+//
+// DRIVES (v0.66.0 semantics, unchanged byte-for-byte): scope to the `userdata/` subtree, pre-create
+// the skeleton, and apply the drive-absent gate — a detached external drive must not get a skeleton
+// written onto the rootfs. Drives always stay in the config source list (pre-R-67 behavior kept).
+//
+// NETWORK SHARES (R-67): the drive-absent gate does NOT apply — an idle automount trigger is
+// HEALTHY (first access mounts it; the old gate skipped an idle share forever). The gate here is
+// the STUB classifier instead, and it is a data-safety gate, not cosmetics: exposing a local stub
+// dir lets a customer upload into a directory the real mount will later SHADOW — their files
+// silently vanish from view. A stub share is excluded from BOTH the mounts and the source list
+// this pass (a source without a mount is a broken sidebar entry). autofs-trigger / real network fs
+// / unknown all include (unknown fails open — a wedged NAS must not hide the share forever).
+// The bind is the share ROOT with :rslave — load-bearing: host-side automount wake and
+// idle-unmount events must propagate into the RUNNING container (Phase-0 probe 2026-07-22 proved
+// an in-container access through an rslave bind wakes the idle trigger). Never a skeleton, never
+// any write toward the NAS — Felhom conventions must not be written onto a customer's own NAS.
+func buildFileBrowserPaths(paths []settings.StoragePath, d fbPathDeps) (storageMounts []string, configPaths []settings.StoragePath) {
+	configPaths = make([]settings.StoragePath, 0, len(paths))
+	for _, sp := range paths {
+		mountName := filepath.Base(sp.Path) // "/mnt/hdd_1" → "hdd_1"; ".../Felhom-Share" → "Felhom-Share"
+		if sp.IsNetwork() {
+			if d.classify != nil && d.classify(sp.Path) == system.FSClassStub {
+				if d.logger != nil {
+					d.logger.Printf("[WARN] [web] FileBrowser: %s namespace sees a local stub, not the NAS — excluded until propagation recovers", mountName)
+				}
+				continue
+			}
+			storageMounts = append(storageMounts, fmt.Sprintf("      - %s:/srv/%s:rslave", sp.Path, mountName))
+			configPaths = append(configPaths, sp)
+			continue
+		}
+		// Drives are ALWAYS in the source list (pre-R-67 behavior: the config listed every
+		// registered path; only the mount obeys the drive-absent gate).
+		configPaths = append(configPaths, sp)
+		// Drive-absent gate: an external drive path that isn't currently a live mountpoint is detached —
+		// don't create its userdata skeleton (would write onto the rootfs) and don't mount it into
+		// FileBrowser this pass. It returns on the next sync after reconnect. Matches planDriveGates'
+		// external-only rule (system paths, not under StableParentDir, are never skipped).
+		if skipFileBrowserPath(sp.Path, d.isMount) {
+			if d.logger != nil {
+				d.logger.Printf("[INFO] [web] FileBrowser: drive %s not mounted — skipping userdata skeleton", sp.Path)
+			}
+			continue
+		}
+		// SCOPE to the drive's `userdata/` subtree (v0.66.0): the customer browses ONLY userdata —
+		// app internals (appdata/) and the recovery units + Tier 2 copies (backups/) are NOT mounted
+		// into FileBrowser. userdata is owned group 1000 mode 2775 (setgid), and FileBrowser runs as
+		// uid 1000 → it can create folders + upload files (the old appdata mount was guest-root 0755
+		// → permission-denied). Pre-create the full skeleton with the convention.
+		if err := d.ensureSkeleton(sp.Path); err != nil {
+			if d.logger != nil {
+				d.logger.Printf("[WARN] [web] FileBrowser: could not ensure userdata skeleton on %s: %v", sp.Path, err)
+			}
+		}
+		userdataSrc := appbackup.UserdataDir(sp.Path)
+		storageMounts = append(storageMounts, fmt.Sprintf("      - %s:/srv/%s", userdataSrc, mountName))
+	}
+	return storageMounts, configPaths
+}
+
 // fbNeedsRecreate reports whether the FileBrowser container must be force-recreated: true when either
 // the config.yaml or the compose file content changed between the pre-sync and post-sync state. On the
 // first-ever run the old files are empty → differs from the freshly generated content → true (creates

controller/internal/web/netstorage_handlers.go

@@ -325,6 +325,9 @@ func (s *Server) handleNetStorageRemove(w http.ResponseWriter, r *http.Request)
 		s.logger.Printf("[WARN] [web] netstorage deregister %q: %v", where, err)
 	}
 	s.logger.Printf("[INFO] [web] network storage removed: %s", name)
+	// R-67: drop the share's FileBrowser source + mount line on the next sync (F2 change-detection
+	// forces the recreate).
+	go s.SyncFileBrowserMounts()
 	writeDiskJSON(w, http.StatusOK, true, "", map[string]any{"removed": true, "name": name})
 }
 

controller/internal/web/netstorage_job.go

@@ -283,6 +283,8 @@ func (s *Server) runNetAdd(agent netAgent, req agentapi.AddNetStorageRequest, la
 	s.netAdd.set(job)
 	logx.Infof(s.logger, "[web] network storage added + verified: %s (%s %s:%s) → %s (warn=%q) in %dms",
 		req.Name, req.Protocol, req.Server, req.Export, res.GuestPath, outcome.Warn, time.Since(start).Milliseconds())
+	// R-67: the fresh share becomes browsable — same debounced/mutexed path the drive flows use.
+	go s.SyncFileBrowserMounts()
 }
 
 // pollAgentVerify polls the agent's verify slot until it leaves `running` (or ctx expires).