v0.328.0 (unreleased): security review round 2 — nearest-ancestor symlink resolve (fail closed), any extends refused, the entry binds a literal image and its exact healthcheck, no variable in a bind's options

Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS

controller/internal/privpolicy/allowlist.json

@@ -1,12 +1,13 @@
 {
   "version": 1,
-  "note": "R-971 (operator decision D2 = A, 2026-10-17). The box's catalog allow-list, compiled into the controller. Each entry allows ONE exact setting for ONE service of ONE app, only while that service runs the entry's image repository with the image's own entrypoint and command; a changed setting is a different one and is refused. The catalog's scripts/privilege-allowlist.json is a byte-identical copy for its CI gate. Survey: felhom.eu documentation/audits/agent-allowlist-rotation-2026-10-17/P2/survey.md",
+  "note": "R-971 (operator decision D2 = A, 2026-10-17). The box's catalog allow-list, compiled into the controller. Each entry allows ONE exact setting for ONE service of ONE app, only while that service runs the entry's image repository with the image's own entrypoint and command and the exact healthcheck named; a changed setting is a different one and is refused. The catalog's scripts/privilege-allowlist.json is a byte-identical copy for its CI gate. Survey: felhom.eu documentation/audits/agent-allowlist-rotation-2026-10-17/P2/survey.md",
   "entries": [
     {
       "app": "homepage",
       "service": "homepage",
       "setting": "bind /var/run/docker.sock -> /var/run/docker.sock ro",
       "image": "ghcr.io/gethomepage/homepage",
+      "healthcheck": "[\"CMD\",\"wget\",\"--spider\",\"-q\",\"http://127.0.0.1:3000\"]",
       "why": "the Docker widget and the docker service discovery list the household's containers and their state",
       "doc": "https://gethomepage.dev/configs/docker/"
     }

controller/internal/privpolicy/privpolicy.go

@@ -33,7 +33,9 @@ package privpolicy
 import (
 	_ "embed"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"io/fs"
 	"path/filepath"
 	"regexp"
 	"sort"
@@ -57,8 +59,11 @@ type Entry struct {
 	// Image is the image REPOSITORY (no tag, no digest) the service must run for the entry to hold. Without it a push
 	// could keep the allowed setting and swap the image under it (security review 2026-10-17, finding 1).
 	Image string `json:"image"`
-	Why   string `json:"why"`
-	Doc   string `json:"doc"`
+	// Healthcheck is the exact `healthcheck.test` (compact JSON) the service may carry — the test runs a command inside
+	// the container holding the allowed right, so a changed one is a changed start (round 2). "" = no healthcheck.
+	Healthcheck string `json:"healthcheck"`
+	Why         string `json:"why"`
+	Doc         string `json:"doc"`
 }
 
 type allowList struct {
@@ -166,10 +171,15 @@ func Admit(app string, compose []byte, env map[string]string, p Policy) error {
 		// Finding 1: the entry holds only for the image it names, and only while the service runs that image's own
 		// start — a changed entrypoint or command would run anything with the allowed right.
 		f := svcs[v.Service]
-		if f.repo != e.Image {
+		switch {
+		case f.imageInterp:
+			refused = append(refused, Violation{Service: v.Service, Setting: v.Setting + " with a variable in its image"})
+		case f.repo != e.Image:
 			refused = append(refused, Violation{Service: v.Service, Setting: v.Setting + " with image " + f.repo + " (allowed only for " + e.Image + ")"})
-		} else if f.startOverride {
+		case f.startOverride:
 			refused = append(refused, Violation{Service: v.Service, Setting: v.Setting + " with a changed entrypoint or command"})
+		case f.healthcheck != e.Healthcheck:
+			refused = append(refused, Violation{Service: v.Service, Setting: v.Setting + " with a changed healthcheck"})
 		}
 	}
 	if len(refused) > 0 {
@@ -180,7 +190,9 @@ func Admit(app string, compose []byte, env map[string]string, p Policy) error {
 
 type facts struct {
 	repo          string // the rendered image's repository (no tag, no digest)
+	imageInterp   bool   // the image reference holds a ${VAR}: a deploy-field default could change it later
 	startOverride bool   // entrypoint or command set
+	healthcheck   string // compact JSON of healthcheck.test, "" when none
 }
 
 // serviceFacts reads, per service, what an allow-list entry is bound to.
@@ -194,7 +206,14 @@ func serviceFacts(compose []byte, env map[string]string) map[string]facts {
 		sd, _ := v.(map[string]any)
 		_, ep := sd["entrypoint"]
 		_, cmd := sd["command"]
-		out[name] = facts{repo: imageRepo(c.render(sd["image"])), startOverride: ep || cmd}
+		hc := ""
+		if h, ok := sd["healthcheck"].(map[string]any); ok {
+			if t, has := h["test"]; has {
+				b, _ := json.Marshal(t)
+				hc = string(b)
+			}
+		}
+		out[name] = facts{repo: imageRepo(c.render(sd["image"])), imageInterp: hasInterp(sd["image"]), startOverride: ep || cmd, healthcheck: hc}
 	}
 	return out
 }
@@ -351,10 +370,17 @@ func (c *checker) service(name string, sd map[string]any) {
 			c.add(name, hook+" "+c.canon(v))
 		}
 	}
-	if ex, ok := sd["extends"].(map[string]any); ok {
+	// Any `extends` (round 2): a second service extending an allowed one INHERITS its rights — its socket bind would
+	// reach a service and an image nobody allowed, and the raw file this check reads would not show it.
+	switch ex := sd["extends"].(type) {
+	case map[string]any:
 		if f, has := ex["file"]; has {
 			c.add(name, "extends_file "+c.shown(f))
+		} else {
+			c.add(name, "extends "+c.shown(ex["service"]))
 		}
+	case string:
+		c.add(name, "extends "+c.shown(ex))
 	}
 	for _, v := range list(sd["env_file"]) {
 		raw := v
@@ -394,6 +420,13 @@ func (c *checker) volume(svc string, v any) {
 			return // an anonymous volume: a container path only
 		}
 		rawSrc = parts[0]
+		if isPathSource(rawSrc) && ((len(parts) > 1 && !rootLed(parts[1])) || (len(parts) > 2 && hasInterp(strings.Join(parts[2:], ":")))) {
+			// round 2: a ${VAR} in the options (or a non-root one in the target) could turn on propagation later — the
+			// options are literal, the target may use a root variable only (the *arr apps mount ${USERDATA_PATH}/media
+			// at the same path inside).
+			c.add(svc, "bind "+t+" with a variable in its target or options")
+			return
+		}
 		src, dst = c.render(parts[0]), c.render(parts[1])
 		mode = "rw"
 		if len(parts) > 2 {
@@ -500,11 +533,13 @@ func (c *checker) underRoot(p string) bool {
 	if !in || c.p.Resolve == nil {
 		return in
 	}
-	// On the box: an existing source is judged by where it really points. A source that does not exist yet (the
-	// deploy belt creates it as a directory) is judged by its name; a root that cannot be resolved is no root.
-	rp, err := c.p.Resolve(p)
-	if err != nil {
-		return true
+	// On the box: a source is judged by where it really points. A path that does not exist yet (the deploy belt — or
+	// Docker — creates it) is judged through its nearest EXISTING ancestor, so a symlink higher up cannot carry a
+	// not-yet-created directory out of the root (security review 2026-10-17, round 2). Any resolve error other than
+	// "does not exist" refuses (fail closed).
+	rp, ok := c.resolveExisting(p)
+	if !ok {
+		return false
 	}
 	for _, r := range c.roots {
 		rr, rerr := c.p.Resolve(r)
@@ -515,6 +550,24 @@ func (c *checker) underRoot(p string) bool {
 	return false
 }
 
+// resolveExisting resolves the nearest existing ancestor of p and re-joins the rest.
+func (c *checker) resolveExisting(p string) (string, bool) {
+	rest := ""
+	for q := p; ; q = filepath.Dir(q) {
+		r, err := c.p.Resolve(q)
+		if err == nil {
+			return filepath.Join(r, rest), true
+		}
+		if !errors.Is(err, fs.ErrNotExist) {
+			return "", false
+		}
+		if q == "/" || q == "." {
+			return "", false
+		}
+		rest = filepath.Join(filepath.Base(q), rest)
+	}
+}
+
 // insideStack: a relative path that stays inside the stack directory.
 func (c *checker) insideStack(p string) bool {
 	return p != "" && !strings.HasPrefix(p, "/") && !strings.HasPrefix(p, "~") && !hasDotDot(p)

controller/internal/privpolicy/privpolicy_test.go

@@ -182,6 +182,8 @@ const homepage = `services:
     volumes:
       - homepage_config:/app/config
       - /var/run/docker.sock:/var/run/docker.sock:ro
+    healthcheck:
+      test: ["CMD", "wget", "--spider", "-q", "http://127.0.0.1:3000"]
 volumes:
   homepage_config:
 `
@@ -348,3 +350,59 @@ func TestAdmit_ARootElsewhereUnderMntIsNoRoot(t *testing.T) {
 		t.Fatal("a root outside /mnt/felhom-drives/ and the system data path was accepted")
 	}
 }
+
+// Round 2 of the security review: the gaps found in the fixes.
+func TestAdmit_Round2(t *testing.T) {
+	refuse := map[string]struct{ app, compose string }{
+		"a second service extends the allowed one":       {"homepage", strings.Replace(homepage, "volumes:\n  homepage_config:", "  evil:\n    image: evil/x:1\n    extends:\n      service: homepage\nvolumes:\n  homepage_config:", 1)},
+		"the allowed service's healthcheck runs a shell": {"homepage", strings.Replace(homepage, `["CMD", "wget", "--spider", "-q", "http://127.0.0.1:3000"]`, `["CMD-SHELL", "id"]`, 1)},
+		"the allowed service's image through a variable": {"homepage", strings.Replace(homepage, "ghcr.io/gethomepage/homepage:v1.13.2", "${HP_IMAGE:-ghcr.io/gethomepage/homepage:v1.13.2}", 1)},
+		"a variable in a bind's options":                 {"app", strings.Replace(base, "      - ${HDD_PATH}/appdata/app:/config\n", "      - ${HDD_PATH}/appdata/app:/config:${MODE}\n", 1)},
+		"a non-root variable in a bind's target":         {"app", strings.Replace(base, "      - ${HDD_PATH}/appdata/app:/config\n", "      - ${HDD_PATH}/appdata/app:${TGT}\n", 1)},
+	}
+	for name, c := range refuse {
+		t.Run(name, func(t *testing.T) {
+			if err := admit(t, c.app, c.compose, env()); err == nil {
+				t.Fatal("admitted")
+			}
+		})
+	}
+	// Positive control: the *arr shape — the same root path inside and out — passes.
+	arr := strings.Replace(base, "      - ${HDD_PATH}/appdata/app:/config\n", "      - ${USERDATA_PATH}/media:${USERDATA_PATH}/media\n", 1)
+	if err := admit(t, "app", arr, env()); err != nil {
+		t.Fatalf("the *arr bind shape was refused: %v", err)
+	}
+}
+
+// Round 2: a path that does not exist yet is judged through its nearest existing ancestor — a symlink above it out of
+// the root refuses; a resolve error that is not "does not exist" refuses too.
+func TestAdmit_ANotYetCreatedPathBehindASymlink(t *testing.T) {
+	root := t.TempDir()
+	ns := filepath.Join(root, "mnt", "felhom-drives", "d", "felhom-data", "userdata")
+	if err := os.MkdirAll(ns, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink("/etc", filepath.Join(ns, "escape")); err != nil {
+		t.Fatal(err)
+	}
+	e := map[string]string{"USERDATA_PATH": "/mnt/felhom-drives/d/felhom-data/userdata"}
+	resolve := func(s string) (string, error) {
+		r, err := filepath.EvalSymlinks(filepath.Join(root, s))
+		if err != nil {
+			return "", err
+		}
+		if rel, rerr := filepath.Rel(root, r); rerr == nil && !strings.HasPrefix(rel, "..") {
+			return "/" + rel, nil
+		}
+		return r, nil
+	}
+	c := "services:\n  app:\n    image: x/y:1\n    volumes:\n      - ${USERDATA_PATH}/escape/new-dir:/x\n"
+	if err := Admit("app", []byte(c), e, Policy{Resolve: resolve}); err == nil {
+		t.Fatal("a not-yet-created directory behind a symlink to /etc was admitted")
+	}
+	broken := func(string) (string, error) { return "", errors.New("permission denied") }
+	ok := "services:\n  app:\n    image: x/y:1\n    volumes:\n      - ${USERDATA_PATH}/media:/m\n"
+	if err := Admit("app", []byte(ok), e, Policy{Resolve: broken}); err == nil {
+		t.Fatal("a resolve error other than not-exist was admitted (fail open)")
+	}
+}