diff --git a/docs/privacy-impact-assessment.md b/docs/privacy-impact-assessment.md new file mode 100644 index 000000000..fae5b515f --- /dev/null +++ b/docs/privacy-impact-assessment.md @@ -0,0 +1,398 @@ +# Privacy Impact Assessment — Clinical KB Database + +**Status:** Draft for review · **Date:** 2026-07-06 · **Branch:** `claude/privacy-tenancy-review` +**Scope:** Clinical data flows through the Clinical KB app (Next.js + Supabase + OpenAI), the live Supabase project `Clinical KB Database` (`sjrfecxgysukkwxsowpy`), and the WA private-clinical deployment context. +**Author:** Automated code-level assessment (multi-agent audit of `src/app/api/**`, `src/lib/*`, `supabase/schema.sql`, `supabase/migrations/**`), cross-checked against the live database. + +> **This is not legal advice.** It is a technical privacy assessment written to be handed to a +> privacy officer / legal reviewer. Statements about the _Privacy Act 1988_ (Cth), the Australian +> Privacy Principles (APPs) and WA health-records obligations are engineering interpretations that +> must be confirmed by a qualified adviser before the app is used with real patients. + +--- + +## 1. Executive summary + +The app is a **clinical knowledge base** — it indexes clinical reference material (guidelines, +drug monographs, protocols) and answers clinician questions over that corpus with retrieval-augmented +generation. It is **not** a patient record system and, by design, does not ask for patient data. + +The dominant privacy risk is therefore **incidental PHI**: a clinician will inevitably type patient +details into a free-text query ("42yo F on clozapine 400mg with rising WCC, next step?"). That query +text is (a) sent to OpenAI in the United States, and (b) written to log tables in Supabase. A secondary +risk is PHI inside **uploaded documents** if users upload anything other than published reference +material. + +**What is already good:** + +- **Data residency**: the Supabase project runs in **`ap-southeast-2` (AWS Sydney, Australia)** — + clinical data at rest stays onshore. Confirmed live via the Supabase API (project region + `ap-southeast-2`). +- **Query redaction**: raw query text is **not** persisted by default. Every log write goes through + `queryTextForStorage()` which stores a hash placeholder unless `RAG_PERSIST_RAW_QUERY_TEXT=true` + ([src/lib/query-privacy.ts:33](src/lib/query-privacy.ts)). +- **The M15 HMAC fix is present** ([src/lib/query-privacy.ts:17-23](src/lib/query-privacy.ts)) — the + stored hash is a keyed HMAC-SHA256 pseudonym **when `RAG_QUERY_HASH_SECRET` is set** (see gap PIA-2). +- **Retention is automated**: nightly `pg_cron` jobs purge `rag_queries` (30d) and + `rag_retrieval_logs` (90d). **Verified running on live** (both jobs `active = true`). +- **OpenAI response storage is off** by default (`OPENAI_STORE_RESPONSES=false`, + [src/lib/env.ts:55-58](src/lib/env.ts)). +- Storage buckets are **private**; files are only reachable via short-lived (10 min) server-minted + signed URLs after an ownership check. + +**Top gaps (full register in §10):** + +| ID | Risk | One-line | +| ----- | ------- | -------------------------------------------------------------------------------------------------------------------------------------------------- | +| PIA-1 | High | Cross-border disclosure to OpenAI (US) has no code-visible DPA/ZDR and no user-facing notice/consent (APP 8, APP 5). | +| PIA-2 | High | Query-hash HMAC downgrades to **unsalted, dictionary-reversible SHA-256** if `RAG_QUERY_HASH_SECRET` is unset — nothing enforces it in production. | +| PIA-3 | Medium | Generated **answer text is stored un-redacted** in `rag_queries`, and can restate the patient details from the query. | +| PIA-4 | Medium | `rag_query_misses` has **no retention/purge job** (only `rag_queries` and `rag_retrieval_logs` do). | +| PIA-5 | Medium | No privacy policy / collection notice / data-handling documentation shipped (APP 1, APP 5). | +| PIA-6 | Low-Med | OpenAI **prompt-cache retention is forced to 24h** for gpt-5.5 regardless of config — query + retrieved excerpts persist ≤24h at OpenAI. | +| PIA-7 | Low | `RAG_PERSIST_RAW_QUERY_TEXT=true` would store raw PHI query text with no secondary safeguard beyond the 30-day purge. | + +--- + +## 2. System overview and data classification + +| Data category | Where it lives | Sensitivity | Notes | +| ------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------- | ------------------------------------------ | ------------------------------------------------------------------------ | +| Clinical reference corpus (documents, chunks, embeddings, images, tables) | Supabase (Sydney) + storage buckets | Low–Medium | Published guidelines are not PHI; **uploaded** docs _could_ contain PHI. | +| Free-text clinical queries | Transient in request; hashed into `rag_queries` / `rag_query_misses` / `rag_retrieval_logs`; sent to OpenAI (US) | **High (potential PHI)** | The primary incidental-PHI vector. | +| Generated answers | `rag_queries.answer`, `rag_response_cache.payload` | **High (derived from PHI query + corpus)** | Stored un-redacted; see PIA-3. | +| User identity | Supabase Auth (`auth.users`), `owner_id` foreign keys | Medium (PII) | Email + SSO identity; managed by Supabase Auth. | +| Audit trail | `audit_logs` | Medium | Append-only, service-role-only, retained indefinitely by design. | +| Operational telemetry | `rag_retrieval_logs`, ingestion job tables | Low–Medium | Redacted query text; per-owner. | + +**Deployment context (from code):** the answer system prompt positions the assistant as _"an +experienced psychiatrist in Perth"_ ([src/lib/rag.ts:7053](src/lib/rag.ts)) — i.e. a **WA psychiatry** +use case. Psychiatric context raises the sensitivity ceiling: mental-health information is squarely +"sensitive information" and "health information" under the _Privacy Act 1988_ (Cth). + +--- + +## 3. Clinical-data flow map + +The end-to-end path for a single clinician query. **Bold** nodes are where PHI can land. + +``` +Clinician browser + │ POST /api/answer { query: "", ... } + ▼ +[Next.js route] src/app/api/answer/route.ts:70 + │ • auth resolved → access.ownerId (or undefined for anon/public) :80 + │ • rate-limit bucket "answer" :83 + │ • resolveSearchScope() → owner-scoped candidate document set :93 + ▼ +[RAG pipeline] answerQuestionWithScope() src/lib/rag.ts + │ + ├──►(A) QUERY EMBEDDING ─────────────────────────────────────────────┐ + │ raw query text → OpenAI embeddings (text-embedding-3-small) │ + │ src/lib/openai.ts:498 embedText → :453 input:batch │ ►► OpenAI API + │ │ (US region, + ├──►(B) RETRIEVAL (Supabase RPCs, owner-filtered in SQL) │ api.openai.com) + │ match_document_chunks* etc. — Sydney, never leaves AU │ + │ │ + ├──►(C) ANSWER SYNTHESIS ─────────────────────────────────────────────┤ + │ **raw query verbatim** ("Question:\n${query}", rag.ts:7144) │ + │ + **retrieved chunk text** (buildRagSourceBlock, rag.ts:6306) │ + │ + system instructions (rag.ts:7053) │ + │ → OpenAI Responses API (gpt-5.5) openai.ts:384 ─┘ + │ store:false (openai.ts:220); prompt_cache_retention:24h (:168) + │ + ├──►(D) LOCAL LOGGING (Supabase, Sydney, owner-stamped) + │ insertRagQuery(): rag.ts:1983 + │ • query = **hash placeholder** (queryTextForStorage) ← redacted + │ • normalized_query= **hash placeholder** ← redacted + │ • **answer = full generated text** ← NOT redacted (PIA-3) + │ • source_chunk_ids= real chunk UUIDs ← owner's own data + │ • metadata.query_hash = HMAC/SHA-256 (query-privacy.ts:51) + │ + └──►(E) RESPONSE CACHE (Supabase rag_response_cache, owner-scoped) + payload = full answer, TTL ~5 min (RAG_ANSWER_CACHE_TTL_MS) + keyed by owner_id predicate (rag.ts:1667) — no cross-tenant serve + ▼ +Clinician browser ← answer + citations +``` + +`/api/search` follows the same shape but writes `rag_queries` / `rag_query_misses` / +`rag_retrieval_logs` (all redacted via the same helpers — +[src/app/api/search/route.ts:450-468, 556-559, 638-643](src/app/api/search/route.ts)). + +**The two egress points that carry PHI off-app are (A) and (C) — both to OpenAI in the US.** +Everything in Supabase stays in Sydney. + +--- + +## 4. What reaches OpenAI, and under what terms + +### 4.1 What is sent + +| Payload | Content | Reference | +| --------------- | ---------------------------------------------------------------------------------------------------- | -------------------------------------------------------------- | +| Embedding input | **Raw query text**, verbatim (normalized whitespace/case only) | [src/lib/openai.ts:498](src/lib/openai.ts) → `embedTexts` :423 | +| Answer input | **Raw query verbatim** (`Question:\n${args.query}`) | [src/lib/rag.ts:7144](src/lib/rag.ts) | +| Answer input | **Retrieved chunk text** (content, capped ~1800 chars, plus title/page/section/table-facts/captions) | [src/lib/rag.ts:6306-6325](src/lib/rag.ts) | +| Instructions | Static system prompt ("experienced psychiatrist in Perth…") | [src/lib/rag.ts:7053](src/lib/rag.ts) | +| Metadata | `{ operation }` only — **no** owner id, **no** patient identifiers added by the app | [src/lib/openai.ts:223](src/lib/openai.ts) | + +The app never _adds_ patient identifiers, but it does not scrub them either: **any PHI the clinician +types into the query, or that exists in an indexed excerpt, is transmitted to OpenAI.** + +### 4.2 Handling controls on the OpenAI request + +- **Model:** `gpt-5.5` for answers, `text-embedding-3-small` for embeddings ([src/lib/env.ts:18-27](src/lib/env.ts)). +- **`store: false`** by default — responses are not retained in OpenAI's dashboard/store + ([src/lib/openai.ts:220](src/lib/openai.ts), [src/lib/env.ts:55-58](src/lib/env.ts)). +- **`prompt_cache_retention: "24h"`** — **forced on for gpt-5.5** regardless of the + `OPENAI_PROMPT_CACHE_RETENTION` env value ([src/lib/openai.ts:168, 208, 221-222](src/lib/openai.ts)). + Prompt prefixes (which include retrieved excerpts and can include the query) are cacheable at OpenAI + for up to 24 hours. See PIA-6. +- **No `baseURL` override and no zero-data-retention (ZDR) header** are set in code — the client is a + plain `new OpenAI({ apiKey, timeout, maxRetries })` ([src/lib/openai.ts:69-73](src/lib/openai.ts)), + so traffic goes to `api.openai.com` (US) under whatever data-processing terms attach to the API + **account/organisation**. + +### 4.3 Data-processing terms — what code can and cannot tell us + +The code shows the _technical_ posture (US endpoint, `store:false`, 24h prompt cache, no ZDR header). +It **cannot** tell us the contractual posture. The following are **operator/legal actions**, not code +facts, and must be confirmed: + +- Whether a **Data Processing Addendum (DPA)** / OpenAI Business/Enterprise agreement is in place for + the account behind `OPENAI_API_KEY`. +- Whether **Zero Data Retention (ZDR)** has been granted for the org (which would also remove the 24h + prompt-cache window). +- OpenAI's standard API commitment (no training on API data by default; limited abuse-monitoring + retention) — this needs to be pinned to the specific contract, not assumed. + +Under **APP 8 (cross-border disclosure)**, the app operator remains accountable for OpenAI's handling +of the disclosed information unless an APP 8.2 exception applies. This is the single most important +privacy item to close before real patient use (PIA-1). + +--- + +## 5. Logging and redaction — per-table verification + +All three log tables are **owner-stamped** and **RLS-enabled** (owner-read for authenticated users; +service-role for writes). Redaction is applied centrally at every write site. + +| Table | Raw query stored? | Redaction mechanism | Other sensitive columns | RLS | +| -------------------- | --------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------- | --------------------------------------------------------- | +| `rag_queries` | No (hash placeholder) | `queryTextForStorage` / `normalizedQueryTextForStorage` ([query-privacy.ts:33-39](src/lib/query-privacy.ts)); write at [rag.ts:1983-1994](src/lib/rag.ts) | **`answer` stored un-redacted** (PIA-3); `source_chunk_ids` (own data) | owner-read, [schema.sql:3932](supabase/schema.sql) | +| `rag_query_misses` | No (hash placeholder) | same helpers; writes in [search/route.ts:558-559](src/app/api/search/route.ts), [interaction/route.ts:88-89](src/app/api/search/interaction/route.ts) | `metadata.query_hash` | owner-read, [schema.sql:3935](supabase/schema.sql) | +| `rag_retrieval_logs` | No (hash placeholder) | same helpers; write at [search/route.ts:556-559](src/app/api/search/route.ts) | retrieval telemetry only | owner-read, [schema.sql:3938](supabase/schema.sql) | +| `audit_logs` | N/A (no query text) | action/resource metadata only; error strings pass through `redactLogValue` ([privacy.ts:5-31](src/lib/privacy.ts)) | `owner_id`, `action`, `resource_id` | service-role-only, [schema.sql:3959](supabase/schema.sql) | + +### 5.1 M15 HMAC query-hash fix — verified present, conditionally active + +The audit's **M15** remediation is in the code +([src/lib/query-privacy.ts:17-23](src/lib/query-privacy.ts)): + +```ts +export function hashQueryText(query: string) { + const normalized = normalizeQueryText(query); + if (env.RAG_QUERY_HASH_SECRET) { + return createHmac("sha256", env.RAG_QUERY_HASH_SECRET).update(normalized).digest("hex"); // keyed pseudonym + } + return createHash("sha256").update(normalized).digest("hex"); // legacy unsalted fallback +} +``` + +- **When `RAG_QUERY_HASH_SECRET` is set:** the stored hash is a keyed HMAC-SHA256 — not + offline-reversible, not correlatable outside this deployment. ✔ This is the intended fix. +- **When it is unset:** the code **silently falls back to unsalted SHA-256**. A short, low-entropy + clinical query ("john smith clozapine") is then **dictionary-reversible** — an attacker (or a + curious insider) with read access to the log tables can hash candidate patient/drug strings offline + and match rows, and can correlate the same query across rows. This defeats the redaction it is + meant to provide. + +**Nothing in the codebase forces the secret to be present in production.** `RAG_QUERY_HASH_SECRET` +is `z.string().min(16).optional()` ([src/lib/env.ts:104](src/lib/env.ts)) with no production guard. +This is gap **PIA-2** — the fix is real but its safety depends on an operator setting that is not +enforced. + +### 5.2 Redaction helper coverage + +`redactLogValue` / `safeErrorLogDetails` ([src/lib/privacy.ts](src/lib/privacy.ts)) strip paths, +URLs, secrets (incl. `sb_secret_` / `sb_publishable_`), and emails from error details before they are +logged, and `redactCaptionIdentifiers` strips emails/MRN/NHS-style ids/phone numbers from image +captions ([privacy.ts:59-74](src/lib/privacy.ts)). These are sound as far as they go, but they are +**pattern-based** and do not attempt to redact free-text clinical narrative (names in prose, etc.) — +which is why the query-hash approach (not raw storage) is the right primary control. + +--- + +## 6. Retention and purge + +| Data | Retention | Mechanism | Live status | +| -------------------- | ---------------------- | ------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------- | +| `rag_queries` | 30 days | `purge_expired_rag_queries(30)`, `pg_cron` `purge-expired-rag-queries` @ 03:30 UTC | **Active** (jobid 11, verified live) | +| `rag_retrieval_logs` | 90 days | `pg_cron` `purge-rag-retrieval-logs` @ 03:00 UTC | **Active** (jobid 12, verified live) | +| `rag_query_misses` | **None** | — | **No purge job** — see PIA-4 | +| `rag_response_cache` | ~5 min TTL (soft) | `expires_at` filtered on read; overwritten per query | Rows expire logically; no hard purge cron (low volume, short TTL) | +| `audit_logs` | Indefinite (by design) | Documented in [migration 20260702120000](supabase/migrations/20260702120000_rag_retrieval_logs_retention.sql):8-12 | Intentional; "do not add purge without compliance review" | + +**Verification (live `cron.job` query, 2026-07-06):** + +``` +jobid 11 purge-expired-rag-queries 30 3 * * * active=true select public.purge_expired_rag_queries(30); +jobid 12 purge-rag-retrieval-logs 0 3 * * * active=true delete from public.rag_retrieval_logs where created_at < now() - interval '90 days'; +``` + +So the answer to "_is anything scheduled?_" is **yes** — the two query-log purges are live and active. +The retention story is sound **except**: + +- **PIA-4:** `rag_query_misses` (which stores the same hashed-query + metadata as `rag_queries`) has + **no** purge job — it accumulates indefinitely. It should get a matching 30–90 day cron. +- The purge functions are installed conditionally (`if to_regnamespace('cron') is null then return`, + [migration 20260629060603](supabase/migrations/20260629060603_rag_queries_retention.sql):27-43) — + fine on live (pg_cron present) but **preview/branch databases silently skip scheduling**. Not a + production risk, but worth noting for any secondary environment that retains real data. + +--- + +## 7. Data residency + +- **Supabase project region: `ap-southeast-2` (AWS Asia Pacific, Sydney).** Confirmed via the Supabase + management API for project `sjrfecxgysukkwxsowpy`. All Postgres data (documents, chunks, embeddings, + logs, auth) and both storage buckets are **onshore in Australia**. This is a strong position for WA + clinical use and directly supports APP 8 / APP 11 expectations for health information. +- **OpenAI: United States.** Query text + retrieved excerpts are disclosed to `api.openai.com` (no + regional/EU endpoint or ZDR configured in code). This is the **only** cross-border flow, and it is + the crux of the APP 8 assessment (PIA-1). + +**Net:** data _at rest_ is Australian; data _in transit for inference_ crosses to the US. A privacy +notice must disclose the OpenAI disclosure and its purpose. + +--- + +## 8. Storage-bucket access paths + +- Buckets `clinical-documents` and `clinical-images` are **private** + ([docs/multi-user-auth-setup.md](docs/multi-user-auth-setup.md) §7). +- No direct client storage access. Files are served only via **server-minted signed URLs** with a + **10-minute TTL** (`signedUrlTtlSeconds = 60 * 10`, + [documents/[id]/signed-url/route.ts:14](src/app/api/documents/[id]/signed-url/route.ts), + [images/[id]/signed-url/route.ts:15](src/app/api/images/[id]/signed-url/route.ts)). +- Every signed-URL mint is **preceded by an ownership check** on the parent document row + (`withOwnerReadScope(...)` before `createSignedUrl`, + [documents/[id]/signed-url/route.ts:40-51](src/app/api/documents/[id]/signed-url/route.ts)) — see the + companion tenancy review for the adversarial verification. +- Storage objects are namespaced by owner (`${uploadOwnerId}/documents/${documentId}/...`, + [upload/route.ts:134](src/app/api/upload/route.ts)), and the DB additionally carries owner-scoped + storage RLS policies ([schema.sql:3967-3973](supabase/schema.sql)) as a backstop for any future + client-direct access. + +Signed-URL handling is well-scoped. The residual consideration is only that a 10-minute URL, once +minted, is bearer-usable by anyone it is shared with in that window — acceptable for this use case. + +--- + +## 9. Assessment against Australian Privacy Act / WA health obligations + +**Framework.** Private-sector health service providers are **APP entities regardless of turnover** — +the small-business exemption does **not** apply where health services are provided and health +information is handled (_Privacy Act 1988_ (Cth), s6D(4)(b)). Health/mental-health information is +**"sensitive information"** attracting the highest APP protections. WA has no equivalent of Victoria's +_Health Records Act 2001_ or NSW's _HRIP Act 2002_ for the private sector; the _Privacy Act_ + APPs are +the operative framework for a WA private clinician. (The WA _Privacy and Responsible Information Sharing +Act 2024_ targets WA **public-sector** entities and may apply to public-health deployments — confirm +with counsel if this is deployed inside a WA Health service.) + +| APP | Obligation | Status in this app | Gap | +| ----------------------------------------- | ---------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------- | +| **APP 1** — open & transparent management | Have a clear, up-to-date APP privacy policy | No privacy policy/collection notice ships in the repo | PIA-5 | +| **APP 3** — collection of sensitive info | Collect health info only with consent + where reasonably necessary | App does not solicit PHI; incidental via free-text. No consent gate/notice on the query box | PIA-5 | +| **APP 5** — notification of collection | Tell individuals what's collected & disclosed (incl. overseas) | No collection notice; OpenAI disclosure not surfaced to the clinician | PIA-1, PIA-5 | +| **APP 6** — use/disclosure | Use only for the primary purpose or a permitted secondary purpose | Query used for answer generation (primary). Log retention = quality/eval (secondary) — defensible but should be documented | PIA-5 | +| **APP 8** — cross-border disclosure | Discloser stays accountable for the overseas recipient unless an exception applies | Disclosure to OpenAI (US); no code-visible DPA/ZDR; accountability unclear | **PIA-1** | +| **APP 11** — security & destruction | Reasonable security; destroy/de-identify when no longer needed | Strong: Sydney residency, RLS, private storage, query hashing, 30/90-day purge. Weakened by PIA-2 (conditional HMAC), PIA-3 (un-redacted answers), PIA-4 (misses never purged) | PIA-2/3/4 | +| **NDB scheme** (Pt IIIC) | Notify OAIC + individuals of eligible breaches of health info | No documented breach-response runbook tied to these tables | Recommend adding | + +**Overall:** the _engineering_ controls for data-at-rest are strong and largely APP-11-aligned. The +material shortfalls are **governance/contractual** (APP 8 cross-border, APP 1/5 notices) plus two +**hardening** items (conditional HMAC, un-redacted answer retention). None of these are cross-tenant +data-leak bugs — the tenancy review found **zero** confirmed cross-tenant leaks — they are +compliance-posture and PHI-minimisation gaps. + +--- + +## 10. Gap register (ranked by risk) + +### PIA-1 — Cross-border disclosure to OpenAI lacks visible DPA/ZDR + notice **(High)** + +- **Risk:** Health/PHI in queries and excerpts is disclosed to OpenAI (US) with no code-visible + contractual data-processing terms and no user notice → APP 8 accountability exposure and APP 5 + notification gap. +- **Evidence:** plain client to `api.openai.com` ([openai.ts:69-73](src/lib/openai.ts)); raw query + + excerpts sent ([rag.ts:7144, 6306](src/lib/rag.ts)); no ZDR/baseURL. +- **Fix (ranked):** (1) Execute an OpenAI DPA and, ideally, obtain **ZDR** for the org; record it in + `docs/`. (2) Add an APP-5 **collection/OpenAI-disclosure notice** at the query UI and in a privacy + policy. (3) Consider an **on-query PHI reminder** ("do not enter identifiable patient details"). + (4) Optionally, a lightweight PHI-scrub / entity-strip on the outbound query as defence-in-depth. + +### PIA-2 — Query-hash HMAC silently downgrades without the secret **(High)** + +- **Risk:** If `RAG_QUERY_HASH_SECRET` is unset in prod, stored query hashes are unsalted SHA-256 → + dictionary-reversible and cross-row correlatable, defeating the redaction (undoes M15). +- **Evidence:** [query-privacy.ts:17-23](src/lib/query-privacy.ts); optional with no prod guard + ([env.ts:104](src/lib/env.ts)). +- **Fix:** Make `RAG_QUERY_HASH_SECRET` **mandatory in production** — fail closed at startup (mirror the + `requireServerEnv` pattern) when `NODE_ENV=production` and the secret is missing. Set it on the live + project now. + +### PIA-3 — Generated answers stored un-redacted in `rag_queries` **(Medium)** + +- **Risk:** The `answer` column holds the full generated text, which can restate patient specifics + echoed from the query; the query itself is hashed but the answer is not. Owner-scoped (not + cross-tenant) and purged at 30 days, but it is un-redacted PHI-derived content at rest. +- **Evidence:** answer-path insert stores `answer` verbatim ([rag.ts:7580-7584](src/lib/rag.ts), traced). +- **Fix:** Gate answer-text persistence behind the same `RAG_PERSIST_RAW_QUERY_TEXT` flag (or a new + `RAG_PERSIST_ANSWER_TEXT`), defaulting off; store only what eval/quality actually needs. Confirm the + eval pipeline's real requirement before changing. + +### PIA-4 — `rag_query_misses` never purged **(Medium)** + +- **Risk:** Hashed-query rows accumulate indefinitely; retention policy is inconsistent with + `rag_queries`/`rag_retrieval_logs`. +- **Evidence:** live `cron.job` has no miss-table purge; only jobids 11/12 exist. +- **Fix:** Add a `pg_cron` purge for `rag_query_misses` (30–90 days) mirroring + [migration 20260702120000](supabase/migrations/20260702120000_rag_retrieval_logs_retention.sql). + +### PIA-5 — No privacy policy / collection notice / data-handling doc **(Medium)** + +- **Risk:** APP 1 (policy) and APP 5 (notification) are unmet; users are not told what is collected, + retained, or disclosed overseas. +- **Fix:** Ship a privacy policy + in-product collection notice; document retention windows and the + OpenAI disclosure. Low engineering cost, high compliance value. + +### PIA-6 — OpenAI prompt-cache retention forced to 24h **(Low-Medium)** + +- **Risk:** Query + retrieved excerpts persist at OpenAI for ≤24h via prompt caching even with + `store:false`; not operator-tunable for gpt-5.5. +- **Evidence:** [openai.ts:168, 208, 221-222](src/lib/openai.ts). +- **Fix:** Resolve via **ZDR** (removes the window) as part of PIA-1; document the 24h window in the + meantime. If a shorter window becomes configurable, expose it. + +### PIA-7 — `RAG_PERSIST_RAW_QUERY_TEXT=true` stores raw PHI query text **(Low, config-gated)** + +- **Risk:** Flipping the flag persists raw queries with only the 30-day purge as a safeguard. +- **Evidence:** [query-privacy.ts:33-47](src/lib/query-privacy.ts), [env.ts:96-99](src/lib/env.ts). +- **Fix:** Keep it **off** in production; if ever enabled, require a documented retention/consent basis + and consider a shorter purge window for raw-text rows. + +--- + +## 11. Recommendation + +Before the app is used with real patients in a WA clinical setting, close **PIA-1** and **PIA-2** as +launch-blockers (cross-border contractual basis + user notice; mandatory HMAC secret), then **PIA-3/4** +as fast follow-ups (minimise answer retention; purge `rag_query_misses`), and ship the **PIA-5** +privacy documentation. The data-at-rest security posture (Sydney residency, RLS, private storage, +query hashing, automated purge) is already strong and should be highlighted in the privacy policy as +evidence of "reasonable steps" under APP 11. + +See the companion **[tenancy defense-in-depth review](docs/tenancy-defense-in-depth-review.md)** for the +cross-tenant isolation analysis referenced above. diff --git a/docs/tenancy-defense-in-depth-review.md b/docs/tenancy-defense-in-depth-review.md new file mode 100644 index 000000000..a5cd937f2 --- /dev/null +++ b/docs/tenancy-defense-in-depth-review.md @@ -0,0 +1,330 @@ +# Tenancy Defense-in-Depth Review — Clinical KB Database + +**Status:** Draft for review · **Date:** 2026-07-06 · **Branch:** `claude/privacy-tenancy-review` +**Scope:** Every API route family under `src/app/api/**`, the Supabase RPCs they call, signed-URL +issuance, the response cache, and the demo/no-auth code paths — audited adversarially for a missed or +bypassable `owner_id` filter. +**Method:** One auditor agent per route family (7 families, all 33 route files), each required to trace +every `supabase.rpc(...)` into `supabase/schema.sql` / `supabase/migrations/**` and confirm the SQL +body itself owner-filters. Every claimed gap was then independently re-verified against the source and +the live database. Verdicts: **verified-scoped** / **gap** / **needs-deeper-look**. + +--- + +## 1. Executive summary + +**Result: 0 confirmed cross-tenant leaks across all 33 API routes.** Every route that reads or mutates +owner-scoped clinical data applies an owner filter, and every retrieval RPC re-applies owner scoping in +its own SQL body. The single deliberately-service-role architecture (RLS bypassed in the app tier, +ownership enforced in application code) is, as implemented today, **correctly and consistently +enforced**. + +That said, this is a **single-layer** design with one structural weakness worth understanding: + +> **The database has no independent tenancy floor.** All 33 routes use the service-role client, which +> bypasses RLS. The retrieval RPCs are owner-_aware_ but **fail _open_** — the shared owner-matching +> helper returns _every_ row when the owner filter is `NULL` +> ([retrieval_owner_matches, migration 20260705210000:11-15](supabase/migrations/20260705210000_retrieval_owner_filter_sentinel.sql)). +> The only thing preventing a cross-tenant leak is that the application layer never passes `NULL` in +> production — enforced by `requireOwnerScope` / `retrievalOwnerFilter` / `assertGlobalSearchAllowed` +> throwing when an owner is absent ([owner-scope.ts:10-13, 27-29](src/lib/owner-scope.ts)). + +So the system is safe today, but a single future PR that adds a route without the owner helper, or that +passes `NULL` into an RPC, would leak cross-tenant clinical data **with no database backstop and no +alarm.** §6 gives a pragmatic, honest recommendation (fail-closed RPCs + CI guard + integration test +before, or instead of, a full RLS refactor). + +**The one non-clean finding** is a **low-severity information disclosure**, not a tenancy leak: +`setup-status` interpolates a raw Postgres RPC error string into its response +([setup-status/route.ts:165](src/app/api/setup-status/route.ts)) — schema-shape only, behind the +local-origin gate (TEN-N1). + +--- + +## 2. The tenancy architecture + +### 2.1 How ownership is resolved + +``` +request ─► publicAccessContext() src/lib/public-api-access.ts:65-80 + │ getOptionalAuthenticatedUser() validates bearer/cookie JWT via supabase.auth.getUser() + ├─ authenticated → { authenticated:true, ownerId: user.id } + └─ anonymous → { authenticated:false, ownerId: undefined } +``` + +Mutating routes instead call `requireAuthenticatedUser()` which **throws** (401) with no session +([auth.ts:136-140](src/lib/supabase/auth.ts)). `owner_id` is **never** taken from the request body/query +— it is always the cryptographically-validated `auth.uid()` or a server-configured value. This closes +the "forge an owner_id" class of attack across every route. + +### 2.2 The two scoping primitives + +- **Reads (public-overlay model):** `withOwnerReadScope(query, ownerId)` + ([public-api-access.ts:60-63](src/lib/public-api-access.ts)): + - authenticated → `.or('owner_id.eq.,owner_id.is.null')` → **own rows + shared public (null-owner) rows** + - anonymous → `.is('owner_id', null)` → **public rows only** +- **Retrieval (RPC filter):** `retrievalOwnerFilter({ownerId, documentIds, allowGlobalSearch})` + ([owner-scope.ts:15-30](src/lib/owner-scope.ts)): + - `ownerId` → that owner (exact) + - demo / local-no-auth / test → `undefined` + - else if `allowGlobalSearch || documentIds` → **`PUBLIC_OWNER_FILTER_SENTINEL` `00000000-…0000`** (public-only) + - else → **throws** (fail-closed) + +**Threat-model note:** null-owner rows are a _deliberately shared public corpus_ (see +[migration 20260705220000](supabase/migrations/20260705220000_promote_locally_reviewed_documents_public.sql) +promoting reviewed documents to public). An authenticated user seeing null-owner rows is **not** a +leak. The leak this review hunts is: **authed user A seeing user B's non-null `owner_id` rows**, an +**anonymous** caller seeing any non-null rows, or any caller **mutating** another owner's rows. + +### 2.3 The SQL-level owner helper (and its fail-open edge) + +Every retrieval RPC gates rows through: + +```sql +-- migration 20260705210000_retrieval_owner_filter_sentinel.sql:4-16 +create function public.retrieval_owner_matches(owner_filter uuid, row_owner_id uuid) returns boolean as $$ + select case + when owner_filter is null then true -- ⚠ FAIL-OPEN: all owners + when owner_filter = '00000000-0000-0000-0000-000000000000' then row_owner_id is null -- public only + else row_owner_id = owner_filter -- exact owner (excludes null) + end; +$$; +``` + +This is faithful to whatever the app passes: `NULL` → everything, sentinel → public, uuid → that owner. +It is a correct _filter_, not an independent _guard_. The safety of the whole system rests on the app +layer never handing it `NULL` in production — which the throw-in-`retrievalOwnerFilter`/ +`requireOwnerScope` design does enforce today. + +--- + +## 3. Route-by-route verdict + +All 33 route files, every exported method. Full per-method reasoning with line cites lives in the audit +transcripts; this is the consolidated verdict. + +### Answer family (OpenAI RAG path) + +| Route · method | Verdict | Owner mechanism | +| ------------------------- | ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `POST /api/answer` | ✅ verified-scoped | `access.ownerId` + `allowGlobalSearch:!ownerId` → RPC `retrieval_owner_matches`; `resolveSearchScope` pre-filters documents ([route.ts:80,93,125-126](src/app/api/answer/route.ts)) | +| `POST /api/answer/stream` | ✅ verified-scoped | Same resolution threaded through `streamAnswer(...ownerId,publicOnly)` ([stream/route.ts:241-252](src/app/api/answer/stream/route.ts)) | + +### Documents read + sub-resources + +| Route · method | Verdict | Owner mechanism | +| ---------------------------------------------- | ------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `GET /api/documents` | ✅ verified-scoped | `withOwnerReadScope(...access.ownerId)` ([documents/route.ts:173](src/app/api/documents/route.ts)); children fetched by owner-scoped documentIds | +| `GET /api/documents/[id]` | ✅ verified-scoped | `withOwnerReadScope(...).eq('id',id)`; 404 before child fetch ([[id]/route.ts:289-295](src/app/api/documents/[id]/route.ts)) | +| `PATCH /api/documents/[id]` | ✅ verified-scoped | `requireAuthenticatedUser` + `.eq('id',id).eq('owner_id',user.id)`; update re-asserts owner ([[id]/route.ts:434-462](src/app/api/documents/[id]/route.ts)) | +| `DELETE /api/documents/[id]` | ✅ verified-scoped | owner-scoped parent fetch + delete re-asserts owner; storage cleanup from owner-verified rows ([[id]/route.ts:493-583](src/app/api/documents/[id]/route.ts)) | +| `POST/PATCH/DELETE /api/documents/[id]/labels` | ✅ verified-scoped | `requireOwnedDocument` + every write triple-scoped `id`+`document_id`+`owner_id` ([labels/route.ts:77-288](src/app/api/documents/[id]/labels/route.ts)) | +| `POST /api/documents/[id]/summarize` | ✅ verified-scoped | `requireAuthenticatedUser`; `summarizeDocument(id,user.id)` filters `owner_id` ([summarize/route.ts:30-34](src/app/api/documents/[id]/summarize/route.ts)); latent note TEN-N2 | +| `GET/PATCH /api/documents/[id]/table-facts` | ✅ verified-scoped | `loadOwnedDocument` (`.eq('owner_id')`); fact writes re-scoped ([table-facts/route.ts:27-116](src/app/api/documents/[id]/table-facts/route.ts)) | +| `GET /api/documents/[id]/search` | ✅ verified-scoped | route owner-scopes parent AND `search_document_chunks` SQL owner-filters ([search/route.ts:190-204](src/app/api/documents/[id]/search/route.ts); [schema.sql:2928-2931](supabase/schema.sql)) | + +### Mutations · signed URLs · upload (highest blast radius) + +| Route · method | Verdict | Owner mechanism | +| ------------------------------------ | ------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `GET /api/documents/[id]/signed-url` | ✅ verified-scoped | `withOwnerReadScope` on doc **before** `createSignedUrl`; `storage_path` from owner-verified row ([signed-url/route.ts:40-51](src/app/api/documents/[id]/signed-url/route.ts)) | +| `GET /api/images/[id]/signed-url` | ✅ verified-scoped | image has no `owner_id`; tenancy via parent-document `withOwnerReadScope` ([images/[id]/signed-url/route.ts:49-55](src/app/api/images/[id]/signed-url/route.ts)) | +| `POST /api/documents/[id]/reindex` | ✅ verified-scoped | `requireAuthenticatedUser` + `.eq('owner_id',user.id)`; every state write re-scoped ([reindex/route.ts:110-255](src/app/api/documents/[id]/reindex/route.ts)) | +| `POST /api/documents/bulk` | ✅ verified-scoped | pre-scoping select `.eq('owner_id',user.id).in('id',ids)`; body ids intersected with ownership ([bulk/route.ts:127-204](src/app/api/documents/bulk/route.ts)) | +| `POST /api/documents/bulk/reindex` | ✅ verified-scoped | pre-scoping select `.eq('owner_id',user.id)`; per-doc writes re-scoped ([bulk/reindex/route.ts:101-247](src/app/api/documents/bulk/reindex/route.ts)) | +| `POST /api/upload` | ✅ verified-scoped | `owner_id` = session id, or configured `PUBLIC_WORKSPACE_OWNER_ID` only if public uploads enabled, else 503 ([upload/route.ts:94-97](src/app/api/upload/route.ts)); operator note TEN-N3 | + +### Search + +| Route · method | Verdict | Owner mechanism | +| ------------------------------ | ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `POST /api/search` | ✅ verified-scoped | `searchChunksWithTelemetry({ownerId,allowGlobalSearch:!ownerId})`; RPCs owner-filter; `assertGlobalSearchAllowed` throws in prod ([search/route.ts:726-728](src/app/api/search/route.ts); [rag.ts:2151-2164](src/lib/rag.ts)) | +| `GET /api/search/universal` | ✅ verified-scoped | live branch only when `access.ownerId` truthy; each domain owner-seeded; static catalogs intended-public ([universal/route.ts:70-82](src/app/api/search/universal/route.ts)) | +| `POST /api/search/interaction` | ✅ verified-scoped | writes hard-pinned to `owner_id:user.id`; clicked doc/chunk validated owner-owned or nulled ([interaction/route.ts:44-84](src/app/api/search/interaction/route.ts)) | + +### Ingestion · jobs + +| Route · method | Verdict | Owner mechanism | +| ------------------------------------- | ------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `GET /api/ingestion/batches` | ✅ verified-scoped | `.eq('owner_id',user.id)` on `import_batches` ([batches/route.ts:62-67](src/app/api/ingestion/batches/route.ts)) | +| `GET /api/ingestion/jobs` | ✅ verified-scoped | `documents!inner` + `.eq('documents.owner_id',user.id)` (jobs have no owner col) ([jobs/route.ts:64-69](src/app/api/ingestion/jobs/route.ts)) | +| `POST /api/ingestion/jobs/[id]/retry` | ✅ verified-scoped | job gated via `documents!inner(owner_id)`+`.eq('id',id)`; requeue re-asserts `.eq('owner_id',user.id)` ([retry/route.ts:23-95](src/app/api/ingestion/jobs/[id]/retry/route.ts)) | +| `GET /api/ingestion/quality` | ✅ verified-scoped | root `documents` `.eq('owner_id',user.id)`; all aggregates `.in('document_id',ownedIds)` ([quality/route.ts:318-361](src/app/api/ingestion/quality/route.ts)) | +| `GET /api/jobs` | ✅ verified-scoped | `documents!inner` + `.eq('documents.owner_id',user.id)` ([jobs/route.ts:67-71](src/app/api/jobs/route.ts)) | + +### Catalogs · eval (owner-scoped private tables with in-memory public fixtures) + +| Route · method | Verdict | Owner mechanism | +| --------------------------------------------------------------- | ------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `GET /api/registry/records` (+ `/[slug]`) | ✅ verified-scoped | authed branch `.eq('owner_id',ownerId)`; anon branch = in-memory fixtures, no DB rows ([registry-seed.ts:65](src/lib/registry-seed.ts); [records/route.ts:79-106](src/app/api/registry/records/route.ts)) | +| `GET /api/medications` (+ `/[slug]`) | ✅ verified-scoped | `.eq('owner_id',ownerId)` ([medication-seed.ts:37](src/lib/medication-seed.ts); [[slug]/route.ts:98](src/app/api/medications/[slug]/route.ts)) | +| `GET /api/differentials` (+ `/[slug]`, `/presentations/[slug]`) | ✅ verified-scoped | `.eq('owner_id',access.ownerId)` on every DB read ([differentials/route.ts:106](src/app/api/differentials/route.ts); [presentations/[slug]/route.ts:113,144](src/app/api/differentials/presentations/[slug]/route.ts)) | +| `POST /api/eval-cases` | ✅ verified-scoped | `requireAuthenticatedUser`; `owner_id:user.id`; referenced doc/chunk validated owner-owned or nulled ([eval-cases/route.ts:124-149](src/app/api/eval-cases/route.ts)) | + +> Catalog correction: the audit brief speculated these tables might be owner-less shared catalogs. +> **False** — `clinical_registry_records`, `medication_records`, `differential_records`, +> `rag_query_misses` all declare `owner_id NOT NULL` with a `unique(owner_id, …)` constraint +> ([migration 20260703020000:10](supabase/migrations/20260703020000_clinical_registry_records.sql), +> [20260705010000:7](supabase/migrations/20260705010000_medication_records.sql), +> [20260705120000:5](supabase/migrations/20260705120000_differential_records.sql)). They are +> owner-scoped private tables; the "public catalog" served to anonymous callers comes from **in-memory +> curated fixtures**, never DB rows. No route exposes a write path to a shared catalog (catalog +> poisoning is not reachable). + +### Infra / misc (info-disclosure, not owner rows) + +| Route · method | Verdict | Notes | +| --------------------------- | ------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `GET /api/health` | ✅ verified-safe | Only presence booleans + coarse status; deep probe behind `HEALTH_DEEP_PROBE_SECRET` with `timingSafeEqual` ([health/route.ts:8-17,29-49](src/app/api/health/route.ts)) | +| `GET /api/setup-status` | ⚠ needs-deeper-look (low) | **TEN-N1** — raw RPC `error.message` in `detail` ([setup-status/route.ts:165](src/app/api/setup-status/route.ts)); schema-shape only, behind local-origin gate | +| `GET /api/local-project-id` | ✅ verified-safe | Returns constants + one-way SHA-256 of cwd path; no secrets, no owner data ([local-server-utils.mjs:20-22](src/lib/local-server-utils.mjs)) | + +**No request-controlled path can flip the app into demo/no-auth mode.** `isDemoMode()` / +`isLocalNoAuthMode()` read only server env + `NODE_ENV`, and both hard-return `false` in production +([env.ts:185-206](src/lib/env.ts)). No auth-bypass surface found. + +--- + +## 4. RPC SQL-body owner enforcement + +Every retrieval RPC reachable from a user route was traced into `supabase/schema.sql` / +`supabase/migrations/**` and confirmed to apply `retrieval_owner_matches(owner_filter, .owner_id)` +(or the inline equivalent) in its `WHERE`. All are `language sql` **SECURITY INVOKER**. + +| RPC | Owner-filters in SQL? | Ref | +| ----------------------------------------- | ---------------------------------------- | ------------------------------------------------------------------------------------------------------------------- | +| `match_document_chunks` (vector) | ✅ | [migration 20260705210000:65](supabase/migrations/20260705210000_retrieval_owner_filter_sentinel.sql), :120 | +| `match_document_chunks_hybrid` | ✅ | :188, :229 | +| `match_document_chunks_text` | ✅ | :591 | +| `match_document_lookup_chunks_text` | ✅ | :723 | +| `match_documents_for_query` | ✅ (`requireOwnerScope`, throws on null) | :511 | +| `match_document_table_facts_text` | ✅ | :862 | +| `match_document_embedding_fields_hybrid` | ✅ | :917, :931 | +| `match_document_index_units_hybrid` | ✅ | :1013 | +| `match_document_memory_cards_hybrid(_v2)` | ✅ | [schema.sql:2228,2248,2330-2337](supabase/schema.sql) | +| `get_related_document_metadata` | ✅ | :765, :775, :781 | +| `search_document_chunks` (single-doc) | ✅ (fail-closed) | [migration 20260705133000:51-52](supabase/migrations/20260705133000_tighten_search_document_chunks_owner_scope.sql) | + +**Ingestion state RPCs** (`claim_ingestion_jobs`, `complete_ingestion_job`, +`fail_or_retry_ingestion_job`, `refresh_import_batch_status`) mutate **by id with no owner predicate** +and are SECURITY INVOKER — but they are **not a route gap**: they are invoked **only from the trusted +worker** (`worker/main.ts`), are **revoked from `anon`/`authenticated`** and granted `service_role` +only ([schema.sql:3772,3805](supabase/schema.sql)), and the user-facing retry route deliberately uses a +direct owner-scoped `UPDATE` instead. No user session can reach them. + +**All execute grants on retrieval RPCs are revoked from `anon`/`authenticated` and granted only to +`service_role`** ([schema.sql:2950-2951](supabase/schema.sql)) — consistent with the "service-role + +app-layer filter" design. + +--- + +## 5. Response-cache cross-tenant analysis + +The memory-flagged concern ("shared null-owner `rag_response_cache` bucket") was checked directly and +does **not** yield a cross-tenant leak: + +- **In-memory answer/search caches:** the cache **key** includes `ownerId` as an explicit component — + `scopedAnswerCacheKey = [depVersion, ownerId ?? "anonymous", scopeKey, modeKey, query]` + ([rag.ts:1453-1459](src/lib/rag.ts)) and `scopedSearchCacheKey` + ([rag.ts:1553-1559](src/lib/rag.ts)). User A's UUID-prefixed key cannot collide with B's. +- **Persisted `rag_response_cache`:** owner enforced as a **column predicate** on both read and write — + `sharedCacheSelector` adds `.eq('owner_id', args.ownerId)` (authed) or `.is('owner_id', null)` (anon) + ([rag.ts:1667](src/lib/rag.ts)); writes stamp `owner_id: args.ownerId ?? null` after a same-owner + delete ([rag.ts:1870-1873](src/lib/rag.ts)). A reads only `owner_id = A` rows — never B's, never the + null bucket. +- The `owner_id IS NULL` cache partition is shared **among anonymous callers only**, and only ever + holds answers built from **public null-owner documents** — the intended public corpus, not private + data. No cross-tenant serve. + +--- + +## 6. Recommendation: does production warrant owner-scoped RLS as a second layer? + +**Short answer: not a full RLS refactor first — but yes to a cheaper, higher-leverage second layer.** +The honest cost/benefit: + +### What RLS would and wouldn't buy today + +The RLS policies in `schema.sql` (owner-read for `authenticated`) are currently **latent**: the API +routes all use the **service-role** client, which bypasses RLS, so those policies protect **nothing on +the current request paths**. They would only bite a _different_ access pattern (client-direct Supabase +access, or an edge function running as the user) — which the app doesn't use. So "RLS exists" is true +but does not currently constitute a second enforcement layer for these routes. + +### The real cost of making RLS bite + +To make RLS an actual second layer, every route would need to stop using the service-role client and +instead run as the user (anon key + user JWT, or a per-request `SET LOCAL` owner GUC). That refactor is +**substantial and risky** because: + +1. **The public-overlay model breaks under naïve RLS.** Current policies grant `owner_id = auth.uid()` + only — **not** null-owner rows. The app's whole "own rows + shared public corpus" read model + ([withOwnerReadScope](src/lib/public-api-access.ts)) would return no public documents unless every + policy is rewritten to `owner_id = auth.uid() OR owner_id IS NULL`. +2. **Anonymous public-catalog reads have no JWT** to present, so an anon-key + RLS path returns nothing + for the intended public/unauthenticated experience unless carefully policy-modelled. +3. **The retrieval RPCs are SECURITY INVOKER but called as service-role**; re-scoping them to run as the + user (or flipping SECURITY DEFINER) is a performance- and correctness-sensitive change. +4. **The worker legitimately needs service-role** and must stay bypassing RLS. + +That is real, multi-week work with its own regression surface — disproportionate while the app serves a +small, largely-cooperative user set with a public shared corpus. + +### The pragmatic second layer (recommended, in priority order) + +1. **Make the retrieval RPCs fail-_closed_ on a null owner filter (cheap, high value).** Change + `retrieval_owner_matches` (or each RPC) so `owner_filter IS NULL` returns **no rows** instead of all + rows, and route the legitimate demo/test path through an explicit sentinel rather than `NULL`. This + converts the one fail-open edge (§2.3) into a fail-safe with a near-one-line SQL change — the + database stops being able to emit cross-tenant rows even if the app passes `NULL` by accident. +2. **Add a CI guard against un-scoped owner tables (cheap, high value).** A lint/test that fails when a + new `src/app/api/**` handler queries an owner-scoped table without a recognised scoping construct + (`withOwnerReadScope`, `.eq('owner_id'`, `requireOwnerScope`, `documents!inner`+`documents.owner_id`). + This directly guards the regression class the single-layer model is exposed to — a future PR + dropping the filter. +3. **Add a live cross-tenant integration test (medium value).** Fixtures for user A + user B; for each + route family assert B cannot read/mutate A's non-null rows and gets 404/empty. This is the + regression harness for the exact property the whole model depends on, and it is what would have + caught any of the (hypothetical) gaps this manual audit looked for. +4. **Full owner-scoped RLS via a per-request user client (larger, do before scaling to many + mutually-distrusting tenants).** This is the textbook defense-in-depth answer and worth doing before + the app hosts many independent clinics on shared infrastructure — but it must preserve the + public-null-owner overlay (policy `OR owner_id IS NULL`), the anonymous public-catalog path, and the + worker's service-role needs. Sequence it **after** 1–3, which deliver most of the safety at a + fraction of the cost and risk. + +**Bottom line:** the current single-layer enforcement is correct today (0/33 gaps), but it is +one careless edit away from a silent, un-backstopped cross-tenant clinical-data leak. Items 1–3 above +are low-cost and close that exposure directly; a full RLS second layer is justified before multi-tenant +scale but should not block launch given the fixes above. + +--- + +## 7. Non-blocking findings + +- **TEN-N1 (low):** `setup-status` interpolates a raw Postgres RPC `error.message` into its response + detail ([setup-status/route.ts:165](src/app/api/setup-status/route.ts)). Worst case is schema-shape + disclosure (a function/relation name), only to a caller past the local-origin gate. Fix: return a + generic message; log the raw error server-side. +- **TEN-N2 (latent):** `summarizeDocument(documentId, ownerId?)` has an **optional** `ownerId` + ([rag.ts:7792](src/lib/rag.ts)) and would skip the owner filter if ever called with `undefined`. The + only caller passes `user.id` ([summarize/route.ts:34](src/app/api/documents/[id]/summarize/route.ts)), + so no live exploit — but make the parameter required (or fail closed) so a future caller can't + reintroduce a gap. +- **TEN-N3 (operator note, intended):** with `NEXT_PUBLIC_PUBLIC_UPLOADS_ENABLED=true`, all anonymous + uploads are pooled under the single configured `PUBLIC_WORKSPACE_OWNER_ID` + ([upload/route.ts:94](src/app/api/upload/route.ts)) — anonymous X's upload is visible to anonymous Y + and the workspace owner. This is the documented public-workspace model, not a private-row A→B leak, + but operators enabling public uploads should understand it. + +--- + +## 8. Method & coverage note + +7 auditor agents (one per route family) covered all 33 `src/app/api/**/route.ts` files and their +methods; each traced its RPCs into the SQL. Every load-bearing claim — the `retrieval_owner_matches` +semantics, one representative RPC body, the cache owner-predicate, the purge crons, and the two soft +findings — was **independently re-verified** against source and the live database (project +`sjrfecxgysukkwxsowpy`, region `ap-southeast-2`) before inclusion here. See the companion +**[privacy impact assessment](docs/privacy-impact-assessment.md)** for the data-flow / PHI / cross-border +analysis.