From 09ac87a843ce793229d0786f6362850e4faba8e8 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sat, 17 Oct 2026 16:49:17 +0200 Subject: [PATCH] =?UTF-8?q?v0.328.0=20(unreleased):=20security=20review=20?= =?UTF-8?q?round=203=20=E2=80=94=20under=20HDD=5FPATH=20only=20appdata/=20?= =?UTF-8?q?is=20a=20data=20root=20(not=20backups/)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- CHANGELOG.md | 5 +++++ controller/internal/privpolicy/privpolicy.go | 6 ++++++ controller/internal/privpolicy/privpolicy_test.go | 11 +++++++++++ 3 files changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01c3fa5..7898961 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,11 @@ still refuses and the app page still says so. options (or a non-root one in its target) is refused. The catalog reader refuses YAML anchors, aliases and merge keys (the box's YAML library expands them, the reader would judge a different document). Red-proved (`P2/redproof-security-review-round2.txt`). +- **Security review round 3:** under `${HDD_PATH}` only `appdata/` is a data root — the drive namespace's `backups/` + (recovery units with the apps' portable secrets) and its other top-level folders are siblings no app may bind; the + household's folders come through `${USERDATA_PATH}`. Every catalog bind already uses `${HDD_PATH}/appdata/…`. + Known limit, stated: one app can still name ANOTHER app's `appdata/` (paperless-ngx's folder is `paperless`, + so a per-app rule needs a name map) — filed. Red-proved (`P2/redproof-security-review-round3.txt`). ## v0.327.1 — files. reaches FileBrowser again: a file route by name, not Docker labels (2026-10-17) diff --git a/controller/internal/privpolicy/privpolicy.go b/controller/internal/privpolicy/privpolicy.go index d470740..adf7976 100644 --- a/controller/internal/privpolicy/privpolicy.go +++ b/controller/internal/privpolicy/privpolicy.go @@ -512,6 +512,12 @@ func (c *checker) validRoots() []string { continue } if strings.HasPrefix(v, DriveNamespace) || (c.p.SystemDataPath != "" && within(v, filepath.Clean(c.p.SystemDataPath))) { + if r == "HDD_PATH" { + // round 3: HDD_PATH is the drive's whole namespace — its backups/ (recovery units carry app secrets) + // and every other top-level folder are siblings an app has no business binding. Only appdata/ is a + // root; the household's folders come through USERDATA_PATH. + v = filepath.Join(v, "appdata") + } out = append(out, v) } } diff --git a/controller/internal/privpolicy/privpolicy_test.go b/controller/internal/privpolicy/privpolicy_test.go index 8c92bfc..955e02b 100644 --- a/controller/internal/privpolicy/privpolicy_test.go +++ b/controller/internal/privpolicy/privpolicy_test.go @@ -406,3 +406,14 @@ func TestAdmit_ANotYetCreatedPathBehindASymlink(t *testing.T) { t.Fatal("a resolve error other than not-exist was admitted (fail open)") } } + +// Round 3: ${HDD_PATH} itself and its siblings of appdata/ (backups/ — the recovery units with app secrets) are no +// root; ${HDD_PATH}/appdata/... is. +func TestAdmit_HDDPathRootIsAppdataOnly(t *testing.T) { + for _, line := range []string{" - ${HDD_PATH}:/drive\n", " - ${HDD_PATH}/backups:/b\n", " - ${HDD_PATH}/appdata-x:/b\n"} { + compose := strings.Replace(base, " - app_data:/data\n", " - app_data:/data\n"+line, 1) + if err := admit(t, "app", compose, env()); err == nil { + t.Fatalf("%q was admitted", line) + } + } +}