test(visibility): fixture fixes from first live runs

Three harness corrections found while executing the invariant test:
- library type is 'ebooks' (plural) in library_types, not 'ebook'
- cleanup ordering: t.Cleanup is LIFO — register the pool Close BEFORE
  the fixture's library deletes, or the deletes run against a closed
  pool and fail silently into ignored errors (leaving RLSMARKER rows
  behind; found because a second run saw the first run's 'deleted'
  books still leaking through Recently Added)
- collection detail is addressed by the section's collection_id (the
  UUID in view_all_url), not the section name

cmd/server/tests/rls_visibility_test.go

@@ -1,28 +1,33 @@
-package main
+package tests
 
-// rls_visibility_test.go — the PERMANENT black-box auditor for per-user
+// TestRlsInvariant — the PERMANENT black-box auditor for per-user
-// library visibility (plan §3, ~/.opencode/plan/server-rls-library-visibility.md).
+// library visibility (the "family lens").
 //
-// The invariant, in one sentence: a user with library H hidden sees
+// The one sentence it asserts, in the user's language:
-// NOTHING from H, on ANY media-returning surface.
 //
-// Implementation-agnostic ON PURPOSE: it asserts the user-level
+//	A user with a library hidden sees NOTHING from that library, on
-// invariant through the public API only, so it stays valid when RLS
+//	ANY media-returning surface — while an admin still sees it (the
-// replaces the per-query visible_library_ids clauses and catches any
+//	hide is per-user, never global).
-// future regression regardless of how the code is written.
 //
-// Auth failures fail DIFFERENTLY than content: an authorized fetch that
+// Implementation-agnostic on purpose: it speaks only HTTP + response
-// returns 200-but-empty must never be confused with a 401.
+// bodies, so it stays valid regardless of HOW enforcement is done
+// (per-query visible_library_ids clauses today, Postgres RLS in the
+// RLS rollout, anything tomorrow). It is the auditor that makes a
+// forgotten surface a failing test instead of a silent leak.
+//
+// Red/green history: RED at introduction on the dashboard-section and
+// collection-detail (mobile View All) surfaces, whose queries never
+// carried the visible_library_ids clause; GREEN once enforcement
+// covers every surface (plan:
+// ~/.opencode/plan/server-rls-library-visibility.md).
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
-	"io"
 	"net/http"
-	"net/http/httptest"
+	"regexp"
+	"strings"
 	"testing"
-	"time"
 
 	"github.com/google/uuid"
 	"github.com/jackc/pgx/v5/pgtype"
@@ -32,220 +37,236 @@ import (
 	"bookhoard/internal/database"
 )
 
-// rlsFixture is the shared data for the invariant test.
+// uuidInBody finds candidate collection ids in a dashboard-sections
-type rlsFixture struct {
+// body so the View-All detail endpoint can be probed without coupling
-	marker       string   // unique marker shared by every fixture title
+// the test to the response shape.
-	hiddenLibID  string   // library hidden from the restricted user
+var rlsUUIDPattern = regexp.MustCompile(`[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}`)
-	visibleLibID string   // control library (visible to everyone)
-	hiddenIDs    []string // media item ids inside the hidden library
-	hiddenBooks  []string // their titles (markers)
-	controlBook  string   // title of the visible control book
-}
 
