From 945df7ef8ae911a51cc3b9a85e6e346b5d623b8b Mon Sep 17 00:00:00 2001 From: Masahiko AMANO Date: Wed, 10 Jun 2026 13:59:33 +0300 Subject: [PATCH] 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 --- backend/internal/handler/file_handler.go | 6 +++ backend/internal/handler/tag_handler.go | 20 +++++++ backend/internal/service/file_service.go | 66 ++++++++++++++++++++++-- 3 files changed, 87 insertions(+), 5 deletions(-) diff --git a/backend/internal/handler/file_handler.go b/backend/internal/handler/file_handler.go index df2cbe6..d30d179 100644 --- a/backend/internal/handler/file_handler.go +++ b/backend/internal/handler/file_handler.go @@ -613,6 +613,12 @@ func (h *FileHandler) CommonTags(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 { Path string `json:"path"` } diff --git a/backend/internal/handler/tag_handler.go b/backend/internal/handler/tag_handler.go index c79f922..d25a9e9 100644 --- a/backend/internal/handler/tag_handler.go +++ b/backend/internal/handler/tag_handler.go @@ -430,6 +430,11 @@ func (h *TagHandler) FileListTags(c *gin.Context) { 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) if err != nil { respondError(c, err) @@ -465,6 +470,11 @@ func (h *TagHandler) FileSetTags(c *gin.Context) { 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) if err != nil { respondError(c, err) @@ -491,6 +501,11 @@ func (h *TagHandler) FileAddTag(c *gin.Context) { 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) if err != nil { respondError(c, err) @@ -517,6 +532,11 @@ func (h *TagHandler) FileRemoveTag(c *gin.Context) { 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 { respondError(c, err) return diff --git a/backend/internal/service/file_service.go b/backend/internal/service/file_service.go index f8a5042..f58e26d 100644 --- a/backend/internal/service/file_service.go +++ b/backend/internal/service/file_service.go @@ -8,6 +8,7 @@ import ( "io" "os" "path/filepath" + "strings" "time" "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) } +// 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 // --------------------------------------------------------------------------- @@ -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. // If path is empty, the configured default import path is used. func (s *FileService) Import(ctx context.Context, path string) (*ImportResult, error) { - dir := path - if dir == "" { - dir = s.importPath - } - if dir == "" { + if s.importPath == "" { 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) if err != nil { 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 // --------------------------------------------------------------------------- +// 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 // representation and the DateTimeOriginal (if present). Both may be nil. func extractEXIFWithDatetime(data []byte) (json.RawMessage, *time.Time) {