R-1281: FileBrowser is served on its own felhom-files network (traefik, and OnlyOffice while linked) and leaves traefik-public, so no app container reaches it. R-1126's check-then-publish gate unchanged. Red-proved. Commit only.

2026-10-25 22:26:45 +01:00
parent 838c834169
commit 7ea942a118
6 changed files with 223 additions and 23 deletions
@@ -1,6 +1,8 @@
 package stacks

 import (
+	"os"
+	"path/filepath"
 	"strings"

 	"gitea.dooplex.hu/admin/felhom-controller/internal/dockerexec"
@@ -68,13 +70,16 @@ func (m *Manager) joinFileBrowserNetwork() {
 	m.logger.Printf("[INFO] [infra] connected %s to %s (the FileBrowser default-login check, R-1126)", controllerContainer, infra.FileBrowserNetwork)
 }

-// ensureFileBrowserPublic joins FileBrowser to traefik-public only when THIS run was checked and its default login is
-// refused. Anything less — the run unknown, not checked yet, the check failed — leaves it unreachable from outside.
+// ensureFileBrowserPublic serves files.<domain> only when THIS run was checked and its default login is refused:
+// traefik joins infra.FilesNetwork, then the checked FileBrowser container does — never traefik-public, where every app
+// container is (R-1281). A FileBrowser an earlier controller joined to traefik-public leaves it once it is on the files
+// network. OnlyOffice joins the files network only while its FileBrowser link is on. Anything less than a checked run
+// leaves FileBrowser unreachable from outside, as R-1126 requires.
 //
 // TOCTOU (found by the commit's security review, 2026-10-17): the connect targets the CONTAINER ID read with the run,
 // never the name — web.SyncFileBrowserMounts can re-create FileBrowser between the check and the connect, and a
 // connect by name would publish the new, unchecked container. By id, a replaced container's connect simply fails.
-// Pinned by TestFileBrowserGate_ConnectsTheCheckedContainerById.
+// Pinned by TestFileBrowserGate_ConnectsTheCheckedContainerById and TestR1281_*.
 func (m *Manager) ensureFileBrowserPublic() {
 	run := m.fbStartedAt()
 	if run == "" || run != m.fbProbedRun {
@@ -84,14 +89,107 @@ func (m *Manager) ensureFileBrowserPublic() {
 	if f := strings.Fields(run); len(f) == 2 {
 		target = f[0]
 	}
+	if err := m.fbNetEnsure(infra.FilesNetwork); err != nil {
+		m.logger.Printf("[WARN] [infra] filebrowser: the %s network is not there (files.<domain> stays as it is): %v", infra.FilesNetwork, err)
+		return
+	}
+	if !m.fbNetOn(traefikContainer, infra.FilesNetwork) {
+		if err := m.fbNetConnect(traefikContainer, infra.FilesNetwork); err != nil {
+			m.logger.Printf("[WARN] [infra] filebrowser: traefik could not join %s (files.<domain> stays as it is): %v", infra.FilesNetwork, err)
+			return
+		}
+		m.logger.Printf("[INFO] [infra] filebrowser: traefik joined %s — the only route to FileBrowser (R-1281)", infra.FilesNetwork)
+	}
+	if !m.fbNetOn(target, infra.FilesNetwork) {
+		if err := m.fbNetConnect(target, infra.FilesNetwork); err != nil {
+			m.logger.Printf("[WARN] [infra] filebrowser: could not join %s (files.<domain> stays unreachable): %v", infra.FilesNetwork, err)
+			return
+		}
+		m.logger.Printf("[INFO] [infra] filebrowser: default login refused for this run — joined %s, files.<domain> served (R-1126, R-1281)", infra.FilesNetwork)
+	}
 	if m.fbNetOn(target, traefikNetwork) {
-		return
+		if err := m.fbNetDisconnect(target, traefikNetwork); err != nil {
+			m.logger.Printf("[WARN] [infra] filebrowser: could not leave %s — every app can still reach it (R-1281): %v", traefikNetwork, err)
+		} else {
+			m.logger.Printf("[INFO] [infra] filebrowser: left %s — no app container reaches it any more (R-1281)", traefikNetwork)
+		}
 	}
-	if err := m.fbNetConnect(target, traefikNetwork); err != nil {
-		m.logger.Printf("[WARN] [infra] filebrowser: could not join %s (files.<domain> stays unreachable): %v", traefikNetwork, err)
-		return
+	m.ensureOfficeOnFilesNetwork()
+}
+
+// traefikContainer is traefik's container (infra/templates/traefik-compose.yml.tmpl container_name).
+const traefikContainer = "traefik"
+
+// officeContainer is OnlyOffice's container (the catalog's container_name).
+const officeContainer = "onlyoffice"
+
+// ensureOfficeOnFilesNetwork (R-1281): OnlyOffice shares the files network while — and only while — its FileBrowser
+// link is on (FileBrowser's config holds the office block). Called after FileBrowser was published, so it never reaches
+// an unchecked FileBrowser.
+func (m *Manager) ensureOfficeOnFilesNetwork() {
+	linked, running := m.fbOfficeLinked()
+	on := m.fbNetOn(officeContainer, infra.FilesNetwork)
+	switch {
+	case linked && running && !on:
+		if err := m.fbNetConnect(officeContainer, infra.FilesNetwork); err != nil {
+			m.logger.Printf("[WARN] [infra] filebrowser: OnlyOffice could not join %s (documents do not open in the file manager): %v", infra.FilesNetwork, err)
+			return
+		}
+		m.logger.Printf("[INFO] [infra] filebrowser: OnlyOffice joined %s — its FileBrowser link is on (R-1281)", infra.FilesNetwork)
+	case !linked && on:
+		if err := m.fbNetDisconnect(officeContainer, infra.FilesNetwork); err != nil {
+			m.logger.Printf("[WARN] [infra] filebrowser: OnlyOffice could not leave %s: %v", infra.FilesNetwork, err)
+			return
+		}
+		m.logger.Printf("[INFO] [infra] filebrowser: OnlyOffice left %s — its FileBrowser link is off (R-1281)", infra.FilesNetwork)
 	}
-	m.logger.Printf("[INFO] [infra] filebrowser: default login refused for this run — joined %s, files.<domain> served (R-1126)", traefikNetwork)
+}
+
+// fbNetDisconnect detaches container from network (test seam: fbNetDisconnectFn; a test FileBrowser and no seam → no-op).
+func (m *Manager) fbNetDisconnect(container, network string) error {
+	if m.fbNetDisconnectFn != nil {
+		return m.fbNetDisconnectFn(container, network)
+	}
+	if m.fbHTTP != nil || m.fbBaseURL != "" {
+		return nil
+	}
+	out, err := dockerexec.Command("docker", "network", "disconnect", network, container).CombinedOutput()
+	if err != nil && !strings.Contains(string(out), "is not connected") {
+		return &netConnectError{out: strings.TrimSpace(string(out)), err: err}
+	}
+	return nil
+}
+
+// fbNetEnsure creates network when it does not exist (test seam: fbNetEnsureFn; a test FileBrowser and no seam → no-op).
+func (m *Manager) fbNetEnsure(network string) error {
+	if m.fbNetEnsureFn != nil {
+		return m.fbNetEnsureFn(network)
+	}
+	if m.fbHTTP != nil || m.fbBaseURL != "" {
+		return nil
+	}
+	if err := dockerexec.Command("docker", "network", "inspect", network).Run(); err == nil {
+		return nil
+	}
+	out, err := dockerexec.Command("docker", "network", "create", network).CombinedOutput()
+	if err != nil && !strings.Contains(string(out), "already exists") {
+		return &netConnectError{out: strings.TrimSpace(string(out)), err: err}
+	}
+	return nil
+}
+
+// fbOfficeLinked reports whether FileBrowser's config holds the OnlyOffice block and whether OnlyOffice runs (test
+// seam: fbOfficeLinkedFn; a test FileBrowser and no seam → neither).
+func (m *Manager) fbOfficeLinked() (bool, bool) {
+	if m.fbOfficeLinkedFn != nil {
+		return m.fbOfficeLinkedFn()
+	}
+	if m.fbHTTP != nil || m.fbBaseURL != "" {
+		return false, false
+	}
+	b, err := os.ReadFile(filepath.Join(m.cfg.Paths.StacksDir, "filebrowser", "config.yaml"))
+	linked := err == nil && strings.Contains(string(b), "\nintegrations:\n  office:")
+	return linked, containerRunning(officeContainer)
 }

 // EnsureFileBrowserGate is the whole check outside the base-stack round — called after web.SyncFileBrowserMounts
@@ -279,20 +279,21 @@ func TestFileBrowserGate_PublishedOnlyAfterTheCheck(t *testing.T) {
 	// A new run whose FileBrowser does not answer: nothing is published.
 	down = true
 	m.fileBrowserGateLocked()
-	if nets["filebrowser@traefik-public"] {
+	if nets["filebrowser@traefik-public"] || nets["filebrowser@felhom-files"] {
 		t.Fatal("published while this run's default login was never checked")
 	}
 	// It answers, on its default: replaced first, published after.
 	down = false
 	m.fileBrowserGateLocked()
-	want := []string{"connect felhom-controller felhom-filebrowser", "password replaced", "connect filebrowser traefik-public"}
+	// R-1281: published on the files network (traefik joins it first), never on traefik-public.
+	want := []string{"connect felhom-controller felhom-filebrowser", "password replaced", "connect traefik felhom-files", "connect filebrowser felhom-files"}
 	if strings.Join(events, " | ") != strings.Join(want, " | ") {
 		t.Fatalf("order:\n got  %v\n want %v", events, want)
 	}
 	// A new run (a restart) on the box's own password: checked, nothing changes, nothing connected twice.
 	run = "run-2"
 	m.fileBrowserGateLocked()
-	if len(events) != 3 {
+	if len(events) != 4 {
 		t.Fatalf("a checked restart did more: %v", events)
 	}
 }
@@ -313,7 +314,82 @@ func TestFileBrowserGate_ConnectsTheCheckedContainerById(t *testing.T) {
 		return nil
 	}
 	m.fileBrowserGateLocked()
-	if len(connected) != 2 || connected[1] != "3f2a9c@traefik-public" {
+	if len(connected) != 3 || connected[2] != "3f2a9c@felhom-files" {
 		t.Fatalf("the checked container must be published by its id, got %v", connected)
 	}
 }
+
+// R-1281 (D44 #23): every app container is on traefik-public, so FileBrowser on it could be reached directly — measured
+// 2026-10-18: from an app container, a forged sign-in header got the drive's listing. The checked FileBrowser is served
+// on the files network only, and one an earlier controller put on traefik-public leaves it.
+// RED-PROOF: skip the traefik-public disconnect → it stays on the app network; connect to traefik-public as before →
+// it is on it.
+func TestR1281_FileBrowserLeavesTheAppNetworkAndIsServedOnTheFilesNetwork(t *testing.T) {
+	fb := &fakeFB{password: "admin"}
+	srv := httptest.NewServer(fb.handler())
+	defer srv.Close()
+	m, _, _ := fbManager(t, srv.URL)
+	m.fbStarted = func() string { return "3f2a9c 2026-10-25T20:00:00Z" }
+	nets := map[string]bool{"3f2a9c@traefik-public": true} // as a 0.343.0 controller left it
+	m.fbNetOnFn = func(c, n string) bool { return nets[c+"@"+n] }
+	m.fbNetConnectFn = func(c, n string) error { nets[c+"@"+n] = true; return nil }
+	m.fbNetDisconnectFn = func(c, n string) error { delete(nets, c+"@"+n); return nil }
+	m.fbNetEnsureFn = func(string) error { return nil }
+	m.fbOfficeLinkedFn = func() (bool, bool) { return false, false }
+	m.fileBrowserGateLocked()
+	if nets["3f2a9c@traefik-public"] {
+		t.Fatal("FileBrowser is still on traefik-public — every app container reaches it")
+	}
+	if !nets["3f2a9c@felhom-files"] || !nets["traefik@felhom-files"] {
+		t.Fatalf("FileBrowser and traefik must share the files network: %v", nets)
+	}
+}
+
+// The files network keeps R-1126's guarantee: an unchecked run joins nothing, so a re-created FileBrowser (default
+// password) is not reachable from outside.
+func TestR1281_AnUncheckedRunJoinsNothing(t *testing.T) {
+	fb := &fakeFB{password: "admin"}
+	srv := httptest.NewServer(fb.handler())
+	defer srv.Close()
+	m, _, _ := fbManager(t, srv.URL)
+	m.fbStarted = func() string { return "" } // the run cannot be read
+	var joined []string
+	m.fbNetOnFn = func(string, string) bool { return false }
+	m.fbNetConnectFn = func(c, n string) error { joined = append(joined, c+"@"+n); return nil }
+	m.fbNetEnsureFn = func(string) error { return nil }
+	m.fileBrowserGateLocked()
+	for _, j := range joined {
+		if strings.HasSuffix(j, "@felhom-files") || strings.HasSuffix(j, "@traefik-public") {
+			t.Fatalf("an unchecked run joined %s", j)
+		}
+	}
+}
+
+// OnlyOffice shares the files network while — and only while — its FileBrowser link is on.
+// RED-PROOF: drop ensureOfficeOnFilesNetwork → the linked case never joins.
+func TestR1281_OnlyOfficeJoinsTheFilesNetworkOnlyWhileLinked(t *testing.T) {
+	fb := &fakeFB{password: "admin"}
+	srv := httptest.NewServer(fb.handler())
+	defer srv.Close()
+	m, _, _ := fbManager(t, srv.URL)
+	m.fbStarted = func() string { return "3f2a9c 2026-10-25T20:00:00Z" }
+	nets := map[string]bool{}
+	m.fbNetOnFn = func(c, n string) bool { return nets[c+"@"+n] }
+	m.fbNetConnectFn = func(c, n string) error { nets[c+"@"+n] = true; return nil }
+	m.fbNetDisconnectFn = func(c, n string) error { delete(nets, c+"@"+n); return nil }
+	m.fbNetEnsureFn = func(string) error { return nil }
+	linked := true
+	m.fbOfficeLinkedFn = func() (bool, bool) { return linked, true }
+	m.fileBrowserGateLocked()
+	if !nets["onlyoffice@felhom-files"] {
+		t.Fatalf("a linked OnlyOffice must reach FileBrowser: %v", nets)
+	}
+	linked = false
+	m.fileBrowserGateLocked()
+	if nets["onlyoffice@felhom-files"] {
+		t.Fatal("an unlinked OnlyOffice must leave the files network")
+	}
+	if nets["onlyoffice@traefik-public"] || nets["3f2a9c@traefik-public"] {
+		t.Fatalf("nothing joins traefik-public here: %v", nets)
+	}
+}
@@ -77,9 +77,10 @@ func TestRestartStack_R1018_FileBrowserPublishedAgainAtOnce(t *testing.T) {
 		return "", nil
 	}

-	// The gate's view of FileBrowser: run 1 was checked and public; a recreate makes run 2, off traefik-public.
+	// The gate's view of FileBrowser: run 1 was checked and public; a recreate makes run 2, off the files network (R-1281:
+	// felhom-files is traefik's only route to it; traefik-public before).
 	var mu sync.Mutex
-	nets := map[string]bool{"filebrowser@traefik-public": true, "felhom-controller@felhom-filebrowser": true}
+	nets := map[string]bool{"filebrowser@felhom-files": true, "traefik@felhom-files": true, "felhom-controller@felhom-filebrowser": true}
 	connected := make(chan string, 8)
 	m.fbStarted = func() string {
 		if recreated() {
@@ -108,9 +109,9 @@ func TestRestartStack_R1018_FileBrowserPublishedAgainAtOnce(t *testing.T) {
 	}
 	os.Remove(marker)

-	// FileBrowser's restart: the recreate drops it from traefik-public; the gate must put run 2 back by itself.
+	// FileBrowser's restart: the recreate drops it from the files network; the gate must put run 2 back by itself.
 	mu.Lock()
-	delete(nets, "filebrowser@traefik-public")
+	delete(nets, "filebrowser@felhom-files")
 	mu.Unlock()
 	if err := m.RestartStack("filebrowser"); err != nil {
 		t.Fatalf("filebrowser restart: %v", err)
@@ -119,7 +120,7 @@ func TestRestartStack_R1018_FileBrowserPublishedAgainAtOnce(t *testing.T) {
 	for {
 		select {
 		case c := <-connected:
-			if c == "filebrowser@traefik-public" {
+			if c == "filebrowser@felhom-files" {
 				if m.fbProbedRun != "run-2" {
 					t.Fatalf("published, but the checked run is %q, not the new run-2", m.fbProbedRun)
 				}
@@ -294,6 +294,10 @@ type Manager struct {
 	// fbNetOnFn / fbNetConnectFn replace the docker network reads and connects of the R-1126 gate in tests.
 	fbNetOnFn      func(container, network string) bool
 	fbNetConnectFn func(container, network string) error
+	// R-1281: the disconnect, the network create, and „is OnlyOffice linked to FileBrowser and running" — test seams.
+	fbNetDisconnectFn func(container, network string) error
+	fbNetEnsureFn     func(network string) error
+	fbOfficeLinkedFn  func() (linked, running bool)
 	// fbSetPassword replaces the stop / CLI / start sequence of ResetFileBrowserAdminPassword in tests; nil in production.
 	fbSetPassword func(ctx context.Context, password string) error
 	// execFn replaces execCommand's process boundary in tests; nil in production.