diff --git a/cmd/server/main.go b/cmd/server/main.go index ea6db199..9a316f48 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -7,7 +7,6 @@ import ( "bookhoard/internal/handlers" "bookhoard/internal/middleware" ratelimit "bookhoard/internal/middleware" - "bookhoard/internal/rls" "bookhoard/internal/router" "bookhoard/internal/services" "bookhoard/internal/sync" @@ -42,7 +41,12 @@ func main() { } defer dbPool.Close() - queries := database.New(database.NewScopedDB(dbPool)) + // The search service's raw DBTX path must ALSO carry the RLS + // identity β€” hand it the ScopedDB itself (a DBTX), never the bare + // pool, or search queries bypass the per-user GUC and raise under + // the policies. + scopedDB := database.NewScopedDB(dbPool) + queries := database.New(scopedDB) // Initialize database schema log.Println("πŸ”§ Ensuring database schema is initialized...") @@ -54,10 +58,11 @@ func main() { // Load tunable settings from the DB into the registry. All values fall back // to compiled defaults if a row is missing, so this never blocks startup. - // INTERNAL context: startup has no end-user identity; system_settings - // is not RLS-governed, so the statement runs without the GUC (plan Β§5). + // (Startup has no end-user identity; system_settings is not RLS- + // governed, and ScopedDB declares nothing when no identity exists β€” + // the engine only raises on governed tables.) registry := database.NewSettingsRegistry(queries) - if err := registry.Load(rls.WithInternal(ctx)); err != nil { + if err := registry.Load(ctx); err != nil { log.Printf("⚠️ Could not load system settings (using defaults): %v", err) } @@ -68,9 +73,7 @@ func main() { if registry.RegistrationEnabled() { return true } - // INTERNAL: a "how many admins exist" read is not user-scoped; - // users is not RLS-governed (plan Β§5). - count, err := queries.CountAdmins(rls.WithInternal(context.Background())) + count, err := queries.CountAdmins(context.Background()) if err != nil { return true // fail open, matching setupstatus } @@ -170,7 +173,7 @@ func main() { dashboardHandler := handlers.NewDashboardHandler(queries) seriesHandler := handlers.NewSeriesHandler(queries) filtersHandler := handlers.NewFiltersHandler(queries) - mediaHandler := handlers.NewMediaHandler(queries, dbPool, libraryService, progressService, annotationService) + mediaHandler := handlers.NewMediaHandler(queries, scopedDB, libraryService, progressService, annotationService) matchingHandler := handlers.NewMatchingHandler(queries, connManager) jobsHandler := handlers.NewJobsHandler(queries, worker) diff --git a/cmd/server/tests/collections_preview_test.go b/cmd/server/tests/collections_preview_test.go index 6f346afd..ef3366e8 100644 --- a/cmd/server/tests/collections_preview_test.go +++ b/cmd/server/tests/collections_preview_test.go @@ -379,7 +379,7 @@ func createPreviewTestMediaItems(t *testing.T, db *database.Queries, libraryID s for i := 0; i < count; i++ { genre := genres[i%len(genres)] - media, err := db.CreateMediaItem(ctx, database.CreateMediaItemParams{ + media, err := db.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: libUUID, Valid: true}, Title: "Preview Test Book " + string(rune('A'+i)), Authors: []string{"Test Author " + string(rune('A'+i))}, diff --git a/cmd/server/tests/collections_smart_test.go b/cmd/server/tests/collections_smart_test.go index 77fe5335..21eb69a6 100644 --- a/cmd/server/tests/collections_smart_test.go +++ b/cmd/server/tests/collections_smart_test.go @@ -108,7 +108,7 @@ func smartCreateLibraryAndBooks(t *testing.T, setup *TestServerSetup, userID uui }) for title, genre := range titlesByGenre { - media, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + media, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: libUUID, Valid: true}, Title: title, Authors: []string{"Smart Rule Author"}, diff --git a/cmd/server/tests/comic_metadata_display_test.go b/cmd/server/tests/comic_metadata_display_test.go index 3007ca64..e03e3258 100644 --- a/cmd/server/tests/comic_metadata_display_test.go +++ b/cmd/server/tests/comic_metadata_display_test.go @@ -113,7 +113,7 @@ func TestComicMetadataDisplay_ComicDetailPage(t *testing.T) { params.AlternateInfo = []byte(v) } - mediaItem, err := setup.DB.CreateMediaItem(ctx, params) + mediaItem, err := setup.DB.CreateMediaItem(seedCtx(), params) require.NoError(t, err, "Should create media item") mediaUUID, err := uuid.FromBytes(mediaItem.ID.Bytes[0:16]) @@ -596,7 +596,7 @@ func TestComicMetadataDisplay_ComicDetailPage(t *testing.T) { Isbn: pgtype.Text{String: "9781234567890", Valid: true}, } - mediaItem, err := setup.DB.CreateMediaItem(ctx, params) + mediaItem, err := setup.DB.CreateMediaItem(seedCtx(), params) require.NoError(t, err, "Should create ebook") mediaUUID, err := uuid.FromBytes(mediaItem.ID.Bytes[0:16]) @@ -740,7 +740,7 @@ func TestComicMetadataDisplay_AllFieldsTogether(t *testing.T) { // Create a media item with ALL metadata fields populated alternateInfoStr := `{"alternate_series":"Ultimate X-Men","alternate_number":1,"alternate_count":12}` var mediaItem database.MediaItems - mediaItem, err = setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + mediaItem, err = setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: parsedLibraryID, Valid: true}, Title: "Complete Metadata Comic", FilePath: "/test/complete.cbz", diff --git a/cmd/server/tests/comic_metadata_test.go b/cmd/server/tests/comic_metadata_test.go index dc6b5c5b..d217d305 100644 --- a/cmd/server/tests/comic_metadata_test.go +++ b/cmd/server/tests/comic_metadata_test.go @@ -25,7 +25,7 @@ func TestComicMetadataExtraction(t *testing.T) { require.NoError(t, err, "Should parse library UUID") // Insert test media item with full comic metadata - _, err = setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err = setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: parsedLibraryID, Valid: true}, Title: "Test Manga", FilePath: "/test/manga.cbz", @@ -64,7 +64,7 @@ func TestComicMetadataExtraction(t *testing.T) { parsedLibraryID, err := uuid.Parse(libraryID) require.NoError(t, err, "Should parse library UUID") - _, err = setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err = setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: parsedLibraryID, Valid: true}, Title: "Test Comic", FilePath: "/test/comic.cbz", @@ -94,7 +94,7 @@ func TestComicMetadataExtraction(t *testing.T) { parsedLibraryID, err := uuid.Parse(libraryID) require.NoError(t, err, "Should parse library UUID") - _, err = setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err = setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: parsedLibraryID, Valid: true}, Title: "Minimal Comic", FilePath: "/test/minimal.cbz", @@ -136,7 +136,7 @@ func TestReadingDirectionAPI(t *testing.T) { } for _, tc := range testCases { - _, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: parsedLibraryID, Valid: true}, Title: tc.title, FilePath: "/test/" + tc.title + ".cbz", @@ -205,7 +205,7 @@ func TestUniversalMetadataFields(t *testing.T) { require.NoError(t, err, "Should parse ebook library UUID") t.Run("Comic with universal fields", func(t *testing.T) { - _, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: parsedComicLibraryID, Valid: true}, Title: "Comic with Universal Metadata", FilePath: "/test/comic.cbz", @@ -231,7 +231,7 @@ func TestUniversalMetadataFields(t *testing.T) { }) t.Run("Ebook with universal fields", func(t *testing.T) { - _, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: parsedEbookLibraryID, Valid: true}, Title: "Ebook with Universal Metadata", FilePath: "/test/book.epub", @@ -270,7 +270,7 @@ func TestComicSpecificFields(t *testing.T) { t.Run("Alternate series info as JSONB", func(t *testing.T) { alternateInfo := `{"alternate_series":"Ultimate X-Men","alternate_number":1,"alternate_count":12}` - _, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: parsedLibraryID, Valid: true}, Title: "X-Men with Alternate Series", FilePath: "/test/xmen.cbz", diff --git a/cmd/server/tests/contract_pins_test.go b/cmd/server/tests/contract_pins_test.go index 17388157..8e5cddc4 100644 --- a/cmd/server/tests/contract_pins_test.go +++ b/cmd/server/tests/contract_pins_test.go @@ -120,7 +120,7 @@ func createContractFixture(t *testing.T, setup *TestServerSetup) contractFixture seriesName := fmt.Sprintf("Test Contract Series %d", time.Now().UnixNano()) itemTitle := fmt.Sprintf("Test Contract Item %d", time.Now().UnixNano()) - media, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + media, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: libUUID, Valid: true}, Title: itemTitle, Authors: []string{"Test Contract Author"}, diff --git a/cmd/server/tests/facets_integration_test.go b/cmd/server/tests/facets_integration_test.go index 735439e2..dbbabca6 100644 --- a/cmd/server/tests/facets_integration_test.go +++ b/cmd/server/tests/facets_integration_test.go @@ -89,7 +89,7 @@ func TestFacetRegistryParamsFilter(t *testing.T) { if mutate != nil { mutate(&p) } - _, err := setup.DB.CreateMediaItem(ctx, p) + _, err := setup.DB.CreateMediaItem(seedCtx(), p) require.NoError(t, err, "create %s", title) } @@ -194,7 +194,7 @@ func TestFacetsEndpoint(t *testing.T) { // Two items share the genre; one carries the language + narrator. for _, title := range []string{"Facet List One", "Facet List Two"} { - _, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: library.ID, Title: title, Authors: []string{"Facet Listing Author"}, @@ -206,7 +206,7 @@ func TestFacetsEndpoint(t *testing.T) { }) require.NoError(t, err) } - _, err = setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err = setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: library.ID, Title: "Facet List Three", Authors: []string{"Facet Listing Author"}, @@ -329,7 +329,7 @@ func facetFixtureBooks(t *testing.T, setup *TestServerSetup, libraryID pgtype.UU {prefix + " Gamma", "history", "Pinecrest"}, } for _, b := range books { - _, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: libraryID, Title: b.title, Authors: []string{prefix + " Author"}, @@ -426,7 +426,7 @@ func TestFacetCountsUnderFilters(t *testing.T) { facetFixtureBooks(t, setup, library.ID, "Counts") // A book in the HIDDEN library: distinctive values that must never // surface for the regular user. - _, err = setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err = setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: hiddenLib.ID, Title: "Counts Hidden Book", Authors: []string{"Counts Secret Author"}, @@ -559,7 +559,7 @@ func TestFieldValuesAllLibraries(t *testing.T) { id pgtype.UUID name string }{{visibleLib.ID, "Visible"}, {hiddenLib.ID, "Hidden"}} { - _, err = setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err = setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: lib.id, Title: fmt.Sprintf("Field Values %s Book", lib.name), Authors: []string{"Verdana Scribe"}, diff --git a/cmd/server/tests/filter_builder_test.go b/cmd/server/tests/filter_builder_test.go index 0979da53..4d34946d 100644 --- a/cmd/server/tests/filter_builder_test.go +++ b/cmd/server/tests/filter_builder_test.go @@ -156,7 +156,7 @@ func builderFixtureLib(t *testing.T, setup *TestServerSetup, userID uuid.UUID, n } books[i].TagsSearch = lowered } - _, err := setup.DB.CreateMediaItem(ctx, books[i]) + _, err := setup.DB.CreateMediaItem(seedCtx(), books[i]) require.NoError(t, err, "create %s", books[i].Title) } return libUUID, library.ID @@ -445,7 +445,7 @@ func TestFilterSuggestionsUnderRules(t *testing.T) { // Add a second distinct author+genre pair (trigram-disjoint). ctx := context.Background() - _, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: libUUID, Valid: true}, Title: "Sug Dune", Authors: []string{"Zephyrine Hale"}, @@ -632,7 +632,7 @@ func TestFilterFamilyLens(t *testing.T) { id pgtype.UUID title string }{{visibleID, "Lens Vis Book"}, {hiddenID, "Lens Hid Book"}} { - _, err = setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err = setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: lib.id, Title: lib.title, Authors: []string{"Lens Secret Author"}, FilePath: fmt.Sprintf("/tmp/builder/%s-%d.epub", lib.title, time.Now().UnixNano()), diff --git a/cmd/server/tests/ownership_and_admin_test.go b/cmd/server/tests/ownership_and_admin_test.go index 6e0cbfbb..2492bb64 100644 --- a/cmd/server/tests/ownership_and_admin_test.go +++ b/cmd/server/tests/ownership_and_admin_test.go @@ -85,7 +85,7 @@ func (s *OwnershipAndAdminTestSuite) createItemViaDB(withFolder bool) (itemID, l } } - item, err := db.CreateMediaItem(ctx, database.CreateMediaItemParams{ + item, err := db.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: library.ID, Title: "Ownership Suite Item", Authors: []string{"Suite Author"}, @@ -269,7 +269,7 @@ func (s *OwnershipAndAdminTestSuite) TestDelete_StorageDownRefused() { }) require.NoError(s.T(), err) - item, err := db.CreateMediaItem(ctx, database.CreateMediaItemParams{ + item, err := db.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: library.ID, Title: "Offline Storage Item", FilePath: "offline.epub", diff --git a/cmd/server/tests/phase13_test.go b/cmd/server/tests/phase13_test.go index a6bee305..dbf07b39 100644 --- a/cmd/server/tests/phase13_test.go +++ b/cmd/server/tests/phase13_test.go @@ -134,7 +134,7 @@ func TestManuallyEditedFilter(t *testing.T) { t.Cleanup(func() { deleteLibraries(t, setup, libID) }) createBook := func(path string) pgtype.UUID { - created, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + created, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: libID, Title: path, Authors: []string{}, diff --git a/cmd/server/tests/queue_test.go b/cmd/server/tests/queue_test.go index d674e9df..0a41f438 100644 --- a/cmd/server/tests/queue_test.go +++ b/cmd/server/tests/queue_test.go @@ -48,7 +48,7 @@ func createTestQueueItem(t *testing.T, db *database.Queries, userID uuid.UUID) ( require.NoError(t, err, "Should create library") // 4. Create media item - mediaItem, err := db.CreateMediaItem(ctx, database.CreateMediaItemParams{ + mediaItem, err := db.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: library.ID, Title: "Test Book for Queue", Authors: []string{"Test Author"}, diff --git a/cmd/server/tests/rls_visibility_test.go b/cmd/server/tests/rls_visibility_test.go index 5be5c790..19f3ecfc 100644 --- a/cmd/server/tests/rls_visibility_test.go +++ b/cmd/server/tests/rls_visibility_test.go @@ -23,9 +23,9 @@ package main import ( "context" + "encoding/json" "fmt" "net/http" - "regexp" "strings" "testing" @@ -39,8 +39,7 @@ import ( // uuidInBody finds candidate collection ids in a dashboard-sections // body so the View-All detail endpoint can be probed without coupling -// the test to the response shape. -var rlsUUIDPattern = regexp.MustCompile(`[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}`) +// the test to unrelated ids in the body. func TestRlsInvariant(t *testing.T) { setup := setupTestServer(t) @@ -77,7 +76,7 @@ func TestRlsInvariant(t *testing.T) { libV := createLib("RLS Test Visible Library " + controlMarker) createBook := func(libraryID pgtype.UUID, title string) { - _, cerr := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, cerr := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: libraryID, Title: title, Authors: []string{"RLS Invariant Author"}, @@ -199,16 +198,27 @@ func TestRlsInvariant(t *testing.T) { 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). + // Structured decode β€” probe exactly the collection ids the + // sections payload declares (its own contract field), not + // whatever UUIDs happen to appear in the body. _, sectionsBody := rlsGet(regularToken, "/api/dashboard/sections?limit=500") + var sections struct { + Sections []struct { + CollectionID string `json:"collection_id"` + } `json:"sections"` + } + require.NoError(t, json.Unmarshal([]byte(sectionsBody), §ions), + "dashboard sections should decode; body: %s", sectionsBody) seen := map[string]bool{} - for _, id := range rlsUUIDPattern.FindAllString(sectionsBody, -1) { - if seen[id] { + for _, sec := range sections.Sections { + id := sec.CollectionID + if id == "" || 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 + continue // not visible to this user β€” absence trivially holds } assert.NotContains(t, body, hiddenMarker, "LEAK: collection detail %s (View All surface) returned hidden-library content", id) diff --git a/cmd/server/tests/scan_state_test.go b/cmd/server/tests/scan_state_test.go index 8ecb6099..21008646 100644 --- a/cmd/server/tests/scan_state_test.go +++ b/cmd/server/tests/scan_state_test.go @@ -78,7 +78,7 @@ func TestScanStateBulkLoad(t *testing.T) { require.NoError(t, os.WriteFile(apiFile, []byte("api made book content"), 0644)) apiStat, err := os.Stat(apiFile) require.NoError(t, err) - created, err := setup.DB.CreateMediaItem(context.Background(), database.CreateMediaItemParams{ + created, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: libID, Title: "API Made Book", Authors: []string{"API Author"}, diff --git a/cmd/server/tests/statistics_test.go b/cmd/server/tests/statistics_test.go index 705c6177..2e08d9cd 100644 --- a/cmd/server/tests/statistics_test.go +++ b/cmd/server/tests/statistics_test.go @@ -253,7 +253,7 @@ func TestReadingStatsSummary(t *testing.T) { media, err = setup.DB.GetMediaItem(ctx, uuidToPGType(uuid.MustParse(reuseMediaID[0]))) require.NoError(t, err, "reuse seeded media item") } else { - media, err = setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + media, err = setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: [16]byte(libUUID), Valid: true}, Title: title, FilePath: fmt.Sprintf("/tmp/stats-%s-%d.epub", title, time.Now().UnixNano()), @@ -448,7 +448,7 @@ func TestReadingStatsWeekLibraryScope(t *testing.T) { libB := uuid.MustParse(createLibraryDirect(t, setup.DB, userID, fmt.Sprintf("Test Scope LibB %d", time.Now().UnixNano()), "ebooks")) seed := func(lib uuid.UUID, title string, daysAgo int, minutes int32) { - media, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + media, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: [16]byte(lib), Valid: true}, Title: title, FilePath: fmt.Sprintf("/tmp/%s-%d.epub", title, time.Now().UnixNano()), @@ -541,7 +541,7 @@ func TestLibraryUsageStats(t *testing.T) { if durationSeconds > 0 { audioMeta = []byte(fmt.Sprintf(`{"duration_seconds":%d,"chapters":[]}`, durationSeconds)) } - media, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + media, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: [16]byte(lib), Valid: true}, Title: title, Authors: []string{author}, @@ -697,7 +697,7 @@ func TestLibraryUsageStats_Visibility(t *testing.T) { fmt.Sprintf("Test Usage Hidden %d", time.Now().UnixNano()), "ebooks")) seed := func(lib uuid.UUID, title string, size int64) { - _, err := setup.DB.CreateMediaItem(ctx, database.CreateMediaItemParams{ + _, err := setup.DB.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: pgtype.UUID{Bytes: [16]byte(lib), Valid: true}, Title: title, FilePath: fmt.Sprintf("/tmp/usage-vis-%s-%d.bin", title, time.Now().UnixNano()), diff --git a/cmd/server/tests/sync_integration_test.go b/cmd/server/tests/sync_integration_test.go index efed65f8..0bab36b6 100644 --- a/cmd/server/tests/sync_integration_test.go +++ b/cmd/server/tests/sync_integration_test.go @@ -92,7 +92,7 @@ func createSyncTestDevice(t *testing.T, db *database.Queries, userID pgtype.UUID func createSyncTestMedia(t *testing.T, db *database.Queries, libraryID pgtype.UUID) pgtype.UUID { ctx := context.Background() - media, err := db.CreateMediaItem(ctx, database.CreateMediaItemParams{ + media, err := db.CreateMediaItem(seedCtx(), database.CreateMediaItemParams{ LibraryID: libraryID, Title: "Sync Test Book", Authors: []string{"Test Author"}, diff --git a/cmd/server/tests/test_helpers_test.go b/cmd/server/tests/test_helpers_test.go index f009ce53..7f451c21 100644 --- a/cmd/server/tests/test_helpers_test.go +++ b/cmd/server/tests/test_helpers_test.go @@ -318,6 +318,17 @@ func createRegularUserOnce(t *testing.T, db *database.Queries) UserTestData { } } +// seedCtx is the context harness-level media seeding runs under. The +// seed identity is a valid UUID with NO library_visibility rows, so +// RLS grants it full visibility β€” needed because sqlc's :one inserts +// use RETURNING, which requires SELECT visibility of the new row. +// Product code never uses this; requests carry real personas. +var seedIdentity = pgtype.UUID{Bytes: [16]byte(uuid.MustParse("00000000-0000-0000-0000-000000000002")), Valid: true} + +func seedCtx() context.Context { + return rls.WithUser(context.Background(), seedIdentity) +} + // uuidToPGType converts uuid.UUID to pgtype.UUID func uuidToPGType(u uuid.UUID) pgtype.UUID { return pgtype.UUID{Bytes: [16]byte(u), Valid: true} @@ -497,10 +508,15 @@ func setupTestServer(t *testing.T) *TestServerSetup { // ScopedDB: the test server must behave EXACTLY like the production // wiring (main.go) β€” every statement through Queries declares the - // acting user for RLS. Helper seeding that must bypass user scoping - // uses direct pool writes (permissive write policies) or - // rls.WithInternal for non-policy'd tables. - queries := database.New(database.NewScopedDB(dbPool)) + // acting user for RLS, including the search service's raw DBTX path. + // TestServerSetup.DB below is a SEPARATE raw-pool handle used ONLY + // for harness seeding/cleanup: write policies are permissive (no GUC + // read) and every seeded table except media_items is RLS-ungoverned, + // so seeding needs no identity; direct media SELECTs through it + // raise loudly, which is correct β€” tests read media through HTTP. + scopedDB := database.NewScopedDB(dbPool) + queries := database.New(scopedDB) + seedDB := database.New(dbPool) // Wire the tunable settings registry exactly like main.go: Load // populates the cache (missing rows fall back to compiled defaults) @@ -509,7 +525,7 @@ func setupTestServer(t *testing.T) *TestServerSetup { // initialized") β€” tests that write settings through the API (the // only sanctioned path) need this wired. settingsRegistry := database.NewSettingsRegistry(queries) - if err := settingsRegistry.Load(rls.WithInternal(context.Background())); err != nil { + if err := settingsRegistry.Load(context.Background()); err != nil { t.Logf("settings registry Load failed (defaults apply): %v", err) } @@ -558,7 +574,7 @@ func setupTestServer(t *testing.T) *TestServerSetup { statisticsService := services.NewStatisticsService(queries) statsHandler := handlers.NewStatsHandler(statisticsService) seriesHandler := handlers.NewSeriesHandler(queries) - mediaHandler := handlers.NewMediaHandler(queries, dbPool, libraryService, progressService, annotationService, worker) + mediaHandler := handlers.NewMediaHandler(queries, scopedDB, libraryService, progressService, annotationService, worker) matchingHandler := handlers.NewMatchingHandler(queries, connManager) // Create conversion service for OPDS @@ -762,7 +778,7 @@ func setupTestServer(t *testing.T) *TestServerSetup { // Create TestServerSetup struct with all resources setup := &TestServerSetup{ Server: ts, - DB: queries, + DB: seedDB, // harness seeding/cleanup handle (see comment above) DBPool: dbPool, Config: cfg, ConnManager: connManager, diff --git a/internal/database/schema.go b/internal/database/schema.go index 5cb25a01..a0729260 100644 --- a/internal/database/schema.go +++ b/internal/database/schema.go @@ -178,6 +178,16 @@ func verifyFunctions(ctx context.Context, db *pgxpool.Pool, expectedFunctions [] return nil } +// schemaMigrationIdentity is the synthetic identity the schema +// migration transaction declares for RLS (plan appendix A11). The +// schema's dedup DO block SELECTs/DELETEs media_items, and with the +// visibility policies live (FORCE applies to the owner) that SELECT +// needs app.user_id set even in this maintenance tx. The identity has +// NO library_visibility rows, so the policy grants it full visibility +// β€” appropriate for the server's own schema pass, and unreachable by +// requests (this tx only ever runs at startup, on the raw pool). +const schemaMigrationIdentity = "00000000-0000-0000-0000-000000000001" + // executeSchema runs the entire schema.sql in a single transaction func executeSchema(ctx context.Context, db *pgxpool.Pool) error { tx, err := db.Begin(ctx) @@ -186,6 +196,12 @@ func executeSchema(ctx context.Context, db *pgxpool.Pool) error { } defer tx.Rollback(ctx) + // Declare the migration identity BEFORE the file executes (A11): + // policy'd tables are queried by schema.sql itself on re-runs. + if _, err := tx.Exec(ctx, `SELECT set_config('app.user_id', $1, true)`, schemaMigrationIdentity); err != nil { + return fmt.Errorf("schema execution failed (declare migration identity): %w", err) + } + _, err = tx.Exec(ctx, SchemaFile) if err != nil { log.Printf("HINT: Run manually to debug: psql -h localhost -U postgres -d bookhoard -f database/schema/schema.sql") diff --git a/internal/database/scoped.go b/internal/database/scoped.go index 933d1aaa..d06496e0 100644 --- a/internal/database/scoped.go +++ b/internal/database/scoped.go @@ -3,14 +3,12 @@ // `app.user_id` GUC β€” the one thing the RLS policies // (database/schema/schema.sql) need to filter media rows per user. // -// Fail-loud, twice: a query whose context carries neither a user -// identity nor the internal marker is refused HERE, and the policies -// themselves raise on an unset/empty GUC. The internal marker -// (rls.WithInternal) declares "not a user" β€” it sets NO GUC, so -// policy'd tables (media_items) still raise at the engine; only -// RLS-ungoverned tables (system_settings, users counts) work, which -// user-less startup paths need. A code path that forgets the auth -// middleware bridge breaks visibly; it never silently sees all rows. +// Fail-loud lives in the ENGINE, not here: a context with no identity +// simply declares nothing, and any RLS-governed statement (media_items) +// then raises on the unset GUC β€” while ungoverned tables (auth lookups, +// settings, sessions) keep working, as they have no policy. That is the +// plan-C property: the database is the one door everything crosses; the +// decorator is only the identity declarer. // // SEMANTICS PRESERVED: every sqlc call was already a single // auto-committed statement; one transaction per call keeps per-call @@ -50,32 +48,27 @@ func NewScopedDB(pool *pgxpool.Pool) *ScopedDB { const zeroUUID = "00000000-0000-0000-0000-000000000000" -// begin opens the per-statement transaction. Identity semantics: -// - user context β†’ declares app.user_id (LOCAL) β€” RLS filters rows -// - internal ctx β†’ NO GUC declared. Grants nothing: policy'd tables -// still raise at the engine (FORCE applies to the owner); only -// RLS-ungoverned tables (system_settings, users, …) are workable β€” -// which is exactly what user-less startup paths need. -// - neither β†’ refused HERE (fail-loud before SQL is even sent) +// begin opens the per-statement transaction. If the context carries a +// user identity, the GUC is declared (LOCAL); if not, NOTHING is +// declared and the ENGINE decides: RLS-governed tables (media_items) +// raise on the unset GUC β€” fail-loud at the database, the layer plan C +// exists to enforce at β€” while RLS-ungoverned tables (users, settings, +// sessions, auth lookups pre-bridge) simply work, as they have no +// policy to evaluate. The decorator therefore never refuses: refusing +// here would re-create a convention gate in code, one more door future +// paths could forget β€” the engine is the one door everything crosses. func (s *ScopedDB) begin(ctx context.Context) (pgx.Tx, error) { - if !rls.IsInternal(ctx) { - uid, ok := rls.FromContext(ctx) - if !ok || !uid.Valid || uid.String() == zeroUUID { - return nil, fmt.Errorf("rls: no authenticated user in context β€” query refused " + - "(route through the auth middleware bridge, or declare an internal " + - "identity via rls.WithUser; see the RLS plan Β§5–6)") - } - tx, err := s.Pool.Begin(ctx) - if err != nil { - return nil, err - } + tx, err := s.Pool.Begin(ctx) + if err != nil { + return nil, err + } + if uid, ok := rls.FromContext(ctx); ok && uid.Valid && uid.String() != zeroUUID { if err := rls.DeclareUser(ctx, tx, uid); err != nil { _ = tx.Rollback(ctx) return nil, fmt.Errorf("rls: declare user: %w", err) } - return tx, nil } - return s.Pool.Begin(ctx) + return tx, nil } func (s *ScopedDB) Exec(ctx context.Context, sql string, args ...any) (pgconn.CommandTag, error) { diff --git a/internal/rls/rls.go b/internal/rls/rls.go index 448cf4d3..d63a7bbd 100644 --- a/internal/rls/rls.go +++ b/internal/rls/rls.go @@ -21,20 +21,9 @@ import ( type ctxKey int -const ( - // CtxKeyUserID is the context key the auth middleware bridge sets and - // ScopedDB reads. Typed key β€” no string-key collisions. - CtxKeyUserID ctxKey = iota - // CtxKeyInternal marks a context as INTERNAL (startup, system - // jobs): the statement runs WITHOUT the RLS identity GUC. This - // grants NOTHING β€” the policies still apply to the table owner - // (FORCE) β€” so any query it runs against a POLICY'D table (today: - // media_items) still RAISES at the engine. It only unblocks - // statements on tables RLS does not govern (system_settings, - // users counts, sessions…), which a user-less startup path must - // be able to read. NEVER wrap a request context with this. - CtxKeyInternal -) +// CtxKeyUserID is the context key the auth middleware bridge sets and +// ScopedDB reads. Typed key β€” no string-key collisions. +const CtxKeyUserID ctxKey = iota // FromContext returns the authenticated end user's UUID. // ok=false means the caller forgot the middleware bridge β€” the @@ -55,20 +44,6 @@ func WithUser(ctx context.Context, uid pgtype.UUID) context.Context { return context.WithValue(ctx, CtxKeyUserID, uid) } -// WithInternal marks a context as internal (startup paths, system -// settings): statements run WITHOUT the RLS GUC. Grants nothing β€” -// policy'd tables still raise at the engine; only RLS-ungoverned -// tables become readable. See CtxKeyInternal. -func WithInternal(ctx context.Context) context.Context { - return context.WithValue(ctx, CtxKeyInternal, true) -} - -// IsInternal reports whether the context opted into internal semantics. -func IsInternal(ctx context.Context) bool { - v, _ := ctx.Value(CtxKeyInternal).(bool) - return v -} - // DeclareUser sets the RLS identity GUC inside an ALREADY-OPEN // transaction. The `true` argument is is_local: transaction-scoped, // evaporating at COMMIT/ROLLBACK β€” NEVER session-scoped on a pooled