From 4bd2788c7de099bb776b5c1ba7c0ee95df779821 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Mon, 2 Nov 2026 14:55:54 -0500 Subject: [PATCH] docs(dev): library-visibility rewritten for the RLS era MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The page still taught the pre-2026-10-29 app-side pattern — visibleLibrarySet helpers, hand-rolled sqlc.narg('visible_library_ids') clauses, per-endpoint 403 checks — which the RLS move made both wrong and counterproductive: media_items, podcast_feeds, and collections are now governed by row-level security filtered on the app.user_id GUC that ScopedDB declares per statement. New media queries need NO visibility plumbing; detached contexts RAISE unless the identity is threaded (rls.WithUser); the NEVER list (session-scoped GUC, bypass flags, superuser connections) and the auditors (TestRlsInvariant, Playwright sweep, contract pins) are documented with the policy mechanics they enforce. The series-per-library rule survives — it is query semantics, not visibility filtering, and the policy makes 'visible' automatic under it. --- docs/developer/library-visibility.md | 116 +++++++++++++++++---------- 1 file changed, 75 insertions(+), 41 deletions(-) diff --git a/docs/developer/library-visibility.md b/docs/developer/library-visibility.md index 581bdf7e..2013a116 100644 --- a/docs/developer/library-visibility.md +++ b/docs/developer/library-visibility.md @@ -1,54 +1,88 @@ -# Library Visibility (family-lens rule) + -Every endpoint that returns media metadata or content must respect -per-user **library visibility**: the `library_visibility` table maps -`(user_id, library_id, is_visible)` and defaults to **ALLOW** — every -user sees every library unless an admin explicitly hides one from them. -The point of the system is the family case: an admin hides the adult -library from a child's account, and no trace of that library (titles, -covers, series, page images) may reach the child's account. +# Library Visibility — how per-user data isolation actually works -## The rule for new endpoints +Every user sees every library **unless an admin hides it from them** +(`library_visibility` table, default **ALLOW** — the family-lens case: +hide the adult library from a child's account, and no trace of it — +titles, covers, series, page images — may reach that account). -Any endpoint that lists, filters, or details media/series/books: +The one thing to understand before writing any code that touches media: -1. Filter by the requesting user's visible-library set. -2. Reject an explicit `library_id` outside that set with **403** — - never silently ignore or fall back. -3. Content endpoints (file bytes, page images) gate per item, like - `ReaderHandler.ensureItemVisible`. +> **Enforcement lives in the DATABASE, as Postgres row-level security. +> The application declares *who* is asking; it never computes *what* +> they may see.** -If you forget, hidden libraries leak. This exact leak shipped: the -series endpoints were name-only across ALL libraries and the PDF -thumbnail endpoint served page images of hidden books (fixed 2026-10-13, -bookhoard `431fec0`). +## The engine -## Helpers — do not hand-roll +`database/schema/schema.sql` carries a labeled RLS block (re-run +idempotently on every boot). Governed tables — `media_items`, +`podcast_feeds`, `collections`, `collection_items` — have policies that +filter every row against the `library_visibility` mapping using the +`app.user_id` GUC: a per-transaction `SET LOCAL` declared by +`ScopedDB` (`internal/database/scoped.go`), which wraps the pool so +**every sqlc statement** runs inside a transaction that declares the +acting user. -- `(*MediaHandler).visibleLibrarySet` / - `(*SeriesHandler).visibleLibrarySet` — returns a membership map plus - the id slice (map for param checks, slice for SQL `ANY()`). -- `router.visibleLibraryIDsFor` — same for SSR routes. -- `resolveLibrary` (router/helpers.go) only honors a requested - `library_id` that is inside the caller's visible set. -- SQL pattern (see `ListMediaItems`, `GetDistinctSeries`, - `SearchMediaItems`): +- A statement whose context carries an identity sees only that user's + visible rows. +- A statement with **no identity raises** on a governed table — the + engine fails loud instead of leaking. Ungoverned tables (auth lookups, + settings, sessions) are unaffected and keep working. +- Collections use predicate functions (`app_can_read_collection`, + `app_can_write_collection`) that are PURE over their arguments — reads + are owner-or-admin, writes are owner (admin escape for SYSTEM + collections only). Future sharing = one EXISTS branch inside the + predicate, no policy migration. -```sql -AND (sqlc.narg('visible_library_ids')::uuid[] IS NULL - OR library_id = ANY(sqlc.narg('visible_library_ids')::uuid[])) -``` +## The rules for new code -## Known-safe surfaces +1. **New media-listing queries need NO visibility clause and NO + plumbing.** If the statement goes through `ScopedDB` with a declared + identity, the policy already filters it. +2. **The app declares identity; it never computes visibility.** Three + bridges put the identity on the request context: the JWT middleware, + the cookie/device middleware, and the KOReader/Kobo device auth + middleware. Everything downstream just carries the context. +3. **A media read on a detached `context.Background()` RAISES.** If you + need detached work (background jobs, websocket fan-in), thread the + acting identity explicitly — `rls.WithUser(ctx, userID)` — the way + scan jobs carry the library owner (first-admin fallback) and the + websocket `initial_state` carries the connecting user. +4. **NEVER**: session-scoped GUCs (pooled-connection identity bleed), a + bypass flag, or connecting the app as a superuser (superusers bypass + RLS unconditionally — the app runs as `bookhoard_srv`, and the + `db-init` compose service provisions that role at startup). +5. **Schema changes go in `schema.sql` in place**, idempotent + (`DROP .. IF EXISTS` + `CREATE`) — it re-runs on every boot. The + migration transaction declares a synthetic identity because the + schema's own dedup pass reads `media_items`. -- OPDS device catalogs filter by the device owner's visible libraries. -- Search (`SearchMediaItems`) is visibility-scoped. -- File download, reader page render, and page content gate per item. +## How this gets audited + +- **`TestRlsInvariant`** — API black-box: two users, one hidden library, + every media-returning surface checked. New media-returning surfaces + must be added to it. +- **The Playwright sweep** (`tests/ui/`) — user-visible pages checked + against the coverage list. +- **`docs/contract_pins.md`** — contract-level pins (a loosening requires + a changelog entry there). + +## Why the rule is written in blood + +The pre-RLS app-side approach shipped a real leak (2026-10-13, +`431fec0`): series endpoints were name-only across ALL libraries and the +PDF thumbnail endpoint served page images of hidden books — every +hand-rolled filter had to be remembered at every call site, and one +forgotten clause leaked. On 2026-10-29 enforcement moved into the +database, and the invariant test went red → green to prove the old +helpers were no longer needed. If you find yourself writing a +`visible_library_ids` filter by hand, you are re-creating the bug. ## Series are per-library -Series are grouped by `(library_id, series)` — the same series name in -two libraries is two entries (manga adaptations, LN splits). Do not -re-merge them cross-library. `/api/series/books` REQUIRES a visible -`library_id`; `/api/series` without one aggregates only the caller's -visible libraries. +Series group by `(library_id, series)` — the same series name in two +libraries is two entries (manga adaptations, LN splits). Do not re-merge +them cross-library. `/api/series/books` requires a visible `library_id`; +`/api/series` without one aggregates only the caller's visible libraries +(the policy makes "visible" automatic).