From f01ad3312767970b829ce7255e06aad5b8fe9768 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Mon, 13 Jul 2026 19:15:16 +0800 Subject: [PATCH 1/8] fix(db): remediate 2026-07-13 audit database findings - Rewrite match_document_chunks_text around two GIN-indexable candidate branches (chunk tsv + title tsv fan-out, unioned by chunk id) instead of the OR-across-relations predicate that sequential-scanned every chunk (~17s warm on live; audit finding 1). Contract, scores, caps, ordering, and grants unchanged; the _v2 wrapper inherits the speedup. Parity/plan/ timing harness: scripts/sql/lexical-rpc-parity-check.sql (10 query shapes identical; no document_chunks seq scan; 8.3x faster on a 30k synthetic corpus). - Pin search_path on retrieval_owner_matches_v2 (advisor function_search_path_mutable; finding 6). - Revoke anon/authenticated future-object default privileges for role supabase_admin with guarded fallback and in-migration probe verification (finding 7); postgres-role defaults were already covered. - Scrub pre-HMAC plaintext query text in rag_queries, rag_query_misses and rag_retrieval_logs with salted irreversible placeholders; delete legacy raw-keyed rag_response_cache rows (finding 5). - Validate the three content_not_blank CHECK constraints (zero violating rows on live) so they are enforced rather than NOT VALID (finding 12). schema.sql mirrored, drift manifest regenerated from a clean scratch replay, schema/drift tests extended. Co-Authored-By: Claude Fable 5 --- docs/process-hardening.md | 7 + scripts/sql/lexical-rpc-parity-check.sql | 441 ++++++++++++++++++ supabase/drift-manifest.json | 16 +- ...00000_index_friendly_lexical_retrieval.sql | 182 ++++++++ ...retrieval_owner_matches_v2_search_path.sql | 11 + ...voke_supabase_admin_default_privileges.sql | 101 ++++ ...0713103000_scrub_legacy_rag_query_text.sql | 83 ++++ ...validate_content_not_blank_constraints.sql | 13 + supabase/schema.sql | 81 +++- tests/supabase-schema.test.ts | 123 +++++ 10 files changed, 1040 insertions(+), 18 deletions(-) create mode 100644 scripts/sql/lexical-rpc-parity-check.sql create mode 100644 supabase/migrations/20260713100000_index_friendly_lexical_retrieval.sql create mode 100644 supabase/migrations/20260713101000_pin_retrieval_owner_matches_v2_search_path.sql create mode 100644 supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql create mode 100644 supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql create mode 100644 supabase/migrations/20260713104000_validate_content_not_blank_constraints.sql diff --git a/docs/process-hardening.md b/docs/process-hardening.md index 582bb96be..d14e47859 100644 --- a/docs/process-hardening.md +++ b/docs/process-hardening.md @@ -308,3 +308,10 @@ hybrid:10}`, all 10 forced-embedding vector cases passed (`force_embedding_failu - The differentials export's trap-tables appendix (`T_Focused_Diagnostic_Trap_Tables.txt`) has no title line, so the parser surfaced its first metadata row ("Urgency: urgent") as a presentation titled "Urgency: urgent" (slug `urgency-urgent`) with detail routes and search visibility. It is a legitimate comparison workflow ("Distinguishes intrusive/obsessional phenomena from psychotic or violent intent"), and 4 of its 7 option diagnoses (`gad-worry-depressive-rumination`, `overvalued-idea`, `psychotic-delusion`, `ptsd-intrusive-memories-flashbacks`) exist in no other entry — so excluding it would orphan them and break the #483 invariant (enforced by `differential-detail.test.ts`) that every diagnosis belongs to a presentation. Fixed at the root: `parseEntryTitle` now falls back to the `=== HEADER ===` text when the first line is a metadata row, retitling the entry "Focused Diagnostic Trap Tables" (slug `focused-diagnostic-trap-tables`). Snapshot stays 31 presentations; no orphans. - **Operator follow-up (live Supabase, confirmation-required):** the retitle changed the slug, and seeding upserts by slug and never deletes, so already-seeded owners keep the stale `differential_records` row (`kind='presentation'`, `slug='urgency-urgent'`) and — where corpus embedding ran — its registry corpus document/chunks in `documents`/`document_chunks`. `npm run differentials:seed -- --owner-id --write` now prunes presentation rows whose slug the current snapshot no longer produces (via `staleSeededPresentations`) and their corpus documents per owner; run it for each seeded owner. + +## 2026-07-13 audit remediation batch (branch claude/audit-remediation-2026-07-13) + +- **Lexical retrieval rewrite (audit finding 1):** `20260713100000_index_friendly_lexical_retrieval.sql` splits `match_document_chunks_text`'s OR-across-relations candidate search into two GIN-index probes unioned by chunk id (same contract, same scores; the `_v2` wrapper inherits the speedup). Parity + plan + timing harness: `scripts/sql/lexical-rpc-parity-check.sql` (scratch databases only; run it against the drift-manifest container kept with `--keep`). Re-run it whenever either lexical body changes. +- **supabase_admin default privileges (audit finding 7, operator caveat):** `20260713102000_revoke_supabase_admin_default_privileges.sql` revokes anon/authenticated future-object defaults for role `supabase_admin` and probes the lockdown with future-object creation. On hosted Supabase the migration degrades to a WARNING if `postgres` cannot act for `supabase_admin`; **operator follow-up:** watch for that warning during live apply and, if present, run the six `ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin` statements once via the dashboard SQL editor, then re-run the migration's probe block. +- **Legacy rag query text scrub (audit finding 5):** `20260713103000_scrub_legacy_rag_query_text.sql` replaces pre-HMAC plaintext in `rag_queries.query`/`normalized_query` and `rag_query_misses` with salted `redacted-query:legacy:` placeholders. **Operator verification after live apply:** count rows not matching `redacted-query:%` (expect 0 unless `RAG_PERSIST_RAW_QUERY_TEXT` is deliberately enabled). +- Remaining operator items from the 2026-07-13 audit (confirmation-required, not automated): apply this batch's migrations live, re-run Supabase advisors, trigger the Eval Canary and require two consecutive green scheduled runs, repair the invalid `storage.idx_objects_bucket_id_name_lower` index via dashboard/support, and dedupe response-cache purge cron jobs if `cron.job` shows duplicates. diff --git a/scripts/sql/lexical-rpc-parity-check.sql b/scripts/sql/lexical-rpc-parity-check.sql new file mode 100644 index 000000000..4bdf4f411 --- /dev/null +++ b/scripts/sql/lexical-rpc-parity-check.sql @@ -0,0 +1,441 @@ +-- Lexical RPC parity + plan harness (2026-07-13 audit, finding 1). +-- +-- Verifies that the index-friendly rewrite of public.match_document_chunks_text +-- (migration 20260713100000_index_friendly_lexical_retrieval.sql) returns the +-- same rows and scores as the previous OR-across-relations body, and that the +-- candidate search is served by the GIN indexes instead of a sequential scan +-- of document_chunks. +-- +-- SCRATCH DATABASES ONLY. The script refuses to run when public.documents has +-- rows. It seeds a synthetic corpus into the real tables, installs the legacy +-- body under public.match_document_chunks_text_legacy_parity, compares both +-- functions across representative queries, and times them. +-- +-- Usage (scratch container kept from the drift-manifest replay): +-- npm run drift:manifest -- --keep +-- docker exec -i clinical-kb-drift-manifest psql -U postgres -d postgres \ +-- -v ON_ERROR_STOP=1 -f - < scripts/sql/lexical-rpc-parity-check.sql +-- +-- Corpus size is configurable: -v docs=2500 -v chunks_per_doc=28 (defaults +-- 1000 x 30 = 30k chunks keep the run fast while still large enough for the +-- planner to prefer the pathological plan on the legacy body). +-- +-- Tie handling: rows tied on text_rank at a truncation boundary were already +-- planner-order dependent in the legacy body, so id parity is asserted +-- strictly for every row scoring above the visible minimum and by count for +-- the boundary tie group. + +\if :{?docs} +\else +\set docs 1000 +\endif +\if :{?chunks_per_doc} +\else +\set chunks_per_doc 30 +\endif + +do $$ +begin + if exists (select 1 from public.documents limit 1) then + raise exception 'lexical-rpc-parity-check must run on an empty scratch database; public.documents has rows'; + end if; +end $$; + +begin; + +-- --------------------------------------------------------------------------- +-- Seed a deterministic synthetic corpus. +-- --------------------------------------------------------------------------- +create temp table parity_docs as +select + gen_random_uuid() as id, + i as seq, + case + when i % 37 = 0 then 'Clozapine Monitoring Protocol ' || i + when i % 11 = 0 then 'Lithium Toxicity Guideline ' || i + when i % 7 = 0 then 'Neutropenia Escalation Pathway ' || i + else 'General Clinical Document ' || i + end as title +from generate_series(1, :docs) as s(i); + +insert into public.documents (id, owner_id, title, file_name, file_type, file_size, storage_path, status, metadata) +select + id, + null, + title, + 'parity-doc-' || seq || '.pdf', + 'application/pdf', + 1024, + 'parity/doc-' || seq || '.pdf', + 'indexed', + '{}'::jsonb +from parity_docs; + +with zero_vec as ( + select ('[' || repeat('0,', 1535) || '0]')::extensions.vector as v +) +insert into public.document_chunks (document_id, chunk_index, content, embedding) +select + d.id, + g.j, + case + when (d.seq + g.j) % 23 = 0 then + 'clozapine monitoring neutropenia baseline bloods weekly schedule marker' || ((d.seq * 31 + g.j) % 9973) + when (d.seq + g.j) % 17 = 0 then + 'lithium level toxicity renal function tremor review marker' || ((d.seq * 29 + g.j) % 9973) + when (d.seq + g.j) % 5 = 0 then + 'clozapine dose titration myocarditis troponin escalation marker' || ((d.seq * 13 + g.j) % 9973) + else + 'routine ward round documentation handover note marker' || ((d.seq * 7 + g.j) % 9973) + end, + zero_vec.v +from parity_docs d +cross join generate_series(1, :chunks_per_doc) as g(j) +cross join zero_vec; + +-- A sprinkling of labels and summaries so the shared doc_labels/doc_summaries +-- CTEs produce non-trivial output in both bodies. +insert into public.document_labels (document_id, label, label_type, source, confidence) +select id, 'parity-label-' || seq, 'topic', 'generated', 0.9 +from parity_docs where seq % 19 = 0; + +insert into public.document_summaries (document_id, summary) +select id, 'Synthetic summary for parity document ' || seq +from parity_docs where seq % 13 = 0; + +analyze public.documents; +analyze public.document_chunks; +analyze public.document_labels; +analyze public.document_summaries; + +commit; + +-- --------------------------------------------------------------------------- +-- Install the legacy body (verbatim candidate search from migration +-- 20260713062107_restore_text_fallback_lexical_score.sql) under a parity name. +-- --------------------------------------------------------------------------- +create or replace function public.match_document_chunks_text_legacy_parity( + query_text text, + match_count integer default 12, + document_filters uuid[] default null, + owner_filter uuid default null +) +returns table ( + id uuid, + document_id uuid, + title text, + file_name text, + page_number integer, + chunk_index integer, + section_heading text, + content text, + retrieval_synopsis text, + image_ids uuid[], + source_metadata jsonb, + document_labels jsonb, + document_summary text, + similarity double precision, + text_rank double precision, + hybrid_score double precision, + lexical_score double precision, + images jsonb +) +language sql +stable +set search_path = public, extensions, pg_temp +as $$ + with query as ( + select websearch_to_tsquery('english', coalesce(query_text, '')) as tsq + ), + ranked as ( + select + c.id, + c.document_id, + d.title, + d.file_name, + c.page_number, + c.chunk_index, + c.section_heading, + c.content, + c.retrieval_synopsis, + c.image_ids, + d.metadata as source_metadata, + ( + ts_rank_cd(c.search_tsv, query.tsq) + + (ts_rank_cd(d.title_search_tsv, query.tsq) * 3.0) + )::double precision as text_rank + from public.document_chunks c + join public.documents d on d.id = c.document_id + cross join query + where (document_filters is null or c.document_id = any(document_filters)) + and public.retrieval_owner_matches(owner_filter, d.owner_id) + and d.status = 'indexed' + and public.is_committed_document_generation(c.index_generation_id, d.metadata) + and (c.search_tsv @@ query.tsq or d.title_search_tsv @@ query.tsq) + order by ( + ts_rank_cd(c.search_tsv, query.tsq) + + (ts_rank_cd(d.title_search_tsv, query.tsq) * 3.0) + ) desc + limit least(greatest(match_count * 2, 24), 96) + ), + doc_labels as ( + select + l.document_id, + coalesce( + jsonb_agg( + jsonb_build_object( + 'id', l.id, + 'document_id', l.document_id, + 'owner_id', l.owner_id, + 'label', l.label, + 'label_type', l.label_type, + 'source', l.source, + 'confidence', l.confidence, + 'metadata', l.metadata, + 'created_at', l.created_at, + 'updated_at', l.updated_at + ) + order by l.confidence desc, l.label + ), + '[]'::jsonb + ) as labels + from public.document_labels l + where l.document_id in (select distinct ranked.document_id from ranked) + and coalesce(l.metadata->>'review_status', 'new') <> 'hidden' + and coalesce(l.metadata->>'hidden', 'false') <> 'true' + group by l.document_id + ), + doc_summaries as ( + select distinct on (s.document_id) + s.document_id, + s.summary + from public.document_summaries s + where s.document_id in (select distinct ranked.document_id from ranked) + order by s.document_id + ) + select + ranked.id, + ranked.document_id, + ranked.title, + ranked.file_name, + ranked.page_number, + ranked.chunk_index, + ranked.section_heading, + ranked.content, + ranked.retrieval_synopsis, + ranked.image_ids, + ranked.source_metadata, + coalesce(doc_labels.labels, '[]'::jsonb) as document_labels, + doc_summaries.summary as document_summary, + 0::double precision as similarity, + ranked.text_rank, + least(0.5, 0.18 + (least(ranked.text_rank, 1) * 0.3))::double precision as hybrid_score, + least(0.99, 0.4 + (least(ranked.text_rank, 1) * 0.59))::double precision as lexical_score, + public.chunk_image_metadata(ranked.image_ids) as images + from ranked + left join doc_labels on doc_labels.document_id = ranked.document_id + left join doc_summaries on doc_summaries.document_id = ranked.document_id + order by lexical_score desc, text_rank desc + limit match_count; +$$; + +-- --------------------------------------------------------------------------- +-- Parity assertion helper. +-- --------------------------------------------------------------------------- +-- The owner gate is fail-closed: a null owner_filter matches nothing, and the +-- seeded corpus is public (owner_id null), which only the zero-UUID public +-- sentinel matches — so the sentinel is the default here. A null-owner case +-- below still asserts the fail-closed behaviour is identical in both bodies. +create or replace function pg_temp.assert_lexical_parity( + p_label text, + p_query text, + p_match_count integer default 12, + p_filters uuid[] default null, + p_owner uuid default '00000000-0000-0000-0000-000000000000'::uuid +) +returns void +language plpgsql +as $fn$ +declare + n_new integer; + n_old integer; + score_mismatches integer; + id_mismatches integer; + boundary_rank numeric; +begin + drop table if exists _parity_new; + drop table if exists _parity_old; + + create temp table _parity_new as + select row_number() over () as pos, + r.id, + round(r.text_rank::numeric, 9) as text_rank, + round(r.hybrid_score::numeric, 9) as hybrid_score, + round(r.lexical_score::numeric, 9) as lexical_score, + r.similarity, + md5(coalesce(r.document_labels::text, '') || '|' || coalesce(r.document_summary, '')) as meta_hash + from public.match_document_chunks_text(p_query, p_match_count, p_filters, p_owner) r; + + create temp table _parity_old as + select row_number() over () as pos, + r.id, + round(r.text_rank::numeric, 9) as text_rank, + round(r.hybrid_score::numeric, 9) as hybrid_score, + round(r.lexical_score::numeric, 9) as lexical_score, + r.similarity, + md5(coalesce(r.document_labels::text, '') || '|' || coalesce(r.document_summary, '')) as meta_hash + from public.match_document_chunks_text_legacy_parity(p_query, p_match_count, p_filters, p_owner) r; + + select count(*) into n_new from _parity_new; + select count(*) into n_old from _parity_old; + if n_new <> n_old then + raise exception '[%] row-count mismatch: rewritten=% legacy=%', p_label, n_new, n_old; + end if; + + select count(*) into score_mismatches + from _parity_new n + join _parity_old o using (pos) + where (n.text_rank, n.hybrid_score, n.lexical_score, n.similarity) + is distinct from (o.text_rank, o.hybrid_score, o.lexical_score, o.similarity); + if score_mismatches > 0 then + raise exception '[%] % positional score mismatches between rewritten and legacy results', p_label, score_mismatches; + end if; + + -- Strict id + metadata parity for every row above the visible minimum rank; + -- the boundary tie group is truncation-order dependent in both bodies. + select min(text_rank) into boundary_rank from _parity_new; + select count(*) into id_mismatches + from ( + select id, meta_hash from _parity_new where text_rank > coalesce(boundary_rank, 0) + ) n + full outer join ( + select id, meta_hash from _parity_old where text_rank > coalesce(boundary_rank, 0) + ) o using (id, meta_hash) + where n.id is null or o.id is null; + if id_mismatches > 0 then + raise exception '[%] % non-boundary id/metadata mismatches between rewritten and legacy results', p_label, id_mismatches; + end if; + + raise notice '[%] parity OK (% rows)', p_label, n_new; +end; +$fn$; + +-- --------------------------------------------------------------------------- +-- Run parity across representative query shapes. +-- --------------------------------------------------------------------------- +do $$ +declare + filter_docs uuid[]; +begin + select array_agg(id) into filter_docs + from (select id from public.documents order by title limit 5) f; + + perform pg_temp.assert_lexical_parity('chunk+title OR query', 'clozapine monitoring'); + perform pg_temp.assert_lexical_parity('and query', 'clozapine neutropenia'); + perform pg_temp.assert_lexical_parity('title-only query', 'escalation pathway'); + perform pg_temp.assert_lexical_parity('chunk-only query', 'troponin'); + perform pg_temp.assert_lexical_parity('lithium query', 'lithium toxicity'); + perform pg_temp.assert_lexical_parity('no-hit query', 'zzzznonexistenttermzzzz'); + perform pg_temp.assert_lexical_parity('empty query', ''); + perform pg_temp.assert_lexical_parity('document-filtered query', 'clozapine monitoring', 12, filter_docs); + perform pg_temp.assert_lexical_parity('large match_count', 'clozapine monitoring', 64); + perform pg_temp.assert_lexical_parity('null owner (fail-closed)', 'clozapine monitoring', 12, null, null); + + -- The seeded corpus is public, so the sentinel must find rows and the + -- fail-closed null owner must find none — in both bodies. + if (select count(*) from public.match_document_chunks_text('clozapine monitoring', 12, null, + '00000000-0000-0000-0000-000000000000'::uuid)) = 0 then + raise exception 'sentinel-owner query unexpectedly returned no rows; the parity run exercised nothing'; + end if; + if (select count(*) from public.match_document_chunks_text('clozapine monitoring', 12, null, null)) <> 0 then + raise exception 'null owner_filter must remain fail-closed'; + end if; +end $$; + +-- --------------------------------------------------------------------------- +-- Plan assertion: the candidate search must be index-driven. EXPLAIN cannot +-- see inside a non-inlined SQL function, so this mirrors the rewritten +-- candidate CTEs (keep in sync with the migration) and asserts the plan shape. +-- --------------------------------------------------------------------------- +do $$ +declare + plan jsonb; +begin + execute $q$ + explain (format json) + with query as ( + select websearch_to_tsquery('english', 'clozapine monitoring') as tsq + ), + chunk_hits as ( + select c.id + from public.document_chunks c + cross join query + where c.search_tsv @@ query.tsq + ), + title_chunk_hits as ( + select c.id + from public.documents d + cross join query + join public.document_chunks c on c.document_id = d.id + where d.title_search_tsv @@ query.tsq + ), + lexical_candidates as ( + select chunk_hits.id from chunk_hits + union + select title_chunk_hits.id from title_chunk_hits + ) + select count(*) from lexical_candidates + $q$ into plan; + + -- The audited defect was the full sequential scan of document_chunks; that + -- must never come back, and chunk candidates must come from the GIN index. + if jsonb_path_exists(plan, '$.** ? (@."Node Type" == "Seq Scan" && @."Relation Name" == "document_chunks")') then + raise exception 'plan check failed: candidate search sequential-scans document_chunks: %', plan; + end if; + if not jsonb_path_exists(plan, '$.** ? (@."Index Name" == "document_chunks_search_idx")') then + raise exception 'plan check failed: document_chunks_search_idx unused: %', plan; + end if; + -- The documents table is orders of magnitude smaller; on small corpora the + -- planner may legitimately prefer scanning it over its GIN index. Informational. + if not jsonb_path_exists(plan, '$.** ? (@."Index Name" == "documents_title_search_idx")') then + raise warning 'documents_title_search_idx unused on this corpus size (planner cost choice; documents scan is bounded by the documents table, not chunks)'; + else + raise notice 'documents_title_search_idx in use for the title branch'; + end if; + raise notice 'plan OK: chunk candidates come from document_chunks_search_idx, no document_chunks seq scan'; +end $$; + +-- --------------------------------------------------------------------------- +-- Timing comparison (informational; warns if the rewrite is not faster). +-- --------------------------------------------------------------------------- +do $$ +declare + sentinel constant uuid := '00000000-0000-0000-0000-000000000000'; + t0 timestamptz; + legacy_ms numeric; + rewritten_ms numeric; +begin + perform count(*) from public.match_document_chunks_text_legacy_parity('clozapine monitoring', 12, null, sentinel); + t0 := clock_timestamp(); + for i in 1..3 loop + perform count(*) from public.match_document_chunks_text_legacy_parity('clozapine monitoring', 12, null, sentinel); + end loop; + legacy_ms := round((extract(epoch from clock_timestamp() - t0) * 1000 / 3)::numeric, 1); + + perform count(*) from public.match_document_chunks_text('clozapine monitoring', 12, null, sentinel); + t0 := clock_timestamp(); + for i in 1..3 loop + perform count(*) from public.match_document_chunks_text('clozapine monitoring', 12, null, sentinel); + end loop; + rewritten_ms := round((extract(epoch from clock_timestamp() - t0) * 1000 / 3)::numeric, 1); + + raise notice 'warm avg over 3 runs: legacy % ms, rewritten % ms', legacy_ms, rewritten_ms; + if rewritten_ms > legacy_ms then + raise warning 'rewritten lexical RPC was not faster on this corpus (legacy % ms vs rewritten % ms)', + legacy_ms, rewritten_ms; + end if; +end $$; + +drop function public.match_document_chunks_text_legacy_parity(text, integer, uuid[], uuid); + +\echo 'lexical-rpc-parity-check complete' diff --git a/supabase/drift-manifest.json b/supabase/drift-manifest.json index c621aa4a8..c47d57207 100644 --- a/supabase/drift-manifest.json +++ b/supabase/drift-manifest.json @@ -1,9 +1,9 @@ { - "generated_at": "2026-07-13T08:29:12.244Z", + "generated_at": "2026-07-13T09:59:28.896Z", "generator": "scripts/generate-drift-manifest.ts", "postgres_image": "supabase/postgres:17.6.1.127", - "schema_sha256": "99d5adc1f21f0c224de4bdd63f3c7475b3ca5d3eaa10da23a0772ac9ed3714e4", - "replay_seconds": 106, + "schema_sha256": "208dbd62d24ec6f851048b303a6c43746e8901781f2a84551020e3f12bb0a103", + "replay_seconds": 138, "snapshot": { "views": [ { @@ -6473,7 +6473,7 @@ "postgres=X/postgres", "service_role=X/postgres" ], - "def_hash": "7df00b3e8e430df3a8a598fa053c7260", + "def_hash": "442329df8bfe935e3876aab627458e3a", "signature": "public.match_document_chunks_text(text,integer,uuid[],uuid)" }, { @@ -6698,7 +6698,7 @@ "postgres=X/postgres", "service_role=X/postgres" ], - "def_hash": "c03491fa86455b3f853a705650a50377", + "def_hash": "1bc5c6e76cdaa83b6831b36413fb61f5", "signature": "public.retrieval_owner_matches_v2(uuid,uuid,boolean)" }, { @@ -6983,7 +6983,7 @@ "table": "differential_records" }, { - "def": "CHECK ((length(btrim(content)) > 0)) NOT VALID", + "def": "CHECK ((length(btrim(content)) > 0))", "name": "document_chunks_content_not_blank", "table": "document_chunks" }, @@ -6998,7 +6998,7 @@ "table": "document_chunks" }, { - "def": "CHECK ((length(btrim(content)) > 0)) NOT VALID", + "def": "CHECK ((length(btrim(content)) > 0))", "name": "document_embedding_fields_content_not_blank", "table": "document_embedding_fields" }, @@ -7068,7 +7068,7 @@ "table": "document_index_quality" }, { - "def": "CHECK ((length(btrim(content)) > 0)) NOT VALID", + "def": "CHECK ((length(btrim(content)) > 0))", "name": "document_index_units_content_not_blank", "table": "document_index_units" }, diff --git a/supabase/migrations/20260713100000_index_friendly_lexical_retrieval.sql b/supabase/migrations/20260713100000_index_friendly_lexical_retrieval.sql new file mode 100644 index 000000000..c70a3814e --- /dev/null +++ b/supabase/migrations/20260713100000_index_friendly_lexical_retrieval.sql @@ -0,0 +1,182 @@ +-- Index-friendly lexical retrieval (2026-07-13 audit, finding 1). +-- +-- The previous body matched with a single predicate that OR-ed conditions +-- across two relations: +-- (c.search_tsv @@ query.tsq or d.title_search_tsv @@ query.tsq) +-- PostgreSQL cannot serve that disjunction from either GIN index, so every +-- call sequential-scanned the full document_chunks table (~17s warm at 70k +-- chunks in production, ~5.5 GB of buffer traffic per call, up to three +-- lexical variants concurrently). Probing document_chunks_search_idx and +-- documents_title_search_idx separately and unioning the candidate chunk ids +-- returns the same rows from two index probes; the visibility gates then run +-- on the narrowed candidate set only. +-- +-- Contract is unchanged: same signature, same 18 output columns, same score +-- formulas (text_rank / similarity 0 / capped hybrid_score / lexical_score), +-- same candidate cap and ordering, service-role-only execution. Rows tied on +-- text_rank at the candidate-cap boundary were already planner-order +-- dependent; that is unchanged. Parity + plan harness: +-- scripts/sql/lexical-rpc-parity-check.sql. + +create or replace function public.match_document_chunks_text( + query_text text, + match_count integer default 12, + document_filters uuid[] default null, + owner_filter uuid default null +) +returns table ( + id uuid, + document_id uuid, + title text, + file_name text, + page_number integer, + chunk_index integer, + section_heading text, + content text, + retrieval_synopsis text, + image_ids uuid[], + source_metadata jsonb, + document_labels jsonb, + document_summary text, + similarity double precision, + text_rank double precision, + hybrid_score double precision, + lexical_score double precision, + images jsonb +) +language sql +stable +set search_path = public, extensions, pg_temp +as $$ + with query as ( + select websearch_to_tsquery('english', coalesce(query_text, '')) as tsq + ), + -- Chunk-content matches probe document_chunks_search_idx directly. + chunk_hits as ( + select c.id + from public.document_chunks c + cross join query + where c.search_tsv @@ query.tsq + and (document_filters is null or c.document_id = any(document_filters)) + ), + -- Title matches probe documents_title_search_idx, then fan out to that + -- document's chunks through document_chunks_document_idx. + title_chunk_hits as ( + select c.id + from public.documents d + cross join query + join public.document_chunks c on c.document_id = d.id + where d.title_search_tsv @@ query.tsq + and (document_filters is null or c.document_id = any(document_filters)) + ), + lexical_candidates as ( + select chunk_hits.id from chunk_hits + union + select title_chunk_hits.id from title_chunk_hits + ), + ranked as ( + select + c.id, + c.document_id, + d.title, + d.file_name, + c.page_number, + c.chunk_index, + c.section_heading, + c.content, + c.retrieval_synopsis, + c.image_ids, + d.metadata as source_metadata, + ( + ts_rank_cd(c.search_tsv, query.tsq) + + (ts_rank_cd(d.title_search_tsv, query.tsq) * 3.0) + )::double precision as text_rank + from lexical_candidates cand + join public.document_chunks c on c.id = cand.id + join public.documents d on d.id = c.document_id + cross join query + where public.retrieval_owner_matches(owner_filter, d.owner_id) + and d.status = 'indexed' + and public.is_committed_document_generation(c.index_generation_id, d.metadata) + order by ( + ts_rank_cd(c.search_tsv, query.tsq) + + (ts_rank_cd(d.title_search_tsv, query.tsq) * 3.0) + ) desc + limit least(greatest(match_count * 2, 24), 96) + ), + -- Batch-fetch label metadata for all distinct document_ids in the result set. + -- One query replaces N per-row calls to document_label_metadata(). + doc_labels as ( + select + l.document_id, + coalesce( + jsonb_agg( + jsonb_build_object( + 'id', l.id, + 'document_id', l.document_id, + 'owner_id', l.owner_id, + 'label', l.label, + 'label_type', l.label_type, + 'source', l.source, + 'confidence', l.confidence, + 'metadata', l.metadata, + 'created_at', l.created_at, + 'updated_at', l.updated_at + ) + order by l.confidence desc, l.label + ), + '[]'::jsonb + ) as labels + from public.document_labels l + where l.document_id in (select distinct ranked.document_id from ranked) + and coalesce(l.metadata->>'review_status', 'new') <> 'hidden' + and coalesce(l.metadata->>'hidden', 'false') <> 'true' + group by l.document_id + ), + -- Batch-fetch summary text for all distinct document_ids in the result set. + -- One query replaces N per-row calls to document_summary_text(). + doc_summaries as ( + select distinct on (s.document_id) + s.document_id, + s.summary + from public.document_summaries s + where s.document_id in (select distinct ranked.document_id from ranked) + order by s.document_id + ) + select + ranked.id, + ranked.document_id, + ranked.title, + ranked.file_name, + ranked.page_number, + ranked.chunk_index, + ranked.section_heading, + ranked.content, + ranked.retrieval_synopsis, + ranked.image_ids, + ranked.source_metadata, + coalesce(doc_labels.labels, '[]'::jsonb) as document_labels, + doc_summaries.summary as document_summary, + -- Text-only fallback has NO vector cosine similarity. Do not fabricate one: + -- a synthetic value here was read downstream as a real semantic score and + -- could label a pure keyword hit as "strong"/"moderate" evidence (>=0.64). + -- Leave similarity at 0; the lexical signal lives in lexical_score. + 0::double precision as similarity, + ranked.text_rank, + -- Cap hybrid_score well below the 0.64 "moderate" threshold so a lexical-only + -- row can order amongst its peers but can never masquerade as a moderate/strong + -- cosine match when merged with vector results. + least(0.5, 0.18 + (least(ranked.text_rank, 1) * 0.3))::double precision as hybrid_score, + least(0.99, 0.4 + (least(ranked.text_rank, 1) * 0.59))::double precision as lexical_score, + public.chunk_image_metadata(ranked.image_ids) as images + from ranked + left join doc_labels on doc_labels.document_id = ranked.document_id + left join doc_summaries on doc_summaries.document_id = ranked.document_id + order by lexical_score desc, text_rank desc + limit match_count; +$$; + +revoke execute on function public.match_document_chunks_text(text, integer, uuid[], uuid) + from public, anon, authenticated; +grant execute on function public.match_document_chunks_text(text, integer, uuid[], uuid) + to service_role; diff --git a/supabase/migrations/20260713101000_pin_retrieval_owner_matches_v2_search_path.sql b/supabase/migrations/20260713101000_pin_retrieval_owner_matches_v2_search_path.sql new file mode 100644 index 000000000..05892e51b --- /dev/null +++ b/supabase/migrations/20260713101000_pin_retrieval_owner_matches_v2_search_path.sql @@ -0,0 +1,11 @@ +-- Pin search_path on public.retrieval_owner_matches_v2 (2026-07-13 audit, +-- finding 6; Supabase advisor: function_search_path_mutable). +-- +-- Every sibling wrapper created in 20260713020000_owner_plus_public_retrieval.sql +-- pins `set search_path = public, extensions, pg_temp`; this helper was the one +-- exception. Its body is a pure boolean expression over its arguments, so the +-- pinned path changes no behaviour — it only removes the mutable-search-path +-- surface and returns the production linter to green. + +alter function public.retrieval_owner_matches_v2(uuid, uuid, boolean) + set search_path = public, extensions, pg_temp; diff --git a/supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql b/supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql new file mode 100644 index 000000000..bfe8a462e --- /dev/null +++ b/supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql @@ -0,0 +1,101 @@ +-- Revoke broad future-object default privileges for supabase_admin +-- (2026-07-13 audit, finding 7). +-- +-- 20260528007000_database_hardening_before_import.sql locked down default +-- privileges for objects created by role `postgres` only. Default privileges +-- are keyed to the creating role, so objects created by `supabase_admin` +-- (dashboard SQL editor, platform tooling) still defaulted to broad +-- anon/authenticated access. No such object exists today; this closes the +-- future-object exposure. +-- +-- Hosted caveat: migrations run as `postgres`, which may not be a member of +-- `supabase_admin` on the hosted platform. In that case the statements below +-- degrade to a WARNING instead of failing the chain, and an operator must run +-- them once via the Supabase dashboard SQL editor (see +-- docs/process-hardening.md). Local replays (CI `supabase db reset`, the +-- drift-manifest scratch container) run as a superuser, apply the change, and +-- exercise the probe verification at the end of this migration. + +do $$ +begin + if not exists (select 1 from pg_roles where rolname = 'supabase_admin') then + raise warning 'role supabase_admin does not exist; default-privilege hardening skipped'; + return; + end if; + + begin + alter default privileges for role supabase_admin in schema public + revoke all privileges on tables from anon, authenticated; + alter default privileges for role supabase_admin in schema public + revoke usage, select on sequences from anon, authenticated; + alter default privileges for role supabase_admin in schema public + revoke execute on functions from public, anon, authenticated; + alter default privileges for role supabase_admin in schema public + grant select, insert, update, delete on tables to service_role; + alter default privileges for role supabase_admin in schema public + grant usage, select on sequences to service_role; + alter default privileges for role supabase_admin in schema public + grant execute on functions to service_role; + raise notice 'supabase_admin future-object default privileges hardened'; + exception + when insufficient_privilege then + raise warning 'cannot alter default privileges for supabase_admin as %; ' + 'operator follow-up required: run the six ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin ' + 'statements from migration 20260713102000 via the Supabase dashboard SQL editor', current_user; + end; +end $$; + +-- Probe verification: create future objects as supabase_admin and assert the +-- client roles receive no automatic access. Runs wherever the current user can +-- assume supabase_admin (CI replay, scratch replay, superuser apply); degrades +-- to a WARNING where it cannot, matching the guarded hardening above. +do $$ +declare + probe_seq text; +begin + if not exists (select 1 from pg_roles where rolname = 'supabase_admin') then + raise warning 'role supabase_admin does not exist; default-privilege probe skipped'; + return; + end if; + + begin + execute 'set local role supabase_admin'; + exception + when insufficient_privilege then + raise warning 'cannot assume supabase_admin as %; default-privilege probe skipped', current_user; + return; + end; + + create table public._defacl_probe_table ( + id bigint generated always as identity primary key, + note text + ); + create function public._defacl_probe_fn() returns integer language sql as 'select 1'; + execute 'reset role'; + + probe_seq := pg_get_serial_sequence('public._defacl_probe_table', 'id'); + + if has_table_privilege('anon', 'public._defacl_probe_table', 'select') + or has_table_privilege('authenticated', 'public._defacl_probe_table', 'select, insert, update, delete') then + raise exception 'future-object default privileges leak: client roles can access a supabase_admin-created table'; + end if; + if probe_seq is not null + and (has_sequence_privilege('anon', probe_seq, 'usage, select') + or has_sequence_privilege('authenticated', probe_seq, 'usage, select')) then + raise exception 'future-object default privileges leak: client roles can access a supabase_admin-created sequence'; + end if; + if has_function_privilege('anon', 'public._defacl_probe_fn()', 'execute') + or has_function_privilege('authenticated', 'public._defacl_probe_fn()', 'execute') then + raise exception 'future-object default privileges leak: client roles can execute a supabase_admin-created function'; + end if; + if not has_table_privilege('service_role', 'public._defacl_probe_table', 'select, insert, update, delete') then + raise exception 'future-object default privileges regression: service_role lost access to a supabase_admin-created table'; + end if; + + execute 'set local role supabase_admin'; + drop function public._defacl_probe_fn(); + drop table public._defacl_probe_table; + execute 'reset role'; + + raise notice 'supabase_admin future-object default privileges verified: no anon/authenticated access, service_role retained'; +end $$; diff --git a/supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql b/supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql new file mode 100644 index 000000000..9c7f3bb23 --- /dev/null +++ b/supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql @@ -0,0 +1,83 @@ +-- Scrub pre-HMAC plaintext query text at rest (2026-07-13 audit, finding 5). +-- +-- Since the HMAC rollout every write path stores `redacted-query:` (see +-- src/lib/query-privacy.ts) unless RAG_PERSIST_RAW_QUERY_TEXT is deliberately +-- enabled — and scripts/production-readiness.ts fails when that flag is on in +-- a production-like environment. Rows written before the rollout still hold +-- raw clinical query text (audit counted 230 rag_queries rows and 10 +-- rag_query_misses rows on live). Their original owner/consent basis cannot be +-- established, so they are replaced, not migrated. +-- +-- Placeholder shape: 'redacted-query:legacy:' || md5(random salt || text). +-- The per-row random salt makes the value irreversible and not +-- dictionary-attackable while keeping rows distinct; the text never leaves the +-- database. The historical rag_queries.answer column was already cleared by +-- 20260713010000_clear_historical_rag_query_answers.sql. +-- +-- rag_response_cache rows are a cache: legacy raw-keyed entries are deleted +-- outright (worst case is a cache miss) rather than re-keyed. + +update public.rag_queries +set query = 'redacted-query:legacy:' || md5(gen_random_uuid()::text || query) +where query not like 'redacted-query:%'; + +update public.rag_query_misses +set + query = case + when query like 'redacted-query:%' then query + else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || query) + end, + normalized_query = case + when normalized_query like 'redacted-query:%' then normalized_query + else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || normalized_query) + end +where query not like 'redacted-query:%' + or normalized_query not like 'redacted-query:%'; + +update public.rag_retrieval_logs +set + query = case + when query like 'redacted-query:%' then query + else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || query) + end, + normalized_query = case + when normalized_query is null or normalized_query like 'redacted-query:%' then normalized_query + else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || normalized_query) + end +where query not like 'redacted-query:%' + or (normalized_query is not null and normalized_query not like 'redacted-query:%'); + +delete from public.rag_response_cache +where normalized_query not like 'redacted-cache:%'; + +do $$ +declare + bad_queries integer; + bad_misses integer; + bad_logs integer; + bad_cache integer; +begin + select count(*) into bad_queries + from public.rag_queries + where query not like 'redacted-query:%'; + + select count(*) into bad_misses + from public.rag_query_misses + where query not like 'redacted-query:%' + or normalized_query not like 'redacted-query:%'; + + select count(*) into bad_logs + from public.rag_retrieval_logs + where query not like 'redacted-query:%' + or (normalized_query is not null and normalized_query not like 'redacted-query:%'); + + select count(*) into bad_cache + from public.rag_response_cache + where normalized_query not like 'redacted-cache:%'; + + if bad_queries + bad_misses + bad_logs + bad_cache > 0 then + raise exception + 'legacy query-text scrub incomplete: rag_queries=%, rag_query_misses=%, rag_retrieval_logs=%, rag_response_cache=%', + bad_queries, bad_misses, bad_logs, bad_cache; + end if; +end $$; diff --git a/supabase/migrations/20260713104000_validate_content_not_blank_constraints.sql b/supabase/migrations/20260713104000_validate_content_not_blank_constraints.sql new file mode 100644 index 000000000..06ca3fb1a --- /dev/null +++ b/supabase/migrations/20260713104000_validate_content_not_blank_constraints.sql @@ -0,0 +1,13 @@ +-- Validate the three content-quality CHECK constraints (2026-07-13 audit, +-- finding 12). They were observed live as NOT VALID and codified as such in +-- 20260707000000_codify_live_observed_drift.sql; the audit's live inspection +-- found zero violating rows in all three tables. VALIDATE CONSTRAINT takes +-- only a SHARE UPDATE EXCLUSIVE lock (reads and writes continue) and turns +-- the guards into enforced invariants for existing rows too. + +alter table public.document_chunks + validate constraint document_chunks_content_not_blank; +alter table public.document_embedding_fields + validate constraint document_embedding_fields_content_not_blank; +alter table public.document_index_units + validate constraint document_index_units_content_not_blank; diff --git a/supabase/schema.sql b/supabase/schema.sql index 4553c19a6..e93d75169 100644 --- a/supabase/schema.sql +++ b/supabase/schema.sql @@ -450,21 +450,23 @@ alter table public.document_labels set ( autovacuum_analyze_scale_factor = 0.02, autovacuum_analyze_threshold = 100 ); --- Content-quality guards observed live (added there as NOT VALID; codified --- byte-identically -- pg_get_constraintdef includes the NOT VALID marker). +-- Content-quality guards observed live. Originally added there as NOT VALID; +-- validated by 20260713104000_validate_content_not_blank_constraints.sql after +-- the 2026-07-13 audit confirmed zero violating rows, so the canonical replay +-- creates them validated. do $guard$ begin if not exists (select 1 from pg_constraint where conname = 'document_chunks_content_not_blank') then alter table public.document_chunks - add constraint document_chunks_content_not_blank check (length(btrim(content)) > 0) not valid; + add constraint document_chunks_content_not_blank check (length(btrim(content)) > 0); end if; if not exists (select 1 from pg_constraint where conname = 'document_embedding_fields_content_not_blank') then alter table public.document_embedding_fields - add constraint document_embedding_fields_content_not_blank check (length(btrim(content)) > 0) not valid; + add constraint document_embedding_fields_content_not_blank check (length(btrim(content)) > 0); end if; if not exists (select 1 from pg_constraint where conname = 'document_index_units_content_not_blank') then alter table public.document_index_units - add constraint document_index_units_content_not_blank check (length(btrim(content)) > 0) not valid; + add constraint document_index_units_content_not_blank check (length(btrim(content)) > 0); end if; end $guard$; @@ -3839,6 +3841,32 @@ as $$ with query as ( select websearch_to_tsquery('english', coalesce(query_text, '')) as tsq ), + -- The chunk/title disjunction is split into two separately indexable probes: + -- OR-ing predicates across document_chunks and documents defeated both GIN + -- indexes and sequential-scanned every chunk (2026-07-13 audit, finding 1). + -- Chunk-content matches probe document_chunks_search_idx directly. + chunk_hits as ( + select c.id + from public.document_chunks c + cross join query + where c.search_tsv @@ query.tsq + and (document_filters is null or c.document_id = any(document_filters)) + ), + -- Title matches probe documents_title_search_idx, then fan out to that + -- document's chunks through document_chunks_document_idx. + title_chunk_hits as ( + select c.id + from public.documents d + cross join query + join public.document_chunks c on c.document_id = d.id + where d.title_search_tsv @@ query.tsq + and (document_filters is null or c.document_id = any(document_filters)) + ), + lexical_candidates as ( + select chunk_hits.id from chunk_hits + union + select title_chunk_hits.id from title_chunk_hits + ), ranked as ( select c.id, @@ -3856,14 +3884,13 @@ as $$ ts_rank_cd(c.search_tsv, query.tsq) + (ts_rank_cd(d.title_search_tsv, query.tsq) * 3.0) )::double precision as text_rank - from public.document_chunks c + from lexical_candidates cand + join public.document_chunks c on c.id = cand.id join public.documents d on d.id = c.document_id cross join query - where (document_filters is null or c.document_id = any(document_filters)) - and public.retrieval_owner_matches(owner_filter, d.owner_id) + where public.retrieval_owner_matches(owner_filter, d.owner_id) and d.status = 'indexed' and public.is_committed_document_generation(c.index_generation_id, d.metadata) - and (c.search_tsv @@ query.tsq or d.title_search_tsv @@ query.tsq) order by ( ts_rank_cd(c.search_tsv, query.tsq) + (ts_rank_cd(d.title_search_tsv, query.tsq) * 3.0) @@ -4890,6 +4917,38 @@ alter default privileges for role postgres in schema public alter default privileges for role postgres in schema public grant execute on functions to service_role; +-- Default privileges are keyed to the creating role, so the postgres block +-- above does not cover objects created by supabase_admin (dashboard SQL +-- editor, platform tooling). Guarded because only a superuser or a member of +-- supabase_admin may alter its defaults (2026-07-13 audit, finding 7). +do $$ +begin + if not exists (select 1 from pg_roles where rolname = 'supabase_admin') then + raise warning 'role supabase_admin does not exist; default-privilege hardening skipped'; + return; + end if; + + begin + alter default privileges for role supabase_admin in schema public + revoke all privileges on tables from anon, authenticated; + alter default privileges for role supabase_admin in schema public + revoke usage, select on sequences from anon, authenticated; + alter default privileges for role supabase_admin in schema public + revoke execute on functions from public, anon, authenticated; + alter default privileges for role supabase_admin in schema public + grant select, insert, update, delete on tables to service_role; + alter default privileges for role supabase_admin in schema public + grant usage, select on sequences to service_role; + alter default privileges for role supabase_admin in schema public + grant execute on functions to service_role; + exception + when insufficient_privilege then + raise warning 'cannot alter default privileges for supabase_admin as %; ' + 'operator follow-up required: run the six ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin ' + 'statements from migration 20260713102000 via the Supabase dashboard SQL editor', current_user; + end; +end $$; + revoke usage on schema public from anon; grant usage on schema public to authenticated, service_role; @@ -6845,7 +6904,9 @@ create or replace function public.retrieval_owner_matches_v2( row_owner_id uuid, include_public boolean default true ) -returns boolean language sql immutable as $$ +returns boolean language sql immutable +set search_path = public, extensions, pg_temp +as $$ select owner_filter is not null and ( row_owner_id = owner_filter or (coalesce(include_public, false) and row_owner_id is null) diff --git a/tests/supabase-schema.test.ts b/tests/supabase-schema.test.ts index 125551aa7..b9c7086e8 100644 --- a/tests/supabase-schema.test.ts +++ b/tests/supabase-schema.test.ts @@ -108,6 +108,26 @@ const deepMemoryCommitReconciliationMigration = readFileSync( new URL("../supabase/migrations/20260713090500_reconcile_deep_memory_commit.sql", import.meta.url), "utf8", ).replace(/\s+/g, " "); +const indexFriendlyLexicalRetrievalMigration = readFileSync( + new URL("../supabase/migrations/20260713100000_index_friendly_lexical_retrieval.sql", import.meta.url), + "utf8", +).replace(/\s+/g, " "); +const pinOwnerMatchesV2SearchPathMigration = readFileSync( + new URL("../supabase/migrations/20260713101000_pin_retrieval_owner_matches_v2_search_path.sql", import.meta.url), + "utf8", +).replace(/\s+/g, " "); +const supabaseAdminDefaultPrivilegesMigration = readFileSync( + new URL("../supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql", import.meta.url), + "utf8", +).replace(/\s+/g, " "); +const scrubLegacyQueryTextMigration = readFileSync( + new URL("../supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql", import.meta.url), + "utf8", +).replace(/\s+/g, " "); +const validateContentNotBlankMigration = readFileSync( + new URL("../supabase/migrations/20260713104000_validate_content_not_blank_constraints.sql", import.meta.url), + "utf8", +).replace(/\s+/g, " "); const clinicalRegistryRecordsMigration = readFileSync( new URL("../supabase/migrations/20260703020000_clinical_registry_records.sql", import.meta.url), "utf8", @@ -972,6 +992,109 @@ describe("Supabase Preview replay guards", () => { ); }); + it("keeps the lexical text path index-friendly (no OR across chunk and title relations)", () => { + // 2026-07-13 audit finding 1: OR-ing chunk and title tsquery predicates across + // two relations defeated both GIN indexes and sequential-scanned every chunk. + // The candidate search must stay split into separately indexable probes. + for (const body of [ + extractTextChunkFunction(schema), + extractTextChunkFunction(indexFriendlyLexicalRetrievalMigration), + ]) { + expect(body).toContain("chunk_hits as ("); + expect(body).toContain("title_chunk_hits as ("); + expect(body).toContain("union"); + expect(body).not.toContain("c.search_tsv @@ query.tsq or d.title_search_tsv @@ query.tsq"); + expect(body).toContain("where public.retrieval_owner_matches(owner_filter, d.owner_id)"); + expect(body).toContain("public.is_committed_document_generation(c.index_generation_id, d.metadata)"); + expect(body).toContain("limit least(greatest(match_count * 2, 24), 96)"); + expect(body).toContain("0::double precision as similarity"); + expect(body).toContain("least(0.5,"); + } + expect(indexFriendlyLexicalRetrievalMigration).toContain( + "revoke execute on function public.match_document_chunks_text(text, integer, uuid[], uuid) from public, anon, authenticated;", + ); + expect(indexFriendlyLexicalRetrievalMigration).toContain( + "grant execute on function public.match_document_chunks_text(text, integer, uuid[], uuid) to service_role;", + ); + }); + + it("pins search_path on retrieval_owner_matches_v2", () => { + // 2026-07-13 audit finding 6 / Supabase advisor function_search_path_mutable: + // this helper was the only owner-plus-public wrapper without a pinned path. + const start = schema.indexOf("create or replace function public.retrieval_owner_matches_v2("); + expect(start).toBeGreaterThanOrEqual(0); + const header = schema.slice(start, schema.indexOf("$$", start)); + expect(header).toContain("set search_path = public, extensions, pg_temp"); + expect(pinOwnerMatchesV2SearchPathMigration).toContain( + "alter function public.retrieval_owner_matches_v2(uuid, uuid, boolean) set search_path = public, extensions, pg_temp;", + ); + }); + + it("locks down supabase_admin future-object default privileges", () => { + // 2026-07-13 audit finding 7: the 20260528007000 hardening covered role + // postgres only; default privileges are keyed to the creating role. + for (const sql of [schema, supabaseAdminDefaultPrivilegesMigration]) { + expect(sql).toContain( + "alter default privileges for role supabase_admin in schema public revoke all privileges on tables from anon, authenticated;", + ); + expect(sql).toContain( + "alter default privileges for role supabase_admin in schema public revoke usage, select on sequences from anon, authenticated;", + ); + expect(sql).toContain( + "alter default privileges for role supabase_admin in schema public revoke execute on functions from public, anon, authenticated;", + ); + expect(sql).toContain( + "alter default privileges for role supabase_admin in schema public grant execute on functions to service_role;", + ); + // Guarded: only a superuser or member of supabase_admin may alter its defaults. + expect(sql).toContain("when insufficient_privilege then"); + } + // The migration also proves the lockdown with future-object probes. + expect(supabaseAdminDefaultPrivilegesMigration).toContain("_defacl_probe_table"); + expect(supabaseAdminDefaultPrivilegesMigration).toContain( + "has_table_privilege('anon', 'public._defacl_probe_table', 'select')", + ); + expect(supabaseAdminDefaultPrivilegesMigration).toContain( + "has_function_privilege('anon', 'public._defacl_probe_fn()', 'execute')", + ); + expect(supabaseAdminDefaultPrivilegesMigration).toContain( + "has_sequence_privilege('anon', probe_seq, 'usage, select')", + ); + }); + + it("scrubs legacy plaintext query text with salted irreversible placeholders", () => { + // 2026-07-13 audit finding 5: rows written before the HMAC rollout still + // held raw clinical query text. Placeholders must be salted (not bare + // md5(query), which is dictionary-attackable) and the migration must + // assert completion for every query-bearing table. + expect(scrubLegacyQueryTextMigration).toContain( + "'redacted-query:legacy:' || md5(gen_random_uuid()::text || query)", + ); + expect(scrubLegacyQueryTextMigration).not.toMatch(/md5\(query\)/); + for (const table of ["rag_queries", "rag_query_misses", "rag_retrieval_logs"]) { + expect(scrubLegacyQueryTextMigration).toContain(`update public.${table}`); + } + // Cache rows are deleted, not re-keyed: a scrubbed key would never be hit again. + expect(scrubLegacyQueryTextMigration).toContain("delete from public.rag_response_cache"); + expect(scrubLegacyQueryTextMigration).toContain("where normalized_query not like 'redacted-cache:%'"); + expect(scrubLegacyQueryTextMigration).toContain("raise exception"); + }); + + it("validates the content_not_blank guards so they are no longer NOT VALID", () => { + // 2026-07-13 audit finding 12: the three content-quality checks were codified + // NOT VALID from live; zero violating rows existed, so they are now validated + // and the canonical replay creates them enforced from the start. + for (const constraint of [ + "document_chunks_content_not_blank", + "document_embedding_fields_content_not_blank", + "document_index_units_content_not_blank", + ]) { + expect(validateContentNotBlankMigration).toContain(`validate constraint ${constraint}`); + expect(schema).toContain(`add constraint ${constraint} check (length(btrim(content)) > 0);`); + } + expect(schema).not.toContain("check (length(btrim(content)) > 0) not valid"); + }); + it("keeps retrieval_synopsis when adding lexical_score to match_document_chunks_text", () => { expect(lexicalScoreMigration).toContain("retrieval_synopsis text"); expect(lexicalScoreMigration).toContain("c.retrieval_synopsis"); From 36a638424b2acebb738958d88ef3ddc8c5cbc6c6 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Mon, 13 Jul 2026 19:15:42 +0800 Subject: [PATCH 2/8] fix(privacy): owner-scope recent queries and purge the legacy unscoped key The standalone search shell still read the unscoped clinical-kb-recent-queries localStorage key, so a historical clinical query written by an older build could resurface for whoever used the browser next (2026-07-13 audit, finding 4). The shell now reads the owner-scoped sessionStorage key with the same owner derivation as the dashboard, and both surfaces purge the legacy unscoped keys on mount without migrating them (the original owner cannot be established). Covered by a unit suite for the shared storage module and a @critical Playwright regression that seeds the legacy value and asserts it is never rendered and gets deleted. Co-Authored-By: Claude Fable 5 --- src/components/ClinicalDashboard.tsx | 10 ++- .../global-search-shell.tsx | 33 +++++--- .../recent-query-storage.ts | 29 +++++++ tests/recent-query-storage.test.ts | 78 +++++++++++++++++++ tests/ui-smoke.spec.ts | 24 ++++++ 5 files changed, 161 insertions(+), 13 deletions(-) create mode 100644 src/components/clinical-dashboard/recent-query-storage.ts create mode 100644 tests/recent-query-storage.test.ts diff --git a/src/components/ClinicalDashboard.tsx b/src/components/ClinicalDashboard.tsx index 83a538681..9183b8f80 100644 --- a/src/components/ClinicalDashboard.tsx +++ b/src/components/ClinicalDashboard.tsx @@ -124,6 +124,7 @@ const DocumentSearchResultsPanel = dynamic( { ssr: false }, ); +import { clearLegacyRecentQueries, demoRecentQueryOwnerId } from "@/components/clinical-dashboard/recent-query-storage"; import type { SearchFacets } from "@/components/clinical-dashboard/document-search-results"; import { isWeakRelevance } from "@/components/clinical-dashboard/relevance"; import { @@ -1019,7 +1020,7 @@ export function ClinicalDashboard({ const localNoAuthMode = isLocalNoAuthMode(); const explicitDemoMode = demoMode || process.env.NEXT_PUBLIC_DEMO_MODE === "true"; const clientDemoMode = explicitDemoMode || browserAuthUnavailableDemoFallback || localNoAuthMode; - const answerThreadOwnerId = auth.session?.user.id ?? (clientDemoMode ? "local-demo-session" : null); + const answerThreadOwnerId = auth.session?.user.id ?? (clientDemoMode ? demoRecentQueryOwnerId : null); const previousAnswerThreadOwnerIdRef = useRef(answerThreadOwnerId); useEffect(() => { const previousOwnerId = previousAnswerThreadOwnerIdRef.current; @@ -1166,6 +1167,13 @@ export function ClinicalDashboard({ return () => window.clearTimeout(timeoutId); }, [prefetchApplications]); + // The dashboard renders directly on "/" without the standalone search shell, + // so it must purge the legacy unscoped recent-queries key too (2026-07-13 + // audit, finding 4). + useEffect(() => { + clearLegacyRecentQueries(); + }, []); + useEffect(() => { if (!answerThreadOwnerId) { queueMicrotask(() => setRecentQueries([])); diff --git a/src/components/clinical-dashboard/global-search-shell.tsx b/src/components/clinical-dashboard/global-search-shell.tsx index 9e0643655..37ad86ccc 100644 --- a/src/components/clinical-dashboard/global-search-shell.tsx +++ b/src/components/clinical-dashboard/global-search-shell.tsx @@ -15,7 +15,11 @@ import { } from "react"; import { AccountSetupDialog } from "@/components/clinical-dashboard/account-setup-dialog"; -import { recentQueryStorageKey } from "@/components/clinical-dashboard/dashboard-contracts"; +import { + clearLegacyRecentQueries, + demoRecentQueryOwnerId, + loadRecentQueries, +} from "@/components/clinical-dashboard/recent-query-storage"; import { SearchCommandProvider } from "@/components/clinical-dashboard/search-command-context"; import { SettingsDialog } from "@/components/clinical-dashboard/settings-dialog"; import { @@ -38,6 +42,7 @@ import { visibleAppModeDefinitions, type AppModeId, } from "@/lib/app-modes"; +import { isLocalNoAuthMode } from "@/lib/client-env"; import { documentsSearchHref } from "@/lib/document-flow-routes"; import { modeHomeDesktopComposerSlotId } from "@/lib/mode-home-composer"; import { readSearchNavigationContext, type SearchNavigationOptions } from "@/lib/search-navigation-context"; @@ -297,25 +302,29 @@ function GlobalStandaloneSearchShellClient({ }; }, [pathname, requestedFocus, searchParamString]); + // Recent queries are owner-scoped session state (2026-07-13 audit, finding 4): + // the legacy unscoped localStorage value could resurface another account's + // clinical queries on a shared workstation, so it is deleted, never read. + const recentQueriesOwnerId = + auth.session?.user.id ?? + (!auth.isConfigured || process.env.NEXT_PUBLIC_DEMO_MODE === "true" || isLocalNoAuthMode() + ? demoRecentQueryOwnerId + : null); + + useEffect(() => { + clearLegacyRecentQueries(); + }, []); + useEffect(() => { let cancelled = false; const frame = window.requestAnimationFrame(() => { - try { - const stored = JSON.parse(window.localStorage.getItem(recentQueryStorageKey) ?? "[]"); - if (Array.isArray(stored) && !cancelled) { - setRecentQueries( - stored.filter((item): item is string => typeof item === "string" && Boolean(item.trim())).slice(0, 5), - ); - } - } catch { - if (!cancelled) setRecentQueries([]); - } + if (!cancelled) setRecentQueries(loadRecentQueries(recentQueriesOwnerId)); }); return () => { cancelled = true; window.cancelAnimationFrame(frame); }; - }, []); + }, [recentQueriesOwnerId]); function prefetchApplications() { router.prefetch("/?mode=tools"); diff --git a/src/components/clinical-dashboard/recent-query-storage.ts b/src/components/clinical-dashboard/recent-query-storage.ts new file mode 100644 index 000000000..10cf29bcd --- /dev/null +++ b/src/components/clinical-dashboard/recent-query-storage.ts @@ -0,0 +1,29 @@ +import { recentQueryStorageKey } from "./dashboard-contracts"; + +export const demoRecentQueryOwnerId = "local-demo-session"; + +export function loadRecentQueries(ownerId: string | null): string[] { + if (typeof window === "undefined" || !ownerId) return []; + try { + const stored = JSON.parse(window.sessionStorage.getItem(`${recentQueryStorageKey}:${ownerId}`) ?? "[]"); + if (!Array.isArray(stored)) return []; + return stored.filter((item): item is string => typeof item === "string" && Boolean(item.trim())).slice(0, 5); + } catch { + return []; + } +} + +// The pre-owner-scoping build persisted recent clinical queries under the +// unscoped key, which survived logout, account switching, and demo/ +// authenticated transitions on shared workstations (2026-07-13 audit, +// finding 4). A surviving value's owner cannot be established, so it is +// deleted, never migrated or displayed. +export function clearLegacyRecentQueries() { + if (typeof window === "undefined") return; + try { + window.localStorage.removeItem(recentQueryStorageKey); + window.sessionStorage.removeItem(recentQueryStorageKey); + } catch { + // Recent queries are a convenience only. + } +} diff --git a/tests/recent-query-storage.test.ts b/tests/recent-query-storage.test.ts new file mode 100644 index 000000000..22d30eabe --- /dev/null +++ b/tests/recent-query-storage.test.ts @@ -0,0 +1,78 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { recentQueryStorageKey } from "@/components/clinical-dashboard/dashboard-contracts"; +import { + clearLegacyRecentQueries, + demoRecentQueryOwnerId, + loadRecentQueries, +} from "@/components/clinical-dashboard/recent-query-storage"; + +describe("recent query storage", () => { + let localStore: Map; + let sessionStore: Map; + + beforeEach(() => { + localStore = new Map(); + sessionStore = new Map(); + const storageFor = (store: Map) => ({ + getItem(key: string) { + return store.get(key) ?? null; + }, + setItem(key: string, value: string) { + store.set(key, value); + }, + removeItem(key: string) { + store.delete(key); + }, + }); + vi.stubGlobal("window", { + localStorage: storageFor(localStore), + sessionStorage: storageFor(sessionStore), + }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("reads only the owner-scoped session key", () => { + sessionStore.set(`${recentQueryStorageKey}:user-a`, JSON.stringify(["clozapine monitoring"])); + sessionStore.set(`${recentQueryStorageKey}:user-b`, JSON.stringify(["lithium toxicity"])); + sessionStore.set(recentQueryStorageKey, JSON.stringify(["legacy unscoped query"])); + localStore.set(recentQueryStorageKey, JSON.stringify(["legacy unscoped query"])); + + expect(loadRecentQueries("user-a")).toEqual(["clozapine monitoring"]); + expect(loadRecentQueries("user-b")).toEqual(["lithium toxicity"]); + expect(loadRecentQueries(demoRecentQueryOwnerId)).toEqual([]); + }); + + it("returns nothing without an owner", () => { + localStore.set(recentQueryStorageKey, JSON.stringify(["legacy unscoped query"])); + expect(loadRecentQueries(null)).toEqual([]); + }); + + it("drops malformed and blank entries and caps at five", () => { + sessionStore.set( + `${recentQueryStorageKey}:user-a`, + JSON.stringify(["one", "", " ", 42, { nested: true }, "two", "three", "four", "five", "six"]), + ); + expect(loadRecentQueries("user-a")).toEqual(["one", "two", "three", "four", "five"]); + + sessionStore.set(`${recentQueryStorageKey}:user-a`, "{not json"); + expect(loadRecentQueries("user-a")).toEqual([]); + }); + + it("purges the legacy unscoped keys without migrating them", () => { + // 2026-07-13 audit finding 4: the legacy value's owner cannot be + // established, so it must be deleted, never surfaced under a new owner. + localStore.set(recentQueryStorageKey, JSON.stringify(["legacy cross-user query"])); + sessionStore.set(recentQueryStorageKey, JSON.stringify(["legacy cross-user query"])); + sessionStore.set(`${recentQueryStorageKey}:user-a`, JSON.stringify(["scoped query"])); + + clearLegacyRecentQueries(); + + expect(localStore.has(recentQueryStorageKey)).toBe(false); + expect(sessionStore.has(recentQueryStorageKey)).toBe(false); + expect(loadRecentQueries("user-a")).toEqual(["scoped query"]); + }); +}); diff --git a/tests/ui-smoke.spec.ts b/tests/ui-smoke.spec.ts index 14b63fea9..e5c919329 100644 --- a/tests/ui-smoke.spec.ts +++ b/tests/ui-smoke.spec.ts @@ -1762,6 +1762,30 @@ test.describe("Clinical KB UI smoke coverage", () => { await expectNoPageHorizontalOverflow(page); }); + test("legacy unscoped recent-query storage is purged and never displayed @critical", async ({ page }) => { + // 2026-07-13 audit finding 4: a historical clinical query written by an + // older build into the unscoped localStorage key must not resurface for + // whoever uses the browser next, and must be deleted on load. + await page.setViewportSize({ width: 1280, height: 900 }); + await mockDemoApi(page); + const legacyQuery = "legacy cross-user clozapine query"; + await page.addInitScript( + ({ storageKey, value }) => { + window.localStorage.setItem(storageKey, JSON.stringify([value])); + window.sessionStorage.setItem(storageKey, JSON.stringify([value])); + }, + { storageKey: recentQueryStorageKey, value: legacyQuery }, + ); + await gotoApp(page, "/"); + await waitForDemoDashboardReady(page); + + await expect(page.getByText(legacyQuery)).toHaveCount(0); + await expect.poll(() => page.evaluate((key) => window.localStorage.getItem(key), recentQueryStorageKey)).toBeNull(); + await expect + .poll(() => page.evaluate((key) => window.sessionStorage.getItem(key), recentQueryStorageKey)) + .toBeNull(); + }); + test("answer search URL opens chat without the answer home copy", async ({ page }) => { await page.setViewportSize({ width: 390, height: 820 }); const answerRequests: string[] = []; From a4ad1291862ab410e57f9c7e2837695b362d6f25 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Mon, 13 Jul 2026 19:15:43 +0800 Subject: [PATCH 3/8] fix(upload): validate OOXML and PDF structure at admission Signature checks only proved the first four bytes: any PK-prefixed stream passed as DOCX/XLSX and any %PDF-prefixed stream passed as PDF (2026-07-13 audit, finding 10). Uploads now must be readable ZIP packages with [Content_Types].xml and the expected main part, no macro content (vbaProject.bin or macroEnabled content types), no non-hyperlink external relationships, safe entry paths, and bounded entry count/decompressed size/compression ratio; PDFs must carry an %%EOF trailer. Existing route tests updated to structurally minimal valid PDF fixtures. Co-Authored-By: Claude Fable 5 --- src/app/api/upload/route.ts | 4 +- src/lib/upload-structure.ts | 134 ++++++++++++++++++++ tests/api-validation-contract.test.ts | 6 +- tests/private-access-routes.test.ts | 30 ++--- tests/upload-structure.test.ts | 170 ++++++++++++++++++++++++++ 5 files changed, 325 insertions(+), 19 deletions(-) create mode 100644 src/lib/upload-structure.ts create mode 100644 tests/upload-structure.test.ts diff --git a/src/app/api/upload/route.ts b/src/app/api/upload/route.ts index 6951bfb4b..8ff02fda8 100644 --- a/src/app/api/upload/route.ts +++ b/src/app/api/upload/route.ts @@ -14,6 +14,7 @@ import { publicAccessContext } from "@/lib/public-api-access"; import { probeSupabaseHealth } from "@/lib/supabase/health"; import { optionalFormText, parseFormDataFields } from "@/lib/validation/form-data"; import { acquireUploadAdmission, parseUploadContentLength } from "@/lib/upload-admission"; +import { assertUploadStructure } from "@/lib/upload-structure"; export const runtime = "nodejs"; @@ -166,8 +167,9 @@ export async function POST(request: Request) { assertUploadNotAborted(request); const buffer = Buffer.from(await file.arrayBuffer()); // The declared MIME type is client-supplied; verify the real byte signature - // before persisting a clinical document. + // and the actual document structure before persisting a clinical document. assertFileContentSignature(file.type, buffer); + await assertUploadStructure(file.type, buffer); const contentHash = createHash("sha256").update(buffer).digest("hex"); const { data: duplicate, error: duplicateError } = await adminSupabase diff --git a/src/lib/upload-structure.ts b/src/lib/upload-structure.ts new file mode 100644 index 000000000..fce7de417 --- /dev/null +++ b/src/lib/upload-structure.ts @@ -0,0 +1,134 @@ +import JSZip from "jszip"; + +import { PublicApiError } from "@/lib/http"; + +// Structural admission checks for uploaded binary documents (2026-07-13 +// audit, finding 10). assertFileContentSignature() only proves the first four +// bytes: any ZIP passed as DOCX/XLSX and any byte stream starting "%PDF" was +// accepted. These checks reject mislabeled archives, macro-enabled OOXML, +// external-content relationships, zip bombs, and truncated PDFs before the +// file is persisted or handed to the extraction workers. + +export const maxOoxmlEntries = 2_000; +// OOXML XML parts compress ~10-30x; archive bombs run into the thousands. +export const maxOoxmlCompressionRatio = 150; +export const maxOoxmlDecompressedBytes = 512 * 1024 * 1024; + +const docxMime = "application/vnd.openxmlformats-officedocument.wordprocessingml.document"; +const xlsxMime = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"; + +const ooxmlRequiredPart: Record = { + [docxMime]: { part: "word/document.xml", label: "DOCX" }, + [xlsxMime]: { part: "xl/workbook.xml", label: "XLSX" }, +}; + +function rejectUpload(reason: string): never { + throw new PublicApiError(`File failed structural validation: ${reason}`, 400, { + code: "invalid_file_structure", + }); +} + +function hasUnsafeEntryPath(name: string) { + return name.startsWith("/") || name.includes("\\") || name.split("/").includes(".."); +} + +// Relationships with TargetMode="External" are how OOXML pulls remote or +// out-of-package content. Plain hyperlinks are legitimate in clinical +// documents; everything else (attached templates, OLE objects, remote +// images/frames) is rejected. +function hasDangerousExternalRelationship(relsXml: string) { + const relationshipTags = relsXml.match(/]*>/g) ?? []; + return relationshipTags.some((tag) => { + if (!/TargetMode\s*=\s*"External"/i.test(tag)) return false; + const type = tag.match(/Type\s*=\s*"([^"]*)"/i)?.[1] ?? ""; + return !/\/hyperlink$/i.test(type); + }); +} + +async function assertOoxmlStructure(fileType: string, content: Uint8Array) { + const { part: requiredPart, label } = ooxmlRequiredPart[fileType]; + + const zip = await JSZip.loadAsync(content).catch(() => { + rejectUpload(`the ${label} archive is corrupt or not a readable ZIP package`); + }); + + const entryNames = Object.keys(zip.files); + if (entryNames.length > maxOoxmlEntries) { + rejectUpload(`the ${label} archive contains ${entryNames.length} entries (limit ${maxOoxmlEntries})`); + } + for (const name of entryNames) { + if (hasUnsafeEntryPath(name)) { + rejectUpload(`the ${label} archive contains an unsafe entry path`); + } + if (/(^|\/)vbaProject\.bin$/i.test(name)) { + rejectUpload(`macro-enabled ${label} content is not supported`); + } + } + + // Zip-bomb guard: JSZip records each entry's declared decompressed size at + // load time. Enforce both an absolute cap and a ratio against the upload. + let declaredDecompressedBytes = 0; + for (const entry of Object.values(zip.files)) { + const size = (entry as unknown as { _data?: { uncompressedSize?: number } })._data?.uncompressedSize; + if (typeof size === "number" && Number.isFinite(size) && size > 0) { + declaredDecompressedBytes += size; + } + } + if (declaredDecompressedBytes > maxOoxmlDecompressedBytes) { + rejectUpload(`the ${label} archive would decompress to more than ${maxOoxmlDecompressedBytes} bytes`); + } + if (content.byteLength > 0 && declaredDecompressedBytes / content.byteLength > maxOoxmlCompressionRatio) { + rejectUpload(`the ${label} archive's compression ratio exceeds ${maxOoxmlCompressionRatio}:1`); + } + + const contentTypesEntry = zip.file("[Content_Types].xml"); + if (!contentTypesEntry) { + rejectUpload(`the ${label} package is missing [Content_Types].xml`); + } + const contentTypesXml = await contentTypesEntry.async("text").catch(() => { + rejectUpload(`the ${label} package's [Content_Types].xml is unreadable`); + }); + if (/macroenabled/i.test(contentTypesXml)) { + rejectUpload(`macro-enabled ${label} content is not supported`); + } + if (!zip.file(requiredPart)) { + rejectUpload(`the ${label} package is missing ${requiredPart}`); + } + + const relsEntries = entryNames.filter((name) => /(^|\/)_rels\/[^/]*\.rels$/i.test(name)); + for (const name of relsEntries) { + const relsXml = await zip + .file(name)! + .async("text") + .catch(() => { + rejectUpload(`the ${label} package's relationship part ${name} is unreadable`); + }); + if (hasDangerousExternalRelationship(relsXml)) { + rejectUpload(`the ${label} package references external content (only plain hyperlinks are allowed)`); + } + } +} + +function assertPdfStructure(content: Uint8Array) { + // assertFileContentSignature already proved the %PDF header. A valid PDF + // ends with an %%EOF marker (possibly followed by a few stray bytes); its + // absence means a truncated or corrupt file that would fail parsing later + // and can smuggle non-PDF payloads past the header check. + const tail = Buffer.from(content.subarray(Math.max(0, content.byteLength - 8192))).toString("latin1"); + if (!tail.includes("%%EOF")) { + rejectUpload("the PDF is truncated or corrupt (missing %%EOF trailer)"); + } +} + +// The declared MIME type has already passed assertAllowedFile and +// assertFileContentSignature; this inspects actual document structure. +export async function assertUploadStructure(fileType: string, content: Uint8Array): Promise { + if (fileType === docxMime || fileType === xlsxMime) { + await assertOoxmlStructure(fileType, content); + return; + } + if (fileType === "application/pdf") { + assertPdfStructure(content); + } + // text/plain has no binary structure to validate. +} diff --git a/tests/api-validation-contract.test.ts b/tests/api-validation-contract.test.ts index dce352385..2a0becd58 100644 --- a/tests/api-validation-contract.test.ts +++ b/tests/api-validation-contract.test.ts @@ -467,7 +467,7 @@ describe("API validation contracts", () => { mockRuntime(uploadClient); const uploadRoute = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const uploadResponse = await uploadRoute.POST( authenticatedRequest("/api/upload", { method: "POST", body: formData }), ); @@ -531,7 +531,7 @@ describe("API validation contracts", () => { mockRuntime(client); const { POST } = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); formData.set("title", "x".repeat(181)); const response = await POST(authenticatedRequest("/api/upload", { method: "POST", body: formData })); @@ -548,7 +548,7 @@ describe("API validation contracts", () => { mockRuntime(client); const { POST } = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); formData.set("title", new File(["Guideline"], "title.txt", { type: "text/plain" })); const response = await POST(authenticatedRequest("/api/upload", { method: "POST", body: formData })); diff --git a/tests/private-access-routes.test.ts b/tests/private-access-routes.test.ts index e01528796..7abef9d2d 100644 --- a/tests/private-access-routes.test.ts +++ b/tests/private-access-routes.test.ts @@ -937,7 +937,7 @@ describe("private document API access", () => { mockRuntime(client); const { POST } = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const response = await POST( request("/api/upload", { @@ -967,7 +967,7 @@ describe("private document API access", () => { mockRuntime(client, undefined, { publicUploadsEnabled: true, publicWorkspaceOwnerId: publicOwnerId }); const { POST } = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const response = await POST( request("/api/upload", { @@ -994,7 +994,7 @@ describe("private document API access", () => { mockRuntime(client, undefined, { publicUploadsEnabled: true, publicWorkspaceOwnerId: publicOwnerId }); const { POST } = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const response = await POST( request("/api/upload", { @@ -1113,7 +1113,7 @@ describe("private document API access", () => { signal: controller.signal, }); const form = new FormData(); - form.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + form.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); vi.spyOn(uploadRequest, "formData").mockImplementation(async () => { controller.abort(); return form; @@ -1139,7 +1139,7 @@ describe("private document API access", () => { mockRuntime(client); const { POST } = await import("../src/app/api/upload/route"); const form = new FormData(); - form.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + form.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const response = await POST( authenticatedRequest("/api/upload", { method: "POST", body: form, signal: controller.signal }), @@ -1184,12 +1184,12 @@ describe("private document API access", () => { expect(rejectedFormData).not.toHaveBeenCalled(); const form = new FormData(); - form.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + form.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); resolveForm(form); expect((await firstResponsePromise).status).toBe(201); const afterRelease = new FormData(); - afterRelease.set("file", new File(["%PDF-1.7 revised"], "second.pdf", { type: "application/pdf" })); + afterRelease.set("file", new File(["%PDF-1.7 revised\n%%EOF"], "second.pdf", { type: "application/pdf" })); const afterResponse = await POST(authenticatedRequest("/api/upload", { method: "POST", body: afterRelease })); expect(afterResponse.status).toBe(201); }); @@ -1214,7 +1214,7 @@ describe("private document API access", () => { expect(failedResponse.status).toBe(400); const form = new FormData(); - form.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + form.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const admittedResponse = await POST( authenticatedRequest("/api/upload", { method: "POST", @@ -1240,7 +1240,7 @@ describe("private document API access", () => { mockRuntime(client); const { POST } = await import("../src/app/api/upload/route"); const form = new FormData(); - form.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + form.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const response = await POST( authenticatedRequest("/api/upload", { method: "POST", body: form, signal: controller.signal }), @@ -1267,7 +1267,7 @@ describe("private document API access", () => { mockRuntime(client); const { POST } = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); formData.set("title", "Guideline"); const response = await POST( @@ -1307,7 +1307,7 @@ describe("private document API access", () => { mockRuntime(client); const { POST } = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7 revised"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7 revised\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const response = await POST( authenticatedRequest("/api/upload", { @@ -1340,7 +1340,7 @@ describe("private document API access", () => { mockRuntime(client); const { POST } = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const response = await POST( authenticatedRequest("/api/upload", { @@ -2395,7 +2395,7 @@ describe("private document API access", () => { mockRuntime(client); const { POST } = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const response = await POST( authenticatedRequest("/api/upload", { @@ -2422,7 +2422,7 @@ describe("private document API access", () => { mockRuntime(client); const { POST } = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const response = await POST( authenticatedRequest("/api/upload", { @@ -2465,7 +2465,7 @@ describe("private document API access", () => { mockRuntime(client); const { POST } = await import("../src/app/api/upload/route"); const formData = new FormData(); - formData.set("file", new File(["%PDF-1.7"], "guideline.pdf", { type: "application/pdf" })); + formData.set("file", new File(["%PDF-1.7\n%%EOF"], "guideline.pdf", { type: "application/pdf" })); const response = await POST( authenticatedRequest("/api/upload", { diff --git a/tests/upload-structure.test.ts b/tests/upload-structure.test.ts new file mode 100644 index 000000000..c11089632 --- /dev/null +++ b/tests/upload-structure.test.ts @@ -0,0 +1,170 @@ +import JSZip from "jszip"; +import { describe, expect, it } from "vitest"; + +import { PublicApiError } from "@/lib/http"; +import { assertUploadStructure, maxOoxmlEntries } from "@/lib/upload-structure"; + +const docxMime = "application/vnd.openxmlformats-officedocument.wordprocessingml.document"; +const xlsxMime = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"; + +const docxContentTypes = ` + + + + +`; + +const xlsxContentTypes = ` + + + +`; + +const packageRels = ` + + +`; + +function buildDocxZip(mutate?: (zip: JSZip) => void) { + const zip = new JSZip(); + zip.file("[Content_Types].xml", docxContentTypes); + zip.file("_rels/.rels", packageRels); + zip.file("word/document.xml", ""); + mutate?.(zip); + return zip; +} + +async function toBuffer(zip: JSZip) { + return zip.generateAsync({ type: "nodebuffer", compression: "DEFLATE" }); +} + +async function expectRejection(promise: Promise, messagePart: string) { + const error = await promise.then( + () => null, + (cause) => cause, + ); + expect(error, `expected structural rejection containing "${messagePart}"`).toBeInstanceOf(PublicApiError); + expect(String((error as PublicApiError).message)).toContain(messagePart); +} + +describe("assertUploadStructure — OOXML", () => { + it("accepts a well-formed DOCX package", async () => { + await expect(assertUploadStructure(docxMime, await toBuffer(buildDocxZip()))).resolves.toBeUndefined(); + }); + + it("accepts a well-formed XLSX package", async () => { + const zip = new JSZip(); + zip.file("[Content_Types].xml", xlsxContentTypes); + zip.file("_rels/.rels", packageRels); + zip.file("xl/workbook.xml", ""); + await expect(assertUploadStructure(xlsxMime, await toBuffer(zip))).resolves.toBeUndefined(); + }); + + it("rejects a non-ZIP byte stream", async () => { + await expectRejection( + assertUploadStructure(docxMime, Buffer.from("PK not really a zip")), + "corrupt or not a readable ZIP", + ); + }); + + it("rejects a ZIP that is missing [Content_Types].xml", async () => { + const zip = new JSZip(); + zip.file("word/document.xml", ""); + await expectRejection(assertUploadStructure(docxMime, await toBuffer(zip)), "missing [Content_Types].xml"); + }); + + it("rejects a DOCX without word/document.xml and an XLSX without xl/workbook.xml", async () => { + const noDocument = new JSZip(); + noDocument.file("[Content_Types].xml", docxContentTypes); + await expectRejection(assertUploadStructure(docxMime, await toBuffer(noDocument)), "missing word/document.xml"); + + const noWorkbook = new JSZip(); + noWorkbook.file("[Content_Types].xml", xlsxContentTypes); + await expectRejection(assertUploadStructure(xlsxMime, await toBuffer(noWorkbook)), "missing xl/workbook.xml"); + }); + + it("rejects macro-enabled content by content type and by vbaProject part", async () => { + const macroTypes = buildDocxZip((zip) => { + zip.file( + "[Content_Types].xml", + docxContentTypes.replace( + "wordprocessingml.document.main+xml", + "wordprocessingml.document.macroEnabled.main+xml", + ), + ); + }); + await expectRejection(assertUploadStructure(docxMime, await toBuffer(macroTypes)), "macro-enabled"); + + const vbaPart = buildDocxZip((zip) => { + zip.file("word/vbaProject.bin", Buffer.from([0xd0, 0xcf, 0x11, 0xe0])); + }); + await expectRejection(assertUploadStructure(docxMime, await toBuffer(vbaPart)), "macro-enabled"); + }); + + it("rejects dangerous external relationships but allows plain hyperlinks", async () => { + const withHyperlink = buildDocxZip((zip) => { + zip.file( + "word/_rels/document.xml.rels", + ` + + +`, + ); + }); + await expect(assertUploadStructure(docxMime, await toBuffer(withHyperlink))).resolves.toBeUndefined(); + + const withRemoteTemplate = buildDocxZip((zip) => { + zip.file( + "word/_rels/settings.xml.rels", + ` + + +`, + ); + }); + await expectRejection( + assertUploadStructure(docxMime, await toBuffer(withRemoteTemplate)), + "references external content", + ); + }); + + it("rejects unsafe entry paths", async () => { + const traversal = buildDocxZip((zip) => { + zip.file("word/../../escape.xml", ""); + }); + await expectRejection(assertUploadStructure(docxMime, await toBuffer(traversal)), "unsafe entry path"); + }); + + it("rejects archives with too many entries", async () => { + const zip = buildDocxZip((bomb) => { + for (let index = 0; index <= maxOoxmlEntries; index += 1) { + bomb.file(`word/media/pad-${index}.xml`, ""); + } + }); + await expectRejection(assertUploadStructure(docxMime, await toBuffer(zip)), "entries (limit"); + }); + + it("rejects high-compression-ratio archives (zip bomb shape)", async () => { + const zip = buildDocxZip((bomb) => { + // 24MB of zeros deflates to a few KB — far beyond the allowed ratio. + bomb.file("word/media/zeros.bin", Buffer.alloc(24 * 1024 * 1024)); + }); + await expectRejection(assertUploadStructure(docxMime, await toBuffer(zip)), "compression ratio"); + }); +}); + +describe("assertUploadStructure — PDF and text", () => { + it("accepts a PDF with an %%EOF trailer", async () => { + const pdf = Buffer.from("%PDF-1.7\n1 0 obj\n<<>>\nendobj\ntrailer\n<<>>\nstartxref\n9\n%%EOF\n"); + await expect(assertUploadStructure("application/pdf", pdf)).resolves.toBeUndefined(); + }); + + it("rejects a truncated PDF without %%EOF", async () => { + const pdf = Buffer.from("%PDF-1.7\n1 0 obj\n<<>>\nendobj\n"); + await expectRejection(assertUploadStructure("application/pdf", pdf), "missing %%EOF"); + }); + + it("leaves text uploads untouched", async () => { + await expect(assertUploadStructure("text/plain", Buffer.from("plain notes"))).resolves.toBeUndefined(); + }); +}); From ff4ce29665780fe5562269f0376244423e313650 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Mon, 13 Jul 2026 19:15:43 +0800 Subject: [PATCH 4/8] fix(security): drop api.openai.com from CSP connect-src OpenAI calls are server-side only; allowing the browser to reach the provider origin needlessly widened the exfiltration surface (2026-07-13 audit, finding 12). Tests now assert the provider origin stays out. Co-Authored-By: Claude Fable 5 --- src/lib/security-headers.ts | 5 +++-- tests/proxy.test.ts | 5 ++++- tests/security-headers.test.ts | 3 +++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/lib/security-headers.ts b/src/lib/security-headers.ts index 42777bcb6..76cf2900b 100644 --- a/src/lib/security-headers.ts +++ b/src/lib/security-headers.ts @@ -73,10 +73,11 @@ export function buildContentSecurityPolicy({ // img-src/media-src are scoped to the Supabase Storage origin that serves the // signed-URL images (document pages) — the app loads no other cross-origin // media, so a bare `https:` would needlessly widen the exfil surface. connect-src - // must include *.supabase.co for the signed-URL/API fetches. + // must include *.supabase.co for the signed-URL/API fetches. OpenAI calls are + // server-side only, so the browser gets no provider origin (2026-07-13 audit). "img-src 'self' data: blob: https://*.supabase.co; " + "media-src 'self' https://*.supabase.co; " + - "connect-src 'self' https://*.supabase.co https://api.openai.com; " + + "connect-src 'self' https://*.supabase.co; " + scriptSrc + "style-src 'self' 'unsafe-inline'" ); diff --git a/tests/proxy.test.ts b/tests/proxy.test.ts index b5b6152f0..fead5901c 100644 --- a/tests/proxy.test.ts +++ b/tests/proxy.test.ts @@ -45,7 +45,10 @@ describe("proxy content-security-policy", () => { expect(csp).toContain("object-src 'none'"); expect(csp).toContain("frame-ancestors 'none'"); expect(csp).toContain("img-src 'self' data: blob: https://*.supabase.co"); - expect(csp).toContain("connect-src 'self' https://*.supabase.co https://api.openai.com"); + expect(csp).toContain("connect-src 'self' https://*.supabase.co;"); + // OpenAI calls are server-side only; the browser must not be allowed to + // reach the provider origin (2026-07-13 audit, finding 12). + expect(csp).not.toContain("api.openai.com"); expect(csp).toContain("style-src 'self' 'unsafe-inline'"); }); diff --git a/tests/security-headers.test.ts b/tests/security-headers.test.ts index b1273bd0b..674f6adb2 100644 --- a/tests/security-headers.test.ts +++ b/tests/security-headers.test.ts @@ -43,6 +43,9 @@ describe("security headers", () => { const connectSrc = csp.split(";").find((directive) => directive.trim().startsWith("connect-src")); expect(connectSrc).toBeDefined(); expect(connectSrc).toContain("https://*.supabase.co"); + // OpenAI calls are server-side only; the browser must not be allowed + // to reach the provider origin (2026-07-13 audit, finding 12). + expect(connectSrc).not.toContain("api.openai.com"); }); it("keeps the baseline hardening headers", () => { From b89f915102a043ccb0df690a34f57f4a494db344 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Mon, 13 Jul 2026 19:15:44 +0800 Subject: [PATCH 5/8] fix(a11y): explicit accessible names for tool-card buttons The split-pane tool cards and wide-section rows exposed only concatenated card text as their accessible name while their Link siblings carried explicit labels (2026-07-13 audit, finding 8 sub-item). Button variants now carry "Preview " labels; the collapse spec asserts the explicit name. Co-Authored-By: Claude Fable 5 --- .../split-pane-refined-mockups.tsx | 1 + .../tools-page-mockup-page.tsx | 18 +++++++++++++++--- tests/ui-tools-collapse.spec.ts | 4 +++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/components/tools-page-mockups/split-pane-refined-mockups.tsx b/src/components/tools-page-mockups/split-pane-refined-mockups.tsx index 58f2a5d4a..8019d1026 100644 --- a/src/components/tools-page-mockups/split-pane-refined-mockups.tsx +++ b/src/components/tools-page-mockups/split-pane-refined-mockups.tsx @@ -215,6 +215,7 @@ function ToolList({ ); @@ -731,14 +737,20 @@ function PhoneBrowserPreview({ if (interactive) { return ( - ); } return ( - + {rowContent} ); diff --git a/tests/ui-tools-collapse.spec.ts b/tests/ui-tools-collapse.spec.ts index 20a690605..b174aea09 100644 --- a/tests/ui-tools-collapse.spec.ts +++ b/tests/ui-tools-collapse.spec.ts @@ -50,6 +50,8 @@ test.describe("Tools mockups collapse the primary region when filtering", () => await page.getByRole("searchbox").first().fill("medication"); await expect(page.getByRole("heading", { name: "Launcher overview" })).toHaveCount(0); await expect(page.getByRole("heading", { name: "Results" })).toBeVisible(); - await expect(page.getByRole("button", { name: /Medication Prescribing/ })).toBeVisible(); + // The split-pane card is a button; it must expose an explicit accessible + // action name, not just its concatenated card text (2026-07-13 audit). + await expect(page.getByRole("button", { name: /^Preview Medication Prescribing/ })).toBeVisible(); }); }); From fb3e3412d1073bdf016bdad472c67662a70f3307 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 13 Jul 2026 13:12:28 +0000 Subject: [PATCH 6/8] fix: apply CodeRabbit auto-fixes Fixed 6 file(s) based on 7 unresolved review comments. Co-authored-by: CodeRabbit --- docs/process-hardening.md | 2 +- src/lib/upload-structure.ts | 41 ++++++++++++------- ...voke_supabase_admin_default_privileges.sql | 1 + ...0713103000_scrub_legacy_rag_query_text.sql | 38 +++++++++-------- supabase/schema.sql | 1 + tests/upload-structure.test.ts | 38 +++++++++++++++++ 6 files changed, 90 insertions(+), 31 deletions(-) diff --git a/docs/process-hardening.md b/docs/process-hardening.md index d14e47859..7091dae4e 100644 --- a/docs/process-hardening.md +++ b/docs/process-hardening.md @@ -313,5 +313,5 @@ hybrid:10}`, all 10 forced-embedding vector cases passed (`force_embedding_failu - **Lexical retrieval rewrite (audit finding 1):** `20260713100000_index_friendly_lexical_retrieval.sql` splits `match_document_chunks_text`'s OR-across-relations candidate search into two GIN-index probes unioned by chunk id (same contract, same scores; the `_v2` wrapper inherits the speedup). Parity + plan + timing harness: `scripts/sql/lexical-rpc-parity-check.sql` (scratch databases only; run it against the drift-manifest container kept with `--keep`). Re-run it whenever either lexical body changes. - **supabase_admin default privileges (audit finding 7, operator caveat):** `20260713102000_revoke_supabase_admin_default_privileges.sql` revokes anon/authenticated future-object defaults for role `supabase_admin` and probes the lockdown with future-object creation. On hosted Supabase the migration degrades to a WARNING if `postgres` cannot act for `supabase_admin`; **operator follow-up:** watch for that warning during live apply and, if present, run the six `ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin` statements once via the dashboard SQL editor, then re-run the migration's probe block. -- **Legacy rag query text scrub (audit finding 5):** `20260713103000_scrub_legacy_rag_query_text.sql` replaces pre-HMAC plaintext in `rag_queries.query`/`normalized_query` and `rag_query_misses` with salted `redacted-query:legacy:` placeholders. **Operator verification after live apply:** count rows not matching `redacted-query:%` (expect 0 unless `RAG_PERSIST_RAW_QUERY_TEXT` is deliberately enabled). +- **Legacy rag query text scrub (audit finding 5):** `20260713103000_scrub_legacy_rag_query_text.sql` performs four redaction/deletion operations for pre-HMAC plaintext query text: (1) scrubs `rag_queries.query` rows not matching `redacted-query:%`, replacing them with salted `redacted-query:legacy:` placeholders; (2) scrubs both `rag_query_misses.query` and `rag_query_misses.normalized_query` not matching `redacted-query:%`; (3) scrubs both `rag_retrieval_logs.query` and `rag_retrieval_logs.normalized_query` not matching `redacted-query:%` (nullable); (4) deletes `rag_response_cache` rows where `normalized_query` does not match `redacted-cache:%` (cache entries, not re-keyed). **Operator verification after live apply:** for each affected table/operation, count rows not matching the expected redacted pattern (expect 0 unless `RAG_PERSIST_RAW_QUERY_TEXT` is deliberately enabled): `select count(*) from rag_queries where query not like 'redacted-query:%';` (expect 0), `select count(*) from rag_query_misses where query not like 'redacted-query:%' or normalized_query not like 'redacted-query:%';` (expect 0), `select count(*) from rag_retrieval_logs where query not like 'redacted-query:%' or (normalized_query is not null and normalized_query not like 'redacted-query:%');` (expect 0), `select count(*) from rag_response_cache where normalized_query not like 'redacted-cache:%';` (expect 0). The migration includes a post-apply assertion block that enforces these exact checks and fails the migration if any unscrubbed/undeleted rows remain. - Remaining operator items from the 2026-07-13 audit (confirmation-required, not automated): apply this batch's migrations live, re-run Supabase advisors, trigger the Eval Canary and require two consecutive green scheduled runs, repair the invalid `storage.idx_objects_bucket_id_name_lower` index via dashboard/support, and dedupe response-cache purge cron jobs if `cron.job` shows duplicates. diff --git a/src/lib/upload-structure.ts b/src/lib/upload-structure.ts index fce7de417..701e04b98 100644 --- a/src/lib/upload-structure.ts +++ b/src/lib/upload-structure.ts @@ -37,10 +37,14 @@ function hasUnsafeEntryPath(name: string) { // documents; everything else (attached templates, OLE objects, remote // images/frames) is rejected. function hasDangerousExternalRelationship(relsXml: string) { - const relationshipTags = relsXml.match(/]*>/g) ?? []; + // Match Relationship tags regardless of namespace prefix (e.g., or ) + // and handle both single- and double-quoted attribute values + const relationshipTags = relsXml.match(/<(?:\w+:)?Relationship\b[^>]*>/g) ?? []; return relationshipTags.some((tag) => { - if (!/TargetMode\s*=\s*"External"/i.test(tag)) return false; - const type = tag.match(/Type\s*=\s*"([^"]*)"/i)?.[1] ?? ""; + // Match TargetMode="External" or TargetMode='External' (case-insensitive) + if (!/TargetMode\s*=\s*["']External["']/i.test(tag)) return false; + // Match Type attribute with either single or double quotes + const type = tag.match(/Type\s*=\s*["']([^"']*)["']/i)?.[1] ?? ""; return !/\/hyperlink$/i.test(type); }); } @@ -52,15 +56,17 @@ async function assertOoxmlStructure(fileType: string, content: Uint8Array) { rejectUpload(`the ${label} archive is corrupt or not a readable ZIP package`); }); - const entryNames = Object.keys(zip.files); - if (entryNames.length > maxOoxmlEntries) { - rejectUpload(`the ${label} archive contains ${entryNames.length} entries (limit ${maxOoxmlEntries})`); + const entries = Object.values(zip.files); + if (entries.length > maxOoxmlEntries) { + rejectUpload(`the ${label} archive contains ${entries.length} entries (limit ${maxOoxmlEntries})`); } - for (const name of entryNames) { - if (hasUnsafeEntryPath(name)) { + for (const entry of entries) { + // Validate the original/unsafe name if available, falling back to sanitized name + const nameToValidate = (entry as unknown as { unsafeOriginalName?: string }).unsafeOriginalName ?? entry.name; + if (hasUnsafeEntryPath(nameToValidate)) { rejectUpload(`the ${label} archive contains an unsafe entry path`); } - if (/(^|\/)vbaProject\.bin$/i.test(name)) { + if (/(^|\/)vbaProject\.bin$/i.test(nameToValidate)) { rejectUpload(`macro-enabled ${label} content is not supported`); } } @@ -91,10 +97,16 @@ async function assertOoxmlStructure(fileType: string, content: Uint8Array) { if (/macroenabled/i.test(contentTypesXml)) { rejectUpload(`macro-enabled ${label} content is not supported`); } - if (!zip.file(requiredPart)) { + const requiredPartEntry = zip.file(requiredPart); + if (!requiredPartEntry) { rejectUpload(`the ${label} package is missing ${requiredPart}`); } + // Attempt to read/decompress the required part; reject if it fails + await requiredPartEntry.async("text").catch(() => { + rejectUpload(`the ${label} package's ${requiredPart} is unreadable or corrupt`); + }); + const entryNames = Object.keys(zip.files); const relsEntries = entryNames.filter((name) => /(^|\/)_rels\/[^/]*\.rels$/i.test(name)); for (const name of relsEntries) { const relsXml = await zip @@ -111,11 +123,12 @@ async function assertOoxmlStructure(fileType: string, content: Uint8Array) { function assertPdfStructure(content: Uint8Array) { // assertFileContentSignature already proved the %PDF header. A valid PDF - // ends with an %%EOF marker (possibly followed by a few stray bytes); its - // absence means a truncated or corrupt file that would fail parsing later - // and can smuggle non-PDF payloads past the header check. + // ends with an %%EOF marker (possibly followed by whitespace); its + // absence or misplacement means a truncated or corrupt file that would fail + // parsing later and can smuggle non-PDF payloads past the header check. const tail = Buffer.from(content.subarray(Math.max(0, content.byteLength - 8192))).toString("latin1"); - if (!tail.includes("%%EOF")) { + const trimmedTail = tail.trimEnd(); + if (!trimmedTail.endsWith("%%EOF")) { rejectUpload("the PDF is truncated or corrupt (missing %%EOF trailer)"); } } diff --git a/supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql b/supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql index bfe8a462e..045444b9d 100644 --- a/supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql +++ b/supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql @@ -42,6 +42,7 @@ begin raise warning 'cannot alter default privileges for supabase_admin as %; ' 'operator follow-up required: run the six ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin ' 'statements from migration 20260713102000 via the Supabase dashboard SQL editor', current_user; + raise; end; end $$; diff --git a/supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql b/supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql index 9c7f3bb23..39332903e 100644 --- a/supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql +++ b/supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql @@ -16,39 +16,45 @@ -- -- rag_response_cache rows are a cache: legacy raw-keyed entries are deleted -- outright (worst case is a cache miss) rather than re-keyed. +-- +-- Valid redacted token format (from src/lib/query-privacy.ts): +-- - redacted-query:<64-char-hex-hash> (SHA-256/HMAC-SHA256, e.g., redacted-query:a1b2c3...) +-- - redacted-cache:<64-char-hex-hash> +-- Only values matching the complete token shape are preserved; prefix-only matches +-- that don't match the full format are treated as legacy plaintext and scrubbed. update public.rag_queries set query = 'redacted-query:legacy:' || md5(gen_random_uuid()::text || query) -where query not like 'redacted-query:%'; +where query !~ '^redacted-query:[0-9a-f]{64}$'; update public.rag_query_misses set query = case - when query like 'redacted-query:%' then query + when query ~ '^redacted-query:[0-9a-f]{64}$' then query else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || query) end, normalized_query = case - when normalized_query like 'redacted-query:%' then normalized_query + when normalized_query ~ '^redacted-query:[0-9a-f]{64}$' then normalized_query else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || normalized_query) end -where query not like 'redacted-query:%' - or normalized_query not like 'redacted-query:%'; +where query !~ '^redacted-query:[0-9a-f]{64}$' + or normalized_query !~ '^redacted-query:[0-9a-f]{64}$'; update public.rag_retrieval_logs set query = case - when query like 'redacted-query:%' then query + when query ~ '^redacted-query:[0-9a-f]{64}$' then query else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || query) end, normalized_query = case - when normalized_query is null or normalized_query like 'redacted-query:%' then normalized_query + when normalized_query is null or normalized_query ~ '^redacted-query:[0-9a-f]{64}$' then normalized_query else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || normalized_query) end -where query not like 'redacted-query:%' - or (normalized_query is not null and normalized_query not like 'redacted-query:%'); +where query !~ '^redacted-query:[0-9a-f]{64}$' + or (normalized_query is not null and normalized_query !~ '^redacted-query:[0-9a-f]{64}$'); delete from public.rag_response_cache -where normalized_query not like 'redacted-cache:%'; +where normalized_query !~ '^redacted-cache:[0-9a-f]{64}$'; do $$ declare @@ -59,21 +65,21 @@ declare begin select count(*) into bad_queries from public.rag_queries - where query not like 'redacted-query:%'; + where query !~ '^redacted-query:[0-9a-f]{64}$'; select count(*) into bad_misses from public.rag_query_misses - where query not like 'redacted-query:%' - or normalized_query not like 'redacted-query:%'; + where query !~ '^redacted-query:[0-9a-f]{64}$' + or normalized_query !~ '^redacted-query:[0-9a-f]{64}$'; select count(*) into bad_logs from public.rag_retrieval_logs - where query not like 'redacted-query:%' - or (normalized_query is not null and normalized_query not like 'redacted-query:%'); + where query !~ '^redacted-query:[0-9a-f]{64}$' + or (normalized_query is not null and normalized_query !~ '^redacted-query:[0-9a-f]{64}$'); select count(*) into bad_cache from public.rag_response_cache - where normalized_query not like 'redacted-cache:%'; + where normalized_query !~ '^redacted-cache:[0-9a-f]{64}$'; if bad_queries + bad_misses + bad_logs + bad_cache > 0 then raise exception diff --git a/supabase/schema.sql b/supabase/schema.sql index e93d75169..b402778be 100644 --- a/supabase/schema.sql +++ b/supabase/schema.sql @@ -4946,6 +4946,7 @@ begin raise warning 'cannot alter default privileges for supabase_admin as %; ' 'operator follow-up required: run the six ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin ' 'statements from migration 20260713102000 via the Supabase dashboard SQL editor', current_user; + raise; end; end $$; diff --git a/tests/upload-structure.test.ts b/tests/upload-structure.test.ts index c11089632..ff0d78c3d 100644 --- a/tests/upload-structure.test.ts +++ b/tests/upload-structure.test.ts @@ -128,6 +128,38 @@ describe("assertUploadStructure — OOXML", () => { ); }); + it("rejects external relationships with namespace prefixes and single-quoted attributes", async () => { + // Test namespace-prefixed Relationship element with single-quoted TargetMode + const withPrefixedTemplate = buildDocxZip((zip) => { + zip.file( + "word/_rels/settings.xml.rels", + ` + + +`, + ); + }); + await expectRejection( + assertUploadStructure(docxMime, await toBuffer(withPrefixedTemplate)), + "references external content", + ); + + // Test mixed single/double quotes with namespace prefix + const withMixedQuotes = buildDocxZip((zip) => { + zip.file( + "word/_rels/settings.xml.rels", + ` + + +`, + ); + }); + await expectRejection( + assertUploadStructure(docxMime, await toBuffer(withMixedQuotes)), + "references external content", + ); + }); + it("rejects unsafe entry paths", async () => { const traversal = buildDocxZip((zip) => { zip.file("word/../../escape.xml", ""); @@ -164,6 +196,12 @@ describe("assertUploadStructure — PDF and text", () => { await expectRejection(assertUploadStructure("application/pdf", pdf), "missing %%EOF"); }); + it("rejects a PDF where %%EOF appears but is followed by non-whitespace content", async () => { + // Simulate %%EOF appearing inside an incomplete object (non-whitespace after %%EOF) + const pdf = Buffer.from("%PDF-1.7\n1 0 obj\n<<>>\nendobj\ntrailer\n<<>>\nstartxref\n9\n%%EOF\n2 0 obj"); + await expectRejection(assertUploadStructure("application/pdf", pdf), "missing %%EOF"); + }); + it("leaves text uploads untouched", async () => { await expect(assertUploadStructure("text/plain", Buffer.from("plain notes"))).resolves.toBeUndefined(); }); From 53672e26093ee1fa3c15d76012ad3f4c32011574 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Mon, 13 Jul 2026 21:30:31 +0800 Subject: [PATCH 7/8] Update supabase/schema.sql Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- supabase/schema.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/supabase/schema.sql b/supabase/schema.sql index b402778be..96e7d2480 100644 --- a/supabase/schema.sql +++ b/supabase/schema.sql @@ -4947,6 +4947,7 @@ begin 'operator follow-up required: run the six ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin ' 'statements from migration 20260713102000 via the Supabase dashboard SQL editor', current_user; raise; + raise; end; end $$; From 547ece255e7782c73b3871bcbd8cf010729267f0 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Mon, 13 Jul 2026 21:36:00 +0800 Subject: [PATCH 8/8] fix(db): correct CodeRabbit auto-fix regressions in audit migrations The auto-fix commit introduced three defects: - 20260713102000 re-raised insufficient_privilege after the warning, turning the deliberate hosted degradation into a hard abort of the migration chain (schema.sql mirror too). Reverted with a comment stating the invariant, and the schema test now rejects any bare re-raise in the guard. - 20260713103000's strict '^redacted-query:[0-9a-f]{64}$' check did not accept the migration's own 'redacted-query:legacy:' placeholders, so the completion assertion aborted on the very rows it had just scrubbed. The format check now accepts both valid shapes and stays idempotent on re-run. - schema.sql changed without regenerating the drift manifest, failing the sha-pinned drift-detection test; the revert restores the manifest match. Kept: the upload-structure hardening (namespaced Relationship tags, quote variants, unsafeOriginalName, required-part decompression check) and its new tests, and the expanded scrub documentation. Co-Authored-By: Claude Fable 5 --- ...voke_supabase_admin_default_privileges.sql | 4 +- ...0713103000_scrub_legacy_rag_query_text.sql | 41 ++++++++++--------- supabase/schema.sql | 1 - tests/supabase-schema.test.ts | 11 ++++- 4 files changed, 34 insertions(+), 23 deletions(-) diff --git a/supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql b/supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql index 045444b9d..1aab2fe66 100644 --- a/supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql +++ b/supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql @@ -39,10 +39,12 @@ begin raise notice 'supabase_admin future-object default privileges hardened'; exception when insufficient_privilege then + -- Deliberately no re-raise: on hosted Supabase `postgres` may not be able + -- to act for supabase_admin, and this hardening must degrade to the + -- documented operator follow-up instead of aborting the migration chain. raise warning 'cannot alter default privileges for supabase_admin as %; ' 'operator follow-up required: run the six ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin ' 'statements from migration 20260713102000 via the Supabase dashboard SQL editor', current_user; - raise; end; end $$; diff --git a/supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql b/supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql index 39332903e..8ec202543 100644 --- a/supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql +++ b/supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql @@ -17,41 +17,44 @@ -- rag_response_cache rows are a cache: legacy raw-keyed entries are deleted -- outright (worst case is a cache miss) rather than re-keyed. -- --- Valid redacted token format (from src/lib/query-privacy.ts): --- - redacted-query:<64-char-hex-hash> (SHA-256/HMAC-SHA256, e.g., redacted-query:a1b2c3...) --- - redacted-cache:<64-char-hex-hash> --- Only values matching the complete token shape are preserved; prefix-only matches --- that don't match the full format are treated as legacy plaintext and scrubbed. +-- Valid redacted token formats: +-- - redacted-query:<64-char-hex> (HMAC-SHA256 from src/lib/query-privacy.ts) +-- - redacted-query:legacy:<32-char-hex> (this migration's salted placeholders, +-- so re-running the migration is a no-op on already-scrubbed rows) +-- - redacted-cache:<64-char-hex> +-- Only values matching a complete token shape are preserved; prefix-only +-- matches that don't match the full format are treated as legacy plaintext +-- and scrubbed. update public.rag_queries set query = 'redacted-query:legacy:' || md5(gen_random_uuid()::text || query) -where query !~ '^redacted-query:[0-9a-f]{64}$'; +where query !~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$'; update public.rag_query_misses set query = case - when query ~ '^redacted-query:[0-9a-f]{64}$' then query + when query ~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$' then query else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || query) end, normalized_query = case - when normalized_query ~ '^redacted-query:[0-9a-f]{64}$' then normalized_query + when normalized_query ~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$' then normalized_query else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || normalized_query) end -where query !~ '^redacted-query:[0-9a-f]{64}$' - or normalized_query !~ '^redacted-query:[0-9a-f]{64}$'; +where query !~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$' + or normalized_query !~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$'; update public.rag_retrieval_logs set query = case - when query ~ '^redacted-query:[0-9a-f]{64}$' then query + when query ~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$' then query else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || query) end, normalized_query = case - when normalized_query is null or normalized_query ~ '^redacted-query:[0-9a-f]{64}$' then normalized_query + when normalized_query is null or normalized_query ~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$' then normalized_query else 'redacted-query:legacy:' || md5(gen_random_uuid()::text || normalized_query) end -where query !~ '^redacted-query:[0-9a-f]{64}$' - or (normalized_query is not null and normalized_query !~ '^redacted-query:[0-9a-f]{64}$'); +where query !~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$' + or (normalized_query is not null and normalized_query !~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$'); delete from public.rag_response_cache where normalized_query !~ '^redacted-cache:[0-9a-f]{64}$'; @@ -65,17 +68,17 @@ declare begin select count(*) into bad_queries from public.rag_queries - where query !~ '^redacted-query:[0-9a-f]{64}$'; + where query !~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$'; select count(*) into bad_misses from public.rag_query_misses - where query !~ '^redacted-query:[0-9a-f]{64}$' - or normalized_query !~ '^redacted-query:[0-9a-f]{64}$'; + where query !~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$' + or normalized_query !~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$'; select count(*) into bad_logs from public.rag_retrieval_logs - where query !~ '^redacted-query:[0-9a-f]{64}$' - or (normalized_query is not null and normalized_query !~ '^redacted-query:[0-9a-f]{64}$'); + where query !~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$' + or (normalized_query is not null and normalized_query !~ '^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$'); select count(*) into bad_cache from public.rag_response_cache diff --git a/supabase/schema.sql b/supabase/schema.sql index b402778be..e93d75169 100644 --- a/supabase/schema.sql +++ b/supabase/schema.sql @@ -4946,7 +4946,6 @@ begin raise warning 'cannot alter default privileges for supabase_admin as %; ' 'operator follow-up required: run the six ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin ' 'statements from migration 20260713102000 via the Supabase dashboard SQL editor', current_user; - raise; end; end $$; diff --git a/tests/supabase-schema.test.ts b/tests/supabase-schema.test.ts index b9c7086e8..abf191e24 100644 --- a/tests/supabase-schema.test.ts +++ b/tests/supabase-schema.test.ts @@ -1046,8 +1046,11 @@ describe("Supabase Preview replay guards", () => { expect(sql).toContain( "alter default privileges for role supabase_admin in schema public grant execute on functions to service_role;", ); - // Guarded: only a superuser or member of supabase_admin may alter its defaults. + // Guarded: only a superuser or member of supabase_admin may alter its + // defaults. The guard must DEGRADE to a warning, never re-raise — a + // re-raise would abort the whole migration chain on hosted Supabase. expect(sql).toContain("when insufficient_privilege then"); + expect(sql).not.toContain("raise;"); } // The migration also proves the lockdown with future-object probes. expect(supabaseAdminDefaultPrivilegesMigration).toContain("_defacl_probe_table"); @@ -1076,8 +1079,12 @@ describe("Supabase Preview replay guards", () => { } // Cache rows are deleted, not re-keyed: a scrubbed key would never be hit again. expect(scrubLegacyQueryTextMigration).toContain("delete from public.rag_response_cache"); - expect(scrubLegacyQueryTextMigration).toContain("where normalized_query not like 'redacted-cache:%'"); + expect(scrubLegacyQueryTextMigration).toContain("where normalized_query !~ '^redacted-cache:[0-9a-f]{64}$'"); expect(scrubLegacyQueryTextMigration).toContain("raise exception"); + // The strict format check must accept this migration's own salted legacy + // placeholders, or the completion assertion aborts on the rows it just wrote. + expect(scrubLegacyQueryTextMigration).toContain("'^redacted-query:([0-9a-f]{64}|legacy:[0-9a-f]{32})$'"); + expect(scrubLegacyQueryTextMigration).not.toContain("'^redacted-query:[0-9a-f]{64}$'"); }); it("validates the content_not_blank guards so they are no longer NOT VALID", () => {