fix(admin): keep account photos out of the media library

internal/store/media.go

@@ -17,19 +17,36 @@ import (
 )
 
 // MediaPath returns the absolute path of a media file, for a caller that
-// serves it. The name must carry an allowed image extension, and the file
-// is opened through the store's root, so a name that would escape the
-// content directory is refused rather than checked for.
+// serves it. The name is either a flat file of the media directory or
+// "avatars/<file>", the one namespace below it the route serves; anything
+// else, and a name that carries no allowed image extension, is refused
+// rather than checked for. The file is opened through the store's root, so
+// a path that would escape it cannot be named.
 func (s *Store) MediaPath(name string) (string, error) {
-	if !imagefile.Allowed(name) {
-		return "", fmt.Errorf("%q is not an allowed image name", name)
+	cleaned := path.Clean(name)
+	if cleaned == "." || cleaned == ".." || strings.HasPrefix(cleaned, "..") {
+		return "", fmt.Errorf("%q is not a media name", name)
+	}
+	dir, base := path.Split(cleaned)
+	dir = path.Clean(dir) // "." for a flat name, "avatars" for an avatar
+	if base == "." || base == ".." || base == "" {
+		return "", fmt.Errorf("%q is not a media name", name)
+	}
+	if !imagefile.Allowed(base) {
+		return "", fmt.Errorf("%q is not an allowed image name", base)
+	}
+	rel := base
+	if dir != "." {
+		if dir != AvatarDirName {
+			return "", fmt.Errorf("%q is not a served media path", name)
+		}
+		rel = path.Join(AvatarDirName, base)
 	}
-	base := filepath.Base(name)
 	root, err := s.openRoot()
 	if err != nil {
 		return "", err
 	}
-	handle, err := root.Open(path.Join(MediaDirName, base))
+	handle, err := root.Open(path.Join(MediaDirName, rel))
 	if err != nil {
 		return "", err
 	}
@@ -41,7 +58,7 @@ func (s *Store) MediaPath(name string) (string, error) {
 	if !info.Mode().IsRegular() {
 		return "", fmt.Errorf("%q is not a regular file", base)
 	}
-	return filepath.Join(s.ContentDir, MediaDirName, base), nil
+	return filepath.Join(s.ContentDir, MediaDirName, rel), nil
 }
 
 // StoreUpload persists an uploaded image and returns its public URL. The
@@ -67,17 +84,65 @@ func (s *Store) StoreUpload(originalName string, data []byte) (string, error) {
 	return "/media/" + name, nil
 }
 
+// StoreAvatar persists an account profile photo and returns its public
+// URL, under avatars/ inside the media directory: the photo is account
+// state and not library content, so it never appears as a media tile.
+// The extension comes from the byte signature as StoreUpload does.
+func (s *Store) StoreAvatar(data []byte) (string, error) {
+	ext := imagefile.Detect(data)
+	if ext == "" {
+		return "", fmt.Errorf("unsupported image format")
+	}
+	root, err := s.openRoot()
+	if err != nil {
+		return "", err
+	}
+	dir := path.Join(MediaDirName, AvatarDirName)
+	if err := root.MkdirAll(dir, 0o755); err != nil {
+		return "", fmt.Errorf("create avatar directory: %w", err)
+	}
+	name := uuid.NewV4().String() + ext
+	if err := atomicWriteIn(root, path.Join(dir, name), data); err != nil {
+		return "", err
+	}
+	return "/media/" + AvatarDirName + "/" + name, nil
+}
+
 // DeleteMedia removes a media file by its public URL and reports whether a
-// file was removed.
+// file was removed. The URL names either a flat file of the media
+// directory or an avatar below avatars/; nothing else is accepted, and a
+// URL that names no file at all removes nothing.
 func (s *Store) DeleteMedia(url string) bool {
 	if url == "" || !strings.HasPrefix(url, "/media/") {
 		return false
 	}
+	name := path.Clean(strings.TrimPrefix(url, "/media/"))
+	if name == "." || name == ".." || strings.HasPrefix(name, "..") {
+		return false
+	}
+	dir, base := path.Split(name)
+	dir = path.Clean(dir)
+	var rel string
+	switch {
+	case dir == "." && base != "" && base != "." && base != "..":
+		rel = base
+	case dir == AvatarDirName && base != "" && base != "." && base != "..":
+		rel = path.Join(AvatarDirName, base)
+	default:
+		return false
+	}
 	root, err := s.openRoot()
 	if err != nil {
 		return false
 	}
-	return root.Remove(path.Join(MediaDirName, filepath.Base(url))) == nil
+	target := path.Join(MediaDirName, rel)
+	// Only a regular file is removed: a URL that resolves to the media
+	// directory, the avatar directory or any other directory is refused,
+	// so a delete can never empty a namespace.
+	if info, err := root.Stat(target); err != nil || !info.Mode().IsRegular() {
+		return false
+	}
+	return root.Remove(target) == nil
 }
 
 // Media is one file in the media library.

internal/store/store.go

@@ -36,6 +36,13 @@ var safeSlugRe = regexp.MustCompile(`[^a-zA-Z0-9._-]`)
 // MediaDirName is the uploads directory inside the content directory.
 const MediaDirName = "media"
 
+// AvatarDirName is the directory inside the media directory that carries
+// the account profile photos. The media library lists only the files of
+// the media directory itself, so an avatar there never appears as a tile;
+// the URL keeps the /media/ prefix, so the public route, the delete path
+// and the backups treat it like any other image.
+const AvatarDirName = "avatars"
+
 // Store manages posts on disk in a flat or language-subdivided
 // directory.
 type Store struct {

internal/store/store_test.go

@@ -5,6 +5,7 @@ package store
 
 import (
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -364,8 +365,8 @@ func TestStoreUploadAndListMedia(t *testing.T) {
 	if _, err := s.MediaPath(name); err != nil {
 		t.Fatalf("MediaPath cannot find the upload: %v", err)
 	}
-	if _, err := s.MediaPath("../" + name); err != nil {
-		t.Fatalf("a base name should still resolve: %v", err)
+	if _, err := s.MediaPath("../" + name); err == nil {
+		t.Fatal("MediaPath accepted a name that climbs out of the media directory")
 	}
 	if _, err := s.MediaPath("notes.txt"); err == nil {
 		t.Fatal("MediaPath accepted a name that is not an image")
@@ -383,6 +384,75 @@ func TestStoreUploadAndListMedia(t *testing.T) {
 	if s.DeleteMedia("/etc/passwd") {
 		t.Fatal("DeleteMedia accepted non-media URL")
 	}
+	// A URL that resolves to a directory removes nothing: the media and
+	// avatar directories are namespaces, not deletable content.
+	if s.DeleteMedia("/media/") {
+		t.Fatal("DeleteMedia accepted the media directory itself")
+	}
+}
+
+// A profile photo is account state, not library content: it lives under
+// avatars/ inside the media directory, which the listing skips, while
+// the public route serves it and the delete path removes it.
+func TestStoreAvatarLivesOutsideTheLibrary(t *testing.T) {
+	s, _ := newStore(t)
+	webpData := append([]byte("RIFF"), 0, 0, 0, 0)
+	webpData = append(webpData, []byte("WEBPVP8 ")...)
+	url, err := s.StoreAvatar(webpData)
+	if err != nil {
+		t.Fatalf("StoreAvatar: %v", err)
+	}
+	if !strings.HasPrefix(url, "/media/avatars/") || !strings.HasSuffix(url, ".webp") {
+		t.Fatalf("url = %q", url)
+	}
+	name := strings.TrimPrefix(url, "/media/") // avatars/<uuid>.webp
+	if got, err := s.MediaPath(name); err != nil {
+		t.Fatalf("MediaPath cannot find the avatar: %v", err)
+	} else if !strings.Contains(filepath.ToSlash(got), "/media/avatars/") {
+		t.Fatalf("avatar path = %q", got)
+	}
+	if _, err := s.MediaPath("avatars/" + path.Base(name)); err != nil {
+		t.Fatalf("MediaPath cannot find the avatar by its route shape: %v", err)
+	}
+	if media := s.ListMedia(); len(media) != 0 {
+		t.Fatalf("the avatar leaked into the library: %v", media)
+	}
+	if !s.DeleteMedia(url) {
+		t.Fatal("DeleteMedia failed for the avatar")
+	}
+	if s.DeleteMedia(url) {
+		t.Fatal("DeleteMedia succeeded twice")
+	}
+	if _, err := s.MediaPath(name); err == nil {
+		t.Fatal("the avatar survived its deletion")
+	}
+}
+
+// The avatar namespace is exactly one level below the media directory:
+// anything deeper, anything outside it and anything without an image
+// extension is refused, so the public route gains no reach.
+func TestMediaPathRefusesNonMediaNames(t *testing.T) {
+	s, _ := newStore(t)
+	for _, name := range []string{
+		"avatars/../x.webp",
+		"avatars/a/b.webp",
+		"sub/x.webp",
+		"avatarss/x.webp",
+		"avatars/x.txt",
+		"avatars/",
+		"avatars",
+		"..",
+		"a/../b.webp",
+	} {
+		if got, err := s.MediaPath(name); err == nil {
+			t.Fatalf("MediaPath(%q) = %q, want a refusal", name, got)
+		}
+	}
+	// The avatar directory itself is not deletable content, with or
+	// without a trailing slash.
+	if s.DeleteMedia("/media/avatars") || s.DeleteMedia("/media/avatars/") {
+		t.Fatal("DeleteMedia removed the avatar directory")
+	}
 }
 
 func TestStoreUploadSVGAndDimensions(t *testing.T) {