fix(backend): enforce file ACL on file-tag and import endpoints
Two broken-access-control holes: - PUT/DELETE /files/:id/tags(/:tag_id) and GET /files/:id/tags went straight to TagService with no ACL check, letting any authenticated user read or rewrite tags on anyone's private files. The handlers now require view (list) or edit (mutate) on the target file via new FileService.AuthorizeView/AuthorizeEdit helpers. - POST /files/import accepted an arbitrary host path from any user, turning it into an arbitrary server-side file read. It is now admin-only and the supplied path is confined to IMPORT_PATH. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -613,6 +613,12 @@ func (h *FileHandler) CommonTags(c *gin.Context) {
|
|||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
func (h *FileHandler) Import(c *gin.Context) {
|
func (h *FileHandler) Import(c *gin.Context) {
|
||||||
|
// Server-side directory import reads arbitrary paths on the host; restrict
|
||||||
|
// it to administrators.
|
||||||
|
if !requireAdmin(c) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
var body struct {
|
var body struct {
|
||||||
Path string `json:"path"`
|
Path string `json:"path"`
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -430,6 +430,11 @@ func (h *TagHandler) FileListTags(c *gin.Context) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if err := h.fileSvc.AuthorizeView(c.Request.Context(), fileID); err != nil {
|
||||||
|
respondError(c, err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
tags, err := h.tagSvc.ListFileTags(c.Request.Context(), fileID)
|
tags, err := h.tagSvc.ListFileTags(c.Request.Context(), fileID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
respondError(c, err)
|
respondError(c, err)
|
||||||
@@ -465,6 +470,11 @@ func (h *TagHandler) FileSetTags(c *gin.Context) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if err := h.fileSvc.AuthorizeEdit(c.Request.Context(), fileID); err != nil {
|
||||||
|
respondError(c, err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
tags, err := h.tagSvc.SetFileTags(c.Request.Context(), fileID, tagIDs)
|
tags, err := h.tagSvc.SetFileTags(c.Request.Context(), fileID, tagIDs)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
respondError(c, err)
|
respondError(c, err)
|
||||||
@@ -491,6 +501,11 @@ func (h *TagHandler) FileAddTag(c *gin.Context) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if err := h.fileSvc.AuthorizeEdit(c.Request.Context(), fileID); err != nil {
|
||||||
|
respondError(c, err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
tags, err := h.tagSvc.AddFileTag(c.Request.Context(), fileID, tagID)
|
tags, err := h.tagSvc.AddFileTag(c.Request.Context(), fileID, tagID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
respondError(c, err)
|
respondError(c, err)
|
||||||
@@ -517,6 +532,11 @@ func (h *TagHandler) FileRemoveTag(c *gin.Context) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if err := h.fileSvc.AuthorizeEdit(c.Request.Context(), fileID); err != nil {
|
||||||
|
respondError(c, err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
if err := h.tagSvc.RemoveFileTag(c.Request.Context(), fileID, tagID); err != nil {
|
if err := h.tagSvc.RemoveFileTag(c.Request.Context(), fileID, tagID); err != nil {
|
||||||
respondError(c, err)
|
respondError(c, err)
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import (
|
|||||||
"io"
|
"io"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/gabriel-vasile/mimetype"
|
"github.com/gabriel-vasile/mimetype"
|
||||||
@@ -407,6 +408,32 @@ func (s *FileService) List(ctx context.Context, params domain.FileListParams) (*
|
|||||||
return s.files.List(ctx, params)
|
return s.files.List(ctx, params)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// AuthorizeView ensures the caller may view the file. Returns ErrNotFound if the
|
||||||
|
// file does not exist or ErrForbidden if the caller lacks view access.
|
||||||
|
func (s *FileService) AuthorizeView(ctx context.Context, id uuid.UUID) error {
|
||||||
|
_, err := s.Get(ctx, id)
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// AuthorizeEdit ensures the caller may edit the file. Returns ErrNotFound if the
|
||||||
|
// file does not exist or ErrForbidden if the caller lacks edit access.
|
||||||
|
func (s *FileService) AuthorizeEdit(ctx context.Context, id uuid.UUID) error {
|
||||||
|
userID, isAdmin, _ := domain.UserFromContext(ctx)
|
||||||
|
|
||||||
|
f, err := s.files.GetByID(ctx, id)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
ok, err := s.acl.CanEdit(ctx, userID, isAdmin, f.CreatorID, fileObjectTypeID, id)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if !ok {
|
||||||
|
return domain.ErrForbidden
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Content / thumbnail / preview streaming
|
// Content / thumbnail / preview streaming
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@@ -468,14 +495,21 @@ func (s *FileService) BulkDelete(ctx context.Context, fileIDs []uuid.UUID) error
|
|||||||
// Import scans a server-side directory and uploads all supported files.
|
// Import scans a server-side directory and uploads all supported files.
|
||||||
// If path is empty, the configured default import path is used.
|
// If path is empty, the configured default import path is used.
|
||||||
func (s *FileService) Import(ctx context.Context, path string) (*ImportResult, error) {
|
func (s *FileService) Import(ctx context.Context, path string) (*ImportResult, error) {
|
||||||
dir := path
|
if s.importPath == "" {
|
||||||
if dir == "" {
|
|
||||||
dir = s.importPath
|
|
||||||
}
|
|
||||||
if dir == "" {
|
|
||||||
return nil, domain.ErrValidation
|
return nil, domain.ErrValidation
|
||||||
}
|
}
|
||||||
|
|
||||||
|
dir := s.importPath
|
||||||
|
if path != "" {
|
||||||
|
// Confine caller-supplied paths to the configured import directory so a
|
||||||
|
// directory-traversal value cannot read arbitrary host files.
|
||||||
|
confined, err := confineToBase(s.importPath, path)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
dir = confined
|
||||||
|
}
|
||||||
|
|
||||||
entries, err := os.ReadDir(dir)
|
entries, err := os.ReadDir(dir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("FileService.Import: read dir %q: %w", dir, err)
|
return nil, fmt.Errorf("FileService.Import: read dir %q: %w", dir, err)
|
||||||
@@ -550,6 +584,28 @@ func (s *FileService) Import(ctx context.Context, path string) (*ImportResult, e
|
|||||||
// Internal helpers
|
// Internal helpers
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
// confineToBase resolves target and verifies it does not escape base (after
|
||||||
|
// cleaning and resolving "..") so a caller cannot read files outside the
|
||||||
|
// configured import directory. Returns the cleaned absolute path on success.
|
||||||
|
func confineToBase(base, target string) (string, error) {
|
||||||
|
absBase, err := filepath.Abs(base)
|
||||||
|
if err != nil {
|
||||||
|
return "", domain.ErrValidation
|
||||||
|
}
|
||||||
|
absTarget, err := filepath.Abs(target)
|
||||||
|
if err != nil {
|
||||||
|
return "", domain.ErrValidation
|
||||||
|
}
|
||||||
|
rel, err := filepath.Rel(absBase, absTarget)
|
||||||
|
if err != nil {
|
||||||
|
return "", domain.ErrValidation
|
||||||
|
}
|
||||||
|
if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
|
||||||
|
return "", domain.ErrForbidden
|
||||||
|
}
|
||||||
|
return absTarget, nil
|
||||||
|
}
|
||||||
|
|
||||||
// extractEXIFWithDatetime parses EXIF from raw bytes, returning both the JSON
|
// extractEXIFWithDatetime parses EXIF from raw bytes, returning both the JSON
|
||||||
// representation and the DateTimeOriginal (if present). Both may be nil.
|
// representation and the DateTimeOriginal (if present). Both may be nil.
|
||||||
func extractEXIFWithDatetime(data []byte) (json.RawMessage, *time.Time) {
|
func extractEXIFWithDatetime(data []byte) (json.RawMessage, *time.Time) {
|
||||||
|
|||||||
Reference in New Issue
Block a user