feat(collections): RLS ownership enforcement at the DB layer

collections + collection_items join the RLS-governed tables, same
doctrine as the media_items family lens (2026-10-29). Policies route
through two PURE predicate functions (app_can_read_collection /
app_can_write_collection) that never query collections — no policy
recursion under FORCE RLS — so a future sharing model is one EXISTS
branch inside the functions, not a schema re-touch. Reads: owner or
admin; writes: owner, admin escape for SYSTEM collections only.

Why: the write path had no app-level ownership gate on Update/Delete/
AddBooks — any authenticated user could mutate another user's
collection by id. The DB is now the enforcement of record; existing
app-level gates (match-mode, reorder, view-settings) stay as
defense-in-depth.

Behavior changes (contract_pins 2026-10-31):
- cross-user GET detail: 200 -> 404 (row invisible)
- cross-user PUT update: silent success -> 404 (ErrNoRows mapping;
  service wrap fixed %v -> %w so errors.Is sees pgx.ErrNoRows)
- cross-user POST books: silent success -> 404
- cross-user DELETE: 204, now a true DB no-op
- anonymous sees nothing; admin persona unchanged

Bootstrap: registration seeds system collections BEFORE the JWT
exists — CreateDefaultCollectionsForUser declares the new user's
identity (rls.WithUser), the same fix mirrored in the test harness
(createDefaultCollectionsForUser owner-scoped; collection sweeps run
under the owner's identity).

Tests: new TestCollectionsRLS pins the cross-user matrix end-to-end
(404 detail/update/add/view-settings, 204 no-op delete, admin escape,
owner positive control, registration seeding under RLS). Harness
fixture writes to non-system collections now run as the row's OWNER
(scopeExecAs/ownerSeedCtx) — the admin write-escape covers system
rows only, and bare-ctx pool reads of collections see nothing.
Filter/playlist/audiobook/scan-state fixtures converted. Full
container suite green (159s).
This commit is contained in:
John O'Keefe committed 2026-10-31 18:30:02 -04:00
1 parent aaf57a66a2
commit ab98015190
13 files changed
+366 -25

No files matched your search

+11
View File
@@ -145,6 +145,17 @@ every user sees every library unless an admin hides one) is enforced by
new pages/controls go into `tests/ui/COVERAGE.md`). Item-scoped new pages/controls go into `tests/ui/COVERAGE.md`). Item-scoped
gates may answer 404 as well as 403 (row invisible vs Go-level gates may answer 404 as well as 403 (row invisible vs Go-level
check — contract_pins 2026-10-29). check — contract_pins 2026-10-29).
- **Collections RLS (2026-10-31)**: `collections` and
`collection_items` are governed too — predicates
`app_can_read_collection` / `app_can_write_collection`, PURE over
their arguments (never query `collections`, so no policy recursion
under FORCE RLS). Reads: owner-or-admin; writes: owner, admin escape
for SYSTEM collections only. Future sharing = one EXISTS branch
inside the predicate functions, no policy migration. Registration
seeding declares the new user's identity (`rls.WithUser` in
`CreateDefaultCollectionsForUser` — the JWT does not exist yet);
test-harness writes to non-system collections must run as the row's
OWNER (`scopeExecAs`/`ownerSeedCtx`, not the admin handle).
History: 2026-10-13 (`431fec0`) the series/media listing surfaces History: 2026-10-13 (`431fec0`) the series/media listing surfaces
leaked hidden libraries; **2026-10-29 enforcement moved to RLS** — the leaked hidden libraries; **2026-10-29 enforcement moved to RLS** — the
+4 -2
View File
@@ -659,12 +659,14 @@ func TestAudiobookFolderSidecarEditDetected(t *testing.T) {
require.NoError(t, os.WriteFile(sidecar, []byte(`{"title":"Sidecar Real Titlz","series":["Sidecar Saga (Reading Order) #1"]}`), 0o644)) require.NoError(t, os.WriteFile(sidecar, []byte(`{"title":"Sidecar Real Titlz","series":["Sidecar Saga (Reading Order) #1"]}`), 0o644))
scanTestFolder(t, setup, admin, folder) scanTestFolder(t, setup, admin, folder)
var collCount int var collCount int
require.NoError(t, setup.DBPool.QueryRow(context.Background(), // Owner-scoped read: collections RLS (2026-10-31) hides rows from
// the pool session's SeedIdentity.
require.NoError(t, database.NewScopedDB(setup.DBPool).QueryRow(ownerSeedCtx(admin),
`SELECT COUNT(*) FROM collections WHERE name = 'Sidecar Saga (Reading Order)' AND user_id = $1`, admin, `SELECT COUNT(*) FROM collections WHERE name = 'Sidecar Saga (Reading Order)' AND user_id = $1`, admin,
).Scan(&collCount)) ).Scan(&collCount))
require.Equal(t, 1, collCount, "reading-order sidecar edit creates the collection for an existing book") require.Equal(t, 1, collCount, "reading-order sidecar edit creates the collection for an existing book")
scanTestFolder(t, setup, admin, folder) scanTestFolder(t, setup, admin, folder)
require.NoError(t, setup.DBPool.QueryRow(context.Background(), require.NoError(t, database.NewScopedDB(setup.DBPool).QueryRow(ownerSeedCtx(admin),
`SELECT COUNT(*) FROM collections WHERE name = 'Sidecar Saga (Reading Order)' AND user_id = $1`, admin, `SELECT COUNT(*) FROM collections WHERE name = 'Sidecar Saga (Reading Order)' AND user_id = $1`, admin,
).Scan(&collCount)) ).Scan(&collCount))
require.Equal(t, 1, collCount, "rescan is idempotent — no duplicate collection") require.Equal(t, 1, collCount, "rescan is idempotent — no duplicate collection")
+127
View File
@@ -0,0 +1,127 @@
package main
import (
"bytes"
"testing"
"encoding/json"
"fmt"
"net/http"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// Collections RLS (2026-10-31): ownership enforcement moved to the DB
// layer (p_collections_* + parent-checked collection_items policies in
// schema.sql, predicates app_can_read/write_collection). The API surface
// answers invisibility with 404 — including the write endpoints that
// had NO app-level gate (Update/AddBooks, the flagged gap). Admin keeps
// the escape; system collections stay per-user; registration still
// seeds them (the seeding identity fix in
// CreateDefaultCollectionsForUser is regression-pinned here).
func TestCollectionsRLS(t *testing.T) {
setup := setupTestServer(t)
ownerToken := setup.RegularToken // user A: the harness regular user
adminToken := setup.Token
marker := fmt.Sprintf("rls%d", time.Now().UnixNano()%100000)
// User B: a second regular user via registration. The register
// endpoint is rate-limited — exactly one call. The response's
// access_token is B's identity for every probe below.
regEmail := fmt.Sprintf("%s@tests.bookhoard.internal", marker)
regBody := fmt.Sprintf(
`{"email":%q,"username":%q,"password":"Test@Pass123!"}`,
regEmail, "rlsb"+marker)
regResp, err := http.Post(setup.Server.URL+"/api/auth/register",
"application/json", bytes.NewReader([]byte(regBody)))
require.NoError(t, err)
require.Equal(t, http.StatusCreated, regResp.StatusCode,
"registration must succeed — including the system-collection seeding that now runs under the new user's RLS identity")
var reg struct {
Token string `json:"access_token"`
}
require.NoError(t, json.NewDecoder(regResp.Body).Decode(&reg))
require.NotEmpty(t, reg.Token)
otherToken := reg.Token
_ = regResp.Body.Close()
// B's registration seeded B's OWN system collections — visible in
// B's list (this is the registration-seeding identity fix, live).
code, body := podcastAuthed(t, setup, otherToken, "GET", "/api/collections", "")
require.Equal(t, http.StatusOK, code)
assert.Contains(t, string(body), "Continue Reading",
"registration seeded the new user's system collections under RLS")
// A creates the target collection.
code, body = podcastAuthed(t, setup, ownerToken, "POST", "/api/collections",
`{"name":"test RLS Owner ` + marker + `"}`)
require.Equal(t, http.StatusCreated, code, "%s", string(body))
var coll struct {
ID string `json:"id"`
}
require.NoError(t, json.Unmarshal(body, &coll))
t.Cleanup(func() {
podcastAuthed(t, setup, ownerToken, "DELETE", "/api/collections/"+coll.ID, "")
})
// ── B (non-owner) probes A's collection: invisible = 404 ──
code, _ = podcastAuthed(t, setup, otherToken, "GET", "/api/collections/"+coll.ID, "")
require.Equal(t, http.StatusNotFound, code, "detail-by-id is RLS-invisible cross-user")
code, body = podcastAuthed(t, setup, otherToken, "GET", "/api/collections", "")
require.Equal(t, http.StatusOK, code)
assert.NotContains(t, string(body), "test RLS Owner "+marker,
"the list query stays user-scoped")
code, body = podcastAuthed(t, setup, otherToken, "PUT", "/api/collections/"+coll.ID,
`{"name":"renamed by B"}`)
require.Equal(t, http.StatusNotFound, code,
"update of an RLS-invisible row answers 404 (was: ungated success): %s", string(body))
code, body = podcastAuthed(t, setup, otherToken, "POST", "/api/collections/"+coll.ID+"/books",
`{"book_ids":[]}`)
require.Equal(t, http.StatusNotFound, code,
"AddBooks on an RLS-invisible collection answers 404 (the flagged ungated-write gap, closed at the DB)")
code, body = podcastAuthed(t, setup, otherToken, "PUT",
"/api/collections/"+coll.ID+"/view-settings", `{"chain_order":"newest"}`)
require.Equal(t, http.StatusNotFound, code,
"chain-order write on an invisible collection answers 404")
code, _ = podcastAuthed(t, setup, otherToken, "DELETE", "/api/collections/"+coll.ID, "")
require.Equal(t, http.StatusNoContent, code,
"DELETE touches no rows under RLS (silent no-op — no destructive action)")
// The no-op delete lied about nothing: A still owns the collection.
code, body = podcastAuthed(t, setup, ownerToken, "GET", "/api/collections/"+coll.ID, "")
require.Equal(t, http.StatusOK, code, "owner retains the collection after B's failed delete")
// ── admin escape: sees everything, as with media ──
code, _ = podcastAuthed(t, setup, adminToken, "GET", "/api/collections/"+coll.ID, "")
require.Equal(t, http.StatusOK, code, "admin persona sees collections (policy escape)")
// ── B's ownership works: B renames B's OWN system collection ──
// (positive control — B is not locked out of what B owns)
code, body = podcastAuthed(t, setup, otherToken, "GET", "/api/collections", "")
require.Equal(t, http.StatusOK, code)
var list struct {
Collections []struct {
ID string `json:"id"`
Name string `json:"name"`
} `json:"collections"`
}
require.NoError(t, json.Unmarshal(body, &list))
var bSystem string
for _, c := range list.Collections {
if c.Name == "Continue Reading" {
bSystem = c.ID
break
}
}
require.NotEmpty(t, bSystem, "B owns the seeded system collections")
code, body = podcastAuthed(t, setup, otherToken, "PUT", "/api/collections/"+bSystem,
`{"name":"Continue Reading"}`)
require.Contains(t, []int{http.StatusOK, http.StatusCreated}, code,
"owner mutation of an OWN collection still works: %s", string(body))
}
+1 -1
View File
@@ -213,7 +213,7 @@ func TestSmartCollections(t *testing.T) {
t.Run("SystemCollectionsBlockedFromTheEditor", func(t *testing.T) { t.Run("SystemCollectionsBlockedFromTheEditor", func(t *testing.T) {
// System collections may never carry rules/trees — the editor // System collections may never carry rules/trees — the editor
// route refuses them (item 3.4: the page IS the editor). // route refuses them (item 3.4: the page IS the editor).
colls, err := setup.DB.GetCollectionsByUser(context.Background(), pgtype.UUID{Bytes: [16]byte(adminID), Valid: true}) colls, err := setup.DB.GetCollectionsByUser(ownerSeedCtx(pgtype.UUID{Bytes: [16]byte(adminID), Valid: true}), pgtype.UUID{Bytes: [16]byte(adminID), Valid: true})
require.NoError(t, err) require.NoError(t, err)
var systemID string var systemID string
for _, c := range colls { for _, c := range colls {
+17 -8
View File
@@ -455,12 +455,17 @@ func TestFilterUserRelativeFields(t *testing.T) {
aID, wickID, aID, tideID) aID, wickID, aID, tideID)
var collID pgtype.UUID var collID pgtype.UUID
err := setup.DBPool.QueryRow(context.Background(), // Seed as the OWNER (collections RLS, 2026-10-31): the insert
// predicate wants actor == owner, and the item/cleanup writes want
// the same identity — the admin write-escape covers SYSTEM rows only.
err := database.NewScopedDB(setup.DBPool).QueryRow(ownerSeedCtx(aID),
`INSERT INTO collections (user_id, name) VALUES ($1, $2) RETURNING id`, `INSERT INTO collections (user_id, name) VALUES ($1, $2) RETURNING id`,
aID, fmt.Sprintf("Test Builder Coll %d", time.Now().UnixNano())).Scan(&collID) aID, fmt.Sprintf("Test Builder Coll %d", time.Now().UnixNano())).Scan(&collID)
require.NoError(t, err) require.NoError(t, err)
t.Cleanup(func() { _ = setup.DB.DeleteCollection(ctx, collID) }) t.Cleanup(func() {
seedExec(t, setup.DBPool, scopeExecAs(t, setup, aID, `DELETE FROM collections WHERE id = $1`, collID)
})
scopeExecAs(t, setup, aID,
`INSERT INTO collection_items (collection_id, media_item_id) VALUES ($1,$2)`, collID, tideID) `INSERT INTO collection_items (collection_id, media_item_id) VALUES ($1,$2)`, collID, tideID)
titlesFor := func(token string, qf string) []string { titlesFor := func(token string, qf string) []string {
@@ -542,7 +547,6 @@ func TestFilterSuggestionsUnderRules(t *testing.T) {
libUUID, _ := builderFixtureLib(t, setup, userID, "Sug") libUUID, _ := builderFixtureLib(t, setup, userID, "Sug")
// Add a second distinct author+genre pair (trigram-disjoint). // Add a second distinct author+genre pair (trigram-disjoint).
ctx := context.Background()
_, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ _, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{
LibraryID: pgtype.UUID{Bytes: libUUID, Valid: true}, LibraryID: pgtype.UUID{Bytes: libUUID, Valid: true},
Title: "Sug Dune", Title: "Sug Dune",
@@ -670,16 +674,21 @@ func TestFilterSuggestionsUnderRules(t *testing.T) {
// offered with the surviving count; zero-match containers never are. // offered with the surviving count; zero-match containers never are.
userA := getRegularUserID(t, setup.DB) userA := getRegularUserID(t, setup.DB)
var collID pgtype.UUID var collID pgtype.UUID
err = setup.DBPool.QueryRow(context.Background(), // Seed as the OWNER (collections RLS, 2026-10-31) — same identity
// rule as the first seeding site in this file.
ownerID := pgtype.UUID{Bytes: [16]byte(userA), Valid: true}
err = database.NewScopedDB(setup.DBPool).QueryRow(ownerSeedCtx(ownerID),
`INSERT INTO collections (user_id, name) VALUES ($1, $2) RETURNING id`, `INSERT INTO collections (user_id, name) VALUES ($1, $2) RETURNING id`,
pgtype.UUID{Bytes: [16]byte(userA), Valid: true}, ownerID,
fmt.Sprintf("Test Builder Sug Coll %d", time.Now().UnixNano())).Scan(&collID) fmt.Sprintf("Test Builder Sug Coll %d", time.Now().UnixNano())).Scan(&collID)
require.NoError(t, err) require.NoError(t, err)
t.Cleanup(func() { _ = setup.DB.DeleteCollection(ctx, collID) }) t.Cleanup(func() {
scopeExecAs(t, setup, ownerID, `DELETE FROM collections WHERE id = $1`, collID)
})
var tideID pgtype.UUID var tideID pgtype.UUID
err = setup.DBPool.QueryRow(context.Background(), `SELECT id FROM media_items WHERE title = $1`, "Sug Tide").Scan(&tideID) err = setup.DBPool.QueryRow(context.Background(), `SELECT id FROM media_items WHERE title = $1`, "Sug Tide").Scan(&tideID)
require.NoError(t, err) require.NoError(t, err)
seedExec(t, setup.DBPool, scopeExecAs(t, setup, ownerID,
`INSERT INTO collection_items (collection_id, media_item_id) VALUES ($1, $2)`, collID, tideID) `INSERT INTO collection_items (collection_id, media_item_id) VALUES ($1, $2)`, collID, tideID)
status, results = suggest("collection", " ", "") status, results = suggest("collection", " ", "")
+13 -6
View File
@@ -13,6 +13,8 @@ import (
"github.com/google/uuid" "github.com/google/uuid"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"bookhoard/internal/database"
) )
// A2c end-to-end: an ABS sidecar whose series array mixes a real series and // A2c end-to-end: an ABS sidecar whose series array mixes a real series and
@@ -67,13 +69,14 @@ func TestPlaylistCollectionsEndToEnd(t *testing.T) {
require.NotEmpty(t, foundPath) require.NotEmpty(t, foundPath)
// (b) The playlist collection exists with provenance, dashboard pin, // (b) The playlist collection exists with provenance, dashboard pin,
// and the item at reading position 7. // and the item at reading position 7. Owner-scoped read: collections
// RLS (2026-10-31) hides the row from the pool session's SeedIdentity.
adminUUIDStr := adminID.String() adminUUIDStr := adminID.String()
var collID uuid.UUID var collID uuid.UUID
var collName, collDesc string var collName, collDesc string
var showOnDashboard bool var showOnDashboard bool
var viewSettingsRaw []byte var viewSettingsRaw []byte
require.NoError(t, setup.DBPool.QueryRow(context.Background(), require.NoError(t, database.NewScopedDB(setup.DBPool).QueryRow(ownerSeedCtx(adminID),
`SELECT id, name, description, show_on_dashboard, view_settings `SELECT id, name, description, show_on_dashboard, view_settings
FROM collections WHERE user_id = $1 AND name = 'The Tagged Saga (Reading Order)'`, FROM collections WHERE user_id = $1 AND name = 'The Tagged Saga (Reading Order)'`,
adminID).Scan(&collID, &collName, &collDesc, &showOnDashboard, &viewSettingsRaw)) adminID).Scan(&collID, &collName, &collDesc, &showOnDashboard, &viewSettingsRaw))
@@ -85,7 +88,9 @@ func TestPlaylistCollectionsEndToEnd(t *testing.T) {
var position *int32 var position *int32
var itemCount int var itemCount int
require.NoError(t, setup.DBPool.QueryRow(context.Background(), // Owner-scoped reads: collections RLS (2026-10-31) — items ride the
// parent's readability, so the pool session's SeedIdentity sees none.
require.NoError(t, database.NewScopedDB(setup.DBPool).QueryRow(ownerSeedCtx(adminID),
`SELECT ci.position, COUNT(*) OVER () FROM collection_items ci `SELECT ci.position, COUNT(*) OVER () FROM collection_items ci
WHERE ci.collection_id = $1`, collID).Scan(&position, &itemCount)) WHERE ci.collection_id = $1`, collID).Scan(&position, &itemCount))
require.Equal(t, 1, itemCount) require.Equal(t, 1, itemCount)
@@ -96,11 +101,11 @@ func TestPlaylistCollectionsEndToEnd(t *testing.T) {
// (c) Rescan: no duplicate collection, no duplicate membership. // (c) Rescan: no duplicate collection, no duplicate membership.
scanTestFolder(t, setup, adminID, folder) scanTestFolder(t, setup, adminID, folder)
var collectionCount, membershipCount int var collectionCount, membershipCount int
require.NoError(t, setup.DBPool.QueryRow(context.Background(), require.NoError(t, database.NewScopedDB(setup.DBPool).QueryRow(ownerSeedCtx(adminID),
`SELECT COUNT(*) FROM collections WHERE user_id = $1 AND name = 'The Tagged Saga (Reading Order)'`, `SELECT COUNT(*) FROM collections WHERE user_id = $1 AND name = 'The Tagged Saga (Reading Order)'`,
adminID).Scan(&collectionCount)) adminID).Scan(&collectionCount))
require.Equal(t, 1, collectionCount) require.Equal(t, 1, collectionCount)
require.NoError(t, setup.DBPool.QueryRow(context.Background(), require.NoError(t, database.NewScopedDB(setup.DBPool).QueryRow(ownerSeedCtx(adminID),
`SELECT COUNT(*) FROM collection_items WHERE collection_id = $1`, collID).Scan(&membershipCount)) `SELECT COUNT(*) FROM collection_items WHERE collection_id = $1`, collID).Scan(&membershipCount))
require.Equal(t, 1, membershipCount) require.Equal(t, 1, membershipCount)
@@ -131,7 +136,9 @@ func TestPlaylistCollectionsEndToEnd(t *testing.T) {
// (e) hide_finished + Up Next + completion marker. // (e) hide_finished + Up Next + completion marker.
// Flip the collection's flag on, mark the single item finished for the // Flip the collection's flag on, mark the single item finished for the
// admin, and the dashboard section becomes Complete with no items. // admin, and the dashboard section becomes Complete with no items.
_, err = setup.DBPool.Exec(context.Background(), // Owner-or-admin UPDATE policy: run as ADMIN (collections RLS,
// 2026-10-31).
adminScopeExec(t, setup, adminCtxValue(t, setup),
`UPDATE collections SET view_settings = view_settings || '{"hide_finished": true}'::jsonb WHERE id = $1`, collID) `UPDATE collections SET view_settings = view_settings || '{"hide_finished": true}'::jsonb WHERE id = $1`, collID)
require.NoError(t, err) require.NoError(t, err)
_, err = setup.DBPool.Exec(context.Background(), _, err = setup.DBPool.Exec(context.Background(),
+3 -2
View File
@@ -207,12 +207,13 @@ func TestScanManifestLifecycle(t *testing.T) {
require.NoError(t, os.WriteFile(opfPath, []byte(`{"title":"Manifesu Book","series":["Lifecycle Saga (Reading Order) #1"]}`), 0644)) require.NoError(t, os.WriteFile(opfPath, []byte(`{"title":"Manifesu Book","series":["Lifecycle Saga (Reading Order) #1"]}`), 0644))
scanTestFolder(t, setup, admin, folder) scanTestFolder(t, setup, admin, folder)
var collCount int var collCount int
require.NoError(t, setup.DBPool.QueryRow(context.Background(), // Owner-scoped read: collections RLS (2026-10-31).
require.NoError(t, database.NewScopedDB(setup.DBPool).QueryRow(ownerSeedCtx(admin),
`SELECT COUNT(*) FROM collections WHERE name = 'Lifecycle Saga (Reading Order)' AND user_id = $1`, admin, `SELECT COUNT(*) FROM collections WHERE name = 'Lifecycle Saga (Reading Order)' AND user_id = $1`, admin,
).Scan(&collCount)) ).Scan(&collCount))
assert.Equal(t, 1, collCount, "reading-order sidecar added to an existing book creates its collection") assert.Equal(t, 1, collCount, "reading-order sidecar added to an existing book creates its collection")
scanTestFolder(t, setup, admin, folder) scanTestFolder(t, setup, admin, folder)
require.NoError(t, setup.DBPool.QueryRow(context.Background(), require.NoError(t, database.NewScopedDB(setup.DBPool).QueryRow(ownerSeedCtx(admin),
`SELECT COUNT(*) FROM collections WHERE name = 'Lifecycle Saga (Reading Order)' AND user_id = $1`, admin, `SELECT COUNT(*) FROM collections WHERE name = 'Lifecycle Saga (Reading Order)' AND user_id = $1`, admin,
).Scan(&collCount)) ).Scan(&collCount))
assert.Equal(t, 1, collCount, "rescan is idempotent — no duplicate collection") assert.Equal(t, 1, collCount, "rescan is idempotent — no duplicate collection")
+32 -5
View File
@@ -329,6 +329,22 @@ func seedCtx() context.Context {
return rls.WithUser(context.Background(), rls.SeedIdentity) return rls.WithUser(context.Background(), rls.SeedIdentity)
} }
// ownerSeedCtx returns a context carrying a fixture user's identity,
// for seeding rows OWNED by that user. Collections RLS (2026-10-31):
// the write predicates want actor == owner for non-system rows (the
// admin write-escape covers SYSTEM collections only), and reads under
// the pool session's SeedIdentity see nothing.
func ownerSeedCtx(userID pgtype.UUID) context.Context {
return rls.WithUser(context.Background(), userID)
}
// scopeExecAs is adminScopeExec's generic form: raw SQL under ANY
// declared identity. Use it (with the row's OWNER) for fixture writes
// to non-system collections — see ownerSeedCtx.
func scopeExecAs(t *testing.T, setup *TestServerSetup, userID pgtype.UUID, sql string, args ...any) {
adminScopeExec(t, setup, userID, sql, args...)
}
// seedExec runs a raw statement against the pool under the seed // seedExec runs a raw statement against the pool under the seed
// identity. The bare pool bypasses ScopedDB (nothing reads the ctx for // identity. The bare pool bypasses ScopedDB (nothing reads the ctx for
// the GUC), so governed tables need the identity declared manually // the GUC), so governed tables need the identity declared manually
@@ -356,7 +372,12 @@ func uuidToPGType(u uuid.UUID) pgtype.UUID {
// createDefaultCollectionsForUser creates the 4 default system collections for a user // createDefaultCollectionsForUser creates the 4 default system collections for a user
func createDefaultCollectionsForUser(t *testing.T, db *database.Queries, userID pgtype.UUID) { func createDefaultCollectionsForUser(t *testing.T, db *database.Queries, userID pgtype.UUID) {
ctx := context.Background() // Owner-scoped seeding: the collections RLS policies (2026-10-31)
// need an acting identity, and the harness seeds before any JWT —
// the same bootstrap fix as CreateDefaultCollectionsForUser in
// internal/handlers/auth.go. Owner == actor, so SELECT sees the
// rows, the INSERT passes, and the heal-UPDATE passes.
ctx := rls.WithUser(context.Background(), userID)
// Canonical Title-case names — the same list the registration seed // Canonical Title-case names — the same list the registration seed
// (CreateDefaultCollectionsForUser) and RestoreSystemCollection use. // (CreateDefaultCollectionsForUser) and RestoreSystemCollection use.
// A stale kebab-case copy lived here once and duplicated every // A stale kebab-case copy lived here once and duplicated every
@@ -772,11 +793,15 @@ func setupTestServer(t *testing.T) *TestServerSetup {
// run's creates die on UNIQUE(user_id, name) — the "collections // run's creates die on UNIQUE(user_id, name) — the "collections
// 500-panic" rot. System collections are skipped (permanent rows). // 500-panic" rot. System collections are skipped (permanent rows).
// Same convention as the library pass: names containing "test". // Same convention as the library pass: names containing "test".
adminColls, cerr := queries.GetCollectionsByUser(ctx, pgtype.UUID{Bytes: adminUUID, Valid: true}) // Swept under the OWNER's identity: collections RLS (2026-10-31)
// makes unscoped reads return nothing, which would silently turn
// this cleanup into a no-op.
adminScope := rls.WithUser(ctx, pgtype.UUID{Bytes: adminUUID, Valid: true})
adminColls, cerr := queries.GetCollectionsByUser(adminScope, pgtype.UUID{Bytes: adminUUID, Valid: true})
if cerr == nil { if cerr == nil {
for _, coll := range adminColls { for _, coll := range adminColls {
if !coll.IsSystemCollection.Bool && strings.Contains(strings.ToLower(coll.Name), "test") { if !coll.IsSystemCollection.Bool && strings.Contains(strings.ToLower(coll.Name), "test") {
queries.DeleteCollection(ctx, coll.ID) queries.DeleteCollection(adminScope, coll.ID)
} }
} }
} }
@@ -850,11 +875,13 @@ func setupTestServer(t *testing.T) *TestServerSetup {
// Same pass for test-created COLLECTIONS (the "collections // Same pass for test-created COLLECTIONS (the "collections
// 500-panic" rot: they lingered forever and poisoned later runs). // 500-panic" rot: they lingered forever and poisoned later runs).
adminColls, cerr := queries.GetCollectionsByUser(ctx, pgtype.UUID{Bytes: adminUUID, Valid: true}) // Owner-scoped for the same RLS reason as the setup sweep.
adminScope := rls.WithUser(ctx, pgtype.UUID{Bytes: adminUUID, Valid: true})
adminColls, cerr := queries.GetCollectionsByUser(adminScope, pgtype.UUID{Bytes: adminUUID, Valid: true})
if cerr == nil { if cerr == nil {
for _, coll := range adminColls { for _, coll := range adminColls {
if !coll.IsSystemCollection.Bool && strings.Contains(strings.ToLower(coll.Name), "test") { if !coll.IsSystemCollection.Bool && strings.Contains(strings.ToLower(coll.Name), "test") {
queries.DeleteCollection(ctx, coll.ID) queries.DeleteCollection(adminScope, coll.ID)
} }
} }
} }
+114
View File
@@ -2180,3 +2180,117 @@ CREATE POLICY p_podcast_feeds_delete ON podcast_feeds
-- policy's lookup table, and a user must always see their own -- policy's lookup table, and a user must always see their own
-- subscription rows (subscribe -> visibility is a single INSERT). -- subscription rows (subscribe -> visibility is a single INSERT).
-- ============================================================ -- ============================================================
-- ── collections + collection_items (2026-10-31): ownership at the DB ──
-- layer, same doctrine as media_items/podcast_feeds above. The
-- predicates live in FUNCTIONS (not inline) so a future sharing model
-- edits exactly one place: add the share-table EXISTS branch inside
-- app_can_read_collection / app_can_write_collection. The functions are
-- PURE over their arguments — they never query collections — so the
-- policies cannot recurse, FORCE RLS safe. Reads are authenticated-
-- allow (household-readable: any signed-in identity, SeedIdentity
-- harness included); WRITES are owner-or-admin — that is the
-- enforcement of record for Update/Delete/AddBooks, which have no
-- app-level gate.
-- ===========================================================
CREATE OR REPLACE FUNCTION app_is_admin() RETURNS BOOLEAN
LANGUAGE sql STABLE SECURITY DEFINER SET search_path = pg_catalog, public AS $$
SELECT EXISTS (
SELECT 1 FROM users u
WHERE u.id = NULLIF(current_setting('app.user_id', true), '')::uuid
AND u.role = 'admin'
);
$$;
CREATE OR REPLACE FUNCTION app_can_read_collection(
p_collection_id UUID, p_owner UUID
) RETURNS BOOLEAN
LANGUAGE sql STABLE SECURITY DEFINER SET search_path = pg_catalog, public AS $$
SELECT NULLIF(current_setting('app.user_id', true), '') IS NOT NULL
AND (
p_owner = current_setting('app.user_id', true)::uuid
OR app_is_admin()
-- FUTURE SHARING slot: OR EXISTS (
-- SELECT 1 FROM collection_shares cs
-- WHERE cs.collection_id = p_collection_id
-- AND cs.shared_with_user_id = current_setting('app.user_id', true)::uuid)
);
$$;
CREATE OR REPLACE FUNCTION app_can_write_collection(
p_collection_id UUID, p_owner UUID, p_system BOOLEAN
) RETURNS BOOLEAN
LANGUAGE sql STABLE SECURITY DEFINER SET search_path = pg_catalog, public AS $$
SELECT NULLIF(current_setting('app.user_id', true), '') IS NOT NULL
AND (
p_owner = current_setting('app.user_id', true)::uuid
OR (COALESCE(p_system, false) AND app_is_admin())
-- FUTURE SHARING slot: OR EXISTS (
-- SELECT 1 FROM collection_shares cs
-- WHERE cs.collection_id = p_collection_id
-- AND cs.shared_with_user_id = current_setting('app.user_id', true)::uuid
-- AND cs.permission IN ('edit'))
);
$$;
ALTER TABLE collections ENABLE ROW LEVEL SECURITY;
ALTER TABLE collections FORCE ROW LEVEL SECURITY;
DROP POLICY IF EXISTS p_collections_select ON collections;
CREATE POLICY p_collections_select ON collections
FOR SELECT USING (app_can_read_collection(id, user_id));
DROP POLICY IF EXISTS p_collections_insert ON collections;
CREATE POLICY p_collections_insert ON collections
FOR INSERT WITH CHECK (
user_id = NULLIF(current_setting('app.user_id', true), '')::uuid
OR app_is_admin()
);
DROP POLICY IF EXISTS p_collections_update ON collections;
CREATE POLICY p_collections_update ON collections
FOR UPDATE
USING (app_can_write_collection(id, user_id, is_system_collection))
WITH CHECK (app_can_write_collection(id, user_id, is_system_collection));
DROP POLICY IF EXISTS p_collections_delete ON collections;
CREATE POLICY p_collections_delete ON collections
FOR DELETE USING (app_can_write_collection(id, user_id, is_system_collection));
ALTER TABLE collection_items ENABLE ROW LEVEL SECURITY;
ALTER TABLE collection_items FORCE ROW LEVEL SECURITY;
-- Items ride their PARENT's accessibility: the collections SELECT
-- policy auto-applies inside these EXISTS subqueries.
DROP POLICY IF EXISTS p_collection_items_select ON collection_items;
CREATE POLICY p_collection_items_select ON collection_items
FOR SELECT USING (EXISTS (
SELECT 1 FROM collections c
WHERE c.id = collection_items.collection_id
));
DROP POLICY IF EXISTS p_collection_items_insert ON collection_items;
CREATE POLICY p_collection_items_insert ON collection_items
FOR INSERT WITH CHECK (EXISTS (
SELECT 1 FROM collections c
WHERE c.id = collection_items.collection_id
AND app_can_write_collection(c.id, c.user_id, c.is_system_collection)
));
DROP POLICY IF EXISTS p_collection_items_update ON collection_items;
CREATE POLICY p_collection_items_update ON collection_items
FOR UPDATE
USING (EXISTS (
SELECT 1 FROM collections c
WHERE c.id = collection_items.collection_id
AND app_can_write_collection(c.id, c.user_id, c.is_system_collection)
))
WITH CHECK (EXISTS (
SELECT 1 FROM collections c
WHERE c.id = collection_items.collection_id
AND app_can_write_collection(c.id, c.user_id, c.is_system_collection)
));
DROP POLICY IF EXISTS p_collection_items_delete ON collection_items;
CREATE POLICY p_collection_items_delete ON collection_items
FOR DELETE USING (EXISTS (
SELECT 1 FROM collections c
WHERE c.id = collection_items.collection_id
AND app_can_write_collection(c.id, c.user_id, c.is_system_collection)
));
+31
View File
@@ -514,3 +514,34 @@ Test-side deletions follow the asymmetry rule: subtests exercising the
deleted endpoints are gone; their surviving SEMANTICS are pinned in deleted endpoints are gone; their surviving SEMANTICS are pinned in
their tree form (TestSmartCollections rewritten on trees, their tree form (TestSmartCollections rewritten on trees,
TestEpisodeCollectionsFilterTree, TestPodcastSmartCollections). TestEpisodeCollectionsFilterTree, TestPodcastSmartCollections).
## 2026-10-31 — collections RLS: cross-user detail/writes tighten to 404/no-op
`collections` and `collection_items` joined the RLS-governed tables
(policies `p_collections_*` / `p_collection_items_*`, predicates
`app_can_read_collection` / `app_can_write_collection` — the same
doctrine as the media_items family lens, 2026-10-29). Contract
changes:
- `GET /api/collections/:id` for another user's collection: was 200
(by-id readability), now 404 (row invisible).
- `PUT /api/collections/:id` cross-user: was ungated success (no
app-level ownership check), now 404 — the RLS update touches no
rows, and the handler maps ErrNoRows to 404.
- `POST /api/collections/:id/books` cross-user: was ungated, now 404
(rejectSmartMutation's GetCollectionData is RLS-invisible).
- `DELETE /api/collections/:id` cross-user: still 204, but now a true
DB-level no-op (zero rows deleted) instead of an app-level free hit.
- `PUT /api/collections/:id/view-settings` cross-user: 404 (the owner
gate stays as defense-in-depth; the DB is enforcement of record).
- `PUT .../match-mode` and `.../items/order` keep their app-level 403
gates; the DB now backs them.
- Reads: owner or admin (policy escape); non-owner non-admin users see
nothing, anonymous included. Future sharing rides the predicate
functions — one EXISTS branch in app_can_read/write_collection, no
policy migration.
Test-side: TestCollectionsRLS pins the cross-user matrix end-to-end;
fixture writes to non-system collections run as the row's OWNER
(scopeExecAs/ownerSeedCtx) because the admin write-escape covers
system collections only.
+6
View File
@@ -6,6 +6,7 @@ package handlers
import ( import (
"bookhoard/internal/database" "bookhoard/internal/database"
"bookhoard/internal/middleware" "bookhoard/internal/middleware"
"bookhoard/internal/rls"
"bookhoard/internal/setupstatus" "bookhoard/internal/setupstatus"
"context" "context"
"errors" "errors"
@@ -1145,6 +1146,11 @@ func (h *AuthHandler) generateJWT(userID string) (string, error) {
} }
func (h *AuthHandler) CreateDefaultCollectionsForUser(ctx context.Context, userID pgtype.UUID) error { func (h *AuthHandler) CreateDefaultCollectionsForUser(ctx context.Context, userID pgtype.UUID) error {
// Registration runs BEFORE the JWT exists — the request context
// carries no identity, and the collections RLS insert policy
// (2026-10-31) would reject the five system-collection seeds.
// Declare the new user as the actor: owner == actor, policy passes.
ctx = rls.WithUser(ctx, userID)
defaultCollections := []struct { defaultCollections := []struct {
Name string Name string
Description string Description string
+6
View File
@@ -545,6 +545,12 @@ func (h *CollectionHandler) UpdateCollection(c *echo.Context) error {
treeJSON, treeJSON,
) )
if err != nil { if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
// Collections RLS: another user's collection is INVISIBLE —
// the update touched no rows. Answer 404, not a 500 that
// reads like a bug (2026-10-31 collections RLS).
return c.JSON(http.StatusNotFound, map[string]string{"error": "collection not found"})
}
return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
} }
+1 -1
View File
@@ -238,7 +238,7 @@ func (s *CollectionService) UpdateCollection(ctx context.Context, collectionID u
}) })
if err != nil { if err != nil {
return database.Collections{}, fmt.Errorf("failed to update collection: %v", err) return database.Collections{}, fmt.Errorf("failed to update collection: %w", err)
} }
return collection, nil return collection, nil