From 6cbabda86bdc04625acb6020f6eacdeed2a3332a Mon Sep 17 00:00:00 2001 From: dtoro Date: Fri, 3 Jul 2026 12:46:00 +0200 Subject: [PATCH] feat(sidecar): enforce per-user BasePath on all filesystem mutations Folder create/rename/delete/move, photo move, heap convert, and file rename now reject paths outside the caller's BasePath (403). Sources resolved via PhotoPrism UIDs are re-checked in movePhotoFiles. The USER_BASEPATHS reconciler also sets upload_path so client-app uploads land inside the user's subtree. Co-Authored-By: Claude Fable 5 --- sidecar/auth.go | 34 +++++++++++++++++++++++++ sidecar/auth_test.go | 50 +++++++++++++++++++++++++++++++++++++ sidecar/handlers_folders.go | 9 +++++++ sidecar/handlers_heap.go | 14 ++++++++--- sidecar/handlers_move.go | 11 +++++++- sidecar/handlers_rename.go | 3 +++ sidecar/users.go | 9 ++++--- 7 files changed, 122 insertions(+), 8 deletions(-) create mode 100644 sidecar/auth_test.go diff --git a/sidecar/auth.go b/sidecar/auth.go index d895065..a16f3d6 100644 --- a/sidecar/auth.go +++ b/sidecar/auth.go @@ -2,6 +2,8 @@ package main import ( "net/http" + "path/filepath" + "strings" "github.com/gin-gonic/gin" ) @@ -85,3 +87,35 @@ func ctxBasePath(c *gin.Context) string { } return s } + +// userScopeRoot returns the absolute directory the session may mutate: +// ORIGINALS_ROOT/ for scoped users, the whole originals root for +// admins (empty BasePath). Lexical join only — callers compare it against +// paths built the same way from cfg.OriginalsRoot. +func userScopeRoot(c *gin.Context, cfg *Config) string { + base := strings.Trim(ctxBasePath(c), "/") + if base == "" { + return cfg.OriginalsRoot + } + return filepath.Join(cfg.OriginalsRoot, base) +} + +// requireUserScope guards an already-root-resolved absolute path against +// the caller's BasePath. PhotoPrism scopes what a session can *see* by +// BasePath, but the sidecar's filesystem endpoints accept raw paths, so +// every mutation must re-check that boundary here. `strict` additionally +// rejects the scope root itself — renaming/deleting/moving the user's own +// base folder would detach their library from auth_users.base_path. +// Writes the 403 response and returns false when out of bounds. +func requireUserScope(c *gin.Context, cfg *Config, abs string, strict bool) bool { + scope := userScopeRoot(c, cfg) + if !sameOrUnder(abs, scope) { + c.JSON(http.StatusForbidden, gin.H{"error": "path outside your library"}) + return false + } + if strict && abs == scope { + c.JSON(http.StatusForbidden, gin.H{"error": "cannot modify your library root"}) + return false + } + return true +} diff --git a/sidecar/auth_test.go b/sidecar/auth_test.go new file mode 100644 index 0000000..1489737 --- /dev/null +++ b/sidecar/auth_test.go @@ -0,0 +1,50 @@ +package main + +import ( + "net/http/httptest" + "path/filepath" + "testing" + + "github.com/gin-gonic/gin" +) + +func scopeCtx(basePath string) *gin.Context { + gin.SetMode(gin.TestMode) + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c.Set("basePath", basePath) + return c +} + +func TestRequireUserScope(t *testing.T) { + cfg := &Config{OriginalsRoot: filepath.FromSlash("/originals")} + abs := func(rel string) string { return filepath.Join(cfg.OriginalsRoot, filepath.FromSlash(rel)) } + + cases := []struct { + name string + basePath string + path string + strict bool + want bool + }{ + {"admin sees root", "", cfg.OriginalsRoot, false, true}, + {"admin anywhere", "", abs("bob/x"), false, true}, + {"admin strict rejects root", "", cfg.OriginalsRoot, true, false}, + {"scoped inside own tree", "alice", abs("alice/2024"), false, true}, + {"scoped own root non-strict", "alice", abs("alice"), false, true}, + {"scoped own root strict", "alice", abs("alice"), true, false}, + {"scoped other user", "alice", abs("bob/2024"), false, false}, + {"scoped sibling prefix", "alice", abs("alice2/2024"), false, false}, + {"scoped originals root", "alice", cfg.OriginalsRoot, false, false}, + {"nested base path", "family/alice", abs("family/alice/x"), false, true}, + {"nested base path parent", "family/alice", abs("family"), false, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := scopeCtx(tc.basePath) + if got := requireUserScope(c, cfg, tc.path, tc.strict); got != tc.want { + t.Errorf("requireUserScope(base=%q, path=%q, strict=%v) = %v, want %v", + tc.basePath, tc.path, tc.strict, got, tc.want) + } + }) + } +} diff --git a/sidecar/handlers_folders.go b/sidecar/handlers_folders.go index ca0fba7..11dd30c 100644 --- a/sidecar/handlers_folders.go +++ b/sidecar/handlers_folders.go @@ -47,6 +47,9 @@ func handleFolderCreate(cfg *Config, pp *ppClient) gin.HandlerFunc { c.JSON(http.StatusBadRequest, gin.H{"error": "invalid path"}) return } + if !requireUserScope(c, cfg, abs, true) { + return + } if _, err := os.Stat(abs); err == nil { c.JSON(http.StatusConflict, gin.H{"error": "already exists"}) return @@ -92,6 +95,9 @@ func handleFolderRename(cfg *Config, pp *ppClient) gin.HandlerFunc { c.JSON(http.StatusBadRequest, gin.H{"error": "invalid path"}) return } + if !requireUserScope(c, cfg, oldAbs, true) { + return + } st, err := os.Stat(oldAbs) if err != nil || !st.IsDir() { c.JSON(http.StatusBadRequest, gin.H{"error": "not a directory"}) @@ -142,6 +148,9 @@ func handleFolderDelete(cfg *Config, pp *ppClient) gin.HandlerFunc { c.JSON(http.StatusBadRequest, gin.H{"error": "refuse to delete root"}) return } + if !requireUserScope(c, cfg, abs, true) { + return + } st, err := os.Stat(abs) if err != nil || !st.IsDir() { c.JSON(http.StatusBadRequest, gin.H{"error": "not a directory"}) diff --git a/sidecar/handlers_heap.go b/sidecar/handlers_heap.go index 9eb57ee..cd2ee97 100644 --- a/sidecar/handlers_heap.go +++ b/sidecar/handlers_heap.go @@ -90,6 +90,9 @@ func handleHeapConvert(cfg *Config, pp *ppClient) gin.HandlerFunc { c.JSON(http.StatusBadRequest, gin.H{"error": "invalid targetFolder"}) return } + if !requireUserScope(c, cfg, targetAbs, false) { + return + } // Pull the heap's membership via the q=album:UID query (count=1000 // covers every realistic heap). We only need the UID list here — the // search's Files array is trimmed and drops videos, so we re-resolve @@ -124,7 +127,7 @@ func handleHeapConvert(cfg *Config, pp *ppClient) gin.HandlerFunc { return } - moved, copied, errs, err := movePhotoFiles(cfg, pp, token, photos, targetAbs, subfolder, mode) + moved, copied, errs, err := movePhotoFiles(cfg, pp, token, photos, targetAbs, subfolder, mode, userScopeRoot(c, cfg)) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return @@ -167,7 +170,10 @@ func handleHeapConvert(cfg *Config, pp *ppClient) gin.HandlerFunc { // and handlePhotosMove (UID-list scoped); both resolve `photos` differently // but move them identically. Returns per-photo errors in `errs`; the returned // top-level error is only for a fatal precondition (subfolder mkdir failed). -func movePhotoFiles(cfg *Config, pp *ppClient, token string, photos []heapPhoto, targetAbs, subfolder, mode string) (moved, copied int, errs []heapErr, err error) { +// `scopeAbs` is the caller's userScopeRoot — source files outside it fail +// per-photo, so a UID that resolves outside the user's BasePath (however +// PhotoPrism came to return it) can't be used to pull files across users. +func movePhotoFiles(cfg *Config, pp *ppClient, token string, photos []heapPhoto, targetAbs, subfolder, mode, scopeAbs string) (moved, copied int, errs []heapErr, err error) { destAbs := targetAbs if subfolder != "" { destAbs = filepath.Join(targetAbs, subfolder) @@ -234,8 +240,8 @@ func movePhotoFiles(cfg *Config, pp *ppClient, token string, photos []heapPhoto, for _, f := range group { srcRel := f.Name srcAbs := filepath.Join(cfg.OriginalsRoot, srcRel) - if !sameOrUnder(srcAbs, cfg.OriginalsRoot) { - failure = "path escapes originals" + if !sameOrUnder(srcAbs, scopeAbs) { + failure = "path outside your library" break } st, statErr := os.Stat(srcAbs) diff --git a/sidecar/handlers_move.go b/sidecar/handlers_move.go index e138171..b72bfe1 100644 --- a/sidecar/handlers_move.go +++ b/sidecar/handlers_move.go @@ -56,6 +56,9 @@ func handlePhotosMove(cfg *Config, pp *ppClient) gin.HandlerFunc { c.JSON(http.StatusBadRequest, gin.H{"error": "invalid targetFolder"}) return } + if !requireUserScope(c, cfg, targetAbs, false) { + return + } // Resolve each photo's FULL file list via the single-photo endpoint // rather than the /photos search (see resolvePhotosFull) — the search @@ -67,7 +70,7 @@ func handlePhotosMove(cfg *Config, pp *ppClient) gin.HandlerFunc { return } - moved, copied, errs, err := movePhotoFiles(cfg, pp, token, photos, targetAbs, subfolder, mode) + moved, copied, errs, err := movePhotoFiles(cfg, pp, token, photos, targetAbs, subfolder, mode, userScopeRoot(c, cfg)) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return @@ -148,6 +151,9 @@ func handleFolderMove(cfg *Config, pp *ppClient) gin.HandlerFunc { c.JSON(http.StatusBadRequest, gin.H{"error": "invalid path"}) return } + if !requireUserScope(c, cfg, oldAbs, true) { + return + } st, err := os.Stat(oldAbs) if err != nil || !st.IsDir() { c.JSON(http.StatusBadRequest, gin.H{"error": "not a directory"}) @@ -158,6 +164,9 @@ func handleFolderMove(cfg *Config, pp *ppClient) gin.HandlerFunc { c.JSON(http.StatusBadRequest, gin.H{"error": "invalid targetParent"}) return } + if !requireUserScope(c, cfg, targetParentAbs, false) { + return + } // Can't move a folder into itself or one of its own descendants. if sameOrUnder(targetParentAbs, oldAbs) { c.JSON(http.StatusBadRequest, gin.H{"error": "cannot move a folder into itself"}) diff --git a/sidecar/handlers_rename.go b/sidecar/handlers_rename.go index f447491..5da3adc 100644 --- a/sidecar/handlers_rename.go +++ b/sidecar/handlers_rename.go @@ -97,6 +97,9 @@ func handleRename(cfg *Config, pp *ppClient) gin.HandlerFunc { c.JSON(http.StatusBadRequest, gin.H{"error": "path escapes originals root"}) return } + if !requireUserScope(c, cfg, oldAbs, false) { + return + } st, err := os.Stat(oldAbs) if err != nil || !st.Mode().IsRegular() { c.JSON(http.StatusNotFound, gin.H{"error": "file missing on disk"}) diff --git a/sidecar/users.go b/sidecar/users.go index 99af5be..929e081 100644 --- a/sidecar/users.go +++ b/sidecar/users.go @@ -86,12 +86,15 @@ func reconcileUserBasepaths(ppDSN, originalsRoot string, mapping map[string]stri // ACL kicks in even if the directory is created later. } + // upload_path rides along with base_path so anything a client app + // uploads (WebDAV sync apps, PhotoPrism's own UI) lands inside the + // user's library subtree instead of the shared originals root. res := db.Exec(`UPDATE auth_users - SET base_path = ? + SET base_path = ?, upload_path = ? WHERE user_name = ? - AND COALESCE(base_path, '') <> ? + AND (COALESCE(base_path, '') <> ? OR COALESCE(upload_path, '') <> ?) AND deleted_at IS NULL`, - path, username, path) + path, path, username, path, path) if res.Error != nil { slog.Error("user-basepath: update failed", "user", username, "err", res.Error) continue