v0.329.0 (review): an app named like another app's appdata folder gets no root; USERDATA_PATH is always derived, never the app's env
gates / gates (push) Successful in 46s

Security review of 2fe6341 (same night, before any release). Both red-proofed.
This commit is contained in:
2026-10-17 23:22:54 +02:00
parent 2fe634141f
commit 03ab121b25
5 changed files with 49 additions and 0 deletions
@@ -90,10 +90,15 @@ func mustLoad(b []byte) (map[string]Entry, map[string]string) {
}
m[key(e.App, e.Service, e.Setting)] = e
}
owner := map[string]string{}
for app, f := range l.AppdataFolders {
if !oneSegment(f) {
panic("privpolicy: allowlist.json appdata folder of " + app + " is not one folder name: " + f)
}
if o, dup := owner[f]; dup {
panic("privpolicy: allowlist.json gives the appdata folder " + f + " to both " + o + " and " + app)
}
owner[f] = app
}
return m, l.AppdataFolders
}
@@ -112,6 +117,13 @@ func AppdataFolder(app string) string {
if !oneSegment(app) {
return ""
}
// A folder the map gives to ANOTHER app is not this app's, even when this app is named like it: a catalog push
// adding a template called "paperless" must not get paperless-ngx's files (security review 2026-10-18).
for _, f := range appdataFolders {
if f == app {
return ""
}
}
return app
}
@@ -527,6 +527,21 @@ func TestAllowList_AppdataFolderMustBeOneName(t *testing.T) {
mustLoad([]byte(`{"version":2,"entries":[],"appdata_folders":{"app":"` + bad + `"}}`))
}()
}
// Security review 2026-10-18: an app NAMED like a folder the map gives to another app gets no root at all.
if got := AppdataFolder("paperless"); got != "" {
t.Errorf("AppdataFolder(paperless) = %q — a template named like paperless-ngx's folder must not own it", got)
}
if err := admit(t, "paperless", "services:\n app:\n image: x/y:1\n volumes:\n - ${HDD_PATH}/appdata/paperless/media:/m\n", env()); err == nil {
t.Error("a template named paperless was admitted to paperless-ngx's folder")
}
func() {
defer func() {
if recover() == nil {
t.Error("one folder given to two apps was loaded")
}
}()
mustLoad([]byte(`{"version":2,"entries":[],"appdata_folders":{"a":"x","b":"x"}}`))
}()
if got := AppdataFolder("paperless-ngx"); got != "paperless" {
t.Errorf("AppdataFolder(paperless-ngx) = %q", got)
}
+4
View File
@@ -34,6 +34,10 @@ func (m *Manager) policyEnv(env map[string]string) map[string]string {
out[k] = v
}
out["DOMAIN"] = m.cfg.Customer.Domain
// Security review 2026-10-18: USERDATA_PATH is ALWAYS the controller's own derivation, never the app's env. A
// catalog push can add a deploy field (InjectMissingFields fills it on the next sync), so an app-env value would
// let a template name any folder — another app's appdata/ included — as its root. No template declares one.
delete(out, "USERDATA_PATH")
if hdd := env["HDD_PATH"]; hdd != "" {
out["USERDATA_PATH"] = appbackup.UserdataDir(appbackup.NamespaceRootFor(hdd, m.sysDataPath))
}
@@ -89,6 +89,17 @@ func TestAllowList_PolicyEnvDerivesTheRootsLikeComposeExec(t *testing.T) {
if e["USERDATA_PATH"] == "/" || !strings.HasPrefix(e["USERDATA_PATH"], "/mnt/felhom-drives/a/") {
t.Fatalf("USERDATA_PATH = %q — the app's own value must not become a data root", e["USERDATA_PATH"])
}
// Security review 2026-10-18: with NO drive, an app-env USERDATA_PATH (a deploy field a catalog push can add) is
// no root either — it could name another app's appdata/. Consequence: such a bind is refused.
// RED-PROOF: without the delete in policyEnv, the bind into nextcloud's folder is admitted.
noHDD := map[string]string{"USERDATA_PATH": "/mnt/felhom-drives/a/felhom-data/appdata"}
if v, ok := m.policyEnv(noHDD)["USERDATA_PATH"]; ok {
t.Fatalf("an app with no drive kept its own USERDATA_PATH %q as a root", v)
}
c := "services:\n web:\n image: x/y:1\n volumes:\n - ${USERDATA_PATH}/nextcloud:/x\n"
if err := m.admitCompose("web", []byte(c), noHDD); err == nil {
t.Fatal("a bind into another app's folder through an app-env USERDATA_PATH was admitted")
}
}
// The install / settings / restore refusal is a bundle key: the reader gets it in their language, with the app and