v0.328.0: the catalog allow-list — the box refuses container rights a template asks for and the box does not give (R-971, D2 = A)

internal/privpolicy checks the RENDERED compose (privileged, cap_add, devices, host namespaces, security_opt, sysctls, volumes_from, build/include/extends, env_file/config files outside the stack dir, aliasing named volumes, binds outside the box's data roots, the Docker socket, propagation, ${VAR} in a privileged key) against an allow-list compiled into the controller (one entry: homepage's read-only socket). Asked before every compose write: the 15-minute sync, the guarded update/ladder step, the install, both settings changes, both restores (before the stop). A refusal writes nothing, keeps the app on its last good compose, raises the operator-only event app_definition_refused once per definition and puts one line on the app page. The sync never copies a template named like a controller infra stack. Structural guard TestAllowList_IsWiredIntoEveryComposeWriter. Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
2026-10-17 16:34:12 +02:00
parent 71fbef6bbe
commit 6d90374f63
26 changed files with 2408 additions and 9 deletions
@@ -0,0 +1,128 @@
+package sync
+
+import (
+	"io"
+	"log"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/config"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/stacks"
+)
+
+// R-971 — the 15-minute correction refuses a catalog definition the box's allow-list does not admit: the deployed
+// app keeps its last good compose AND its .felhom.yml (nothing of the app moves this cycle), read back off disk.
+
+const tplOldFixedPrivileged = `services:
+  web:
+    image: nextcloud:31.0.14-apache
+    privileged: true
+    healthcheck:
+      test: ["CMD", "curl", "-fsS", "http://127.0.0.1:80/status.php"]
+`
+
+// RED-PROOF (P2/redproof-allowlist.txt): with the admit call removed from copyTemplates, the live compose gains
+// `privileged: true` and this test fails.
+func TestR971_RefusedCorrectionWritesNothing(t *testing.T) {
+	for _, wired := range []bool{true, false} { // the manager's check, and the syncer's own placeholder-env check
+		s, stackDir, _ := renderFixture(t, tplOldFixedPrivileged)
+		live := filepath.Join(stackDir, "docker-compose.yml")
+		meta := filepath.Join(stackDir, ".felhom.yml")
+		write(t, live, tplOld)
+		write(t, meta, "OLD META\n")
+		write(t, stacks.AppliedComposePath(stackDir), tplOld)
+		s.SetRenderPlanFn(pinnedPlan(stackDir, map[string]string{"web": "nextcloud:31.0.14-apache"}, true))
+		asked := 0
+		if wired {
+			s.SetAdmitFn(func(app string, data []byte) error {
+				asked++
+				return s.admit2(app, data)
+			})
+		}
+		_, updated, err := s.copyTemplates()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if got := readFile(t, live); got != tplOld {
+			t.Fatalf("[wired=%v] the refused definition reached the live compose:\n%s", wired, got)
+		}
+		// The app is not reported updated, so the post-sync hook does not redeploy it: it keeps running as it was.
+		for _, u := range updated {
+			if u == "nextcloud" {
+				t.Fatalf("[wired=%v] a refused app was reported updated — the post-sync redeploy would touch it", wired)
+			}
+		}
+		if got := readFile(t, meta); got != "OLD META\n" {
+			t.Fatalf("[wired=%v] the .felhom.yml of a refused app moved: %q", wired, got)
+		}
+		if wired && asked == 0 {
+			t.Fatal("the manager's check was never asked")
+		}
+	}
+}
+
+// admit2 is the placeholder-env check (what an unwired syncer uses), callable from the wired case.
+func (s *Syncer) admit2(app string, data []byte) error {
+	saved := s.admitFn
+	s.admitFn = nil
+	defer func() { s.admitFn = saved }()
+	return s.admit(app, data)
+}
+
+// Positive control: the same fix without the privilege flows exactly as before (Group A's assertion).
+func TestR971_AdmittedCorrectionStillFlows(t *testing.T) {
+	s, stackDir, _ := renderFixture(t, tplOldFixed)
+	live := filepath.Join(stackDir, "docker-compose.yml")
+	write(t, live, tplOld)
+	write(t, stacks.AppliedComposePath(stackDir), tplOld)
+	s.SetRenderPlanFn(pinnedPlan(stackDir, map[string]string{"web": "nextcloud:31.0.14-apache"}, true))
+	if _, _, err := s.copyTemplates(); err != nil {
+		t.Fatal(err)
+	}
+	if got := readFile(t, live); !strings.Contains(got, "status.php") {
+		t.Fatalf("an admitted fix did not flow:\n%s", got)
+	}
+}
+
+// A catalog template named like a controller infra stack never lands in that stack's directory.
+func TestR971_CatalogNeverWritesAnInfraStack(t *testing.T) {
+	root := t.TempDir()
+	cfg := &config.Config{}
+	cfg.Paths.DataDir = filepath.Join(root, "data")
+	cfg.Paths.StacksDir = filepath.Join(root, "stacks")
+	for _, name := range []string{"traefik", "cloudflared", "filebrowser", "samba", "felhom-controller"} {
+		cat := filepath.Join(cfg.Paths.DataDir, "catalog-cache", "templates", name)
+		dst := filepath.Join(cfg.Paths.StacksDir, name)
+		for _, d := range []string{cat, dst} {
+			if err := os.MkdirAll(d, 0o755); err != nil {
+				t.Fatal(err)
+			}
+		}
+		write(t, filepath.Join(cat, "docker-compose.yml"), "services:\n  x:\n    image: evil/x:1\n")
+		write(t, filepath.Join(dst, "docker-compose.yml"), "CONTROLLER RENDERED\n")
+	}
+	s := New(cfg, log.New(io.Discard, "", 0), func() error { return nil }, nil)
+	if _, _, err := s.copyTemplates(); err != nil {
+		t.Fatal(err)
+	}
+	for _, name := range []string{"traefik", "cloudflared", "filebrowser", "samba", "felhom-controller"} {
+		if got := readFile(t, filepath.Join(cfg.Paths.StacksDir, name, "docker-compose.yml")); got != "CONTROLLER RENDERED\n" {
+			t.Errorf("%s: the catalog overwrote the controller's own compose: %q", name, got)
+		}
+	}
+}
+
+// An app that is not installed: its refused template is not written either (the install would refuse it again).
+func TestR971_RefusedTemplateOfAnAppNotInstalled(t *testing.T) {
+	s, stackDir, _ := renderFixture(t, tplOldFixedPrivileged)
+	live := filepath.Join(stackDir, "docker-compose.yml")
+	write(t, live, tplOld)
+	if _, _, err := s.copyTemplates(); err != nil {
+		t.Fatal(err)
+	}
+	if got := readFile(t, live); got != tplOld {
+		t.Fatalf("a refused template was written for an app not installed:\n%s", got)
+	}
+}
@@ -22,6 +22,7 @@ import (
 	// must not reach for the Manager through it: everything it needs about an app arrives through
 	// renderPlanFn. Importing the package rather than re-writing a compose parser is deliberate;
 	// a second image parser is exactly the duplication REUSE.md exists to prevent.
+	"gitea.dooplex.hu/admin/felhom-controller/internal/privpolicy"
 	"gitea.dooplex.hu/admin/felhom-controller/internal/stacks"
 )

