From f4b581872f0974be649f93dc4b05a6c0247d4f87 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Thu, 29 Oct 2026 08:11:20 -0400 Subject: [PATCH] wip(rls): ScopedDB + rls package + schema policies + echo bridge [pin compiling state] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3 of the RLS plan, pinned mid-flight because a parallel writer was actively editing cmd/server/tests/rls_visibility_test.go (A14): the file flip-flopped between two implementations and gained a compile error after the previous commit. This snapshot makes the whole tree compile; functional verification (two-boot gate, invariant green, full suite) lands in the next commit(s). - internal/rls: typed ctx keys β€” user identity (bridged from echo by the router) and an INTERNAL marker whose semantics grant nothing: no GUC is declared, so policy'd tables still raise at the engine; only RLS-ungoverned tables (system_settings, users counts) work for user-less startup paths (settings registry Load, CountAdmins). - internal/database/scoped.go: DBTX decorator β€” per-statement tx, commit-on-Close (:many) / commit-on-Scan (:one), error paths always terminate the tx, refusal before SQL when no identity and not internal. - database/schema/schema.sql: idempotent policy block (DROP IF EXISTS + CREATE β€” schema.sql re-runs every boot): ENABLE + FORCE, read policy exactly mirroring COALESCE(is_visible,true), permissive per-command write stand-ins (FOR ALL would OR into SELECT). - router.go: RLS bridge at BOTH c.Set("user") sites (JWT + device auth) β€” echo-local storage never reaches the stdlib ctx. - main.go + test harness: database.New over ScopedDB at both wiring sites; startup reads use rls.WithInternal. --- cmd/server/main.go | 11 +- cmd/server/tests/rls_visibility_test.go | 9 +- cmd/server/tests/test_helpers_test.go | 10 +- database/schema/schema.sql | 69 +++++++++++ internal/database/scoped.go | 154 ++++++++++++++++++++++++ internal/rls/rls.go | 80 ++++++++++++ internal/router/router.go | 13 ++ 7 files changed, 337 insertions(+), 9 deletions(-) create mode 100644 internal/database/scoped.go create mode 100644 internal/rls/rls.go diff --git a/cmd/server/main.go b/cmd/server/main.go index f01ab502..ea6db199 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -7,6 +7,7 @@ import ( "bookhoard/internal/handlers" "bookhoard/internal/middleware" ratelimit "bookhoard/internal/middleware" + "bookhoard/internal/rls" "bookhoard/internal/router" "bookhoard/internal/services" "bookhoard/internal/sync" @@ -41,7 +42,7 @@ func main() { } defer dbPool.Close() - queries := database.New(dbPool) + queries := database.New(database.NewScopedDB(dbPool)) // Initialize database schema log.Println("πŸ”§ Ensuring database schema is initialized...") @@ -53,8 +54,10 @@ 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). registry := database.NewSettingsRegistry(queries) - if err := registry.Load(ctx); err != nil { + if err := registry.Load(rls.WithInternal(ctx)); err != nil { log.Printf("⚠️ Could not load system settings (using defaults): %v", err) } @@ -65,7 +68,9 @@ func main() { if registry.RegistrationEnabled() { return true } - count, err := queries.CountAdmins(context.Background()) + // 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())) if err != nil { return true // fail open, matching setupstatus } diff --git a/cmd/server/tests/rls_visibility_test.go b/cmd/server/tests/rls_visibility_test.go index 73d23375..3fcdd261 100644 --- a/cmd/server/tests/rls_visibility_test.go +++ b/cmd/server/tests/rls_visibility_test.go @@ -1,6 +1,6 @@ -package tests +package main -// TestRlsInvariant β€” the PERMANENT black-box auditor for per-user +// rls_visibility_test.go β€” the PERMANENT black-box auditor for per-user // library visibility (the "family lens"). // // The one sentence it asserts, in the user's language: @@ -93,11 +93,12 @@ func TestRlsInvariant(t *testing.T) { // Hide libH from the REGULAR user β€” the same row the admin // visibility UI writes (harness idiom: SetLibraryVisibility). - require.NoError(t, setup.DB.SetLibraryVisibility(ctx, database.SetLibraryVisibilityParams{ + _, err := setup.DB.SetLibraryVisibility(ctx, database.SetLibraryVisibilityParams{ UserID: pgtype.UUID{Bytes: [16]byte(regularID), Valid: true}, LibraryID: libH.ID, IsVisible: false, - }), "hide libH from regular user") + }) + require.NoError(t, err, "hide libH from regular user") // OPDS device owned by the REGULAR user (OPDS identity = device owner). opdsToken := "dev_" + uuid.New().String() diff --git a/cmd/server/tests/test_helpers_test.go b/cmd/server/tests/test_helpers_test.go index a155282c..f009ce53 100644 --- a/cmd/server/tests/test_helpers_test.go +++ b/cmd/server/tests/test_helpers_test.go @@ -6,6 +6,7 @@ import ( "bookhoard/internal/handlers" "bookhoard/internal/middleware" ratelimit "bookhoard/internal/middleware" + "bookhoard/internal/rls" "bookhoard/internal/router" "bookhoard/internal/services" wsync "bookhoard/internal/sync" @@ -494,7 +495,12 @@ func setupTestServer(t *testing.T) *TestServerSetup { dbPool, err := pgxpool.NewWithConfig(context.Background(), dbConfig) require.NoError(t, err, "Failed to connect to test database") - queries := database.New(dbPool) + // 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)) // Wire the tunable settings registry exactly like main.go: Load // populates the cache (missing rows fall back to compiled defaults) @@ -503,7 +509,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(context.Background()); err != nil { + if err := settingsRegistry.Load(rls.WithInternal(context.Background())); err != nil { t.Logf("settings registry Load failed (defaults apply): %v", err) } diff --git a/database/schema/schema.sql b/database/schema/schema.sql index 9370a117..f13e2b06 100644 --- a/database/schema/schema.sql +++ b/database/schema/schema.sql @@ -1877,3 +1877,72 @@ CREATE INDEX IF NOT EXISTS idx_hash_conflicts_status ON hash_conflicts(status); -- Widen for databases created before the resolution format settled (no-op otherwise) ALTER TABLE hash_conflicts ALTER COLUMN resolution TYPE VARCHAR(50); + +-- ============================================================ +-- RLS: PER-USER LIBRARY VISIBILITY (ONE SOURCE OF TRUTH) +-- Plan: ~/.opencode/plan/server-rls-library-visibility.md REV 2. +-- The policy joins library_visibility DIRECTLY β€” the same table the +-- visibility UI writes. The app declares the acting user per +-- transaction via the LOCAL GUC app.user_id (ScopedDB, set_config +-- is_local=true). Unset identity FAILS LOUD, never fails open: +-- - GUC never set in the session -> current_setting() raises +-- ("unrecognized configuration parameter") +-- - GUC set-then-transaction-ended -> reverts to '' -> the ::uuid +-- cast raises +-- - NULL GUC (unreachable via ScopedDB) -> IS NOT NULL guard -> +-- zero rows (fail-closed) +-- The runtime role bookhoard_srv OWNS these tables and is subject to +-- the policies via FORCE (z_rls_role.sh). Never connect the app as +-- the bootstrap superuser β€” superusers bypass RLS unconditionally. +-- IDEMPOTENT: schema.sql re-executes on EVERY app boot +-- (database.Initialize -> executeSchema); DROP .. IF EXISTS + CREATE +-- matches the repo's trigger idiom. DO NOT make this block +-- non-idempotent β€” the second boot would brick the app. +-- ============================================================ + +DROP POLICY IF EXISTS p_media_items_visible ON media_items; +DROP POLICY IF EXISTS p_media_items_insert ON media_items; +DROP POLICY IF EXISTS p_media_items_update ON media_items; +DROP POLICY IF EXISTS p_media_items_delete ON media_items; + +ALTER TABLE media_items ENABLE ROW LEVEL SECURITY; +ALTER TABLE media_items FORCE ROW LEVEL SECURITY; + +-- READ policy: EXACTLY the app's COALESCE(lv.is_visible, true) +-- semantics β€” row absent -> visible; row true -> visible; row false +-- -> hidden. No new index needed: library_visibility's +-- UNIQUE(user_id, library_id) constraint already indexes the join. +CREATE POLICY p_media_items_visible ON media_items + FOR SELECT + USING ( + current_setting('app.user_id') IS NOT NULL + AND NOT EXISTS ( + SELECT 1 FROM library_visibility lv + WHERE lv.user_id = current_setting('app.user_id')::uuid + AND lv.library_id = media_items.library_id + AND lv.is_visible = false + ) + ); + +-- WRITE policies: PERMISSIVE STAND-INS for the deferred scoped write +-- lens. MANDATORY at ENABLE time β€” with RLS enabled and no applicable +-- policy, INSERT fails (42501) and UPDATE/DELETE silently affect zero +-- rows, bricking the media scanner. Per-command split is deliberate: +-- a single FOR ALL policy with USING (true) would OR into SELECT and +-- disable the read policy above. Writes still need SELECT visibility +-- of the target rows (the read policy governs that), which internal +-- identities satisfy (they have no hidden rows). +-- ON CONFLICT upserts (scanner: (library_id, file_path)) need the +-- conflicting row SELECT-visible to the acting identity β€” holds for +-- every internal identity by construction. +CREATE POLICY p_media_items_insert ON media_items + FOR INSERT WITH CHECK (true); +CREATE POLICY p_media_items_update ON media_items + FOR UPDATE USING (true) WITH CHECK (true); +CREATE POLICY p_media_items_delete ON media_items + FOR DELETE USING (true); + +-- Other tables: NONE. collection_items reaches media only via INNER +-- JOIN media_items (the join drops filtered rows). collections +-- metadata (book_count) is a separate counts question β€” plan A13. +-- ============================================================ diff --git a/internal/database/scoped.go b/internal/database/scoped.go new file mode 100644 index 00000000..933d1aaa --- /dev/null +++ b/internal/database/scoped.go @@ -0,0 +1,154 @@ +// ScopedDB wraps a pgx pool so every sqlc statement runs inside a +// transaction that declares the acting user via the LOCAL +// `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. +// +// SEMANTICS PRESERVED: every sqlc call was already a single +// auto-committed statement; one transaction per call keeps per-call +// atomicity identical. pgx v5 rows are bound to their transaction, so +// the transaction ends when the ROWS end: commit-on-Close for :many +// results, commit-on-Scan for :one results, plain commit for Exec. +// +// Rollout plan: ~/.opencode/plan/server-rls-library-visibility.md. +package database + +import ( + "context" + "fmt" + "log" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" + "github.com/jackc/pgx/v5/pgxpool" + + "bookhoard/internal/rls" +) + +// Compile-pin: ScopedDB is a drop-in DBTX for database.New. +var _ DBTX = (*ScopedDB)(nil) + +// ScopedDB implements DBTX over a pool, declaring the acting user for +// the RLS policies on every statement. +type ScopedDB struct { + Pool *pgxpool.Pool +} + +// NewScopedDB is the constructor used at the single Queries +// construction site (cmd/server/main.go). +func NewScopedDB(pool *pgxpool.Pool) *ScopedDB { + return &ScopedDB{Pool: pool} +} + +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) +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 + } + 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) +} + +func (s *ScopedDB) Exec(ctx context.Context, sql string, args ...any) (pgconn.CommandTag, error) { + tx, err := s.begin(ctx) + if err != nil { + return pgconn.CommandTag{}, err + } + // Rollback after a successful Commit is a no-op (ErrTxClosed ignored): + // this defer only acts on the error paths. + defer func() { _ = tx.Rollback(ctx) }() + tag, err := tx.Exec(ctx, sql, args...) + if err != nil { + return pgconn.CommandTag{}, err + } + return tag, tx.Commit(ctx) +} + +func (s *ScopedDB) Query(ctx context.Context, sql string, args ...any) (pgx.Rows, error) { + tx, err := s.begin(ctx) + if err != nil { + return nil, err + } + rows, err := tx.Query(ctx, sql, args...) + if err != nil { + _ = tx.Rollback(ctx) + return nil, err + } + return &scopedRows{Rows: rows, tx: tx, ctx: ctx}, nil +} + +// scopedRows commits the transaction when the rows close β€” committing +// earlier would kill the rows mid-read. sqlc's :many path fully +// consumes rows then Close()s. pgx v5 Rows.Close returns nothing, so a +// commit error is logged, not propagated (a commit failure on a +// read-only transaction is log-worthy only). +type scopedRows struct { + pgx.Rows + tx pgx.Tx + ctx context.Context +} + +func (r *scopedRows) Close() { + r.Rows.Close() + if err := r.tx.Commit(r.ctx); err != nil { + log.Printf("rls: tx commit after rows: %v", err) + } +} + +// scopedRow terminates the transaction on EVERY Scan path for sqlc's +// :one results (pgx.Row is just { Scan(dest ...) error }). +type scopedRow struct { + row pgx.Row + tx pgx.Tx + ctx context.Context +} + +func (r scopedRow) Scan(dest ...any) error { + err := r.row.Scan(dest...) + if err == nil { + return r.tx.Commit(r.ctx) + } + _ = r.tx.Rollback(r.ctx) // best effort; the tx ends either way + return err +} + +type errRow struct{ err error } + +func (r errRow) Scan(_ ...any) error { return r.err } + +func (s *ScopedDB) QueryRow(ctx context.Context, sql string, args ...any) pgx.Row { + tx, err := s.begin(ctx) + if err != nil { + return errRow{err} + } + return scopedRow{row: tx.QueryRow(ctx, sql, args...), tx: tx, ctx: ctx} +} diff --git a/internal/rls/rls.go b/internal/rls/rls.go new file mode 100644 index 00000000..448cf4d3 --- /dev/null +++ b/internal/rls/rls.go @@ -0,0 +1,80 @@ +// Package rls carries the authenticated end user's identity through +// the stdlib context so the database layer can declare it to Postgres +// per transaction (the `app.user_id` GUC the RLS policies read β€” see +// database/schema/schema.sql and +// ~/.opencode/plan/server-rls-library-visibility.md). +// +// WHY A SEPARATE PACKAGE: Echo's c.Set/c.Get is echo-local storage and +// never reaches the context.Context handed to database calls; the auth +// middleware therefore bridges the user id into the request context +// here, and ScopedDB (internal/database) reads it back. A query whose +// context carries no identity is REFUSED (fail-loud) β€” a code path +// that forgets the middleware breaks visibly, it never leaks. +package rls + +import ( + "context" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" +) + +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 +) + +// FromContext returns the authenticated end user's UUID. +// ok=false means the caller forgot the middleware bridge β€” the +// decorator will refuse the query loudly. +func FromContext(ctx context.Context) (pgtype.UUID, bool) { + uid, ok := ctx.Value(CtxKeyUserID).(pgtype.UUID) + if !ok || !uid.Valid { + return pgtype.UUID{}, false + } + return uid, true +} + +// WithUser re-attaches an identity to a (possibly detached) context. +// Background jobs MUST capture the acting user's UUID from the request +// context BEFORE `go func` detaches β€” context.Background() carries +// nothing β€” and rebuild their job context with this. +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 +// connection (cross-request identity bleed). For the manual-tx paths +// the ScopedDB decorator cannot wrap. +func DeclareUser(ctx context.Context, tx pgx.Tx, uid pgtype.UUID) error { + _, err := tx.Exec(ctx, `SELECT set_config('app.user_id', $1, true)`, uid.String()) + return err +} diff --git a/internal/router/router.go b/internal/router/router.go index 482267ab..0f142825 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -6,6 +6,7 @@ import ( "bookhoard/internal/handlers" "bookhoard/internal/middleware" ratelimit "bookhoard/internal/middleware" + "bookhoard/internal/rls" "bookhoard/internal/services" "bookhoard/internal/sync" "bookhoard/templates" @@ -155,6 +156,12 @@ func createOptionalAuthMiddleware(cfg *Config) echo.MiddlewareFunc { Timezone: liveUser.Timezone, Theme: liveUser.Theme, // SSR body class; see the required middleware }) + // RLS bridge: echo's c.Set is echo-local and never reaches + // the stdlib context.Context the database layer receives β€” + // ScopedDB refuses media queries without this (plan Β§5.1). + c.SetRequest(c.Request().WithContext(context.WithValue( + c.Request().Context(), rls.CtxKeyUserID, + pgtype.UUID{Bytes: userUUID, Valid: true}))) return next(c) } } @@ -222,6 +229,12 @@ func createJWTMiddleware(cfg *Config) echo.MiddlewareFunc { // admin/docs theme flash. Theme: liveUser.Theme, }) + // RLS bridge (same as the JWT path above): echo-local c.Set + // never reaches the stdlib context.Context the database + // layer receives β€” ScopedDB refuses media queries without it. + c.SetRequest(c.Request().WithContext(context.WithValue( + c.Request().Context(), rls.CtxKeyUserID, + pgtype.UUID{Bytes: userUUID, Valid: true}))) return nil }, ErrorHandler: func(c *echo.Context, err error) error {