From b409f5eee225166ac9c6201c3fa39a31c228cb23 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sat, 18 Jul 2026 11:54:35 +0200 Subject: [PATCH] fix(samba): separate storage-ROOT validation from share-TARGET validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live validation caught it: the 'new folder' flow passed the storage root through sharingResolvePath, which (correctly) refuses the drive root as a share target — so share creation silently failed. sharingResolveStorageRoot accepts EXACTLY a registered live root (strictly tighter) and is used only as the new-folder parent. Regression test asserts both halves. --- controller/internal/web/sharing_handlers.go | 35 +++++++++++++++++- .../internal/web/sharing_handlers_test.go | 37 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/controller/internal/web/sharing_handlers.go b/controller/internal/web/sharing_handlers.go index 6625527..38be82b 100644 --- a/controller/internal/web/sharing_handlers.go +++ b/controller/internal/web/sharing_handlers.go @@ -82,6 +82,39 @@ func (s *Server) sharingResolvePath(raw string) (string, error) { return resolved, nil } +// sharingResolveStorageRoot validates a storage ROOT chosen for the "new folder" flow. +// +// This is deliberately NOT sharingResolvePath: that one validates a SHARE TARGET and therefore +// refuses the drive root itself (a whole drive is never shareable). Here the root is not the share — +// the new folder is created UNDER it — so the accept condition is "is EXACTLY a registered, live +// storage root", which is strictly tighter than the share-target guard. +func (s *Server) sharingResolveStorageRoot(raw string) (string, error) { + if strings.TrimSpace(raw) == "" { + return "", errNotShareable + } + clean := filepath.Clean(raw) + if !filepath.IsAbs(clean) { + return "", errNotShareable + } + resolved, err := filepath.EvalSymlinks(clean) + if err != nil { + return "", errNotShareable + } + for _, sp := range s.settings.GetStoragePaths() { + if sp.Decommissioned || sp.Disconnected { + continue + } + spResolved, serr := filepath.EvalSymlinks(sp.Path) + if serr != nil { + spResolved = filepath.Clean(sp.Path) + } + if filepath.Clean(resolved) == filepath.Clean(spResolved) { + return resolved, nil + } + } + return "", errNotShareable +} + // sharingPageData assembles the „Megosztás" page state. func (s *Server) sharingPageData() map[string]interface{} { data := s.settingsBaseData("sharing", "Hálózati megosztás") @@ -216,7 +249,7 @@ func (s *Server) sharingShareCreateHandler(w http.ResponseWriter, r *http.Reques var target string switch mode { case "new": - root, err := s.sharingResolvePath(r.FormValue("storage_root")) + root, err := s.sharingResolveStorageRoot(r.FormValue("storage_root")) if err != nil { sharingRedirect(w, r, errNotShareable.Error()) return diff --git a/controller/internal/web/sharing_handlers_test.go b/controller/internal/web/sharing_handlers_test.go index 45daa76..2cc1c6e 100644 --- a/controller/internal/web/sharing_handlers_test.go +++ b/controller/internal/web/sharing_handlers_test.go @@ -115,6 +115,43 @@ func TestSharingResolvePath_UniformRefusal(t *testing.T) { } } +// The "new folder" flow validates the storage ROOT, which the share-target guard rightly refuses. +// Regression guard for the live-validation bug: reusing sharingResolvePath there broke share +// creation entirely (the root is never a valid share TARGET, but is the valid PARENT). +func TestSharingResolveStorageRoot(t *testing.T) { + s, drive := newSharingServer(t) + + // The registered root is accepted here... + if got, err := s.sharingResolveStorageRoot(drive); err != nil || got == "" { + t.Errorf("a registered storage root must be accepted as a new-folder parent, got %q err=%v", got, err) + } + // ...even though it is (correctly) refused as a share TARGET. + if _, err := s.sharingResolvePath(drive); err == nil { + t.Error("the drive root must still be refused as a share target") + } + + // Anything that is not EXACTLY a registered live root is refused. + for _, bad := range []string{ + filepath.Join(drive, "media"), // a subdir is not a root + filepath.Join(drive, "appdata"), + t.TempDir(), // unregistered + "relative", + "", + } { + if _, err := s.sharingResolveStorageRoot(bad); err == nil { + t.Errorf("%q must not be accepted as a storage root", bad) + } + } + + // A decommissioned root stops being a valid parent. + if err := s.settings.SetDecommissioned(drive, ""); err != nil { + t.Fatal(err) + } + if _, err := s.sharingResolveStorageRoot(drive); err == nil { + t.Error("a decommissioned root must not accept new folders") + } +} + // pathWithin must be segment-wise: a sibling directory sharing a name PREFIX is not containment. func TestPathWithin_SiblingPrefixIsNotContainment(t *testing.T) { if pathWithin("/mnt/drive-evil/x", "/mnt/drive") {