From 3dcb794032eb4834239e0fca1695d659a817ce1d Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Thu, 29 Oct 2026 07:59:05 -0400 Subject: [PATCH] =?UTF-8?q?test(visibility):=20black-box=20invariant=20?= =?UTF-8?q?=E2=80=94=20hidden=20library=20absent=20from=20every=20surface?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the permanent auditor for per-user library visibility: a restricted user with a hidden library sees none of its items on any media-returning surface (media list, search, dashboard sections, series, facets, OPDS device catalog, collection detail, scoped/detail access). RED now, on exactly the surfaces the audit predicted: the dashboard section queries (GetNotStartedItems / GetRecentlyAddedItems / GetCollectionItemsForDashboard et al.) never carried the visible_library_ids clause, and the collection-detail endpoint (mobile View All) exposes the full section query results. Clause-protected surfaces (media list, search, series) pass, proving the test itself isn't a false positive. Implementation-agnostic on purpose: it asserts the user-level invariant through the public API, so it stays valid when RLS replaces the per-query clauses and catches any future regression regardless of how the code is written. Also pins: hidden library_id=403 (never silently passed), item detail denial, and that the hide is USER-scoped (admin still sees the hidden library). Test-lesson encoded: cleanup ordering is LIFO — the pool Close must be registered BEFORE fixture cleanups, or library deletes run against a closed pool and fail silently into ignored errors. --- cmd/server/tests/rls_visibility_test.go | 251 ++++++++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 cmd/server/tests/rls_visibility_test.go diff --git a/cmd/server/tests/rls_visibility_test.go b/cmd/server/tests/rls_visibility_test.go new file mode 100644 index 00000000..444e85a8 --- /dev/null +++ b/cmd/server/tests/rls_visibility_test.go @@ -0,0 +1,251 @@ +package main + +// rls_visibility_test.go — the PERMANENT black-box auditor for per-user +// library visibility (plan §3, ~/.opencode/plan/server-rls-library-visibility.md). +// +// The invariant, in one sentence: a user with library H hidden sees +// NOTHING from H, on ANY media-returning surface. +// +// Implementation-agnostic ON PURPOSE: it asserts the user-level +// invariant through the public API only, so it stays valid when RLS +// replaces the per-query visible_library_ids clauses and catches any +// future regression regardless of how the code is written. +// +// Auth failures fail DIFFERENTLY than content: an authorized fetch that +// returns 200-but-empty must never be confused with a 401. + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgtype" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "bookhoard/internal/database" +) + +// rlsFixture is the shared data for the invariant test. +type rlsFixture struct { + marker string // unique marker shared by every fixture title + hiddenLibID string // library hidden from the restricted user + 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) { + t.Helper() + ctx := context.Background() + + admin := createTestUserOnce(t, setup.DB) // shared admin — never edited (§1.8) + restricted := createRegularUserOnce(t, setup.DB) + token := loginUserWithCredentials(t, setup.Server, restricted.Email, restricted.Password) + require.NotEmpty(t, token, "restricted user login must succeed") + + marker := fmt.Sprintf("RLSMARKER%x", time.Now().UnixNano()) + hiddenLib := createLibraryDirect(t, setup.DB, admin.ID, marker+"-hidden-lib", "ebooks") + visibleLib := createLibraryDirect(t, setup.DB, admin.ID, marker+"-visible-lib", "ebooks") + // createLibraryDirect does not self-clean (mirrors its other callers): + // media items + visibility rows cascade with the library delete. + hiddenLibPG := uuidToPGType(uuid.MustParse(hiddenLib)) + visibleLibPG := uuidToPGType(uuid.MustParse(visibleLib)) + t.Cleanup(func() { + _ = setup.DB.DeleteLibrary(context.Background(), hiddenLibPG) + _ = setup.DB.DeleteLibrary(context.Background(), visibleLibPG) + }) + + fix := &rlsFixture{ + marker: marker, + hiddenLibID: hiddenLib, + visibleLibID: visibleLib, + hiddenBooks: []string{marker + "-hidden-one", marker + "-hidden-two"}, + controlBook: marker + "-visible-control", + } + + createBook := func(libID, title string) string { + media, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + LibraryID: uuidToPGType(uuid.MustParse(libID)), + Title: title, + Authors: []string{"RLS Invariant Author"}, + FilePath: fmt.Sprintf("/tmp/%s.epub", title), + 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") + id, err := uuid.FromBytes(media.ID.Bytes[0:16]) + require.NoError(t, err) + return id.String() + } + for _, title := range fix.hiddenBooks { + fix.hiddenIDs = append(fix.hiddenIDs, createBook(hiddenLib, title)) + } + createBook(visibleLib, fix.controlBook) + + // Hide the hidden library from the restricted user — the exact + // write the admin visibility route performs, via the same sqlc query. + _, err := setup.DB.SetLibraryVisibility(ctx, database.SetLibraryVisibilityParams{ + UserID: uuidToPGType(restricted.ID), + LibraryID: uuidToPGType(uuid.MustParse(hiddenLib)), + IsVisible: false, + }) + require.NoError(t, err, "should hide library from restricted user") + + return restricted, token, fix +} + +func rlsGet(t *testing.T, ts *httptest.Server, token, path string) (int, string) { + t.Helper() + 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, + AuthToken: deviceToken, + SyncEnabled: pgtype.Bool{Bool: true, Valid: true}, + AutoSync: pgtype.Bool{Bool: true, Valid: true}, + SyncFrequencyMinutes: pgtype.Int4{Int32: 5, Valid: true}, + DeviceMetadata: []byte("{}"), + }) + require.NoError(t, err) + deviceID, err := uuid.FromBytes(device.ID.Bytes[0:16]) + require.NoError(t, err) + t.Cleanup(func() { + _ = setup.DB.DeleteDevice(context.Background(), device.ID) + }) + + req, _ := http.NewRequest(http.MethodGet, + setup.Server.URL+"/opds/devices/"+deviceID.String()+"/catalog", nil) + req.Header.Set("Authorization", "Bearer "+deviceToken) + 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) + require.Equal(t, http.StatusOK, resp.StatusCode, "device-authenticated catalog must succeed") + + assert.NotContains(t, string(body), fix.hiddenBooks[0], "hidden library book leaked via OPDS") + assert.NotContains(t, string(body), fix.hiddenBooks[1], "hidden library book leaked via OPDS") + }) +}