fix(db): fail closed on NULL retrieval owner_filter

src/lib/owner-scope.ts

@@ -2,10 +2,17 @@ import { isDemoMode, isLocalNoAuthMode } from "@/lib/env";
 
 export const PUBLIC_OWNER_FILTER_SENTINEL = "00000000-0000-0000-0000-000000000000";
 
-export function requireOwnerScope(ownerId: string | null | undefined): string | undefined {
+// Demo / local-no-auth / test have no authenticated owner. These used to return
+// `undefined`, which reaches the retrieval RPCs as a NULL owner_filter — and
+// retrieval_owner_matches(NULL, …) previously failed OPEN (matched every tenant's
+// rows). Return the public sentinel instead, so these modes see only the shared
+// public (null-owner) corpus. Combined with the DB fail-closed change (migration
+// 20260708160000_retrieval_owner_matches_fail_closed), no legitimate caller ever
+// passes NULL. See docs/tenancy-defense-in-depth-review.md §6.
+export function requireOwnerScope(ownerId: string | null | undefined): string {
   if (ownerId) return ownerId;
   if (isDemoMode() || isLocalNoAuthMode() || process.env.NODE_ENV === "test") {
-    return undefined;
+    return PUBLIC_OWNER_FILTER_SENTINEL;
   }
   throw new Error(
     "Owner-scoped retrieval was called without an ownerId; refusing to run to avoid returning another tenant's data.",
@@ -16,10 +23,10 @@ export function retrievalOwnerFilter(args: {
   ownerId?: string | null;
   documentIds?: string[];
   allowGlobalSearch?: boolean;
-}): string | null | undefined {
+}): string {
   if (args.ownerId) return requireOwnerScope(args.ownerId);
   if (isDemoMode() || isLocalNoAuthMode() || process.env.NODE_ENV === "test") {
-    return undefined;
+    return PUBLIC_OWNER_FILTER_SENTINEL;
   }
   if (args.allowGlobalSearch || args.documentIds?.length) {
     return PUBLIC_OWNER_FILTER_SENTINEL;

supabase/drift-allowlist.json

@@ -110,7 +110,7 @@
       "category": "functions",
       "key": "public.retrieval_owner_matches(uuid,uuid)",
       "kind": "mismatch",
-      "reason": "ACL-only drift: live retains a default PUBLIC execute grant (=X/postgres) that schema.sql (service-role-only) does not. Same posture as search_document_chunks — this is a security-invoker helper, RLS still applies; revoke on live via an approved hardening migration, then remove. The prior search_path half of this drift (live pg_catalog vs schema.sql pg_temp) was reconciled 2026-07-08: schema.sql + drift-manifest def_hash now match live's body (verified read-only via schema_drift_snapshot). This function is a stable boolean owner-scoping helper — NOT one of the raw-SQL retrieval RPCs (match_document_chunks et al.) whose bodies genuinely diverge; do not conflate.",
+      "reason": "Body+ACL drift until migration 20260708160000 is applied on live: schema.sql now fails CLOSED on NULL owner_filter (tenancy defense-in-depth §6 item 1); live still has the prior fail-open body until that migration runs. Independently, live retains a default PUBLIC execute grant (=X/postgres) that schema.sql (service-role-only) does not — same ACL posture as search_document_chunks; revoke on live via an approved hardening migration, then remove this entry. Security-invoker helper; RLS still applies. Not one of the raw-SQL retrieval RPCs whose bodies genuinely diverge.",
       "ref": "docs/database-drift-detection.md#reconciliation-backlog"
     },
     {

supabase/drift-manifest.json

@@ -2,7 +2,7 @@
   "generated_at": "2026-07-08T06:45:01.039Z",
   "generator": "scripts/generate-drift-manifest.ts",
   "postgres_image": "supabase/postgres:17.6.1.127",
-  "schema_sha256": "f7ca50611471d4ced647bfc7731d72f87856bd03890f03eaca8e0ccb8affc6b0",
+  "schema_sha256": "cb34f64fc531fc042f545475ddaa7cc65edeeae0dfa4df0b8d7d58edb7123adf",
   "replay_seconds": 13,
   "snapshot": {
     "views": [
@@ -5930,7 +5930,7 @@
           "postgres=X/postgres",
           "service_role=X/postgres"
         ],
-        "def_hash": "1d88b539bded5aa40393125f672b8cab",
+        "def_hash": "ca2877374851b432c4ca971e1e15f746",
         "signature": "public.retrieval_owner_matches(uuid,uuid)"
       },
       {

supabase/migrations/20260708160000_retrieval_owner_matches_fail_closed.sql

@@ -0,0 +1,61 @@
+-- Tenancy defense-in-depth (docs/tenancy-defense-in-depth-review.md §6, item 1).
+--
+-- retrieval_owner_matches(owner_filter, row_owner_id) previously returned TRUE for
+-- EVERY row when owner_filter IS NULL (fail-open). Because RLS is service-role-only,
+-- the retrieval RPCs had no database-level tenant floor: a future app-layer regression
+-- that passed a NULL owner_filter would silently return every tenant's rows with no
+-- alarm. Make the NULL case fail CLOSED (match no rows).
+--
+-- This is behaviour-neutral for every real path — no legitimate caller passes NULL:
+--   * production authed      -> the owner's uuid            (exact-owner match)
+--   * anon / public / demo / local-no-auth / test -> the public sentinel
+--     '00000000-…' (src/lib/owner-scope.ts now returns the sentinel, never null)
+--   * production without an owner -> throws before the RPC is called
+-- Only the unreachable fail-open branch changes; the sentinel and exact-owner
+-- branches are untouched, so the golden retrieval eval (which passes the sentinel)
+-- is unaffected.
+
+set search_path = public, extensions, pg_temp;
+
+create or replace function public.retrieval_owner_matches(owner_filter uuid, row_owner_id uuid)
+returns boolean
+language sql
+immutable
+parallel safe
+set search_path = public, pg_catalog
+as $$
+  select case
+    when owner_filter is null then false -- fail CLOSED (was: true) — no DB-level global escape hatch
+    when owner_filter = '00000000-0000-0000-0000-000000000000'::uuid then row_owner_id is null -- public corpus only
+    else row_owner_id = owner_filter -- exact owner
+  end;
+$$;
+
+-- Self-verify the truth table so a bad redefinition fails the migration (on live and
+-- on Supabase Preview) instead of silently shipping a tenancy regression.
+do $verify$
+declare
+  a uuid := '11111111-1111-1111-1111-111111111111';
+  b uuid := '22222222-2222-2222-2222-222222222222';
+  sentinel uuid := '00000000-0000-0000-0000-000000000000';
+begin
+  if public.retrieval_owner_matches(null, a) is not false then
+    raise exception 'retrieval_owner_matches(NULL, uuid) must be FALSE (fail-closed)';
+  end if;
+  if public.retrieval_owner_matches(null, null) is not false then
+    raise exception 'retrieval_owner_matches(NULL, NULL) must be FALSE (fail-closed)';
+  end if;
+  if public.retrieval_owner_matches(sentinel, null) is not true then
+    raise exception 'retrieval_owner_matches(sentinel, NULL) must be TRUE (public row)';
+  end if;
+  if public.retrieval_owner_matches(sentinel, a) is not false then
+    raise exception 'retrieval_owner_matches(sentinel, owned) must be FALSE (public-only)';
+  end if;
+  if public.retrieval_owner_matches(a, a) is not true then
+    raise exception 'retrieval_owner_matches(owner, same owner) must be TRUE';
+  end if;
+  if public.retrieval_owner_matches(a, b) is not false then
+    raise exception 'retrieval_owner_matches(owner, other owner) must be FALSE';
+  end if;
+end
+$verify$;

supabase/schema.sql

@@ -2050,9 +2050,9 @@ parallel safe
 set search_path = public, pg_catalog
 as $$
   select case
-    when owner_filter is null then true
-    when owner_filter = '00000000-0000-0000-0000-000000000000'::uuid then row_owner_id is null
-    else row_owner_id = owner_filter
+    when owner_filter is null then false -- fail CLOSED (was: true) — no DB-level global escape hatch
+    when owner_filter = '00000000-0000-0000-0000-000000000000'::uuid then row_owner_id is null -- public corpus only
+    else row_owner_id = owner_filter -- exact owner
   end;
 $$;
 

tests/owner-scope.test.ts

@@ -12,11 +12,14 @@ describe("requireOwnerScope (fail-closed owner scoping)", () => {
     expect(requireOwnerScope("owner-1")).toBe("owner-1");
   });
 
-  it("stays permissive (undefined) without an owner in demo mode", async () => {
+  it("returns the public sentinel (never null) without an owner in demo mode", async () => {
     vi.doMock("@/lib/env", () => ({ isDemoMode: () => true, isLocalNoAuthMode: () => false }));
-    const { requireOwnerScope } = await import("../src/lib/owner-scope");
-    expect(requireOwnerScope(undefined)).toBeUndefined();
-    expect(requireOwnerScope(null)).toBeUndefined();
+    const { requireOwnerScope, PUBLIC_OWNER_FILTER_SENTINEL } = await import("../src/lib/owner-scope");
+    // Must not return undefined/null: that reaches the retrieval RPCs as a NULL
+    // owner_filter, which now fails closed (migration 20260708160000). The public
+    // sentinel scopes these modes to the shared public corpus instead.
+    expect(requireOwnerScope(undefined)).toBe(PUBLIC_OWNER_FILTER_SENTINEL);
+    expect(requireOwnerScope(null)).toBe(PUBLIC_OWNER_FILTER_SENTINEL);
   });
 
   it("throws when a real (non-demo, non-local) deployment omits the ownerId", async () => {
@@ -35,4 +38,10 @@ describe("retrievalOwnerFilter", () => {
     const { retrievalOwnerFilter, PUBLIC_OWNER_FILTER_SENTINEL } = await import("../src/lib/owner-scope");
     expect(retrievalOwnerFilter({ allowGlobalSearch: true })).toBe(PUBLIC_OWNER_FILTER_SENTINEL);
   });
+
+  it("returns the public sentinel (never null) in demo/test mode without an owner", async () => {
+    vi.doMock("@/lib/env", () => ({ isDemoMode: () => true, isLocalNoAuthMode: () => false }));
+    const { retrievalOwnerFilter, PUBLIC_OWNER_FILTER_SENTINEL } = await import("../src/lib/owner-scope");
+    expect(retrievalOwnerFilter({})).toBe(PUBLIC_OWNER_FILTER_SENTINEL);
+  });
 });

tests/retrieval-owner-filter-guard.test.ts

@@ -5,13 +5,13 @@ import { describe, expect, it } from "vitest";
 
 // Guard for the retrieval owner-scope boundary (48h-review finding #3).
 //
-// The SQL `retrieval_owner_matches(owner_filter, row_owner_id)` returns TRUE for EVERY row when
-// `owner_filter IS NULL` (fails open by design; RLS is service-role-only, so the app filter is the
-// sole tenant boundary). Until a migration makes null fail closed — gated on the Supabase Preview
-// drift fix so the SQL change can be verified — this test is the tripwire: no `.rpc(...)` call in
-// `src/` may pass a *literal* null/undefined `owner_filter`, and every owner_filter value must come
-// from the sanctioned scope helpers (which fail closed in production) or the public sentinel — never
-// a raw/unaudited value that could reach the RPC as null and silently return another tenant's rows.
+// The SQL `retrieval_owner_matches(owner_filter, row_owner_id)` now fails CLOSED when
+// `owner_filter IS NULL` (migration 20260708160000_retrieval_owner_matches_fail_closed), and
+// src/lib/owner-scope.ts no longer emits null — so the database has a real tenant floor. This
+// test remains as defense-in-depth on the app side: no `.rpc(...)` call in `src/` may pass a
+// *literal* null/undefined `owner_filter`, and every owner_filter value must come from the
+// sanctioned scope helpers (which fail closed in production) or the public sentinel — never a
+// raw/unaudited value.
 
 const SRC_DIR = join(process.cwd(), "src");
 

tests/supabase-schema.test.ts

@@ -564,6 +564,8 @@ describe("Supabase schema Data API grants", () => {
     expect(schema).toContain(
       "create or replace function public.retrieval_owner_matches(owner_filter uuid, row_owner_id uuid)",
     );
+    expect(schema).toContain("when owner_filter is null then false");
+    expect(schema).not.toContain("when owner_filter is null then true");
     expect(schema).toContain(
       "when owner_filter = '00000000-0000-0000-0000-000000000000'::uuid then row_owner_id is null",
     );