From 7ea942a11843c020be88609fd85fe5638c687286 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sun, 25 Oct 2026 22:26:45 +0100 Subject: [PATCH] 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. --- CHANGELOG.md | 14 +++ controller/internal/infra/infra.go | 19 ++- .../internal/stacks/filebrowser_gate.go | 114 ++++++++++++++++-- .../stacks/filebrowser_password_test.go | 84 ++++++++++++- .../stacks/filebrowser_restart_gate_test.go | 11 +- controller/internal/stacks/manager.go | 4 + 6 files changed, 223 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6fdccb1..0454a737 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ **MinAgent: 0.158.1** (unchanged). The hold acts only on a box whose agent reports `waiting_for_quiet` (≥ 0.165.0). +### FileBrowser is off the app network: only traefik (and a linked OnlyOffice) reach it (R-1281, P2, D44 #23) + +- **Measured before** (the night's rig, 0.344.0-rc.2): from the qBittorrent and Transmission containers (on + traefik-public, as every app), `GET http://filebrowser:80/api/resources` with the box's secret sign-in header set to + the household user → **200, the drive's listing** (FileBrowser's log names the user `felhom-household`); without the + header 401. The guard was only the header NAME's secrecy (`infra/filebrowser_sso.go`). +- **Now** (`internal/stacks/filebrowser_gate.go` `ensureFileBrowserPublic`): a new network `felhom-files` + (`infra.FilesNetwork`); traefik joins it; the CHECKED FileBrowser container joins it by id after this run's default + login was refused (R-1126's gate, unchanged) — never traefik-public any more; one an earlier controller put on + traefik-public leaves it. OnlyOffice joins `felhom-files` while — and only while — its FileBrowser link is on (it + fetches and saves documents at `http://filebrowser:80`, R-1018) — the one stated exception. The route stays by name. +- Tests: `TestR1281_*` (three) + the two R-1126 gate tests updated to the files network. Red-proofs: felhom.eu + `documentation/audits/night-2026-10-25/redproof-r1281.txt`. + ### A held app says WHY it is held (R-1286, found reading demo-hp's Kimai) - `cmd/controller/main.go` `holdWhy`: the start gate (and through it the boot sweep's line and `--restore-holds`) now diff --git a/controller/internal/infra/infra.go b/controller/internal/infra/infra.go index efb9b2de..1cae8117 100644 --- a/controller/internal/infra/infra.go +++ b/controller/internal/infra/infra.go @@ -192,6 +192,13 @@ func RenderCloudflared(d CloudflaredData) (map[string]FileSpec, error) { // FileBrowser is put on traefik-public. const FileBrowserNetwork = "felhom-filebrowser" +// FilesNetwork (R-1281, operator D44 #23, 2026-10-25) is the ONLY network on which traefik reaches FileBrowser: traefik +// is on it, the checked FileBrowser container is joined to it after the default-login check (by id, as R-1126 did with +// traefik-public), and no app is — except OnlyOffice while its FileBrowser link is on (it fetches and saves documents at +// http://filebrowser:80, and FileBrowser calls http://onlyoffice:80). FileBrowser is never on traefik-public, where every +// app container is: an app that learnt the secret header name could otherwise sign in as the household. +const FilesNetwork = "felhom-files" + func RenderFileBrowserCompose(domain string, storageMounts []string, groupAdd ...int) string { // R-691 (v0.275.0): supplementary groups — the owning groups of kept folders the view must READ (a // 0770 folder another user owns, e.g. nextcloud's www-data). Their binds are `:ro`. Absent → the @@ -229,10 +236,10 @@ services: volumes: - filebrowser_data:/home/filebrowser/data - ./config.yaml:/home/filebrowser/config.yaml:ro%s - # R-1126 (D11 = A): NOT on traefik-public at start. The box joins it to traefik-public only after it has checked - # THIS run's default login is refused (stacks.ensureFileBrowserPublic), so files. never answers with - # FileBrowser's default password — a re-created container always starts unreachable from outside. The box reaches - # it for that check over its own network. + # R-1126 (D11 = A): NOT reachable from traefik at start. The box joins it to felhom-files (traefik's route to it, + # R-1281 — never traefik-public, where every app is) only after it has checked THIS run's default login is refused + # (stacks.ensureFileBrowserPublic), so files. never answers with FileBrowser's default password — a + # re-created container always starts unreachable from outside. The box reaches it for that check over its own network. networks: - %s deploy: @@ -249,7 +256,7 @@ services: # BY NAME. Docker labels made Traefik pick FileBrowser's address once, at container start — on its own network, # before the box joined it to traefik-public — and a later "network connect" did not refresh it (demo-hp and the # N100 timed out after v0.327.0). By name, the address is resolved per connection, and only while FileBrowser is on - # traefik-public. + # felhom-files (R-1281; traefik-public before). volumes: filebrowser_data: @@ -261,7 +268,7 @@ networks: } // RenderFileBrowserRoute is the traefik file-provider route for files. → http://filebrowser:80 by NAME (R-1126, -// v0.327.1). The name resolves only while FileBrowser is on traefik-public, which the box allows only after this run's +// v0.327.1). The name resolves only while FileBrowser is on felhom-files (R-1281), which the box allows only after this run's // default login was checked — so the route can stay in place and still never reach an unchecked FileBrowser (502 then). // TLS as the old Docker label had it (`tls: {}`, the default certificate store / the wildcard). // diff --git a/controller/internal/stacks/filebrowser_gate.go b/controller/internal/stacks/filebrowser_gate.go index 8f98f140..b15ea992 100644 --- a/controller/internal/stacks/filebrowser_gate.go +++ b/controller/internal/stacks/filebrowser_gate.go @@ -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. 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. 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. 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. stays unreachable): %v", infra.FilesNetwork, err) + return + } + m.logger.Printf("[INFO] [infra] filebrowser: default login refused for this run — joined %s, files. 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. 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. 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 diff --git a/controller/internal/stacks/filebrowser_password_test.go b/controller/internal/stacks/filebrowser_password_test.go index a411a9e0..b4aa9977 100644 --- a/controller/internal/stacks/filebrowser_password_test.go +++ b/controller/internal/stacks/filebrowser_password_test.go @@ -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) + } +} diff --git a/controller/internal/stacks/filebrowser_restart_gate_test.go b/controller/internal/stacks/filebrowser_restart_gate_test.go index 824674a1..a5544aa4 100644 --- a/controller/internal/stacks/filebrowser_restart_gate_test.go +++ b/controller/internal/stacks/filebrowser_restart_gate_test.go @@ -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) } diff --git a/controller/internal/stacks/manager.go b/controller/internal/stacks/manager.go index 37fda838..fb9843d3 100644 --- a/controller/internal/stacks/manager.go +++ b/controller/internal/stacks/manager.go @@ -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.