diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 95e697361..db23b23aa 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -8,7 +8,7 @@ - [ ] `npm run verify:ui` when UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed - [ ] `npm run verify:release` before release or handoff confidence claims - [ ] `npm run format:check` -- [ ] **`npm run eval:retrieval:quality` (must stay 23/23) when retrieval, ranking, selection, chunking, or scoring behavior changed** — CI cannot run it (needs live keys), so run it locally and paste the summary. A metadata/governance-weighting change once buried correct docs (recall 1.0→0.76) and only this eval caught it. +- [ ] **`npm run eval:retrieval:quality` (must stay 36/36) when retrieval, ranking, selection, chunking, or scoring behavior changed** — CI cannot run it (needs live keys), so run it locally and paste the summary. A metadata/governance-weighting change once buried correct docs (recall 1.0→0.76) and only this eval caught it. - [ ] `npm run eval:rag -- --limit 15` + `npm run eval:quality -- --rag-only` when answer generation, the synthesis prompt, or answer post-processing changed (grounded-supported must not drop; citation-failure 0) - [ ] `npm run check:production-readiness` when clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changed - [ ] `npm run check:deployment-readiness` when deployment startup, hosting, or rollout behavior changed diff --git a/docs/ingestion-concurrency-fix-workorder.md b/docs/ingestion-concurrency-fix-workorder.md index 065a9c59a..6c8b67c9c 100644 --- a/docs/ingestion-concurrency-fix-workorder.md +++ b/docs/ingestion-concurrency-fix-workorder.md @@ -7,17 +7,25 @@ Sequenced, operator-applied plan for the state-machine violations that could Author date: 2026-07-08. All facts below were read from the **live** project `Clinical KB Database` (`sjrfecxgysukkwxsowpy`) via read-only `execute_sql`; -nothing here was applied to live. Function bodies are quoted from +nothing here was applied to live at author time. **Status refresh 2026-07-09:** July 8 migrations are merged to `main` but pending live apply — see [`docs/operator-apply-july8-batch.md`](operator-apply-july8-batch.md). Function bodies are quoted from `pg_get_functiondef` so migrations are derived from live truth, not `schema.sql` (which is known-drifted — see R24e and `docs/database-drift-detection.md`). -## Already landed (do not redo) +## Already landed on `main` (do not redo) - **R11, R15/R16, R22, R24d-gate** — merged (PR #346). - **R11 janitor-side guard** (`scripts/cleanup-storage.ts` + `src/lib/storage-cleanup-safety.ts`) and **R1 lease heartbeat** (`shouldPersistJobProgress` in `src/lib/ingestion.ts`, worker refresh of `locked_at`) — merged (PR #369). +- **R1/R2/R7/R9/R23 RPC hardening** — migration `20260708130000` merged (PR #380); **pending live apply**. +- **R24e** — phantom `ingestion_job_stages.job_id` FK dropped from `schema.sql` (PR #380 batch); **pending live apply** (no-op on live). +- **R17** — partial unique index + reindex-route 409 handling merged (PR #405); migration `20260708170000` — **pending live apply** (normal `db push` when queue quiet). +- **R5** — metadata deep-merge RPC + worker merged (PR #408); **pending live apply** + worker redeploy. + +## Still open (not merged or needs design) + +- **deep-memory delete-scoping** — design required (see bottom of this doc). ## Global rules for this release @@ -298,13 +306,12 @@ naive scope — it can crash live enrichment.** ## Suggested landing order -1. **R24e** (repo-only, unblocks fresh/preview envs) — via drift backlog #8. -2. **R9**, **R23** (self-contained, behavior-preserving RPCs, DB-only). -3. **R2 fences** (expand/contract: migration, then worker deploy) — the root fix. -4. **R7** (small; pick option b if retry/recovery is the source). -5. **R17** (with the reindex-route 409 change) — via drift backlog #7. -6. **R5** (coordinated worker + commit RPC). -7. **deep-memory scoping** — only after the section-ownership design is agreed. +**Superseded for merged items** — use [`docs/operator-apply-july8-batch.md`](operator-apply-july8-batch.md) +for live apply of R24e → RPC hardening → fail-closed → R5 → R17 (`20260708170000`). + +Remaining repo work: + +1. **deep-memory scoping** — only after the section-ownership design is agreed. Every DB item ends with `npm run check:drift` green and (for retrieval-affecting items) `npm run eval:retrieval:quality` unchanged. diff --git a/docs/operator-apply-july8-batch.md b/docs/operator-apply-july8-batch.md new file mode 100644 index 000000000..6c6afe2da --- /dev/null +++ b/docs/operator-apply-july8-batch.md @@ -0,0 +1,86 @@ +# Operator apply — July 8 ingestion & tenancy batch + +Consolidated runbook for migrations merged to `main` in PRs **#380**, **#405**, +**#408**, and **#409** that are **in the repo but not yet verified on live** as +of 2026-07-09. Companion to +[`docs/supabase-migration-reconciliation.md`](supabase-migration-reconciliation.md) +and [`docs/ingestion-concurrency-fix-workorder.md`](ingestion-concurrency-fix-workorder.md). + +## Pre-flight + +- Live env vars must target **production** (verify with `npm run check:supabase-project`). `npm run check:july8-live-batch` refuses staging or other non-production targets so a green probe cannot mask an unapplied production database. Set `SUPABASE_ENVIRONMENT=production` (or leave unset — production is the default). + +```bash +npx supabase migration list --linked +npm run supabase:recovery-status # or npm run check:supabase-project +npm run reindex:health # jobs_pending = 0 AND jobs_processing = 0 before R17 +``` + +Do **not** redeploy the ingestion worker from current `main` until step 2 below +is live — `worker/main.ts` already passes `p_worker_id` to completion RPCs. + +## Apply order + +All steps below are safe through a single `supabase db push` when the ingestion +queue is quiet. R17 uses its **own migration version** (`20260708170000`, not +`20260708160000`) so history/repair cannot collide with the fail-closed tenancy +migration at `20260708160000`. + +| Step | Migration | What | How | +| ---- | --------------------------------------------------------- | -------------------------------------------- | ---------------------------------------------- | +| 1 | `20260708140000_drop_ingestion_job_stages_job_id_fk.sql` | R24e — drop phantom FK from fresh-env schema | Normal `supabase db push` (no-op on live) | +| 2 | `20260708130000_ingestion_concurrency_rpc_hardening.sql` | R1/R2 lease fences, R7/R9/R23 RPC hardening | Normal push — **apply before worker redeploy** | +| 3 | `20260708150000_ensure_retrieval_owner_matches.sql` | Ensure helper exists before fail-closed | Normal push | +| 4 | `20260708160000_retrieval_owner_matches_fail_closed.sql` | Tenancy fail-closed (#409) | Normal push | +| 5 | `20260708310000_r5_document_metadata_merge.sql` | R5 metadata deep-merge (#408) | Normal push (safe before worker) | +| 6 | `20260708170000_ingestion_jobs_one_open_per_document.sql` | R17 one-open-job index (#405) | Normal push when queue quiet — see below | + +Reindex routes for R17 (409 on duplicate job) are already on `main`. + +### R17 on a busy live queue (optional) + +When `jobs_pending` / `jobs_processing` are non-zero, or you want a lock-free +build, apply the index manually **before** recording migration history: + +```sql +create unique index concurrently if not exists ingestion_jobs_one_open_per_document_uidx + on public.ingestion_jobs (document_id) + where status in ('pending', 'processing'); +``` + +Then mark only the R17 version as applied (never `20260708160000`): + +```bash +supabase migration repair --linked --status applied 20260708170000 +``` + +If the queue is quiet, skip the manual SQL — step 6 in the table applies the +transactional `CREATE UNIQUE INDEX` via `db push`. + +## Post-apply verification + +```bash +SUPABASE_ENVIRONMENT=production npm run check:july8-live-batch # requires live keys — fails if absent or staging +npm run check:drift +npm run check:indexing +npm run eval:retrieval:quality # must stay 36/36 (retrieval-affecting: step 4 only) +``` + +Then redeploy the ingestion **worker** so R5 merge + `p_worker_id` fences are +active end-to-end. + +## Probe semantics (`check:july8-live-batch`) + +| Check | Pass means | +| ---------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `retrieval_owner_matches(null, …)` | Returns **false** (fail-closed live) | +| `jsonb_merge_deep` | RPC exists and merges objects | +| `complete_ingestion_job` + `p_worker_id` | Accepts the lease-fence parameter (returns `ok:false`, not signature error) | +| R17 index | Named in `schema_drift_snapshot` with the partial-unique definition on `ingestion_jobs(document_id)` for **only** `pending`/`processing`, duplicate open-job insert rejected with a violation citing `ingestion_jobs_one_open_per_document_uidx`, probe rows tagged with stage `july8-live-batch-probe` and swept before/after enforcement (cleanup errors fail the check) | + +## Still open (not this batch) + +- Forward-codify live-ahead retrieval RPC bodies (`docs/database-drift-detection.md` backlog #0) +- deep-memory delete scoping design (`docs/ingestion-concurrency-fix-workorder.md`) +- Staging soak (`docs/staging-setup.md`, `docs/capacity-review.md`) +- `registry:seed` per owner (`docs/process-hardening.md`) diff --git a/docs/process-hardening.md b/docs/process-hardening.md index 97ea39824..67c8b5f9d 100644 --- a/docs/process-hardening.md +++ b/docs/process-hardening.md @@ -93,16 +93,16 @@ All approved render-surface modules are extracted. `ClinicalDashboard.tsx` went - **Known follow-up debts (documented, not actioned):** - Live migration history has duplicate-version churn (two each of `api_rate_limits`, `audit_logs`, `rag_queries_retention`, `audit_logs_service_role_policy`, `indexing_reliability_recovery`) from the same raw-apply habit. Do not rewrite history; treat as a caution for future applies. - Auth server is capped at 10 absolute DB connections (Supabase advisor); switch to percentage-based allocation in the dashboard before scaling instance size (not settable via SQL/MCP). - - `storage_cleanup_jobs` live indexes drifted from `supabase/schema.sql`: live carries legacy auto-names (`storage_cleanup_jobs_document_id_idx`, `storage_cleanup_jobs_owner_id_idx`, a non-partial `storage_cleanup_jobs_status_created_idx`) that the hardening defs superseded. Migration `20260703030000_reconcile_storage_cleanup_jobs_indexes` is **prepared but NOT applied** — it drops the legacy names and (re)creates the intended named/partial indexes to match schema.sql. Functional-not-broken (the document_id FK is covered), so apply to live only with explicit approval. `20260703000000`/`010000` are also absent from live `schema_migrations` and will self-heal on the next `supabase db push`. + - **`storage_cleanup_jobs` indexes: RESOLVED 2026-07-08** — `20260703030000` + `20260708000000_reapply_storage_cleanup_jobs_indexes` applied and verified on live (see `docs/archive/operator-decisions-2026-07-06.md`). ## Live database drift reconciliation (2026-07-05) - **RESOLVED:** `indexing_v3_agent_jobs` table + `claim_indexing_v3_agent_jobs` + `update_indexing_v3_agent_job_status` were recorded as applied but absent on live. Migration `20260705230000_reconcile_live_database_drift` idempotently re-applied them; live verified post-push. - **RESOLVED:** `match_document_embedding_fields_text` codified with service_role-only execute. -- **Follow-up (2026-07-06):** the codified `match_document_embedding_fields_text` kept the legacy `(owner_filter is null or d.owner_id = owner_filter)` predicate instead of `retrieval_owner_matches`, so it ignores the public-owner sentinel (anonymous sentinel would match zero rows; a real owner id excludes public docs). Migration `20260706130000_fix_embedding_fields_text_owner_sentinel` recreates it with the shared predicate — **prepared but NOT applied to live**; latent until the `_text` RPC is wired into app code, so apply with the next approved push. +- **RESOLVED (2026-07-08):** `match_document_embedding_fields_text` owner-sentinel fix (`20260706130000`) applied to live; latent until the `_text` RPC is wired into app code. - **RESOLVED:** `rag_visual_eval_*` tables codified with service_role-only RLS. - **RESOLVED:** Live-only `20260705133000_tighten_search_document_chunks_owner_scope` mirrored in `schema.sql`. -- **Edge function follow-up:** deploy `indexing-v3-agent` after merge so JSONB status RPC parsing is live. +- **RESOLVED (2026-07-08):** `indexing-v3-agent` edge function deployed (version 53, JSONB status-RPC parsing live — see `docs/archive/operator-decisions-2026-07-06.md`). - **Operator-only:** publishable key rotation (`docs/archive/operator-decisions-2026-07-04.md`). ## Full-inventory drift detection & DR rehearsal (2026-07-07) @@ -119,6 +119,15 @@ All approved render-surface modules are extracted. `ClinicalDashboard.tsx` went - **Live is under active concurrent multi-session editing** — during this work a new `corpus_topic_term_stats` function appeared and `search_schema_health` re-drifted minutes after apply. The drift allowlist is therefore an explicit point-in-time snapshot needing periodic regeneration. - `apply_migration` records history rows under a current-UTC version with the passed name (documented churn), not the repo file's version number; re-applying the repo files later is a harmless idempotent no-op. +### July 8 ingestion & tenancy batch — merged to main, pending live apply (2026-07-09) + +PRs **#380**, **#405**, **#408**, **#409** landed ingestion RPC hardening (R1/R2/R7/R9/R23), R17 +one-open-job index, R5 metadata deep-merge, and `retrieval_owner_matches` fail-closed. Migrations are +in `supabase/migrations/` but **not yet verified on live** as of 2026-07-09. **Do not redeploy the +ingestion worker from current `main` until `20260708130000` is live** — `worker/main.ts` already +passes `p_worker_id`. Ordered apply steps, R17 manual `CONCURRENTLY` index, and post-apply probes: +[`docs/operator-apply-july8-batch.md`](operator-apply-july8-batch.md) · `npm run check:july8-live-batch`. + ## PR merge gate: tiered CI + required checks (2026-07-02) - CI is now two parallel PR jobs instead of one serial 6-7 minute job: `verify` (runtime alignment, edge typecheck, CI-safe production readiness, lint, typecheck, unit tests with coverage gate, build — ~3 min) and `ui-smoke` (Chromium Playwright smoke against its own dev server — ~4.5 min). Wall-clock PR feedback drops to the slower of the two, and a flaky smoke rerun no longer repeats lint/typecheck/tests/build. @@ -169,7 +178,7 @@ All approved render-surface modules are extracted. `ClinicalDashboard.tsx` went ## Retrieval changes must pass the golden eval before merge (2026-07-03) -- **Any PR that touches retrieval, ranking, selection, chunking, or scoring MUST run `npm run eval:retrieval:quality` (23/23) locally before merge** and paste the summary in the PR. CI cannot run it (it needs live Supabase + OpenAI keys), so it is a manual gate — now a checkbox in the PR template. +- **Any PR that touches retrieval, ranking, selection, chunking, or scoring MUST run `npm run eval:retrieval:quality` (36/36) locally before merge** and paste the summary in the PR. CI cannot run it (it needs live Supabase + OpenAI keys), so it is a manual gate — now a checkbox in the PR template. - **Why (measured):** PR #118 caught a main-side change (uncapped candidate score + blanket source-governance metadata weighting in `retrieval-selection.ts`) that regressed the golden set 23/23 → 16/23 (doc-recall@5 1.0 → 0.76) on the partially-enriched corpus. `verify:cheap` was green throughout — only the golden retrieval eval surfaced it. Unit tests do not exercise live ranking, so they cannot substitute. - **Standing constraint (do not relearn):** source-governance metadata (`document_status`/`clinical_validation_status`/`extraction_quality`) must NOT weight retrieval **selection ordering**, and candidate relevance scores must stay clamped. Live scores saturate at 1.0 and the corpus is only partially enriched (unenriched → unknown/unverified), so metadata weighting buries correct documents. Governance belongs in ranking penalties and the answer/source-governance layer. See [[no-governance-weighting-in-retrieval-selection]] and `docs/rag-hybrid-findings-and-todo.md` (RC8). - **Answer-generation changes** (synthesis prompt, post-processing) additionally run `eval:rag --limit 15` + `eval:quality --rag-only` (grounded-supported must not drop; citation-failure 0). A new opt-in `npm run eval:answer-quality` reports a structural per-intent **targeting** metric (informational) for measuring how precisely answers hit the asked question. diff --git a/docs/redesign/04-deferred.md b/docs/redesign/04-deferred.md index cb28fd880..84ecd01bd 100644 --- a/docs/redesign/04-deferred.md +++ b/docs/redesign/04-deferred.md @@ -17,7 +17,12 @@ There was a lockfile/install mismatch around ESLint that predated and was indepe - **Mitigation used:** redesign code was additionally linted via the eslint 9.39.4 engine against the same flat config — all changed TS/TSX files are lint-clean. - **Recommended fix:** run the `dependency` shortcut, or bump `eslint-plugin-react` to a release compatible with ESLint 10, then reconcile the lockfile. -## 2. ClinicalDashboard decomposition (Tier 2, see decision log D5) +## 2. ClinicalDashboard decomposition (resolved July 6, 2026) + +**DONE** — all approved render-surface modules extracted to +`src/components/clinical-dashboard/`; admin surfaces cut over 2026-07-06. See +`docs/process-hardening.md` (Phase 3) for the full progress log (~8.8k → ~3.5k +lines in the orchestrator). Original deferral note kept below for history. The 4,655-line `src/components/ClinicalDashboard.tsx` was not split into `clinical-dashboard/` modules this pass. It remains fully functional; this is maintainability-only. diff --git a/docs/supabase-migration-reconciliation.md b/docs/supabase-migration-reconciliation.md index 5d3b675c5..a782d024e 100644 --- a/docs/supabase-migration-reconciliation.md +++ b/docs/supabase-migration-reconciliation.md @@ -140,4 +140,9 @@ npm run check:indexing ## Operator follow-ups -Manual key rotation and live migration apply decisions are recorded in [`docs/archive/operator-decisions-2026-07-04.md`](archive/operator-decisions-2026-07-04.md). Do not execute those steps from CI or agent automation without explicit operator approval. +Manual key rotation and live migration apply decisions are recorded in +[`docs/archive/operator-decisions-2026-07-04.md`](archive/operator-decisions-2026-07-04.md) +and [`docs/archive/operator-decisions-2026-07-06.md`](archive/operator-decisions-2026-07-06.md). +The **July 8 ingestion & tenancy batch** (merged to `main`, pending live apply as of +2026-07-09) is in [`docs/operator-apply-july8-batch.md`](operator-apply-july8-batch.md). +Do not execute live applies from CI or agent automation without explicit operator approval. diff --git a/docs/tenancy-defense-in-depth-review.md b/docs/tenancy-defense-in-depth-review.md index a5cd937f2..b2047541e 100644 --- a/docs/tenancy-defense-in-depth-review.md +++ b/docs/tenancy-defense-in-depth-review.md @@ -1,6 +1,6 @@ # Tenancy Defense-in-Depth Review — Clinical KB Database -**Status:** Draft for review · **Date:** 2026-07-06 · **Branch:** `claude/privacy-tenancy-review` +**Status:** Review complete · **Date:** 2026-07-06 (fail-closed RPC landed **2026-07-08**, PR #409) · **Branch:** `claude/privacy-tenancy-review` **Scope:** Every API route family under `src/app/api/**`, the Supabase RPCs they call, signed-URL issuance, the response cache, and the demo/no-auth code paths — audited adversarially for a missed or bypassable `owner_id` filter. @@ -19,20 +19,18 @@ its own SQL body. The single deliberately-service-role architecture (RLS bypasse ownership enforced in application code) is, as implemented today, **correctly and consistently enforced**. -That said, this is a **single-layer** design with one structural weakness worth understanding: +That said, this is a **single-layer** design with one structural weakness that **was closed on +2026-07-08** (PR #409): -> **The database has no independent tenancy floor.** All 33 routes use the service-role client, which -> bypasses RLS. The retrieval RPCs are owner-_aware_ but **fail _open_** — the shared owner-matching -> helper returns _every_ row when the owner filter is `NULL` -> ([retrieval_owner_matches, migration 20260705210000:11-15](supabase/migrations/20260705210000_retrieval_owner_filter_sentinel.sql)). -> The only thing preventing a cross-tenant leak is that the application layer never passes `NULL` in -> production — enforced by `requireOwnerScope` / `retrievalOwnerFilter` / `assertGlobalSearchAllowed` -> throwing when an owner is absent ([owner-scope.ts:10-13, 27-29](src/lib/owner-scope.ts)). +> **The database had no independent tenancy floor for NULL `owner_filter`.** Before #409, the shared +> `retrieval_owner_matches` helper returned _every_ row when `owner_filter IS NULL` (fail-open). PR +> #409 (`20260708160000_retrieval_owner_matches_fail_closed.sql`) makes `NULL` match **no rows**; the +> app routes demo/test/local-no-auth through the public sentinel (`00000000-…`) instead of `NULL` +> ([owner-scope.ts](src/lib/owner-scope.ts)). Production paths that lack an owner still throw before +> any RPC is called. -So the system is safe today, but a single future PR that adds a route without the owner helper, or that -passes `NULL` into an RPC, would leak cross-tenant clinical data **with no database backstop and no -alarm.** §6 gives a pragmatic, honest recommendation (fail-closed RPCs + CI guard + integration test -before, or instead of, a full RLS refactor). +**Historical note (pre-#409):** the review below describes the fail-open edge that existed at audit +time. Item 1 in §6 is **DONE**; items 2–4 remain the recommended follow-ups. **The one non-clean finding** is a **low-severity information disclosure**, not a tenancy leak: `setup-status` interpolates a raw Postgres RPC error string into its response @@ -76,25 +74,25 @@ promoting reviewed documents to public). An authenticated user seeing null-owner leak. The leak this review hunts is: **authed user A seeing user B's non-null `owner_id` rows**, an **anonymous** caller seeing any non-null rows, or any caller **mutating** another owner's rows. -### 2.3 The SQL-level owner helper (and its fail-open edge) +### 2.3 The SQL-level owner helper (fail-closed since #409) -Every retrieval RPC gates rows through: +Every retrieval RPC gates rows through `retrieval_owner_matches`. **As of PR #409** the helper is +fail-closed on `NULL`: ```sql --- migration 20260705210000_retrieval_owner_filter_sentinel.sql:4-16 +-- migration 20260708160000_retrieval_owner_matches_fail_closed.sql create function public.retrieval_owner_matches(owner_filter uuid, row_owner_id uuid) returns boolean as $$ select case - when owner_filter is null then true -- ⚠ FAIL-OPEN: all owners + when owner_filter is null then false -- fail-closed (was fail-open pre-#409) when owner_filter = '00000000-0000-0000-0000-000000000000' then row_owner_id is null -- public only else row_owner_id = owner_filter -- exact owner (excludes null) end; $$; ``` -This is faithful to whatever the app passes: `NULL` → everything, sentinel → public, uuid → that owner. -It is a correct _filter_, not an independent _guard_. The safety of the whole system rests on the app -layer never handing it `NULL` in production — which the throw-in-`retrievalOwnerFilter`/ -`requireOwnerScope` design does enforce today. +Legitimate public/demo paths pass the sentinel, not `NULL`. Verify live with +`npm run check:july8-live-batch` after applying the July 8 batch +([operator runbook](operator-apply-july8-batch.md)). --- @@ -272,11 +270,9 @@ small, largely-cooperative user set with a public shared corpus. ### The pragmatic second layer (recommended, in priority order) -1. **Make the retrieval RPCs fail-_closed_ on a null owner filter (cheap, high value).** Change - `retrieval_owner_matches` (or each RPC) so `owner_filter IS NULL` returns **no rows** instead of all - rows, and route the legitimate demo/test path through an explicit sentinel rather than `NULL`. This - converts the one fail-open edge (§2.3) into a fail-safe with a near-one-line SQL change — the - database stops being able to emit cross-tenant rows even if the app passes `NULL` by accident. +1. **Make the retrieval RPCs fail-_closed_ on a null owner filter — DONE (2026-07-08, PR #409).** + `retrieval_owner_matches` now returns no rows when `owner_filter IS NULL`; the app uses the public + sentinel for legitimate unauthenticated paths. Verify: `npm run check:july8-live-batch`. 2. **Add a CI guard against un-scoped owner tables (cheap, high value).** A lint/test that fails when a new `src/app/api/**` handler queries an owner-scoped table without a recognised scoping construct (`withOwnerReadScope`, `.eq('owner_id'`, `requireOwnerScope`, `documents!inner`+`documents.owner_id`). @@ -293,10 +289,10 @@ small, largely-cooperative user set with a public shared corpus. worker's service-role needs. Sequence it **after** 1–3, which deliver most of the safety at a fraction of the cost and risk. -**Bottom line:** the current single-layer enforcement is correct today (0/33 gaps), but it is -one careless edit away from a silent, un-backstopped cross-tenant clinical-data leak. Items 1–3 above -are low-cost and close that exposure directly; a full RLS second layer is justified before multi-tenant -scale but should not block launch given the fixes above. +**Bottom line:** the current single-layer enforcement is correct today (0/33 gaps). Item 1 (fail-closed +RPC) is live in the repo (#409); **apply to production** per +[`docs/operator-apply-july8-batch.md`](operator-apply-july8-batch.md). Items 2–3 close the remaining +app-layer regression exposure; full RLS (item 4) is justified before multi-tenant scale. --- diff --git a/package.json b/package.json index 987bcba5a..636054bd4 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "check:supabase-project": "tsx scripts/check-supabase-project.ts", "check:indexing": "tsx scripts/check-indexing.ts", "check:m13-migration": "tsx scripts/check-m13-migration.ts", + "check:july8-live-batch": "tsx scripts/check-july8-live-batch.ts", "check:type-scale": "node scripts/check-type-scale.mjs", "recover:ingestion": "tsx scripts/recover-ingestion-queue.ts", "registry:seed": "tsx scripts/seed-registry-records.ts", diff --git a/scripts/check-july8-live-batch.ts b/scripts/check-july8-live-batch.ts new file mode 100644 index 000000000..ef226c71a --- /dev/null +++ b/scripts/check-july8-live-batch.ts @@ -0,0 +1,375 @@ +import { loadEnvConfig } from "@next/env"; +import { randomUUID } from "node:crypto"; +import { pathToFileURL } from "node:url"; + +loadEnvConfig(process.cwd()); + +const R17_INDEX = "ingestion_jobs_one_open_per_document_uidx"; +export const R17_PROBE_STAGE = "july8-live-batch-probe"; +const LEGACY_R17_PROBE_JOB_IDS = [ + "00000000-0000-0000-0000-000000000097", + "00000000-0000-0000-0000-000000000096", +] as const; +const PROBE_JOB_ID = "00000000-0000-0000-0000-000000000099"; +const PROBE_DOC_ID = "00000000-0000-0000-0000-000000000098"; + +type DriftIndex = { + name?: unknown; + def?: unknown; +}; + +type DriftSnapshot = { + indexes?: DriftIndex[]; +}; + +export function normalizeIndexDef(def: string) { + return def.replace(/\s+/g, " ").trim().toLowerCase(); +} + +export function isExpectedR17IndexDef(def: string) { + const normalized = normalizeIndexDef(def); + if (!normalized.includes("create unique index")) return false; + if (!normalized.includes("ingestion_jobs")) return false; + if (!normalized.includes("(document_id)")) return false; + + const whereIdx = normalized.indexOf(" where "); + if (whereIdx === -1) return false; + const whereClause = normalized.slice(whereIdx + " where ".length); + + // Reject broader predicates that block reindex behind a stale failed job. + for (const status of ["failed", "completed", "cancelled", "queued", "indexed"]) { + if (whereClause.includes(status)) return false; + } + + let statuses: string[] = []; + const inMatch = whereClause.match(/status\s+in\s*\(([^)]+)\)/); + const anyMatch = whereClause.match(/status\s*=\s*any\s*\(\s*(?:array\s*)?\[([^\]]+)\]/); + + if (inMatch) { + statuses = inMatch[1].split(",").map((value) => + value + .split("::")[0] + .trim() + .replace(/^['"]|['"]$/g, ""), + ); + } else if (anyMatch) { + statuses = anyMatch[1].split(",").map((value) => + value + .split("::")[0] + .trim() + .replace(/^['"]|['"]$/g, ""), + ); + } else { + return false; + } + + statuses = statuses.sort(); + return statuses.length === 2 && statuses[0] === "pending" && statuses[1] === "processing"; +} + +export function isR17IndexUniqueViolation(error: { code?: string | null; message: string }) { + if (error.code !== "23505") return false; + const message = error.message.toLowerCase(); + return message.includes(R17_INDEX) || message.includes("one_open_per_document"); +} + +type AdminClient = ReturnType["createAdminClient"]>; + +function isSignatureError(message: string) { + const lower = message.toLowerCase(); + return ( + lower.includes("could not find the function") || + lower.includes("does not exist") || + lower.includes("unknown argument") || + lower.includes("not found in the schema cache") + ); +} + +async function checkFailClosedRetrieval(supabase: AdminClient) { + const { data, error } = await supabase.rpc( + "retrieval_owner_matches" as never, + { + owner_filter: null, + row_owner_id: PROBE_DOC_ID, + } as never, + ); + + if (error) { + throw new Error(`retrieval_owner_matches probe failed: ${error.message}`); + } + + if (data === true) { + throw new Error( + "retrieval_owner_matches still fail-OPEN on NULL owner_filter — apply 20260708160000_retrieval_owner_matches_fail_closed.sql", + ); + } + + if (data !== false) { + throw new Error(`retrieval_owner_matches returned unexpected value: ${String(data)}`); + } + + console.log("[July8 Live Batch] PASS: retrieval_owner_matches is fail-closed on NULL."); +} + +async function checkJsonbMergeDeep(supabase: AdminClient) { + const { data, error } = await supabase.rpc( + "jsonb_merge_deep" as never, + { + target_obj: { a: 1, nested: { keep: true } }, + patch_obj: { b: 2, nested: { add: "x" } }, + } as never, + ); + + if (error) { + if (isSignatureError(error.message)) { + throw new Error("jsonb_merge_deep RPC missing — apply 20260708310000_r5_document_metadata_merge.sql"); + } + throw new Error(`jsonb_merge_deep probe failed: ${error.message}`); + } + + const merged = data as { a?: number; b?: number; nested?: { keep?: boolean; add?: string } } | null; + if (merged?.a !== 1 || merged?.b !== 2 || merged?.nested?.keep !== true || merged?.nested?.add !== "x") { + throw new Error("jsonb_merge_deep returned an unexpected merge result."); + } + + console.log("[July8 Live Batch] PASS: jsonb_merge_deep is live."); +} + +async function checkWorkerLeaseFence(supabase: AdminClient) { + const { data, error } = await supabase.rpc("complete_ingestion_job", { + p_job_id: PROBE_JOB_ID, + p_document_id: PROBE_DOC_ID, + p_stage: "indexed", + p_worker_id: "july8-live-batch-probe", + }); + + if (error) { + if (isSignatureError(error.message)) { + throw new Error( + "complete_ingestion_job does not accept p_worker_id — apply 20260708130000_ingestion_concurrency_rpc_hardening.sql before redeploying the worker", + ); + } + throw new Error(`complete_ingestion_job probe failed: ${error.message}`); + } + + const payload = data as { ok?: boolean; reason?: string } | null; + if (payload?.ok !== false) { + throw new Error( + `complete_ingestion_job probe expected ok:false for a missing job, got: ${JSON.stringify(payload)}`, + ); + } + + console.log("[July8 Live Batch] PASS: complete_ingestion_job accepts p_worker_id lease fence."); +} + +async function deleteR17ProbeJobIds(supabase: AdminClient, ids: readonly string[], context: string) { + const uniqueIds = [...new Set(ids.filter(Boolean))]; + if (uniqueIds.length === 0) { + return; + } + + const { error } = await supabase.from("ingestion_jobs").delete().in("id", uniqueIds); + if (error) { + throw new Error(`R17 probe: ${context} cleanup failed: ${error.message}`); + } +} + +async function sweepStrandedR17ProbeRows(supabase: AdminClient) { + const { error } = await supabase + .from("ingestion_jobs") + .delete() + .eq("stage", R17_PROBE_STAGE) + .in("status", ["pending", "processing"]); + if (error) { + throw new Error(`R17 probe: could not sweep stranded probe rows: ${error.message}`); + } +} + +async function assertNoStrandedR17ProbeRows(supabase: AdminClient) { + const { data, error } = await supabase + .from("ingestion_jobs") + .select("id") + .eq("stage", R17_PROBE_STAGE) + .in("status", ["pending", "processing"]) + .limit(1); + if (error) { + throw new Error(`R17 probe: could not verify probe cleanup: ${error.message}`); + } + if (data?.length) { + throw new Error(`R17 probe: stranded probe job ${data[0]?.id} remains after cleanup — manual delete required`); + } +} + +async function prepareR17ProbeCleanup(supabase: AdminClient) { + await deleteR17ProbeJobIds(supabase, LEGACY_R17_PROBE_JOB_IDS, "legacy"); + await sweepStrandedR17ProbeRows(supabase); +} + +async function findR17ProbeDocument(supabase: AdminClient) { + const pageSize = 100; + let page = 0; + + while (true) { + const from = page * pageSize; + const to = from + pageSize - 1; + + const { data: docs, error } = await supabase.from("documents").select("id").order("id").range(from, to); + + if (error) { + throw new Error(`R17 probe: could not list documents: ${error.message}`); + } + if (!docs?.length) { + break; + } + + for (const doc of docs) { + const { data: openJobs, error: openError } = await supabase + .from("ingestion_jobs") + .select("id") + .eq("document_id", doc.id) + .in("status", ["pending", "processing"]) + .limit(1); + if (openError) { + throw new Error(`R17 probe: open-job lookup failed: ${openError.message}`); + } + if (!openJobs?.length) { + return doc.id; + } + } + + page++; + } + + throw new Error("R17 probe: could not find a document without open ingestion jobs"); +} + +async function checkR17IndexEnforcement(supabase: AdminClient) { + const maxAttempts = 5; + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + await prepareR17ProbeCleanup(supabase); + + const documentId = await findR17ProbeDocument(supabase); + const primaryJobId = randomUUID(); + const duplicateJobId = randomUUID(); + const nextRunAt = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(); + const baseRow = { + document_id: documentId, + status: "pending" as const, + stage: R17_PROBE_STAGE, + progress: 0, + next_run_at: nextRunAt, + }; + + const { error: firstError } = await supabase.from("ingestion_jobs").insert({ + id: primaryJobId, + ...baseRow, + }); + + if (firstError) { + if (isR17IndexUniqueViolation(firstError) && attempt < maxAttempts) { + console.log( + `[July8 Live Batch] Race detected on doc ${documentId} (attempt ${attempt}/${maxAttempts}). Retrying...`, + ); + continue; + } + throw new Error(`R17 probe: could not insert primary open job: ${firstError.message}`); + } + + try { + const { error: duplicateError } = await supabase.from("ingestion_jobs").insert({ + id: duplicateJobId, + ...baseRow, + }); + + if (!duplicateError) { + await deleteR17ProbeJobIds(supabase, [duplicateJobId], "unexpected duplicate"); + throw new Error( + `${R17_INDEX} is not enforcing uniqueness — duplicate open job insert succeeded for document ${documentId}`, + ); + } + + if (!isR17IndexUniqueViolation(duplicateError)) { + throw new Error( + `R17 probe: duplicate insert must violate ${R17_INDEX}, not a generic key conflict: ${duplicateError.message}`, + ); + } + + // Success, break out of loop + break; + } finally { + await deleteR17ProbeJobIds(supabase, [primaryJobId, duplicateJobId], "enforcement"); + } + } + + await assertNoStrandedR17ProbeRows(supabase); +} + +async function checkR17Index(supabase: AdminClient) { + const { data, error } = await supabase.rpc("schema_drift_snapshot" as never); + if (error) { + throw new Error(`schema_drift_snapshot unavailable — cannot verify ${R17_INDEX}: ${error.message}`); + } + + const indexes = (data as DriftSnapshot | null)?.indexes ?? []; + const entry = indexes.find((index) => index?.name === R17_INDEX); + if (!entry) { + throw new Error( + `${R17_INDEX} missing on live — apply 20260708170000_ingestion_jobs_one_open_per_document.sql (or manual CONCURRENTLY + repair 20260708170000)`, + ); + } + + const definition = typeof entry.def === "string" ? entry.def : ""; + if (!isExpectedR17IndexDef(definition)) { + throw new Error( + `${R17_INDEX} definition mismatch — expected partial unique index on ingestion_jobs(document_id) for open statuses; got: ${definition || ""}`, + ); + } + + await checkR17IndexEnforcement(supabase); + console.log("[July8 Live Batch] PASS: R17 partial unique index is live, valid, and enforcing."); +} + +async function main() { + const { env, requireServerEnv } = await import("@/lib/env"); + const { checkSupabaseProjectConfig, expectedSupabaseProject, formatSupabaseProjectCheck } = + await import("@/lib/supabase/project"); + + requireServerEnv(); + + const projectCheck = checkSupabaseProjectConfig( + { + NEXT_PUBLIC_SUPABASE_URL: env.NEXT_PUBLIC_SUPABASE_URL, + SUPABASE_PROJECT_REF: env.SUPABASE_PROJECT_REF, + SUPABASE_PROJECT_NAME: env.SUPABASE_PROJECT_NAME, + SUPABASE_STAGING_PROJECT_REF: env.SUPABASE_STAGING_PROJECT_REF, + SUPABASE_STAGING_PROJECT_NAME: env.SUPABASE_STAGING_PROJECT_NAME, + }, + { requireMetadata: true }, + ); + + if (projectCheck.status === "missing" || projectCheck.status === "mismatch") { + throw new Error(formatSupabaseProjectCheck(projectCheck)); + } + + if (projectCheck.observed.environment !== "production") { + throw new Error( + `[July8 Live Batch] must target production ${expectedSupabaseProject.name} (${expectedSupabaseProject.ref}), not staging ${projectCheck.expected.ref}.`, + ); + } + + const { createAdminClient } = await import("@/lib/supabase/admin"); + const supabase = createAdminClient(); + await checkFailClosedRetrieval(supabase); + await checkJsonbMergeDeep(supabase); + await checkWorkerLeaseFence(supabase); + await checkR17Index(supabase); + console.log("[July8 Live Batch] PASS: all July 8 batch probes green."); +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main().catch((error) => { + console.error("[July8 Live Batch] FAIL:", error instanceof Error ? error.message : error); + console.error("[July8 Live Batch] Runbook: docs/operator-apply-july8-batch.md"); + process.exit(1); + }); +} diff --git a/scripts/check-retrieval-owner-migration.ts b/scripts/check-retrieval-owner-migration.ts index 632fd5c01..ecf51018a 100644 --- a/scripts/check-retrieval-owner-migration.ts +++ b/scripts/check-retrieval-owner-migration.ts @@ -31,6 +31,36 @@ async function main() { process.exit(1); } + const { data: nullFilterCheck, error: nullFilterError } = await supabase.rpc( + "retrieval_owner_matches" as never, + { + owner_filter: null, + row_owner_id: "00000000-0000-0000-0000-000000000001", + } as never, + ); + + if (nullFilterError) { + console.error( + "[Retrieval Owner Migration] FAIL: retrieval_owner_matches null-filter probe failed:", + nullFilterError.message, + ); + process.exit(1); + } + + if (nullFilterCheck === true) { + console.error("[Retrieval Owner Migration] FAIL: retrieval_owner_matches is still fail-OPEN on NULL owner_filter."); + console.error( + "Apply 20260708160000_retrieval_owner_matches_fail_closed.sql — see docs/operator-apply-july8-batch.md", + ); + process.exit(1); + } + + if (nullFilterCheck !== false) { + console.error("[Retrieval Owner Migration] FAIL: unexpected null-filter result:", String(nullFilterCheck)); + process.exit(1); + } + + console.log("[Retrieval Owner Migration] PASS: retrieval_owner_matches is fail-closed on NULL."); console.log("[Retrieval Owner Migration] PASS: retrieval_owner_matches sentinel is live."); console.log("[Retrieval Owner Migration] search_schema_health:", JSON.stringify(data)); } diff --git a/supabase/drift-manifest.json b/supabase/drift-manifest.json index d1978e577..2ceafc72f 100644 --- a/supabase/drift-manifest.json +++ b/supabase/drift-manifest.json @@ -2,7 +2,7 @@ "generated_at": "2026-07-08T17:13:03.597Z", "generator": "scripts/generate-drift-manifest.ts", "postgres_image": "supabase/postgres:17.6.1.127", - "schema_sha256": "c004ebf92005475495394533bed3880f5372f16b90aab24f1a790e4d748d052c", + "schema_sha256": "7a6c44a8a2bb236356ce2728accf063e5afcecffc4e16af30448d484f304f854", "replay_seconds": 31, "snapshot": { "views": [ diff --git a/supabase/migrations/20260708160000_ingestion_jobs_one_open_per_document.sql b/supabase/migrations/20260708160000_ingestion_jobs_one_open_per_document.sql deleted file mode 100644 index ebbc5805a..000000000 --- a/supabase/migrations/20260708160000_ingestion_jobs_one_open_per_document.sql +++ /dev/null @@ -1,28 +0,0 @@ --- R17 (docs/ingestion-concurrency-fix-workorder.md): no constraint currently --- prevents multiple open ingestion_jobs rows per document; the reindex --- enqueue race (R13) and duplicate reindex POSTs previously relied on --- application-side advisory checks (a SELECT immediately before INSERT), --- which is a check-then-act race. This index makes the invariant structural. --- --- NOT applied via `supabase db push` — CLI migrations run inside a --- transaction and `CONCURRENTLY` cannot run inside one --- (docs/supabase-migration-reconciliation.md, "Index replacements" rule). --- Operator: apply the statement below manually against live (outside a --- transaction, e.g. `supabase db query --linked` or the SQL editor with --- autocommit), after confirming `npm run reindex:health` reports --- jobs_pending = 0 and jobs_processing = 0 (live had 0/0 as of 2026-07-08, --- so creation will not fail on existing duplicates). Then run --- `supabase migration repair --linked --status applied 20260708160000` so --- history matches effect, per policy. --- --- Paired app change (same release, already shipped in this PR): the --- single-document and bulk reindex routes translate a 23505 unique --- violation on job insert into the same 409 "already queued" response the --- pre-check produces, instead of a raw constraint 500 --- (src/app/api/documents/[id]/reindex/route.ts, --- src/app/api/documents/bulk/reindex/route.ts). - -create unique index concurrently if not exists - ingestion_jobs_one_open_per_document_uidx - on public.ingestion_jobs (document_id) - where status in ('pending', 'processing'); diff --git a/supabase/migrations/20260708170000_ingestion_jobs_one_open_per_document.sql b/supabase/migrations/20260708170000_ingestion_jobs_one_open_per_document.sql new file mode 100644 index 000000000..62c04a6f3 --- /dev/null +++ b/supabase/migrations/20260708170000_ingestion_jobs_one_open_per_document.sql @@ -0,0 +1,23 @@ +-- R17 (docs/ingestion-concurrency-fix-workorder.md): no constraint currently +-- prevents multiple open ingestion_jobs rows per document; the reindex +-- enqueue race (R13) and duplicate reindex POSTs previously relied on +-- application-side advisory checks (a SELECT immediately before INSERT), +-- which is a check-then-act race. This index makes the invariant structural. +-- +-- Uses a transactional CREATE UNIQUE INDEX (not CONCURRENTLY) so `supabase db +-- push` can apply it with the rest of the July 8 batch. This version is +-- intentionally separate from 20260708160000_retrieval_owner_matches_fail_closed +-- so migration history/repair cannot collide on the same timestamp. +-- +-- Before applying on live, confirm `npm run reindex:health` reports +-- jobs_pending = 0 and jobs_processing = 0 (live had 0/0 as of 2026-07-08). +-- On a busy queue, prefer the lock-free form from +-- docs/operator-apply-july8-batch.md (CREATE INDEX CONCURRENTLY outside a +-- transaction), then `supabase migration repair --linked --status applied +-- 20260708170000`. +-- +-- Paired app change (already on main): reindex routes translate 23505 into 409. + +create unique index if not exists ingestion_jobs_one_open_per_document_uidx + on public.ingestion_jobs (document_id) + where status in ('pending', 'processing'); diff --git a/supabase/schema.sql b/supabase/schema.sql index 483caecff..65d5d7452 100644 --- a/supabase/schema.sql +++ b/supabase/schema.sql @@ -779,10 +779,9 @@ create index if not exists ingestion_jobs_document_status_idx on public.ingestion_jobs(document_id, status, created_at); -- R17 (docs/ingestion-concurrency-fix-workorder.md): structural guard against -- more than one open job per document. Migration --- 20260708160000_ingestion_jobs_one_open_per_document.sql creates the live --- equivalent with CONCURRENTLY (manual apply — see that file's header); this --- non-concurrent form is for fresh/scratch replay only, where there is no --- concurrent traffic to block. +-- 20260708170000_ingestion_jobs_one_open_per_document.sql applies the same +-- index transactionally via `db push`; operators on a busy queue may use the +-- CONCURRENTLY variant documented in docs/operator-apply-july8-batch.md instead. create unique index if not exists ingestion_jobs_one_open_per_document_uidx on public.ingestion_jobs(document_id) where status in ('pending', 'processing'); diff --git a/tests/check-july8-live-batch.test.ts b/tests/check-july8-live-batch.test.ts new file mode 100644 index 000000000..fa1799a82 --- /dev/null +++ b/tests/check-july8-live-batch.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vitest"; + +import { + isExpectedR17IndexDef, + isR17IndexUniqueViolation, + normalizeIndexDef, + R17_PROBE_STAGE, +} from "../scripts/check-july8-live-batch"; + +describe("check-july8-live-batch R17 index definition probe", () => { + it("accepts the canonical partial unique index definition", () => { + const definition = ` + CREATE UNIQUE INDEX ingestion_jobs_one_open_per_document_uidx + ON public.ingestion_jobs (document_id) + WHERE status IN ('pending', 'processing') + `; + expect(isExpectedR17IndexDef(definition)).toBe(true); + expect(normalizeIndexDef(definition)).toContain("create unique index"); + }); + + it("accepts the ANY(ARRAY) partial unique index definition format", () => { + const definition = ` + CREATE UNIQUE INDEX ingestion_jobs_one_open_per_document_uidx + ON public.ingestion_jobs (document_id) + WHERE (status = ANY (ARRAY['pending'::text, 'processing'::text])) + `; + expect(isExpectedR17IndexDef(definition)).toBe(true); + }); + + it("rejects a same-named index on the wrong columns", () => { + const definition = + "CREATE UNIQUE INDEX ingestion_jobs_one_open_per_document_uidx ON public.ingestion_jobs (batch_id)"; + expect(isExpectedR17IndexDef(definition)).toBe(false); + }); + + it("rejects broader predicates that include failed jobs", () => { + const definition = + "CREATE UNIQUE INDEX ingestion_jobs_one_open_per_document_uidx ON public.ingestion_jobs (document_id) WHERE status IN ('pending', 'processing', 'failed')"; + expect(isExpectedR17IndexDef(definition)).toBe(false); + }); + + it("rejects predicates that only include pending", () => { + const definition = + "CREATE UNIQUE INDEX ingestion_jobs_one_open_per_document_uidx ON public.ingestion_jobs (document_id) WHERE status IN ('pending')"; + expect(isExpectedR17IndexDef(definition)).toBe(false); + }); +}); + +describe("isR17IndexUniqueViolation", () => { + it("accepts unique violations that cite the R17 index", () => { + expect( + isR17IndexUniqueViolation({ + code: "23505", + message: 'duplicate key value violates unique constraint "ingestion_jobs_one_open_per_document_uidx"', + }), + ).toBe(true); + }); + + it("rejects unique violations from other constraints (e.g. primary key)", () => { + expect( + isR17IndexUniqueViolation({ + code: "23505", + message: 'duplicate key value violates unique constraint "ingestion_jobs_pkey"', + }), + ).toBe(false); + }); +}); + +describe("R17 probe row tagging", () => { + it("uses a dedicated stage marker for mark-and-sweep cleanup", () => { + expect(R17_PROBE_STAGE).toBe("july8-live-batch-probe"); + }); +});