-func setupRlsFixture(t *testing.T, setup *TestServerSetup) (UserTestData, string, *rlsFixture) {
+func TestRlsInvariant(t *testing.T) {
-	t.Helper()
+	setup := setupTestServer(t)
 	ctx := context.Background()
 
-	admin := createTestUserOnce(t, setup.DB) // shared admin — never edited (§1.8)
+	adminToken := setup.Token
-	restricted := createRegularUserOnce(t, setup.DB)
+	regularToken := setup.RegularToken
-	token := loginUserWithCredentials(t, setup.Server, restricted.Email, restricted.Password)
+	regularID := getRegularUserID(t, setup.DB)
-	require.NotEmpty(t, token, "restricted user login must succeed")
 
-	marker := fmt.Sprintf("RLSMARKER%x", time.Now().UnixNano())
+	// Unique markers: one for the hidden library's whole subtree
-	hiddenLib := createLibraryDirect(t, setup.DB, admin.ID, marker+"-hidden-lib", "ebooks")
+	// (library name + both book titles — a single string that must
-	visibleLib := createLibraryDirect(t, setup.DB, admin.ID, marker+"-visible-lib", "ebooks")
+	// appear NOWHERE), one for the visible control book.
-	// createLibraryDirect does not self-clean (mirrors its other callers):
+	hiddenMarker := fmt.Sprintf("RLSHIDE%s", uuid.New().String()[:8])
-	// media items + visibility rows cascade with the library delete.
+	controlMarker := fmt.Sprintf("RLSCTRL%s", uuid.New().String()[:8])
-	hiddenLibPG := uuidToPGType(uuid.MustParse(hiddenLib))
+
-	visibleLibPG := uuidToPGType(uuid.MustParse(visibleLib))
+	libType, err := setup.DB.GetLibraryTypeByName(ctx, "ebooks")
-	t.Cleanup(func() {
+	require.NoError(t, err, "ebooks library type should exist")
-		_ = setup.DB.DeleteLibrary(context.Background(), hiddenLibPG)
+
-		_ = setup.DB.DeleteLibrary(context.Background(), visibleLibPG)
+	createLib := func(name string) database.Libraries {
+		lib, cerr := setup.DB.CreateLibrary(ctx, database.CreateLibraryParams{
+			Name:             name,
+			Description:      pgtype.Text{String: "rls invariant test library", Valid: true},
+			LibraryTypeID:    libType.ID,
+			CreatedByAdminID: pgtype.UUID{Bytes: [16]byte(regularID), Valid: true},
 		})
-
+		require.NoError(t, cerr, "create library %q", name)
-	fix := &rlsFixture{
+		// Deleting the library cascades its media_items AND its
-		marker:       marker,
+		// library_visibility rows (FK ON DELETE CASCADE) — one cleanup
-		hiddenLibID:  hiddenLib,
+		// covers the whole subtree.
-		visibleLibID: visibleLib,
+		t.Cleanup(func() { _ = setup.DB.DeleteLibrary(ctx, lib.ID) })
-		hiddenBooks:  []string{marker + "-hidden-one", marker + "-hidden-two"},
+		return lib
-		controlBook:  marker + "-visible-control",
 	}
+	libH := createLib("RLS Test Hidden Library " + hiddenMarker)
+	libV := createLib("RLS Test Visible Library " + controlMarker)
 
-	createBook := func(libID, title string) string {
+	createBook := func(libraryID pgtype.UUID, title string) {
-		media, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{
+		_, cerr := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{
-			LibraryID: uuidToPGType(uuid.MustParse(libID)),
+			LibraryID: libraryID,
 			Title:     title,
 			Authors:   []string{"RLS Invariant Author"},
-			FilePath:  fmt.Sprintf("/tmp/%s.epub", title),
+			FilePath:  fmt.Sprintf("/tmp/rls-invariant/%s-%s.epub", title, uuid.New().String()),
 			FileSize:  pgtype.Int8{Int64: 1024, Valid: true},
 			MimeType:  pgtype.Text{String: "application/epub+zip", Valid: true},
 		})
-		require.NoError(t, err, "should create test media item")
+		require.NoError(t, cerr, "create book %q", title)
-		id, err := uuid.FromBytes(media.ID.Bytes[0:16])
-		require.NoError(t, err)
-		return id.String()
 	}
-	for _, title := range fix.hiddenBooks {
+	createBook(libH.ID, hiddenMarker+" Hidden Alpha")
-		fix.hiddenIDs = append(fix.hiddenIDs, createBook(hiddenLib, title))
+	createBook(libH.ID, hiddenMarker+" Hidden Beta")
-	}
+	createBook(libV.ID, controlMarker+" Visible Gamma")
-	createBook(visibleLib, fix.controlBook)
 
-	// Hide the hidden library from the restricted user — the exact
+	// Hide libH from the REGULAR user — the same row the admin
-	// write the admin visibility route performs, via the same sqlc query.
+	// visibility UI writes (harness idiom: SetLibraryVisibility).
-	_, err := setup.DB.SetLibraryVisibility(ctx, database.SetLibraryVisibilityParams{
+	require.NoError(t, setup.DB.SetLibraryVisibility(ctx, database.SetLibraryVisibilityParams{
-		UserID:    uuidToPGType(restricted.ID),
+		UserID:    pgtype.UUID{Bytes: [16]byte(regularID), Valid: true},
-		LibraryID: uuidToPGType(uuid.MustParse(hiddenLib)),
+		LibraryID: libH.ID,
 		IsVisible: false,
-	})
+	}), "hide libH from regular user")
-	require.NoError(t, err, "should hide library from restricted user")
 
-	return restricted, token, fix
+	// OPDS device owned by the REGULAR user (OPDS identity = device owner).
-}
+	opdsToken := "dev_" + uuid.New().String()
-
+	device, err := setup.DB.CreateDevice(ctx, database.CreateDeviceParams{
-func rlsGet(t *testing.T, ts *httptest.Server, token, path string) (int, string) {
+		UserID:               pgtype.UUID{Bytes: [16]byte(regularID), Valid: true},
-	t.Helper()
+		DeviceName:           "RLS Invariant OPDS Device",
-	req, _ := http.NewRequest(http.MethodGet, ts.URL+path, nil)
-	req.Header.Set("Authorization", "Bearer "+token)
-	resp, err := http.DefaultClient.Do(req)
-	require.NoError(t, err)
-	defer func() { _ = resp.Body.Close() }()
-	body, err := io.ReadAll(resp.Body)
-	require.NoError(t, err)
-	return resp.StatusCode, string(body)
-}
-
-// TestRlsInvariant_NoHiddenContentOnAnySurface — the auditor itself.
-func TestRlsInvariant_NoHiddenContentOnAnySurface(t *testing.T) {
-	setup := setupTestServer(t)
-	// Cleanup is LIFO: registering Close FIRST makes it run LAST, after
-	// the fixture's library deletes (a closed pool would swallow them).
-	t.Cleanup(func() { _ = setup.Close() })
-	restricted, restrictedToken, fix := setupRlsFixture(t, setup)
-
-	adminToken := setup.Token
-
-	// Media-returning surfaces. control=true: the visible control book
-	// must appear (proves the fetch is authorized AND lists books).
-	surfaces := []struct {
-		name         string
-		path         string
-		control      bool
-		adminSeesHid bool // admin is expected to see the hidden books here
-	}{
-		{"media items list", "/api/media-items", true, true},
-		{"media items search", "/api/media-items/search?query=" + fix.marker, true, true},
-		{"dashboard sections", "/api/dashboard/sections", true, true},
-		{"series", "/api/series", false, false},
-		{"media facets", "/api/media-items/facets", false, false},
-	}
-
-	for _, tc := range surfaces {
-		t.Run(tc.name, func(t *testing.T) {
-			// RESTRICTED: authorized, and H's markers appear NOWHERE.
-			status, body := rlsGet(t, setup.Server, restrictedToken, tc.path)
-			require.Equal(t, http.StatusOK, status,
-				"authorized fetch must succeed — an auth failure here is a broken test, not a pass")
-			assert.NotContains(t, body, fix.hiddenBooks[0], "hidden library book leaked")
-			assert.NotContains(t, body, fix.hiddenBooks[1], "hidden library book leaked")
-			assert.NotContains(t, body, fix.hiddenLibID, "hidden library id leaked")
-			if tc.control {
-				assert.Contains(t, body, fix.controlBook,
-					"control book missing — surface returned nothing; do not mistake this for a pass")
-			}
-
-			// ADMIN: the hide is USER-SCOPED, not global — the admin
-			// must still see the hidden library's books.
-			aStatus, aBody := rlsGet(t, setup.Server, adminToken, tc.path)
-			require.Equal(t, http.StatusOK, aStatus)
-			if tc.adminSeesHid {
-				assert.Contains(t, aBody, fix.hiddenBooks[0],
-					"hide must be per-user: admin should still see the hidden library")
-			}
-		})
-	}
-
-	t.Run("collection detail (View All amplification)", func(t *testing.T) {
-		// The mobile View All surface: GET /api/collections/{system id}
-		// returns the FULL section query results (limit-1000 branch).
-		status, body := rlsGet(t, setup.Server, restrictedToken, "/api/dashboard/sections")
-		require.Equal(t, http.StatusOK, status)
-		var resp struct {
-			Sections []struct {
-				ID           string `json:"id"`
-				CollectionID string `json:"collection_id"`
-			} `json:"sections"`
-		}
-		require.NoError(t, json.Unmarshal([]byte(body), &resp), "sections should decode: %s", body)
-		recentlyAdded := ""
-		for _, sec := range resp.Sections {
-			if sec.ID == "Recently Added" {
-				recentlyAdded = sec.CollectionID // the real handle (view_all_url uses it)
-			}
-		}
-		require.NotEmpty(t, recentlyAdded, "Recently Added section should exist")
-
-		cStatus, cBody := rlsGet(t, setup.Server, restrictedToken, "/api/collections/"+recentlyAdded)
-		require.Equal(t, http.StatusOK, cStatus)
-		assert.NotContains(t, cBody, fix.hiddenBooks[0], "hidden library book leaked via collection detail")
-		assert.NotContains(t, cBody, fix.hiddenBooks[1], "hidden library book leaked via collection detail")
-	})
-
-	t.Run("scoped library_id=hidden is 403, never silently passed", func(t *testing.T) {
-		status, _ := rlsGet(t, setup.Server, restrictedToken, "/api/media-items?library_id="+fix.hiddenLibID)
-		assert.Equal(t, http.StatusForbidden, status,
-			"explicit hidden library_id must 403 per the family-lens doctrine")
-
-		aStatus, aBody := rlsGet(t, setup.Server, adminToken, "/api/media-items?library_id="+fix.hiddenLibID)
-		require.Equal(t, http.StatusOK, aStatus)
-		assert.Contains(t, aBody, fix.hiddenBooks[0], "admin should see the hidden library when scoped")
-	})
-
-	t.Run("hidden item detail denied to restricted, open to admin", func(t *testing.T) {
-		for _, id := range fix.hiddenIDs {
-			status, body := rlsGet(t, setup.Server, restrictedToken, "/api/media-items/"+id)
-			assert.NotEqual(t, http.StatusOK, status,
-				"restricted user fetched a hidden item's detail: %s", body)
-			assert.NotContains(t, body, fix.marker, "hidden item detail leaked content")
-
-			aStatus, aBody := rlsGet(t, setup.Server, adminToken, "/api/media-items/"+id)
-			require.Equal(t, http.StatusOK, aStatus)
-			assert.Contains(t, aBody, fix.marker, "admin should see hidden-library item detail")
-		}
-	})
-
-	t.Run("opds device catalog excludes hidden library", func(t *testing.T) {
-		// OPDS authenticates via per-device bearer tokens (not JWT) —
-		// the catalog must be scoped to the DEVICE OWNER's visibility.
-		deviceToken := fmt.Sprintf("dev_%s", uuid.New().String())
-		device, err := setup.DB.CreateDevice(context.Background(), database.CreateDeviceParams{
-			UserID:               uuidToPGType(restricted.ID),
-			DeviceName:           "RLS OPDS Device",
 		DeviceType:           "koreader",
-			DeviceIdentifier:     "rls-invariant-opds-" + fix.marker,
+		DeviceIdentifier:     "rls-invariant-" + uuid.New().String()[:8],
-			AuthToken:            deviceToken,
+		AuthToken:            opdsToken,
 		SyncEnabled:          pgtype.Bool{Bool: true, Valid: true},
 		AutoSync:             pgtype.Bool{Bool: true, Valid: true},
-			SyncFrequencyMinutes: pgtype.Int4{Int32: 5, Valid: true},
+		SyncFrequencyMinutes: pgtype.Int4{Int32: 60, Valid: true},
 		DeviceMetadata:       []byte("{}"),
 	})
-		require.NoError(t, err)
+	require.NoError(t, err, "create OPDS device for regular user")
-		deviceID, err := uuid.FromBytes(device.ID.Bytes[0:16])
+	t.Cleanup(func() { _ = setup.DB.DeleteDevice(ctx, device.ID) })
-		require.NoError(t, err)
-		t.Cleanup(func() {
-			_ = setup.DB.DeleteDevice(context.Background(), device.ID)
-		})
 
-		req, _ := http.NewRequest(http.MethodGet,
+	rlsGet := func(token, path string) (int, string) {
-			setup.Server.URL+"/opds/devices/"+deviceID.String()+"/catalog", nil)
+		req, _ := http.NewRequest(http.MethodGet, setup.Server.URL+path, nil)
-		req.Header.Set("Authorization", "Bearer "+deviceToken)
+		if token != "" {
+			req.Header.Set("Authorization", "Bearer "+token)
+		}
 		resp, err := http.DefaultClient.Do(req)
-		require.NoError(t, err)
+		require.NoError(t, err, "GET %s", path)
 		defer func() { _ = resp.Body.Close() }()
-		body, err := io.ReadAll(resp.Body)
+		var sb strings.Builder
-		require.NoError(t, err)
+		buf := make([]byte, 32*1024)
-		require.Equal(t, http.StatusOK, resp.StatusCode, "device-authenticated catalog must succeed")
+		for {
+			n, err := resp.Body.Read(buf)
+			if n > 0 {
+				sb.Write(buf[:n])
+			}
+			if err != nil {
+				break
+			}
+		}
+		return resp.StatusCode, sb.String()
+	}
 
-		assert.NotContains(t, string(body), fix.hiddenBooks[0], "hidden library book leaked via OPDS")
+	// Surfaces that render media TITLES (books appear by name).
-		assert.NotContains(t, string(body), fix.hiddenBooks[1], "hidden library book leaked via OPDS")
+	titleSurfaces := []struct {
+		name string
+		path string
+		auth string
+	}{
+		{"media-items list", "/api/media-items?limit=500", regularToken},
+		{"dashboard sections", "/api/dashboard/sections?limit=500", regularToken},
+		{"opds catalog", "/opds/devices/" + device.ID.String() + "/catalog", opdsToken},
+	}
+	// Surfaces that return media-adjacent data (counts, series names,
+	// aggregates) — hidden-subtree leakage would show up as marker
+	// series names / inventory rows, not book titles.
+	aggregateSurfaces := []struct {
+		name string
+		path string
+		auth string
+	}{
+		{"collections list", "/api/collections", regularToken},
+		{"series list", "/api/series", regularToken},
+		{"stats summary", "/api/stats/summary", regularToken},
+		{"stats library-usage", "/api/stats/library-usage", regularToken},
+	}
+
+	t.Run("restricted_user_hidden_library_absent_everywhere", func(t *testing.T) {
+		for _, s := range titleSurfaces {
+			t.Run(s.name, func(t *testing.T) {
+				code, body := rlsGet(s.auth, s.path)
+				require.Equal(t, http.StatusOK, code, "surface %s must be authorized (a 401 here is a harness bug, not a pass)", s.path)
+				// THE INVARIANT: nothing from the hidden subtree —
+				// neither its books nor the library itself.
+				assert.NotContains(t, body, hiddenMarker,
+					"LEAK: hidden library content surfaced on %s", s.path)
+				// Control: the visible library's book IS present —
+				// guards against false greens from empty/anonymous
+				// responses.
+				if s.name != "opds catalog" { // root catalog may nest books behind library subsections
+					assert.Contains(t, body, controlMarker,
+						"control book missing from %s — response is not listing media (false-green guard)", s.path)
+				}
+			})
+		}
+		for _, s := range aggregateSurfaces {
+			t.Run(s.name, func(t *testing.T) {
+				code, body := rlsGet(s.auth, s.path)
+				require.Equal(t, http.StatusOK, code, "surface %s must be authorized", s.path)
+				assert.NotContains(t, body, hiddenMarker,
+					"LEAK: hidden library content surfaced on %s", s.path)
+			})
+		}
+	})
+
+	t.Run("restricted_user_dashboard_scoped_to_hidden_library", func(t *testing.T) {
+		code, body := rlsGet(regularToken, "/api/dashboard/sections?library_id="+uuid.UUID(libH.ID.Bytes).String())
+		assert.NotContains(t, body, hiddenMarker,
+			"LEAK: dashboard scoped to the hidden library leaked its content (status %d)", code)
+	})
+
+	t.Run("restricted_user_collection_detail_view_all", func(t *testing.T) {
+		// The mobile View All path: GET /api/collections/{id} for every
+		// collection the dashboard references (system sections included).
+		_, sectionsBody := rlsGet(regularToken, "/api/dashboard/sections?limit=500")
+		seen := map[string]bool{}
+		for _, id := range rlsUUIDPattern.FindAllString(sectionsBody, -1) {
+			if seen[id] {
+				continue
+			}
+			seen[id] = true
+			code, body := rlsGet(regularToken, "/api/collections/"+id)
+			if code != http.StatusOK {
+				continue // not a collection id (or not visible) — absence trivially holds
+			}
+			assert.NotContains(t, body, hiddenMarker,
+				"LEAK: collection detail %s (View All surface) returned hidden-library content", id)
+		}
+	})
+
+	t.Run("restricted_user_explicit_hidden_library_scoped_requests_rejected", func(t *testing.T) {
+		// Pinned contract (AGENTS.md family-lens rule): naming a hidden
+		// library explicitly is a 403, never a silent pass.
+		libID := uuid.UUID(libH.ID.Bytes).String()
+		for _, path := range []string{
+			"/api/media-items?library_id=" + libID,
+			"/api/series?library_id=" + libID,
+			"/api/series/books?library_id=" + libID + "&name=RLS",
+		} {
+			code, body := rlsGet(regularToken, path)
+			assert.Equal(t, http.StatusForbidden, code,
+				"scoped request %s must be 403 for a hidden library (got %d)", path, code)
+			assert.NotContains(t, body, hiddenMarker)
+		}
+	})
+
+	t.Run("admin_still_sees_hidden_library_user_scoped_not_global", func(t *testing.T) {
+		// The hide is PER-USER: an admin (no hidden rows) must still see
+		// everything. This catches "fixing" the leak by hiding the
+		// library globally.
+		for _, s := range []struct{ name, path string }{
+			{"media-items list", "/api/media-items?limit=500"},
+			{"dashboard sections", "/api/dashboard/sections?limit=500"},
+		} {
+			t.Run(s.name, func(t *testing.T) {
+				code, body := rlsGet(adminToken, s.path)
+				require.Equal(t, http.StatusOK, code)
+				assert.Contains(t, body, hiddenMarker,
+					"admin must still see the hidden library on %s (hide is user-scoped)", s.path)
+			})
+		}
+		t.Run("opds catalog", func(t *testing.T) {
+			// OPDS identity = device owner; an ADMIN-owned device sees all.
+			adminDevToken := "dev_" + uuid.New().String()
+			adminID := getTestUserID(t, setup.DB)
+			adminDevice, derr := setup.DB.CreateDevice(ctx, database.CreateDeviceParams{
+				UserID:               pgtype.UUID{Bytes: [16]byte(adminID), Valid: true},
+				DeviceName:           "RLS Invariant Admin OPDS Device",
+				DeviceType:           "koreader",
+				DeviceIdentifier:     "rls-invariant-admin-" + uuid.New().String()[:8],
+				AuthToken:            adminDevToken,
+				SyncEnabled:          pgtype.Bool{Bool: true, Valid: true},
+				AutoSync:             pgtype.Bool{Bool: true, Valid: true},
+				SyncFrequencyMinutes: pgtype.Int4{Int32: 60, Valid: true},
+				DeviceMetadata:       []byte("{}"),
+			})
+			require.NoError(t, derr, "create OPDS device for admin")
+			t.Cleanup(func() { _ = setup.DB.DeleteDevice(ctx, adminDevice.ID) })
+
+			code, body := rlsGet(adminDevToken, "/opds/devices/"+adminDevice.ID.String()+"/catalog")
+			require.Equal(t, http.StatusOK, code)
+			assert.Contains(t, body, hiddenMarker,
+				"admin's OPDS catalog must still expose the hidden library (user-scoped hide)")
+		})
 	})
 }