@@ -48,12 +49,16 @@ type Syncer struct {
 	// behaviour. Every existing test that constructs a Syncer without one keeps passing, and a
 	// wiring mistake degrades to the old product rather than to a broken one.
 	renderPlanFn func(appName string) stacks.RenderPlan
-	mu           sync.Mutex
-	lastSync     time.Time
-	lastErr      error
-	syncing      bool
-	stopCh       chan struct{}
-	stopOnce     sync.Once
+	// admitFn (R-971) is the box's catalog allow-list check for one app's compose, asked BEFORE the file is written.
+	// The stack manager's (stacks.Manager.AdmitSyncedCompose) renders a deployed app with its own env and raises the
+	// operator event; nil → the same check on the placeholder env (privpolicy.PlaceholderEnv), never "no check".
+	admitFn  func(appName string, data []byte) error
+	mu       sync.Mutex
+	lastSync time.Time
+	lastErr  error
+	syncing  bool
+	stopCh   chan struct{}
+	stopOnce sync.Once
 }

 // SyncStatus holds information about the last sync operation.
@@ -94,6 +99,17 @@ func New(cfg *config.Config, logger *log.Logger, rescanFn func() error, postSync
 // seam has to be optional so a Syncer without one keeps the pre-v0.235.0 behaviour exactly.
 func (s *Syncer) SetRenderPlanFn(fn func(appName string) stacks.RenderPlan) { s.renderPlanFn = fn }

+// SetAdmitFn injects the R-971 check (see Syncer.admitFn).
+func (s *Syncer) SetAdmitFn(fn func(appName string, data []byte) error) { s.admitFn = fn }
+
+// admit runs the R-971 check: the injected one, or the placeholder-env check when none is wired.
+func (s *Syncer) admit(appName string, data []byte) error {
+	if s.admitFn != nil {
+		return s.admitFn(appName, data)
+	}
+	return privpolicy.Admit(appName, data, privpolicy.PlaceholderEnv(), privpolicy.Policy{SystemDataPath: s.cfg.Paths.SystemDataPath})
+}
+
 // isDebug returns true if the logging level is set to "debug".
 func (s *Syncer) isDebug() bool { return s.cfg.Logging.Level == "debug" }

@@ -430,6 +446,12 @@ func (s *Syncer) copyTemplates() (newApps []string, updated []string, err error)
 		}

 		appName := entry.Name()
+		if privpolicy.IsControllerInfra(appName) {
+			// R-971: the controller renders its own infra stacks; a catalog template with one of their names must never
+			// land in their directory (traefik holds the Docker socket at the internet edge).
+			s.logger.Printf("[WARN] [sync] catalog template %q has the name of a controller infra stack — not copied", appName)
+			continue
+		}
 		srcDir := filepath.Join(templatesDir, appName)
 		dstDir := filepath.Join(s.cfg.Paths.StacksDir, appName)

@@ -485,7 +507,22 @@ func (s *Syncer) copyTemplates() (newApps []string, updated []string, err error)
 				} else {
 					rendered = stacks.RenderWithLadderDigests(srcDir, raw)
 				}
+				if aerr := s.admit(appName, rendered); aerr != nil {
+					s.logger.Printf("[WARN] [sync] %s: catalog definition REFUSED by the allow-list — the app keeps its last good compose, nothing written: %v", appName, aerr)
+					break // neither the compose nor the .felhom.yml of this app moves this cycle
+				}
 				changed, err = writeIfChanged(rendered, dst)
+			} else if filename == "docker-compose.yml" {
+				data, rerr := os.ReadFile(src)
+				if rerr != nil {
+					s.logger.Printf("[WARN] [sync] Failed to read %s for %s: %v", src, appName, rerr)
+					continue
+				}
+				if aerr := s.admit(appName, data); aerr != nil {
+					s.logger.Printf("[WARN] [sync] %s: stored definition REFUSED by the allow-list — the app keeps its last good compose, nothing written: %v", appName, aerr)
+					break
+				}
+				changed, err = writeIfChanged(data, dst)
 			} else {
 				changed, err = copyIfChanged(src, dst)
 			}