From 040c10ac76ff03bb67e24630514b341a54714396 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sun, 18 Oct 2026 13:10:57 -0400 Subject: [PATCH] feat(matching): family-lens scoping for book matching and linking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /api/books/match returned media metadata from ALL libraries regardless of visibility: the resolver lookups (GetMediaItemBySHA256 and friends) are global, so match results leaked hidden-library titles, covers and ids to any authenticated user, and LinkBook accepted a link target from a hidden library. Both now enforce the family-lens rule: - QueryBooks post-filters the cascade by the requester's visible-library set (absent-from-results — a hidden library's book simply never matches), before determineAction so the suggested action reflects the filtered set. BookQueryRequest gains a server-side-only VisibleLibraryIDs field; nil keeps legacy callers unfiltered. - LinkBook verifies the device belongs to the caller (DeviceExistsForUser) and the target item's library is visible — 403 otherwise. Linking was previously unscoped on both axes. - GetUnlinkedBooks likewise verifies device ownership (its service is still a stub, but the route no longer answers for other users' devices). Also: the collections books-grid partial uses a light loader — requested library validated against the visible set directly instead of the page-only library resolution (folder reachability, counts, dropdown payload), removing the extra queries that made library switching hold the spinner longer. And the dashboard library switch now folds the This Week widget's htmx refresh behind the spinner (refreshWeekWidget awaits the widget's swap, with a safety timeout) instead of letting it pop in after the fade-in — the two-stage update introduced with the widget's library-scoped re-fetch. Guardrail: the rendering lint now counts all innerHTML assignments (not just template-literal ones) — the old pattern undercounted 20 files at 80 sites, including .join("") builders like renderDashboardCollections. Baseline rebalanced accordingly. --- Makefile | 4 +- internal/handlers/book_matching.go | 99 +++++++++++++++++- internal/router/frontend.go | 16 ++- internal/router/helpers.go | 155 ++++++++++++++++++----------- internal/services/book_matching.go | 29 +++++- web/src/dashboard.ts | 33 +++++- web/src/rendering-allowlist.txt | 40 +++++--- 7 files changed, 293 insertions(+), 83 deletions(-) diff --git a/Makefile b/Makefile index dcfb0a94..74f04c81 100644 --- a/Makefile +++ b/Makefile @@ -58,8 +58,8 @@ test: lint-rendering # partials; grow one only with a written reason. lint-rendering: @violations=""; \ - for f in $$(grep -rlE 'innerHTML[[:space:]]*=[[:space:]]*\`' web/src --include='*.ts' | grep -v 'web/src/reader/'); do \ - count=$$(grep -cE 'innerHTML[[:space:]]*=[[:space:]]*\`' $$f); \ + for f in $$(grep -rlE 'innerHTML[[:space:]]*=[[:space:]]*' web/src --include='*.ts' | grep -v 'web/src/reader/'); do \ + count=$$(grep -E 'innerHTML[[:space:]]*=[[:space:]]*' $$f | grep -cvE 'innerHTML[[:space:]]*=[[:space:]]*""[[:space:]]*$$'); \ allowed=$$(grep -F "$$f:" web/src/rendering-allowlist.txt | cut -d: -f2); \ allowed=$${allowed:-0}; \ if [ $$count -gt $$allowed ]; then \ diff --git a/internal/handlers/book_matching.go b/internal/handlers/book_matching.go index 7aaf1396..538e18df 100644 --- a/internal/handlers/book_matching.go +++ b/internal/handlers/book_matching.go @@ -80,6 +80,48 @@ func (h *Handler) LinkBook(c *echo.Context) error { }) } + // Ownership: the device must belong to the caller. + user, ok := c.Get("user").(database.Users) + if !ok { + return c.JSON(http.StatusUnauthorized, map[string]string{ + "error": "user not authenticated", + }) + } + deviceOwned, err := h.db.DeviceExistsForUser(c.Request().Context(), database.DeviceExistsForUserParams{ + DeviceID: pgtype.UUID{Bytes: deviceID, Valid: true}, + UserID: user.ID, + }) + if err != nil { + return c.JSON(http.StatusInternalServerError, map[string]string{ + "error": "Failed to verify device", + }) + } + if !deviceOwned { + return c.JSON(http.StatusForbidden, map[string]string{ + "error": "Device not found", + }) + } + + // Family-lens: the link target must sit in a library the caller can + // see — a hidden library's book can never be linked. + visibleSet, visErr := h.visibleLibrarySetFor(c) + if visErr != nil { + return c.JSON(http.StatusInternalServerError, map[string]string{ + "error": "Failed to resolve visible libraries", + }) + } + item, itemErr := h.db.GetMediaItem(c.Request().Context(), pgtype.UUID{Bytes: req.MediaItemID, Valid: true}) + if itemErr != nil { + return c.JSON(http.StatusNotFound, map[string]string{ + "error": "Media item not found", + }) + } + if !item.LibraryID.Valid || !visibleSet[uuid.UUID(item.LibraryID.Bytes).String()] { + return c.JSON(http.StatusForbidden, map[string]string{ + "error": "Book is not available to you", + }) + } + alias, err := matchingService.LinkBook(c.Request().Context(), deviceID, &req) if err != nil { return c.JSON(http.StatusInternalServerError, map[string]string{ @@ -112,6 +154,27 @@ func (h *Handler) GetUnlinkedBooks(c *echo.Context) error { }) } + user, ok := c.Get("user").(database.Users) + if !ok { + return c.JSON(http.StatusUnauthorized, map[string]string{ + "error": "user not authenticated", + }) + } + deviceOwned, devErr := h.db.DeviceExistsForUser(c.Request().Context(), database.DeviceExistsForUserParams{ + DeviceID: pgtype.UUID{Bytes: deviceID, Valid: true}, + UserID: user.ID, + }) + if devErr != nil { + return c.JSON(http.StatusInternalServerError, map[string]string{ + "error": "Failed to verify device", + }) + } + if !deviceOwned { + return c.JSON(http.StatusForbidden, map[string]string{ + "error": "Device not found", + }) + } + unlinked, err := matchingService.GetUnlinkedBooks(c.Request().Context(), deviceID) if err != nil { return c.JSON(http.StatusInternalServerError, map[string]string{ @@ -509,10 +572,35 @@ func (h *Handler) DeleteDeviceFileAlias(c *echo.Context) error { }) } +// visibleLibrarySetFor resolves the requesting user's visible-library set +// (family-lens) from the request context identity. +func (h *Handler) visibleLibrarySetFor(c *echo.Context) (map[string]bool, error) { + user, ok := c.Get("user").(database.Users) + if !ok { + return nil, echo.NewHTTPError(http.StatusUnauthorized, "user not authenticated") + } + set, _, err := services.NewLibraryService(h.db).VisibleLibrarySet( + c.Request().Context(), user.ID) + return set, err +} + // GetBookMatches handles GET /api/books/match func (h *Handler) GetBookMatches(c *echo.Context) error { matchingService := h.getMatchingService() + // Family-lens: restrict matches to libraries the requester can see + // (absent-from-results — a hidden library's book simply never matches). + visibleSet, visErr := h.visibleLibrarySetFor(c) + if visErr != nil { + return c.JSON(http.StatusInternalServerError, map[string]string{ + "error": "Failed to resolve visible libraries", + }) + } + visibleIDs := make([]string, 0, len(visibleSet)) + for id := range visibleSet { + visibleIDs = append(visibleIDs, id) + } + // Get query parameters identifiers := c.QueryParams()["identifier"] sha256 := c.QueryParam("sha256") @@ -532,11 +620,12 @@ func (h *Handler) GetBookMatches(c *echo.Context) error { } req := &services.BookQueryRequest{ - Identifiers: identifiers, - SHA256: sha256, - Title: title, - Author: author, - FileSize: fileSize, + Identifiers: identifiers, + SHA256: sha256, + Title: title, + Author: author, + FileSize: fileSize, + VisibleLibraryIDs: visibleIDs, } response, err := matchingService.QueryBooks(c.Request().Context(), req) diff --git a/internal/router/frontend.go b/internal/router/frontend.go index 8c053640..6049bb24 100644 --- a/internal/router/frontend.go +++ b/internal/router/frontend.go @@ -702,10 +702,15 @@ func registerFrontendRoutes(cfg *Config) { // Books-grid partial: the collection detail page's #books-container // content. The library switcher swaps this in (htmx.ajax) instead of // re-rendering the grid client-side from JSON. Family-lens scoped via - // loadCollectionBooks — same resolution rules as the detail page. + // loadCollectionBooksLite — the requested library must be in the + // caller's visible set, without the page-only library resolution. frontendProtected.GET("/collections/:id/books-grid", func(c *echo.Context) error { - user, err := getTemplateUserWithTheme(c, cfg) - if err != nil { + user, ok := c.Get("user").(database.Users) + if !ok { + return c.HTML(http.StatusUnauthorized, "
Unauthorized
") + } + userUUID, uuidErr := uuid.Parse(user.ID.String()) + if uuidErr != nil { return c.HTML(http.StatusUnauthorized, "
Unauthorized
") } collectionID := c.Param("id") @@ -717,7 +722,10 @@ func registerFrontendRoutes(cfg *Config) { if err != nil { return c.HTML(http.StatusNotFound, "
Collection not found
") } - books, _, isSmart, errCode := cfg.loadCollectionBooks(c, user, collection, collectionID) + books, isSmart, errCode := cfg.loadCollectionBooksLite(c, userUUID, collection, collectionID) + if errCode == "library_forbidden" { + return c.HTML(http.StatusForbidden, "
Library not available
") + } if errCode != "" { return c.HTML(http.StatusBadRequest, "
Error loading books
") } diff --git a/internal/router/helpers.go b/internal/router/helpers.go index 8e4e6cfc..90a494a9 100644 --- a/internal/router/helpers.go +++ b/internal/router/helpers.go @@ -275,11 +275,69 @@ func collectionDataFromRow(collection database.Collections, collectionID string, } } -// loadCollectionBooks resolves the books shown for a collection: smart -// evaluation, a dashboard query section, or manual membership — scoped to -// the requesting user's visible libraries (family-lens) exactly like the -// collection detail page. Shared by the detail page and the books-grid -// partial route so the two can never drift. +// collectionBooksForScope runs the per-scope book loading shared by the +// detail page and the books-grid partial: smart evaluation, a dashboard +// query section, or manual membership. Callers resolve the scope: +// libUUID/libScope/isAll/scoped per their own resolution rules. +func (cfg *Config) collectionBooksForScope(c *echo.Context, userUUID uuid.UUID, collUUID pgtype.UUID, collection database.Collections, collectionID string, libUUID pgtype.UUID, libScope pgtype.UUID, scoped bool, visibleIDs []pgtype.UUID) (books []handlers.BookInfo, isSmart bool, errCode string) { + colService := cfg.CollectionHandler.CollectionService() + isSmart = colService.IsSmartCollection(collection.AutoAssignRules) + libTypes := handlers.LibraryTypeMap(c.Request().Context(), cfg.Queries) + + if isSmart { + // Smart (dynamic) collection: membership is evaluated live — + // visible libraries, live items only, any enabled rule matches. + matches, evalErr := colService.EvaluateSmartCollection(c.Request().Context(), + colService.ParseRules(collection.AutoAssignRules), services.SmartEvalOptions{ + LibraryID: libScope, + VisibleIDs: visibleIDs, + }) + if evalErr != nil { + return nil, isSmart, "books_load_error" + } + books = handlers.BookCardsFromMediaItems(matches, libTypes) + } else if collection.IsSystemCollection.Bool && collection.QueryType.Valid && collection.QueryType.String != "" && collection.QueryType.String != "filter" { + dashboardSvc := services.NewDashboardService(cfg.Queries) + sections, secErr := dashboardSvc.GetDashboardSections(c.Request().Context(), userUUID, libUUID, 1000, []string{}, []string{}) + if secErr != nil { + return nil, isSmart, "books_load_error" + } + + for _, section := range sections { + if section.CollectionID.String() == collectionID { + books = handlers.BookCardsFromMediaItems(section.Items, libTypes) + break + } + } + } else { + if scoped && libUUID.Valid { + collItems, collErr := cfg.Queries.GetCollectionItemsForDashboard(c.Request().Context(), + database.GetCollectionItemsForDashboardParams{ + CollectionID: collUUID, + LibraryID: libUUID, + Limit: pgtype.Int4{Int32: 1000, Valid: true}, + }) + if collErr != nil { + books = handlers.BookCardsFromCollectionDashRows(nil, libTypes) + } else { + // The query filters excluded rows in SQL. + books = handlers.BookCardsFromCollectionDashRows(collItems, libTypes) + } + } else { + collItems, collErr := cfg.Queries.GetCollectionItems(c.Request().Context(), collUUID) + if collErr != nil { + collItems = nil + } + books = handlers.BookCardsFromCollectionItemRows(collItems, libTypes) + } + } + return books, isSmart, "" +} + +// loadCollectionBooks resolves the books shown for a collection detail +// page: full library resolution (visible-library payload, folder +// reachability, counts) because the page render needs it. The books-grid +// partial uses the lighter loadCollectionBooksLite instead. // errCode is "" on success, otherwise a renderErrorPage code. func (cfg *Config) loadCollectionBooks(c *echo.Context, user templates.User, collection database.Collections, collectionID string) (books []handlers.BookInfo, libRes LibraryResolution, isSmart bool, errCode string) { collUUID, parseErr := uuid.Parse(collectionID) @@ -297,8 +355,6 @@ func (cfg *Config) loadCollectionBooks(c *echo.Context, user templates.User, col libScope = pgtype.UUID{Bytes: parsed, Valid: true} } } - colService := cfg.CollectionHandler.CollectionService() - isSmart = colService.IsSmartCollection(collection.AutoAssignRules) visibleIDs, visErr := cfg.CollectionHandler.UserVisibleLibraryIDs(c) if visErr != nil { @@ -306,59 +362,46 @@ func (cfg *Config) loadCollectionBooks(c *echo.Context, user templates.User, col visibleIDs = nil } - libTypes := handlers.LibraryTypeMap(c.Request().Context(), cfg.Queries) + scoped := libraryID != "" && !libRes.IsAll + books, isSmart, errCode = cfg.collectionBooksForScope(c, userUUID, pgtype.UUID{Bytes: collUUID, Valid: true}, collection, collectionID, libRes.LibUUID, libScope, scoped, visibleIDs) + return books, libRes, isSmart, errCode +} - if isSmart { - // Smart (dynamic) collection: membership is evaluated live — - // visible libraries, live items only, any enabled rule matches. - matches, evalErr := colService.EvaluateSmartCollection(c.Request().Context(), - colService.ParseRules(collection.AutoAssignRules), services.SmartEvalOptions{ - LibraryID: libScope, - VisibleIDs: visibleIDs, - }) - if evalErr != nil { - return nil, libRes, isSmart, "books_load_error" +// loadCollectionBooksLite is the books-grid partial's loader. The switcher +// refresh only needs "is the requested library visible, and its books" — +// so it validates the requested library against the caller's visible set +// directly (family-lens: out-of-set is rejected) and skips the page-only +// library resolution (folder reachability, counts, dropdown payload). +func (cfg *Config) loadCollectionBooksLite(c *echo.Context, userUUID uuid.UUID, collection database.Collections, collectionID string) (books []handlers.BookInfo, isSmart bool, errCode string) { + collUUID, parseErr := uuid.Parse(collectionID) + if parseErr != nil { + return nil, isSmart, "invalid_id" + } + libraryID := c.QueryParam("library_id") + + var libUUID pgtype.UUID + if libraryID != "" { + parsed, parseErr := uuid.Parse(libraryID) + if parseErr != nil { + return nil, isSmart, "invalid_library_id" } - books = handlers.BookCardsFromMediaItems(matches, libTypes) - } else if collection.IsSystemCollection.Bool && collection.QueryType.Valid && collection.QueryType.String != "" && collection.QueryType.String != "filter" { - dashboardSvc := services.NewDashboardService(cfg.Queries) - sections, secErr := dashboardSvc.GetDashboardSections(c.Request().Context(), userUUID, libRes.LibUUID, 1000, []string{}, []string{}) - if secErr != nil { - return nil, libRes, isSmart, "books_load_error" + set, _, setErr := cfg.LibraryService.VisibleLibrarySet( + c.Request().Context(), pgtype.UUID{Bytes: userUUID, Valid: true}) + if setErr != nil { + return nil, isSmart, "books_load_error" } - - for _, section := range sections { - if section.CollectionID.String() == collectionID { - books = handlers.BookCardsFromMediaItems(section.Items, libTypes) - break - } + if !set[libraryID] { + return nil, isSmart, "library_forbidden" } - } else { - if libraryID != "" && !libRes.IsAll { - libUUID, parseErr := uuid.Parse(libraryID) - if parseErr != nil { - return nil, libRes, isSmart, "invalid_library_id" - } + libUUID = pgtype.UUID{Bytes: parsed, Valid: true} + } - collItems, collErr := cfg.Queries.GetCollectionItemsForDashboard(c.Request().Context(), - database.GetCollectionItemsForDashboardParams{ - CollectionID: pgtype.UUID{Bytes: collUUID, Valid: true}, - LibraryID: pgtype.UUID{Bytes: libUUID, Valid: true}, - Limit: pgtype.Int4{Int32: 1000, Valid: true}, - }) - if collErr != nil { - books = handlers.BookCardsFromCollectionDashRows(nil, libTypes) - } else { - // The query filters excluded rows in SQL. - books = handlers.BookCardsFromCollectionDashRows(collItems, libTypes) - } - } else { - collItems, collErr := cfg.Queries.GetCollectionItems(c.Request().Context(), pgtype.UUID{Bytes: collUUID, Valid: true}) - if collErr != nil { - collItems = nil - } - books = handlers.BookCardsFromCollectionItemRows(collItems, libTypes) - } + visibleIDs, visErr := cfg.CollectionHandler.UserVisibleLibraryIDs(c) + if visErr != nil { + log.Printf("UserVisibleLibraryIDs failed: %v", visErr) + visibleIDs = nil } - return books, libRes, isSmart, "" + + return cfg.collectionBooksForScope(c, userUUID, pgtype.UUID{Bytes: collUUID, Valid: true}, collection, collectionID, + libUUID, libUUID, libraryID != "", visibleIDs) } diff --git a/internal/services/book_matching.go b/internal/services/book_matching.go index 75f87470..f457f250 100644 --- a/internal/services/book_matching.go +++ b/internal/services/book_matching.go @@ -24,7 +24,11 @@ type BookQueryRequest struct { SHA256 string `json:"sha256"` Title string `json:"title"` Author string `json:"author"` - FileSize int64 `json:"file_size"` + // VisibleLibraryIDs scopes results to the requesting user's visible + // libraries (family-lens, absent-from-results). Set server-side by the + // handler — never from client input. nil keeps legacy callers unfiltered. + VisibleLibraryIDs []string `json:"-"` + FileSize int64 `json:"file_size"` } // BookQueryResponse represents the response to a book query @@ -106,6 +110,29 @@ func (s *BookMatchingService) QueryBooks(ctx context.Context, req *BookQueryRequ } } + // Family-lens: matches may only surface items from libraries the + // requesting user can see — a hidden library's book simply never + // matches (absent-from-results). Runs before determineAction so the + // suggested action reflects the filtered set. nil (unset) keeps + // legacy callers unfiltered. + if req.VisibleLibraryIDs != nil { + allowed := make(map[string]bool, len(req.VisibleLibraryIDs)) + for _, id := range req.VisibleLibraryIDs { + allowed[id] = true + } + filtered := matches[:0] + for _, m := range matches { + item, err := s.db.GetMediaItem(ctx, pgtype.UUID{Bytes: m.MediaItemID, Valid: true}) + if err != nil || !item.LibraryID.Valid { + continue // vanished between cascade and filter, or unscoped — drop + } + if allowed[uuid.UUID(item.LibraryID.Bytes).String()] { + filtered = append(filtered, m) + } + } + matches = filtered + } + // Determine action action := s.determineAction(matches) diff --git a/web/src/dashboard.ts b/web/src/dashboard.ts index c671f925..49f8aea6 100644 --- a/web/src/dashboard.ts +++ b/web/src/dashboard.ts @@ -435,6 +435,34 @@ function initDragAndDrop(): void { }); } +// The week widget re-fetches itself when the body sees `week-refresh` +// (hx-trigger on #week-widget). Resolve once that swap has landed so the +// library switch fades in with the refreshed widget already in place. +// Safety timeout keeps a stalled request from holding the switch hostage, +// and a missing widget (stats disabled) resolves immediately. +function refreshWeekWidget(): Promise { + return new Promise((resolve) => { + if (!document.getElementById("week-widget")) { + resolve(); + return; + } + let settled = false; + const finish = (): void => { + if (settled) return; + settled = true; + document.removeEventListener("htmx:afterSwap", onSwap); + resolve(); + }; + const onSwap = (evt: Event): void => { + const target = (evt as CustomEvent).detail?.target as HTMLElement | undefined; + if (target && target.id === "week-widget") finish(); + }; + document.addEventListener("htmx:afterSwap", onSwap); + setTimeout(finish, 4000); + document.body.dispatchEvent(new CustomEvent("week-refresh")); + }); +} + function initDashboard() { initDragAndDrop(); @@ -444,7 +472,10 @@ function initDashboard() { await fetchAndRenderSections(libraryId); // The "This Week" widget tracks the switched library: htmx // re-fetches its fragment with the current #library-select value. - document.body.dispatchEvent(new CustomEvent("week-refresh")); + // Await the widget's swap so it lands behind the spinner — + // otherwise it pops in during/after the fade-in and the switch + // reads as two disjoint updates. + await refreshWeekWidget(); }), }); diff --git a/web/src/rendering-allowlist.txt b/web/src/rendering-allowlist.txt index 5dcac303..1c2d6406 100644 --- a/web/src/rendering-allowlist.txt +++ b/web/src/rendering-allowlist.txt @@ -1,15 +1,27 @@ # Rendering-ownership baseline (AGENTS.md → Rendering ownership). -# Files allowed to build HTML via template literals, with their current -# site counts. `make lint-rendering` fails when a count grows above its -# baseline. Shrink counts as fragments move to templ partials — the end -# state of the triage is an empty list. The reader (web/src/reader/) is a -# sanctioned island and not counted here. -web/src/admin.ts:1 -web/src/api-explorer.ts:2 -web/src/collections.ts:1 -web/src/custom-section-builder.ts:2 -web/src/linking.ts:1 -web/src/search.ts:3 -web/src/stats.ts:6 -web/src/toast.ts:1 -web/src/unlinked_books.ts:1 +# Files allowed to build HTML in TypeScript, with their current innerHTML +# assignment counts (trivial `innerHTML = ""` clears don't count; the +# reader — web/src/reader/ — is a sanctioned island and not counted). +# `make lint-rendering` fails when a count grows above its baseline. +# Shrink counts as fragments move to templ partials — the end state of +# the triage is an empty list. +web/src/admin.ts:3 +web/src/api-explorer.ts:4 +web/src/bookshelf.ts:1 +web/src/collection-rules.ts:6 +web/src/collections.ts:5 +web/src/custom-section-builder.ts:10 +web/src/device-management.ts:4 +web/src/docs.ts:5 +web/src/dashboard.ts:2 +web/src/linking.ts:3 +web/src/markdown.ts:1 +web/src/dom.ts:1 +web/src/password_validation.ts:1 +web/src/queue.ts:3 +web/src/search.ts:4 +web/src/series.ts:1 +web/src/setup.ts:1 +web/src/stats.ts:9 +web/src/toast.ts:2 +web/src/unlinked_books.ts:13