From 5c5593644d5997c85e365d3cb8a56079840ea341 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sun, 30 Aug 2026 11:44:36 -0400 Subject: [PATCH] fix(media): gate file serving by library visibility; proper download URL ServeFile previously authenticated only ("any logged-in user") and never checked that the user can actually see the library owning the file, so knowing a library UUID + path was enough to fetch content from hidden libraries. Library visibility is the permission model - the library is what grants access to its media. - ServeFile now resolves two URL forms through one flow: /uploads/library-{id}/{path} (covers, reader files) /api/media-items/{id}/download (explicit book download, new) The item form looks up the media item, derives its library and file path, and adds a Content-Disposition attachment header. - Both forms enforce GetUserVisibleLibraries for the authenticated user, mirroring the OPDS download handler (403 when not visible). - Deleted the dead MediaHandler.DownloadBook handler (never routed). Also widen media_highlights.start_position/end_position from VARCHAR(100) to TEXT: the API handlers validate up to 1000 characters (full Readium locators, KOReader CRE xpointers) but the column rejected anything longer at the database layer. Metadata-only change applied idempotently at startup; existing rows are untouched. Verified against the running server: download 200 + attachment headers + epub bytes, unauthenticated 401, user hidden from the library 403 on both URL forms, visible user 200, covers unchanged, and a 334-char locator JSON now round-trips through the highlights API. --- database/schema/schema.sql | 11 ++- internal/handlers/media.go | 138 +++++++++++++++++++------------------ internal/router/media.go | 3 + 3 files changed, 84 insertions(+), 68 deletions(-) diff --git a/database/schema/schema.sql b/database/schema/schema.sql index c6baa42..aca6ae7 100644 --- a/database/schema/schema.sql +++ b/database/schema/schema.sql @@ -335,8 +335,8 @@ CREATE TABLE IF NOT EXISTS media_highlights ( media_item_id UUID NOT NULL REFERENCES media_items(id) ON DELETE CASCADE, user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE, selection_text TEXT NOT NULL, - start_position VARCHAR(100), -- position (page:offset or CFI) where highlight starts - end_position VARCHAR(100), -- position (page:offset or CFI) where highlight ends + start_position TEXT, -- position (page:offset, CFI, or locator JSON) where highlight starts + end_position TEXT, -- position (page:offset, CFI, or locator JSON) where highlight ends color VARCHAR(7) DEFAULT '#ffff00', -- hex color code for highlight note_id UUID REFERENCES media_notes(id) ON DELETE SET NULL, -- optional associated note created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW(), @@ -1364,6 +1364,13 @@ ALTER TABLE media_highlights ADD COLUMN IF NOT EXISTS note_text TEXT; ALTER TABLE media_highlights ADD COLUMN IF NOT EXISTS deleted BOOLEAN DEFAULT FALSE; ALTER TABLE media_highlights ADD COLUMN IF NOT EXISTS deleted_at TIMESTAMPTZ; +-- Widen position columns for existing databases: the API handlers +-- validate up to 1000 characters (full Readium locators, KOReader CRE +-- xpointers) but VARCHAR(100) rejected anything longer at the database +-- layer. VARCHAR -> TEXT is a metadata-only change, safe to re-run. +ALTER TABLE media_highlights ALTER COLUMN start_position TYPE TEXT; +ALTER TABLE media_highlights ALTER COLUMN end_position TYPE TEXT; + ALTER TABLE media_notes ADD COLUMN IF NOT EXISTS dedup_key VARCHAR(40); ALTER TABLE media_notes ADD COLUMN IF NOT EXISTS last_modified_at TIMESTAMPTZ; ALTER TABLE media_notes ADD COLUMN IF NOT EXISTS last_modified_source VARCHAR(30); diff --git a/internal/handlers/media.go b/internal/handlers/media.go index f4c7251..3642ecf 100644 --- a/internal/handlers/media.go +++ b/internal/handlers/media.go @@ -11,7 +11,6 @@ import ( "fmt" "io" "log" - "mime" "mime/multipart" "net/http" "net/url" @@ -191,55 +190,6 @@ func (mh *MediaHandler) SetAnnotationService(svc *wsync.AnnotationService) { mh.annotationSvc = svc } -func (h *MediaHandler) DownloadBook(c *echo.Context) error { - bookUUID, err := uuid.Parse(c.Param("uuid")) - if err != nil { - return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid book UUID"}) - } - - pgBookUUID := pgtype.UUID{Bytes: bookUUID, Valid: true} - - mediaItem, err := h.db.GetMediaItem(c.Request().Context(), pgBookUUID) - if err != nil { - return c.JSON(http.StatusNotFound, map[string]string{"error": "book not found"}) - } - - // Resolve relative path to absolute filesystem path - fullPath, err := h.getFullFilePath(c.Request().Context(), mediaItem.LibraryID, mediaItem.FilePath) - if err != nil { - return c.JSON(http.StatusNotFound, map[string]string{"error": "book file not found on disk"}) - } - - if _, err := os.Stat(fullPath); os.IsNotExist(err) { - return c.JSON(http.StatusNotFound, map[string]string{"error": "book file not found on disk"}) - } - - file, err := os.Open(fullPath) - if err != nil { - return c.JSON(http.StatusInternalServerError, map[string]string{"error": "failed to open book file"}) - } - defer file.Close() - - mimeType := mediaItem.MimeType.String - if !mediaItem.MimeType.Valid || mimeType == "" { - mimeType = mime.TypeByExtension(filepath.Ext(mediaItem.FilePath)) - } - - c.Response().Header().Set("Content-Type", mimeType) - c.Response().Header().Set("Content-Disposition", "attachment; filename=\""+filepath.Base(mediaItem.FilePath)+"\"") - - if mediaItem.FileSize.Valid && mediaItem.FileSize.Int64 > 0 { - c.Response().Header().Set("Content-Length", strconv.FormatInt(mediaItem.FileSize.Int64, 10)) - } - - _, err = io.Copy(c.Response(), file) - if err != nil { - return c.JSON(http.StatusInternalServerError, map[string]string{"error": "failed to stream file"}) - } - - return nil -} - // ExecuteSearch performs search and returns results with count // Public wrapper for shared search logic used by both JSON and HTML endpoints func (h *MediaHandler) ExecuteSearch(ctx context.Context, params services.SearchParams) ([]database.SearchMediaItemsUnifiedRow, int, error) { @@ -2073,30 +2023,86 @@ func (mh *MediaHandler) getFullFilePath(ctx context.Context, libraryID pgtype.UU return mh.libraryService.ResolveMediaPath(ctx, libraryID, relativePath) } -// ServeFile serves files (covers or books) via /uploads/library-{id}/path -// Requires JWT authentication +// ServeFile serves stored library files (covers and books). +// +// Two URL forms funnel into this handler: +// +// /uploads/library-{libraryID}/{relativePath} (covers, reader files) +// /api/media-items/{mediaItemID}/download (explicit book download) +// +// Both require JWT authentication and that the authenticated user can see +// the library owning the file - library visibility is the permission gate. func (mh *MediaHandler) ServeFile(c *echo.Context) error { - // URL format: /uploads/library-{libraryID}/{relativePath} - // Get library ID directly from route parameter - libraryIDStr := c.Param("id") - libraryUUID, err := uuid.Parse(libraryIDStr) - if err != nil { - return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid library ID"}) - } + var libraryUUID pgtype.UUID + var relativePath string - // Get remaining path from URL rawPath := c.Param("*") - relativePath, err := url.QueryUnescape(rawPath) - if err != nil { - relativePath = rawPath + if rawPath != "" { + // Path form: /uploads/library-{libraryID}/{relativePath} + libraryIDStr := c.Param("id") + parsed, err := uuid.Parse(libraryIDStr) + if err != nil { + return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid library ID"}) + } + libraryUUID = pgtype.UUID{Bytes: parsed, Valid: true} + + relativePath, err = url.QueryUnescape(rawPath) + if err != nil { + relativePath = rawPath + } + + if relativePath == "" { + return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid path"}) + } + } else { + // Item form: /api/media-items/{mediaItemID}/download + itemUUID, err := uuid.Parse(c.Param("id")) + if err != nil { + return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid media item ID"}) + } + + mediaItem, err := mh.db.GetMediaItem(c.Request().Context(), pgtype.UUID{Bytes: itemUUID, Valid: true}) + if err != nil { + return c.JSON(http.StatusNotFound, map[string]string{"error": "book not found"}) + } + + if !mediaItem.LibraryID.Valid || mediaItem.FilePath == "" { + return c.JSON(http.StatusNotFound, map[string]string{"error": "book file not found"}) + } + + libraryUUID = mediaItem.LibraryID + relativePath = mediaItem.FilePath + + // Explicit download endpoint: suggest saving instead of inline display. + filename := strings.Map(func(r rune) rune { + if r == '"' || r == '\\' || r == '/' { + return -1 + } + return r + }, filepath.Base(relativePath)) + c.Response().Header().Set("Content-Disposition", `attachment; filename="`+filename+`"`) } - if relativePath == "" { - return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid path"}) + // Library visibility gate: the library is what grants permission to + // see and download media. + user := c.Get("user").(database.Users) + visibleLibraries, err := mh.libraryService.GetUserVisibleLibraries(c.Request().Context(), user.ID) + if err != nil { + return c.JSON(http.StatusInternalServerError, map[string]string{"error": "failed to check library access"}) + } + libraryVisible := false + for _, lib := range visibleLibraries { + if lib.ID.Valid && lib.ID.Bytes == libraryUUID.Bytes { + libraryVisible = true + break + } + } + if !libraryVisible { + return c.JSON(http.StatusForbidden, map[string]string{"error": "library not accessible"}) } // Resolve using service - fullPath, err := mh.getFullFilePath(c.Request().Context(), pgtype.UUID{Bytes: libraryUUID, Valid: true}, relativePath) + fullPath, err := mh.getFullFilePath(c.Request().Context(), libraryUUID, relativePath) if err != nil { return c.JSON(http.StatusNotFound, map[string]string{"error": "file not found"}) } diff --git a/internal/router/media.go b/internal/router/media.go index e71e2a6..6998b80 100644 --- a/internal/router/media.go +++ b/internal/router/media.go @@ -15,6 +15,9 @@ func registerMediaRoutes(cfg *Config) { // Media item routes (all authenticated users) protected.GET("/media-items", cfg.MediaHandler.ListMediaItems) protected.GET("/media-items/:id", cfg.MediaHandler.GetMediaItem) + // Book download endpoint - same ServeFile flow as /uploads/library-:id/* + // (JWT + library-visibility gated), addressed by media item ID. + protected.GET("/media-items/:id/download", cfg.MediaHandler.ServeFile) // Media rating routes (all authenticated users) protected.POST("/media-items/:id/rating", cfg.MediaHandler.CreateMediaRating)