From c246dbff0b3238e791880b7c9c1e6e6764ac4904 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 13:57:26 +0000 Subject: [PATCH 01/10] test(db): two-user privacy test and a runner for the SQL behaviour tests Adds tests/sql/two-user-privacy.sql, which signs in as two synthetic doctors and proves browser roles hold no access to any public table, the callable function allowlist, administrator functions refusing a doctor, and every browser-facing owner policy isolating the two doctors. Adds npm run test:sql (bare-image replay) and runs tests/sql in the Migration replay CI job. Repairs enrichment-repair.sql fixtures for the newer lease-tuple checks. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KhogQUCHgdtFBHFtpjnd1n --- .github/workflows/ci.yml | 16 ++ package.json | 1 + scripts/ci-change-scope.mjs | 1 + scripts/test-sql-behaviour.ts | 226 ++++++++++++++++++++++ tests/sql/enrichment-repair.sql | 18 +- tests/sql/two-user-privacy.sql | 325 ++++++++++++++++++++++++++++++++ 6 files changed, 583 insertions(+), 4 deletions(-) create mode 100644 scripts/test-sql-behaviour.ts create mode 100644 tests/sql/two-user-privacy.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 49cf949424..d2d57aa14f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1501,6 +1501,22 @@ jobs: - name: Verify Migration Replay run: supabase migration up --local + # Database behaviour tests (tests/sql), including the two-user privacy test that signs in + # as two synthetic doctors and proves neither can read or change the other's rows. Each + # file seeds synthetic rows and rolls back, so they share the replayed chain. The bare-image + # equivalent is `npm run test:sql`. + - name: Run database behaviour tests against the replayed chain + run: | + set -euo pipefail + db_container="$(docker ps --format '{{.Names}}' --filter 'name=^supabase_db_' | head -n 1)" + test -n "${db_container}" + for test_file in tests/sql/*.sql; do + echo "::group::${test_file}" + docker exec -i "${db_container}" \ + psql -X -U postgres -d postgres -v ON_ERROR_STOP=1 -q -1 < "${test_file}" + echo "::endgroup::" + done + # `drift:manifest` needs the same Docker-capable runner as migration # replay. Generating it here gives a schema-changing PR an authoritative # replacement artifact when the offline freshness check fails, rather diff --git a/package.json b/package.json index b6b0a4025b..c4a3d39a01 100644 --- a/package.json +++ b/package.json @@ -80,6 +80,7 @@ "test:e2e:visual:update": "node scripts/run-playwright.mjs --config=playwright.visual.config.ts --update-snapshots", "test:e2e:style-contract": "node scripts/run-playwright.mjs tests/ui-style-contract.spec.ts --project=chromium", "test:cross-tenant:staging": "node scripts/run-tsx.mjs scripts/test-cross-tenant-staging.ts", + "test:sql": "node scripts/run-tsx.mjs scripts/test-sql-behaviour.ts", "worktrees:report": "node scripts/clean-worktree.mjs", "worktrees:inventory": "node scripts/worktree-inventory.mjs", "clean:worktree": "npm run worktrees:report", diff --git a/scripts/ci-change-scope.mjs b/scripts/ci-change-scope.mjs index df87a0b572..a7edc92fc0 100644 --- a/scripts/ci-change-scope.mjs +++ b/scripts/ci-change-scope.mjs @@ -497,6 +497,7 @@ const dbPatterns = [ "src/lib/supabase", "docs/database-drift-detection.md", "docs/supabase-migration-reconciliation.md", + "tests/sql", /^scripts\/(check-drift|check-chain-mirror-parity|generate-drift-manifest|check-m13-migration|check-retrieval-owner-migration|check-supabase-project|audit-tables|reindex|reindex-health|cleanup-abandoned-reindex-generations)\.ts$/, /^tests\/(supabase|drift|private-rag|private-access|retrieval-owner).*\.test\.ts$/, ]; diff --git a/scripts/test-sql-behaviour.ts b/scripts/test-sql-behaviour.ts new file mode 100644 index 0000000000..9b2fc876a2 --- /dev/null +++ b/scripts/test-sql-behaviour.ts @@ -0,0 +1,226 @@ +import { execFileSync } from "node:child_process"; +import { createHash, randomBytes } from "node:crypto"; +import { readdirSync, readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +/** + * test:sql — runs the database behaviour tests in tests/sql/*.sql. + * + * Replays supabase/schema.sql into a disposable, digest-pinned Supabase Postgres container (the + * same bootstrap as `npm run drift:manifest`), then runs each SQL test in its own transaction. + * Every test seeds synthetic rows and rolls back, so they share one replay. Requires Docker and + * the cached image. Never touches the live project. CI runs the same files against the full + * migration chain in the Migration replay job. + * + * Flags: + * --files run only these tests (default: every tests/sql/*.sql) + * --keep leave the container running for inspection + * --image override the digest-pinned Postgres image reference + */ + +const IMAGE_DEFAULT = + "supabase/postgres:17.6.1.127@sha256:be60aee15997daca475b710b734bc6bfe52cd544dcd7e9fd2ff58210b6747d83"; +const OWNER_LABEL = "com.psychiatry-tools.sql-behaviour.worktree"; +const repoUrl = (relative: string) => fileURLToPath(new URL(`../${relative}`, import.meta.url)); +// Same identity scheme as drift:manifest, so concurrent worktrees never touch each other's containers. +const WORKTREE_IDENTITY = createHash("sha256").update(repoUrl("").toLowerCase()).digest("hex").slice(0, 12); + +// The bare image ships auth.uid() but not auth.jwt(), which the administrator functions read. +// This is the hosted platform's definition, installed only into the scratch container. +const AUTH_JWT_SHIM = ` +do $$ begin + if to_regprocedure('auth.jwt()') is null then + create function auth.jwt() returns jsonb language sql stable as $f$ + select coalesce(nullif(current_setting('request.jwt.claim', true), ''), + nullif(current_setting('request.jwt.claims', true), ''))::jsonb + $f$; + grant execute on function auth.jwt() to anon, authenticated; + end if; +end $$;`; + +function arg(flag: string): string | undefined { + const index = process.argv.indexOf(flag); + if (index < 0) return undefined; + const value = process.argv[index + 1]; + if (!value || value.startsWith("--")) throw new Error(`${flag} requires a value`); + return value; +} + +function docker(args: string[], input?: string, environment?: Record): string { + return execFileSync("docker", args, { + encoding: "utf8", + env: environment ? { ...process.env, ...environment } : process.env, + input, + maxBuffer: 64 * 1024 * 1024, + stdio: ["pipe", "pipe", "pipe"], + }); +} + +function removeOwnedContainer(container: string) { + let labels: Record | undefined; + try { + labels = ( + JSON.parse(docker(["container", "inspect", container])) as Array<{ Config?: { Labels?: Record } }> + )[0]?.Config?.Labels; + } catch { + return; + } + if (labels?.[OWNER_LABEL] !== WORKTREE_IDENTITY) { + throw new Error(`Refusing to remove Docker container ${container}: it is not owned by this worktree.`); + } + docker(["rm", "-f", container]); +} + +function selectedTests(): string[] { + const requested = arg("--files"); + if (requested) + return requested + .split(",") + .map((file) => file.trim()) + .filter(Boolean); + return readdirSync(repoUrl("tests/sql")) + .filter((name) => name.endsWith(".sql")) + .sort() + .map((name) => `tests/sql/${name}`); +} + +async function main() { + const image = arg("--image") ?? IMAGE_DEFAULT; + if (!/@sha256:[a-f0-9]{64}$/i.test(image)) throw new Error("--image must use an immutable @sha256 digest reference"); + const keep = process.argv.includes("--keep"); + const container = `clinical-kb-sqltest-${WORKTREE_IDENTITY}-${process.pid}`; + const tests = selectedTests(); + if (tests.length === 0) throw new Error("No SQL behaviour tests found in tests/sql."); + + try { + docker(["info", "--format", "{{.ServerVersion}}"]); + } catch { + throw new Error( + "Docker is required for test:sql (it replays schema.sql into a scratch container). Start Docker and retry.", + ); + } + try { + docker(["image", "inspect", image]); + } catch { + throw new Error( + `Required image is not cached: ${image}. Pull it explicitly after approving registry access, then retry.`, + ); + } + + const query = (user: string, sql: string) => + docker([ + "exec", + "-i", + container, + "psql", + "-U", + user, + "-d", + "postgres", + "-tA", + "-v", + "ON_ERROR_STOP=1", + "-c", + sql, + ]).trim(); + const psql = (user: string, sql: string, singleTransaction = false) => + docker( + [ + "exec", + "-i", + container, + "psql", + "-X", + "-U", + user, + "-d", + "postgres", + "-v", + "ON_ERROR_STOP=1", + "-q", + ...(singleTransaction ? ["-1"] : []), + "-f", + "-", + ], + sql, + ); + + const failures: string[] = []; + try { + console.log(`Starting scratch container ${container} (${image})…`); + docker( + [ + "run", + "-d", + "--pull=never", + "--name", + container, + "--label", + `${OWNER_LABEL}=${WORKTREE_IDENTITY}`, + "-e", + "POSTGRES_PASSWORD", + image, + ], + undefined, + { POSTGRES_PASSWORD: randomBytes(16).toString("hex") }, + ); + // The image restarts Postgres once during init; require sustained readiness. + let ready = 0; + for (let attempt = 0; attempt < 60 && ready < 5; attempt += 1) { + try { + docker(["exec", container, "pg_isready", "-U", "postgres", "-q"]); + ready += 1; + } catch { + ready = 0; + } + await new Promise((resolve) => setTimeout(resolve, 2000)); + } + if (ready < 5) throw new Error("scratch Postgres did not remain ready after initialization"); + + psql("postgres", readFileSync(repoUrl("supabase/roles.sql"), "utf8")); + const storageOwner = query( + "postgres", + "select pg_catalog.pg_get_userbyid(nspowner) from pg_catalog.pg_namespace where nspname = 'storage';", + ); + const authOwner = query( + "postgres", + "select pg_catalog.pg_get_userbyid(nspowner) from pg_catalog.pg_namespace where nspname = 'auth';", + ); + if (!storageOwner || !authOwner) + throw new Error("bare Supabase image does not expose owners for the storage and auth schemas"); + psql(storageOwner, readFileSync(repoUrl("scripts/sql/drift-replay-scaffold.sql"), "utf8")); + console.log("Replaying supabase/schema.sql…"); + psql("postgres", readFileSync(repoUrl("supabase/schema.sql"), "utf8")); + psql(authOwner, AUTH_JWT_SHIM); + + for (const test of tests) { + try { + const output = psql("postgres", readFileSync(repoUrl(test), "utf8"), true); + const passLine = + output + .split("\n") + .map((line) => line.trim()) + // Skip psql's table furniture so the test's own result line is what gets reported. + .filter((line) => line && !/^\(\d+ rows?\)$/.test(line) && !/^[-+]+$/.test(line)) + .at(-1) ?? ""; + console.log(`PASS ${test} ${passLine}`); + } catch (error) { + const stderr = (error as { stderr?: string }).stderr ?? String(error); + const reason = stderr.split("\n").find((line) => line.includes("ERROR")) ?? stderr.trim(); + console.error(`FAIL ${test} ${reason}`); + failures.push(test); + } + } + } finally { + if (keep) console.log(`Container ${container} kept running (--keep).`); + else removeOwnedContainer(container); + } + + if (failures.length > 0) throw new Error(`${failures.length} of ${tests.length} SQL behaviour test(s) failed.`); + console.log(`All ${tests.length} SQL behaviour tests passed.`); +} + +main().catch((error) => { + console.error(error instanceof Error ? error.message : String(error)); + process.exitCode = 1; +}); diff --git a/tests/sql/enrichment-repair.sql b/tests/sql/enrichment-repair.sql index e61c5a0260..3b687ec79b 100644 --- a/tests/sql/enrichment-repair.sql +++ b/tests/sql/enrichment-repair.sql @@ -42,18 +42,28 @@ insert into public.document_index_quality(document_id, quality_score, extraction select id, 1, 'good' from cases where n in (4, 6, 7, 8); insert into public.indexing_v3_agent_jobs(document_id, status, enrichment_status, attempt_count, max_attempts, - locked_by, locked_at, created_at, updated_at) + locked_by, locked_at, created_at, updated_at, lease_token, lease_expected_generation_id, + lease_write_generation_id, lease_authority_digest, lease_expires_at, lease_duration_seconds) select id, case n when 1 then 'needs_enrichment_artifacts' when 2 then 'failed' when 7 then 'processing' else 'pending' end, case n when 1 then 'needs_enrichment_artifacts' when 2 then 'failed' when 7 then 'processing' else 'pending' end, case when n in (1, 2, 3) then 3 else 1 end, 3, case when n = 7 then 'synthetic-worker' end, case when n = 7 then now() end, - now() - interval '2 hours', now() - interval '2 hours' + now() - interval '2 hours', now() - interval '2 hours', + -- A processing row must carry the full lease tuple (indexing_v3_agent_jobs_lease_tuple). + case when n = 7 then gen_random_uuid() end, case when n = 7 then gen_random_uuid() end, + case when n = 7 then gen_random_uuid() end, case when n = 7 then 'sha256:' || repeat('0', 64) end, + case when n = 7 then now() + interval '1 hour' end, case when n = 7 then 3600 end from cases where n in (1, 2, 3, 6, 7); -insert into public.ingestion_jobs(document_id, status, stage, next_run_at, created_at, locked_at, locked_by) +insert into public.ingestion_jobs(document_id, status, stage, next_run_at, created_at, locked_at, locked_by, + lease_token, lease_write_generation_id, lease_authority_digest, lease_expires_at, lease_duration_seconds) select id, case when n = 8 then 'processing' else 'pending' end, 'queued', case when n = 5 then now() + interval '1 day' else now() end, now() - interval '2 hours', case when n = 8 then now() - interval '2 hours' end, - case when n = 8 then 'expired-worker' end + case when n = 8 then 'expired-worker' end, + -- Full lease tuple (ingestion_jobs_lease_tuple), expired an hour ago. + case when n = 8 then gen_random_uuid() end, case when n = 8 then gen_random_uuid() end, + case when n = 8 then 'sha256:' || repeat('0', 64) end, + case when n = 8 then now() - interval '1 hour' end, case when n = 8 then 3600 end from cases where n in (4, 5, 8); create temp table before_documents as select d.* from public.documents d join cases c on c.id = d.id where n in (4, 5, 6, 7); create temp table before_jobs as select j.* from public.ingestion_jobs j join cases c on c.id = j.document_id where n in (4, 5); diff --git a/tests/sql/two-user-privacy.sql b/tests/sql/two-user-privacy.sql new file mode 100644 index 0000000000..431bd51911 --- /dev/null +++ b/tests/sql/two-user-privacy.sql @@ -0,0 +1,325 @@ +-- Two-user privacy behaviour test. Signs in as two different synthetic doctors (A and B) and +-- proves, by actually querying, that neither can see or change the other's rows. +-- +-- Part 1 Browser roles fail closed. The signed-in role (`authenticated`) and the public role +-- (`anon`) hold no table, view or sequence privilege in `public`, so every read and +-- write a browser could attempt is refused. All Work mode data (roster, on call, CPD, +-- teaching, admin) is reachable only through server routes; this proves the database +-- itself gives a browser nothing to read around them. +-- Part 2 The only functions a browser role may call are the known allowlist, and the +-- administrator-only ones refuse an ordinary signed-in doctor. +-- Part 3 Defence in depth. Every owner policy written for the browser roles is exercised as +-- if a future change granted access: each doctor sees only their own rows, cannot +-- change or delete the other's, and cannot plant a row in the other's name. +-- +-- Run with psql ON_ERROR_STOP against a disposable database replayed from schema.sql or the +-- migration chain (`npm run test:sql`, or the Migration replay CI job). Never against a live +-- database. Synthetic fixtures and every grant roll back. +\set ON_ERROR_STOP on +begin; +set local statement_timeout = '120s'; + +create function pg_temp.assert_true(ok boolean, message text) returns void +language plpgsql as $$ begin + if ok is distinct from true then raise exception '%', message; end if; +end $$; +select pg_temp.assert_true(not exists (select 1 from public.documents), 'requires an empty disposable database'); + +-- The administrator functions read auth.jwt(). The emulator and hosted platform ship it; the +-- bare-image runner (scripts/test-sql-behaviour.ts) adds the platform definition first. +select pg_temp.assert_true(to_regprocedure('auth.jwt()') is not null, + 'auth.jwt() is missing: run through npm run test:sql, which installs the platform definition'); + +create temp table people (k text primary key, id uuid not null) on commit drop; +insert into people values + ('a', 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa'), + ('b', 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb'); +insert into auth.users(id, email) select id, 'doctor-' || k || '@example.org' from people; +grant select on people to anon, authenticated; + +-- Act as a signed-in doctor (or as the public role when p_user is null), the way PostgREST +-- does: role plus JWT claims. Both claim spellings are set because the bare image's auth.uid() +-- reads request.jwt.claim.sub and the platform's reads request.jwt.claims. +create function pg_temp.act_as(p_user uuid) returns void language plpgsql as $$ begin + perform set_config('request.jwt.claim.sub', coalesce(p_user::text, ''), true); + perform set_config('request.jwt.claims', + case when p_user is null then '{"role":"anon"}' + else pg_catalog.json_build_object('sub', p_user, 'role', 'authenticated', 'aal', 'aal1')::text end, true); + perform set_config('role', case when p_user is null then 'anon' else 'authenticated' end, true); +end $$; +create function pg_temp.uid(p_k text) returns uuid language sql stable as $$ select id from people where k = p_k $$; +grant execute on function pg_temp.assert_true(boolean, text), pg_temp.uid(text) to anon, authenticated; + +-- --------------------------------------------------------------------------------------------- +-- Part 1: browser roles fail closed on every public relation and sequence. +-- --------------------------------------------------------------------------------------------- +select pg_temp.assert_true(not exists ( + select 1 from pg_class c join pg_namespace n on n.oid = c.relnamespace, (values ('anon'), ('authenticated')) r(role) + where n.nspname = 'public' and c.relkind in ('r', 'p', 'v', 'm', 'f') + and has_table_privilege(r.role, c.oid, 'SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES,TRIGGER') +), 'a browser role holds a privilege on a public table or view: ' || coalesce(( + select string_agg(r.role || ' on ' || c.oid::regclass::text, ', ') + from pg_class c join pg_namespace n on n.oid = c.relnamespace, (values ('anon'), ('authenticated')) r(role) + where n.nspname = 'public' and c.relkind in ('r', 'p', 'v', 'm', 'f') + and has_table_privilege(r.role, c.oid, 'SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES,TRIGGER')), '')); +select pg_temp.assert_true(not exists ( + select 1 from pg_class c join pg_namespace n on n.oid = c.relnamespace, (values ('anon'), ('authenticated')) r(role) + where n.nspname = 'public' and c.relkind = 'S' and has_sequence_privilege(r.role, c.oid, 'USAGE,SELECT,UPDATE') +), 'a browser role holds a privilege on a public sequence'); + +-- The catalogue says no; now prove it by trying every read and write as each doctor and as +-- the public role. Permission is checked before any row or constraint, so empty statements +-- are enough to provoke the refusal. +create function pg_temp.expect_refused_everywhere(p_who text) returns integer language plpgsql as $$ +declare r record; v_checked integer := 0; +begin + for r in + select c.oid::regclass::text as rel, c.relkind + from pg_class c join pg_namespace n on n.oid = c.relnamespace + where n.nspname = 'public' and c.relkind in ('r', 'p', 'v', 'm', 'f') and not c.relispartition + order by 1 + loop + begin + execute format('select 1 from %s limit 1', r.rel); + raise exception 'PRIVACY LEAK: % can read %', p_who, r.rel; + exception when insufficient_privilege then null; + end; + if r.relkind in ('r', 'p') then + begin + execute format('insert into %s default values', r.rel); + raise exception 'PRIVACY LEAK: % can insert into %', p_who, r.rel; + exception when insufficient_privilege then null; + end; + begin + execute format('delete from %s where false', r.rel); + raise exception 'PRIVACY LEAK: % can delete from %', p_who, r.rel; + exception when insufficient_privilege then null; + end; + end if; + v_checked := v_checked + 1; + end loop; + return v_checked; +end $$; +grant execute on function pg_temp.expect_refused_everywhere(text) to anon, authenticated; + +select pg_temp.act_as(pg_temp.uid('a')); +select pg_temp.assert_true(pg_temp.expect_refused_everywhere('doctor A') > 100, + 'expected to probe every public table as doctor A'); +reset role; +select pg_temp.act_as(pg_temp.uid('b')); +select pg_temp.assert_true(pg_temp.expect_refused_everywhere('doctor B') > 100, + 'expected to probe every public table as doctor B'); +reset role; +select pg_temp.act_as(null); +select pg_temp.assert_true(pg_temp.expect_refused_everywhere('the public role') > 100, + 'expected to probe every public table as the public role'); +reset role; + +-- --------------------------------------------------------------------------------------------- +-- Part 2: callable functions are an explicit allowlist, and admin-only ones refuse a doctor. +-- --------------------------------------------------------------------------------------------- +select pg_temp.assert_true(not exists ( + select 1 from pg_proc p join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' and has_function_privilege('anon', p.oid, 'EXECUTE') +), 'the public role can call a public function: ' || coalesce(( + select string_agg(p.oid::regprocedure::text, ', ') from pg_proc p join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' and has_function_privilege('anon', p.oid, 'EXECUTE')), '')); +select pg_temp.assert_true(not exists ( + select 1 from pg_proc p join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' and has_function_privilege('authenticated', p.oid, 'EXECUTE') + and p.proname not in ( + 'admit_administrator_document', -- administrator plus recent sign-in, checked inside + 'publish_site_content_record', -- administrator, checked inside + 'record_site_content_reconciliation_plan', -- administrator, checked inside + 'retire_site_content_record', -- administrator, checked inside + 'is_visible_document_page' -- pure helper for the page policy, reads nothing + ) +), 'a signed-in doctor can call a function outside the reviewed allowlist: ' || coalesce(( + select string_agg(p.oid::regprocedure::text, ', ') from pg_proc p join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' and has_function_privilege('authenticated', p.oid, 'EXECUTE') + and p.proname not in ('admit_administrator_document', 'publish_site_content_record', + 'record_site_content_reconciliation_plan', 'retire_site_content_record', 'is_visible_document_page')), '')); +select pg_temp.assert_true(not (select prosecdef from pg_proc where oid = 'public.is_visible_document_page(uuid, uuid, text)'::regprocedure), + 'is_visible_document_page must stay an ordinary (not SECURITY DEFINER) helper'); + +create function pg_temp.expect_admin_refusal(p_sql text) returns void language plpgsql as $$ begin + execute p_sql; + raise exception 'PRIVACY LEAK: an ordinary doctor ran an administrator function: %', p_sql; +exception when others then + if sqlerrm not like '%administrator%' then + raise exception 'expected an administrator refusal, got "%" from %', sqlerrm, p_sql; + end if; +end $$; +grant execute on function pg_temp.expect_admin_refusal(text) to authenticated; + +select pg_temp.act_as(pg_temp.uid('b')); +select pg_temp.expect_admin_refusal($q$select public.admit_administrator_document('{}'::jsonb, '{}'::jsonb, false)$q$); +select pg_temp.expect_admin_refusal($q$select * from public.publish_site_content_record('service', gen_random_uuid(), 'v', 0, null, null, null)$q$); +select pg_temp.expect_admin_refusal($q$select * from public.retire_site_content_record('service', gen_random_uuid(), 'v', 0, null, null, null)$q$); +select pg_temp.expect_admin_refusal($q$select public.record_site_content_reconciliation_plan('{}'::jsonb)$q$); +reset role; + +-- --------------------------------------------------------------------------------------------- +-- Part 3: owner policies, exercised as if a browser grant existed. Each doctor gets a full set +-- of synthetic rows, written as the database owner so no policy helps or hinders the fixture. +-- --------------------------------------------------------------------------------------------- +insert into public.documents(id, owner_id, title, file_name, file_type, storage_path, status) +select id, id, 'Synthetic privacy fixture', 'fixture.txt', 'text/plain', id || '/fixture.txt', 'indexed' from people; +insert into public.document_pages(document_id, page_number) select id, 1 from public.documents; +insert into public.document_images(document_id, storage_path) select id, owner_id || '/image.png' from public.documents; +insert into public.document_labels(document_id, owner_id, label, label_type, source) +select id, owner_id, 'fixture', 'custom', 'manual' from public.documents; +insert into public.document_summaries(document_id, owner_id, summary) select id, owner_id, 'Synthetic summary' from public.documents; +insert into public.document_sections(document_id, owner_id, section_index, heading) +select id, owner_id, 0, 'Synthetic section' from public.documents; +insert into public.document_memory_cards(document_id, owner_id, card_type, title, content, embedding) +select id, owner_id, 'definition', 'Synthetic card', 'Fixture', array_fill(0.1::real, array[1536])::extensions.vector +from public.documents; +insert into public.document_chunks(document_id, chunk_index, content, embedding) +select id, 0, 'Synthetic chunk', array_fill(0.1::real, array[1536])::extensions.vector from public.documents; +insert into public.document_table_facts(document_id, owner_id, row_label) select id, owner_id, 'Synthetic fact' from public.documents; +insert into public.document_embedding_fields(document_id, owner_id, field_type, content, content_hash, embedding) +select id, owner_id, 'document_title', 'Synthetic title', md5(id::text), array_fill(0.1::real, array[1536])::extensions.vector +from public.documents; +insert into public.document_index_quality(document_id, owner_id) select id, owner_id from public.documents; +insert into public.document_index_units(document_id, owner_id, unit_type, title, content, embedding) +select id, owner_id, 'document_profile', 'Synthetic unit', 'Fixture', array_fill(0.1::real, array[1536])::extensions.vector +from public.documents; +insert into public.ingestion_jobs(document_id) select id from public.documents; +insert into public.import_batches(owner_id, name) select id, 'Synthetic batch' from people; +insert into public.image_caption_cache(owner_id, image_hash, model, caption) select id, md5(id::text), 'fixture', 'Synthetic' from people; +insert into public.rag_queries(owner_id, query) select id, 'synthetic query' from people; +insert into public.rag_query_misses(owner_id, query, normalized_query) select id, 'synthetic miss', 'synthetic miss' from people; +insert into public.rag_aliases(owner_id, alias, canonical, alias_type) select id, 'fx' || k, 'fixture', 'custom' from people; +insert into public.rag_aliases(owner_id, alias, canonical, alias_type) values (null, 'fxshared', 'fixture', 'custom'); +insert into public.rag_retrieval_logs(owner_id, query) select id, 'synthetic log' from people; +insert into public.storage_cleanup_jobs(owner_id, document_paths) +select id, array[id || '/orphan.txt'] from people; +insert into public.user_favourite_sets(user_id, name) select id, 'Synthetic set' from people; +insert into public.user_favourites(user_id, content_type, content_key) select id, 'form', 'fixture-form' from people; +insert into public.user_preferences(user_id) select id from people; +insert into storage.objects(bucket_id, name, owner) +select b, id || '/fixture.' || b, id from people, (values ('clinical-documents'), ('clinical-images')) v(b); + +-- How to tell whose row it is, per table. `$me` is the acting doctor; shared rows (an alias with +-- no owner) count as the actor's own. Every table carrying a browser-role policy must be listed +-- here, so a new owner policy cannot ship without two-user coverage. +create temp table owner_rule (rel regclass primary key, owner_expr text not null, owner_col text) on commit drop; +insert into owner_rule values + ('public.documents', 'owner_id', 'owner_id'), + ('public.document_pages', '(select d.owner_id from public.documents d where d.id = document_id)', null), + ('public.document_images', '(select d.owner_id from public.documents d where d.id = document_id)', null), + ('public.document_chunks', '(select d.owner_id from public.documents d where d.id = document_id)', null), + ('public.document_table_facts', '(select d.owner_id from public.documents d where d.id = document_id)', null), + ('public.document_embedding_fields', '(select d.owner_id from public.documents d where d.id = document_id)', null), + ('public.document_index_units', '(select d.owner_id from public.documents d where d.id = document_id)', null), + ('public.ingestion_jobs', '(select d.owner_id from public.documents d where d.id = document_id)', null), + ('public.document_labels', 'owner_id', 'owner_id'), + ('public.document_summaries', 'owner_id', 'owner_id'), + ('public.document_sections', 'owner_id', 'owner_id'), + ('public.document_memory_cards', 'owner_id', 'owner_id'), + ('public.document_index_quality', 'owner_id', 'owner_id'), + ('public.import_batches', 'owner_id', 'owner_id'), + ('public.image_caption_cache', 'owner_id', 'owner_id'), + ('public.rag_queries', 'owner_id', 'owner_id'), + ('public.rag_query_misses', 'owner_id', 'owner_id'), + ('public.rag_aliases', 'coalesce(owner_id, $me)', 'owner_id'), + ('public.rag_retrieval_logs', 'owner_id', 'owner_id'), + ('public.storage_cleanup_jobs', 'owner_id', 'owner_id'), + ('public.user_favourite_sets', 'user_id', 'user_id'), + ('public.user_favourites', 'user_id', 'user_id'), + ('public.user_preferences', 'user_id', 'user_id'), + ('storage.objects', '(storage.foldername(name))[1]::uuid', null); +grant select on owner_rule to authenticated; + +select pg_temp.assert_true(not exists ( + select distinct p.polrelid from pg_policy p + where exists (select 1 from unnest(p.polroles) r where r in ('anon'::regrole, 'authenticated'::regrole)) + and p.polrelid not in (select rel from owner_rule) + -- A blanket deny is not an owner policy: it is proved by the refusal check further down. + and p.polname <> 'cme_evidence_no_direct_access' +), 'a browser-role policy has no two-user coverage here; add its table to owner_rule: ' || coalesce(( + select string_agg(distinct p.polrelid::regclass::text, ', ') from pg_policy p + where exists (select 1 from unnest(p.polroles) r where r in ('anon'::regrole, 'authenticated'::regrole)) + and p.polrelid not in (select rel from owner_rule) and p.polname <> 'cme_evidence_no_direct_access'), '')); + +-- Simulate the broadest browser grant a future change could add. Rolled back with everything else. +do $$ declare r record; begin + for r in select rel from owner_rule loop + execute format('grant select, insert, update, delete on %s to authenticated', r.rel); + end loop; +end $$; + +-- Totals as the owner, before either doctor acts. +create temp table owner_totals on commit drop as select rel, 0::bigint as total from owner_rule; +do $$ declare r record; v bigint; begin + for r in select rel from owner_rule loop + execute format('select count(*) from %s', r.rel) into v; + update owner_totals set total = v where rel = r.rel; + end loop; +end $$; + +create function pg_temp.check_isolation(p_me text, p_other text) returns void language plpgsql as $$ +declare r record; v_me uuid := pg_temp.uid(p_me); v_other uuid := pg_temp.uid(p_other); + v_own bigint; v_foreign bigint; v_rows bigint; v_expr text; +begin + for r in select rel, owner_expr, owner_col from owner_rule order by rel::text loop + v_expr := replace(r.owner_expr, '$me', quote_literal(v_me) || '::uuid'); + execute format('select count(*) filter (where %1$s = %2$L::uuid), count(*) filter (where %1$s is distinct from %2$L::uuid) from %3$s', + v_expr, v_me, r.rel) into v_own, v_foreign; + if v_own = 0 then raise exception 'doctor % cannot see their own rows in % (fixture or policy is broken)', p_me, r.rel; end if; + if v_foreign > 0 then raise exception 'PRIVACY LEAK: doctor % can see % of doctor %''s rows in %', p_me, v_foreign, p_other, r.rel; end if; + + -- Changing or deleting the other doctor's rows must touch nothing. + begin + execute format('update %s set %s = %s where %s = %L::uuid', + r.rel, coalesce(r.owner_col, 'document_id'), coalesce(r.owner_col, 'document_id'), v_expr, v_other); + get diagnostics v_rows = row_count; + if v_rows > 0 then raise exception 'PRIVACY LEAK: doctor % changed % of doctor %''s rows in %', p_me, v_rows, p_other, r.rel; end if; + exception when insufficient_privilege or undefined_column then null; + end; + begin + execute format('delete from %s where %s is distinct from %L::uuid', r.rel, v_expr, v_me); + get diagnostics v_rows = row_count; + if v_rows > 0 then raise exception 'PRIVACY LEAK: doctor % deleted % of doctor %''s rows in %', p_me, v_rows, p_other, r.rel; end if; + exception when insufficient_privilege then null; + end; + + -- Handing your own row to the other doctor (planting data in their name) must be refused. + if r.owner_col is not null and r.rel <> 'public.rag_aliases'::regclass then + begin + execute format('update %s set %I = %L::uuid where %I = %L::uuid', r.rel, r.owner_col, v_other, r.owner_col, v_me); + get diagnostics v_rows = row_count; + if v_rows > 0 then + raise exception 'PRIVACY LEAK: doctor % moved % row(s) into doctor %''s name in %', p_me, v_rows, p_other, r.rel; + end if; + exception when insufficient_privilege then null; + end; + end if; + end loop; +end $$; +grant execute on function pg_temp.check_isolation(text, text) to authenticated; + +select pg_temp.act_as(pg_temp.uid('a')); +select pg_temp.check_isolation('a', 'b'); +reset role; +select pg_temp.act_as(pg_temp.uid('b')); +select pg_temp.check_isolation('b', 'a'); +reset role; + +-- Nothing either doctor did above added, removed or reassigned anyone's rows. +do $$ declare r record; v bigint; begin + for r in select t.rel, t.total from owner_totals t loop + execute format('select count(*) from %s', r.rel) into v; + if v is distinct from r.total then + raise exception 'PRIVACY LEAK: row count in % changed from % to % while the doctors probed it', r.rel, r.total, v; + end if; + end loop; +end $$; +select pg_temp.assert_true((select count(*) from public.documents where owner_id = pg_temp.uid('a')) = 1 + and (select count(*) from public.documents where owner_id = pg_temp.uid('b')) = 1, + 'each doctor must still own exactly their own document'); + +select 'TWO_USER_PRIVACY_SQL_PASS: browser roles fail closed on every table, function allowlist, ' + || 'admin functions refuse doctors, owner policies isolate two doctors' as result; +rollback; From 6bc6b20fcdfe634ea8c99dfe83ecfa20933f4827 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 13:58:02 +0000 Subject: [PATCH 02/10] chore(org): place the SQL behaviour runner in Data platform Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KhogQUCHgdtFBHFtpjnd1n --- docs/organisation/systems/data-platform.json | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/organisation/systems/data-platform.json b/docs/organisation/systems/data-platform.json index 0da72009ea..f82c8a0b6f 100644 --- a/docs/organisation/systems/data-platform.json +++ b/docs/organisation/systems/data-platform.json @@ -51,6 +51,7 @@ "scripts/check-retrieval-owner-migration.ts", "scripts/check-supabase-project.ts", "scripts/generate-drift-manifest.ts", + "scripts/test-sql-behaviour.ts", "scripts/lib/cross-tenant-*", "scripts/lib/find-owner-id-by-email.ts", "scripts/lib/operational-alerts.mjs", From 46ef4ea98b3affeecf1a9c1d33f93caef79e779d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 15:20:04 +0000 Subject: [PATCH 03/10] test(db): count PUBLIC (no TO clause) policies as browser-facing in the coverage guard Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KhogQUCHgdtFBHFtpjnd1n --- tests/sql/two-user-privacy.sql | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/sql/two-user-privacy.sql b/tests/sql/two-user-privacy.sql index 431bd51911..c067735966 100644 --- a/tests/sql/two-user-privacy.sql +++ b/tests/sql/two-user-privacy.sql @@ -232,15 +232,19 @@ insert into owner_rule values ('storage.objects', '(storage.foldername(name))[1]::uuid', null); grant select on owner_rule to authenticated; +-- A policy with no TO clause is stored as PUBLIC (role oid 0) and applies to both browser roles. +-- Extension schemas (pg_cron's own cron.job policies) are outside the app's data and not checked. select pg_temp.assert_true(not exists ( select distinct p.polrelid from pg_policy p - where exists (select 1 from unnest(p.polroles) r where r in ('anon'::regrole, 'authenticated'::regrole)) + where exists (select 1 from unnest(p.polroles) r where r = 0 or r in ('anon'::regrole, 'authenticated'::regrole)) + and (select c.relnamespace from pg_class c where c.oid = p.polrelid) in ('public'::regnamespace, 'storage'::regnamespace) and p.polrelid not in (select rel from owner_rule) -- A blanket deny is not an owner policy: it is proved by the refusal check further down. and p.polname <> 'cme_evidence_no_direct_access' ), 'a browser-role policy has no two-user coverage here; add its table to owner_rule: ' || coalesce(( select string_agg(distinct p.polrelid::regclass::text, ', ') from pg_policy p - where exists (select 1 from unnest(p.polroles) r where r in ('anon'::regrole, 'authenticated'::regrole)) + where exists (select 1 from unnest(p.polroles) r where r = 0 or r in ('anon'::regrole, 'authenticated'::regrole)) + and (select c.relnamespace from pg_class c where c.oid = p.polrelid) in ('public'::regnamespace, 'storage'::regnamespace) and p.polrelid not in (select rel from owner_rule) and p.polname <> 'cme_evidence_no_direct_access'), '')); -- Simulate the broadest browser grant a future change could add. Rolled back with everything else. From a1a0e3826d165db6f74f7eda5914b59c7aa4afd7 Mon Sep 17 00:00:00 2001 From: "qodo-code-review[bot]" <151058649+qodo-code-review[bot]@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:30:00 +0000 Subject: [PATCH 04/10] =?UTF-8?q?fix:=203=20findings=20=E2=80=94=20Prevent?= =?UTF-8?q?=20administrator=20refusal=20helper=20from=20swallowing=20p?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Prevent administrator refusal helper from swallowing privacy leaks - Probe cross-owner inserts explicitly - Use valid storage columns for update probes --- tests/sql/two-user-privacy.sql | 54 ++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 12 deletions(-) diff --git a/tests/sql/two-user-privacy.sql b/tests/sql/two-user-privacy.sql index c067735966..da05773d91 100644 --- a/tests/sql/two-user-privacy.sql +++ b/tests/sql/two-user-privacy.sql @@ -142,13 +142,17 @@ select pg_temp.assert_true(not exists ( select pg_temp.assert_true(not (select prosecdef from pg_proc where oid = 'public.is_visible_document_page(uuid, uuid, text)'::regprocedure), 'is_visible_document_page must stay an ordinary (not SECURITY DEFINER) helper'); -create function pg_temp.expect_admin_refusal(p_sql text) returns void language plpgsql as $$ begin - execute p_sql; +create function pg_temp.expect_admin_refusal(p_sql text) returns void language plpgsql as $$ +begin + begin + execute p_sql; + exception when others then + if sqlerrm not like '%administrator%' then + raise exception 'expected an administrator refusal, got "%" from %', sqlerrm, p_sql; + end if; + return; + end; raise exception 'PRIVACY LEAK: an ordinary doctor ran an administrator function: %', p_sql; -exception when others then - if sqlerrm not like '%administrator%' then - raise exception 'expected an administrator refusal, got "%" from %', sqlerrm, p_sql; - end if; end $$; grant execute on function pg_temp.expect_admin_refusal(text) to authenticated; @@ -204,7 +208,12 @@ select b, id || '/fixture.' || b, id from people, (values ('clinical-documents') -- How to tell whose row it is, per table. `$me` is the acting doctor; shared rows (an alias with -- no owner) count as the actor's own. Every table carrying a browser-role policy must be listed -- here, so a new owner policy cannot ship without two-user coverage. -create temp table owner_rule (rel regclass primary key, owner_expr text not null, owner_col text) on commit drop; +create temp table owner_rule ( + rel regclass primary key, + owner_expr text not null, + owner_col text, + update_col text default null +) on commit drop; insert into owner_rule values ('public.documents', 'owner_id', 'owner_id'), ('public.document_pages', '(select d.owner_id from public.documents d where d.id = document_id)', null), @@ -229,7 +238,7 @@ insert into owner_rule values ('public.user_favourite_sets', 'user_id', 'user_id'), ('public.user_favourites', 'user_id', 'user_id'), ('public.user_preferences', 'user_id', 'user_id'), - ('storage.objects', '(storage.foldername(name))[1]::uuid', null); + ('storage.objects', '(storage.foldername(name))[1]::uuid', null, 'metadata'); grant select on owner_rule to authenticated; -- A policy with no TO clause is stored as PUBLIC (role oid 0) and applies to both browser roles. @@ -267,7 +276,7 @@ create function pg_temp.check_isolation(p_me text, p_other text) returns void la declare r record; v_me uuid := pg_temp.uid(p_me); v_other uuid := pg_temp.uid(p_other); v_own bigint; v_foreign bigint; v_rows bigint; v_expr text; begin - for r in select rel, owner_expr, owner_col from owner_rule order by rel::text loop + for r in select rel, owner_expr, owner_col, update_col from owner_rule order by rel::text loop v_expr := replace(r.owner_expr, '$me', quote_literal(v_me) || '::uuid'); execute format('select count(*) filter (where %1$s = %2$L::uuid), count(*) filter (where %1$s is distinct from %2$L::uuid) from %3$s', v_expr, v_me, r.rel) into v_own, v_foreign; @@ -276,12 +285,33 @@ begin -- Changing or deleting the other doctor's rows must touch nothing. begin - execute format('update %s set %s = %s where %s = %L::uuid', - r.rel, coalesce(r.owner_col, 'document_id'), coalesce(r.owner_col, 'document_id'), v_expr, v_other); + execute format('update %s set %I = %I where %s = %L::uuid', + r.rel, coalesce(r.update_col, r.owner_col, 'document_id'), + coalesce(r.update_col, r.owner_col, 'document_id'), v_expr, v_other); get diagnostics v_rows = row_count; if v_rows > 0 then raise exception 'PRIVACY LEAK: doctor % changed % of doctor %''s rows in %', p_me, v_rows, p_other, r.rel; end if; - exception when insufficient_privilege or undefined_column then null; + exception when insufficient_privilege then null; + end; + -- An INSERT policy must not let a doctor plant a new row under the other doctor's owner. + begin + if r.rel = 'public.documents'::regclass then + execute format($sql$ + insert into %s (id, owner_id, title, file_name, file_type, storage_path, status) + values (gen_random_uuid(), %L::uuid, 'Cross-owner insert probe', 'probe.txt', 'text/plain', + %L || '/probe.txt', 'indexed')$sql$, r.rel, v_other, v_other); + elsif r.rel = 'storage.objects'::regclass then + execute format($sql$ + insert into %s (bucket_id, name, owner) + values ('clinical-documents', %L || '/cross-owner-probe-' || gen_random_uuid() || '.txt', %L::uuid)$sql$, + r.rel, v_other, v_other); + end if; + get diagnostics v_rows = row_count; + if v_rows > 0 then + raise exception 'PRIVACY LEAK: doctor % inserted % row(s) for doctor % in %', p_me, v_rows, p_other, r.rel; + end if; + exception when insufficient_privilege then null; end; + begin execute format('delete from %s where %s is distinct from %L::uuid', r.rel, v_expr, v_me); get diagnostics v_rows = row_count; From 590a31afab8048e94d00a921db83608804fcde27 Mon Sep 17 00:00:00 2001 From: "qodo-code-review[bot]" <151058649+qodo-code-review[bot]@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:30:04 +0000 Subject: [PATCH 05/10] fix: Roll back roster behavior fixtures --- tests/sql/roster-behaviour.sql | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/sql/roster-behaviour.sql b/tests/sql/roster-behaviour.sql index 4e8e018f7e..5f5d1f3eae 100644 --- a/tests/sql/roster-behaviour.sql +++ b/tests/sql/roster-behaviour.sql @@ -543,3 +543,7 @@ $delete_account$; set local role service_role; select 'roster behaviour: all checks passed' as result; + +-- The local runner and CI use psql -1, but keep this explicit so selecting this file twice +-- (or running it outside the runner) can never leave its synthetic fixtures behind. +rollback; From b53c879da4b65bcd2ca829f7b659754cef19f4f7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 15:33:36 +0000 Subject: [PATCH 06/10] test(db): probe cross-owner inserts on every covered table and fix the owner_rule rows Qodo's insert probe only touched documents and storage objects, which have no insert policy, so it could not catch a permissive one. Clone one of the actor's own rows, point it at the other doctor and require a row-level security refusal on every covered table. Also gives every owner_rule row the new update_col value, which Qodo's commit left mismatched (VALUES lists must all be the same length). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KhogQUCHgdtFBHFtpjnd1n --- tests/sql/two-user-privacy.sql | 90 +++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 40 deletions(-) diff --git a/tests/sql/two-user-privacy.sql b/tests/sql/two-user-privacy.sql index da05773d91..a7e578b747 100644 --- a/tests/sql/two-user-privacy.sql +++ b/tests/sql/two-user-privacy.sql @@ -215,29 +215,29 @@ create temp table owner_rule ( update_col text default null ) on commit drop; insert into owner_rule values - ('public.documents', 'owner_id', 'owner_id'), - ('public.document_pages', '(select d.owner_id from public.documents d where d.id = document_id)', null), - ('public.document_images', '(select d.owner_id from public.documents d where d.id = document_id)', null), - ('public.document_chunks', '(select d.owner_id from public.documents d where d.id = document_id)', null), - ('public.document_table_facts', '(select d.owner_id from public.documents d where d.id = document_id)', null), - ('public.document_embedding_fields', '(select d.owner_id from public.documents d where d.id = document_id)', null), - ('public.document_index_units', '(select d.owner_id from public.documents d where d.id = document_id)', null), - ('public.ingestion_jobs', '(select d.owner_id from public.documents d where d.id = document_id)', null), - ('public.document_labels', 'owner_id', 'owner_id'), - ('public.document_summaries', 'owner_id', 'owner_id'), - ('public.document_sections', 'owner_id', 'owner_id'), - ('public.document_memory_cards', 'owner_id', 'owner_id'), - ('public.document_index_quality', 'owner_id', 'owner_id'), - ('public.import_batches', 'owner_id', 'owner_id'), - ('public.image_caption_cache', 'owner_id', 'owner_id'), - ('public.rag_queries', 'owner_id', 'owner_id'), - ('public.rag_query_misses', 'owner_id', 'owner_id'), - ('public.rag_aliases', 'coalesce(owner_id, $me)', 'owner_id'), - ('public.rag_retrieval_logs', 'owner_id', 'owner_id'), - ('public.storage_cleanup_jobs', 'owner_id', 'owner_id'), - ('public.user_favourite_sets', 'user_id', 'user_id'), - ('public.user_favourites', 'user_id', 'user_id'), - ('public.user_preferences', 'user_id', 'user_id'), + ('public.documents', 'owner_id', 'owner_id', null), + ('public.document_pages', '(select d.owner_id from public.documents d where d.id = document_id)', null, null), + ('public.document_images', '(select d.owner_id from public.documents d where d.id = document_id)', null, null), + ('public.document_chunks', '(select d.owner_id from public.documents d where d.id = document_id)', null, null), + ('public.document_table_facts', '(select d.owner_id from public.documents d where d.id = document_id)', null, null), + ('public.document_embedding_fields', '(select d.owner_id from public.documents d where d.id = document_id)', null, null), + ('public.document_index_units', '(select d.owner_id from public.documents d where d.id = document_id)', null, null), + ('public.ingestion_jobs', '(select d.owner_id from public.documents d where d.id = document_id)', null, null), + ('public.document_labels', 'owner_id', 'owner_id', null), + ('public.document_summaries', 'owner_id', 'owner_id', null), + ('public.document_sections', 'owner_id', 'owner_id', null), + ('public.document_memory_cards', 'owner_id', 'owner_id', null), + ('public.document_index_quality', 'owner_id', 'owner_id', null), + ('public.import_batches', 'owner_id', 'owner_id', null), + ('public.image_caption_cache', 'owner_id', 'owner_id', null), + ('public.rag_queries', 'owner_id', 'owner_id', null), + ('public.rag_query_misses', 'owner_id', 'owner_id', null), + ('public.rag_aliases', 'coalesce(owner_id, $me)', 'owner_id', null), + ('public.rag_retrieval_logs', 'owner_id', 'owner_id', null), + ('public.storage_cleanup_jobs', 'owner_id', 'owner_id', null), + ('public.user_favourite_sets', 'user_id', 'user_id', null), + ('public.user_favourites', 'user_id', 'user_id', null), + ('public.user_preferences', 'user_id', 'user_id', null), ('storage.objects', '(storage.foldername(name))[1]::uuid', null, 'metadata'); grant select on owner_rule to authenticated; @@ -275,6 +275,7 @@ end $$; create function pg_temp.check_isolation(p_me text, p_other text) returns void language plpgsql as $$ declare r record; v_me uuid := pg_temp.uid(p_me); v_other uuid := pg_temp.uid(p_other); v_own bigint; v_foreign bigint; v_rows bigint; v_expr text; + v_row jsonb; v_cols text; v_refused boolean; begin for r in select rel, owner_expr, owner_col, update_col from owner_rule order by rel::text loop v_expr := replace(r.owner_expr, '$me', quote_literal(v_me) || '::uuid'); @@ -292,25 +293,34 @@ begin if v_rows > 0 then raise exception 'PRIVACY LEAK: doctor % changed % of doctor %''s rows in %', p_me, v_rows, p_other, r.rel; end if; exception when insufficient_privilege then null; end; - -- An INSERT policy must not let a doctor plant a new row under the other doctor's owner. + -- An INSERT policy must not let a doctor plant a new row in the other doctor's name. Clone one + -- of your own rows, point it at the other doctor (owner column, parent document, or storage + -- folder) and insert it. Only a row-level security refusal counts: Postgres checks the policy + -- before constraints, so any other error or a success means the policy let the row through. + -- Fixture documents share their owner's id, so the other doctor's document id is v_other. + execute format('select to_jsonb(t) from %s t where %s = %L::uuid limit 1', r.rel, v_expr, v_me) into v_row; + if r.owner_col is not null then v_row := v_row || jsonb_build_object(r.owner_col, v_other); end if; + if v_row ? 'document_id' then v_row := v_row || jsonb_build_object('document_id', v_other); end if; + if r.rel = 'storage.objects'::regclass then + v_row := v_row || jsonb_build_object('name', v_other || '/planted.txt', 'owner', v_other); + end if; + select string_agg(quote_ident(a.attname), ', ' order by a.attnum) into v_cols + from pg_attribute a + where a.attrelid = r.rel and a.attnum > 0 and not a.attisdropped and a.attgenerated = '' + and a.attidentity = '' and a.attname <> 'id'; + v_refused := false; begin - if r.rel = 'public.documents'::regclass then - execute format($sql$ - insert into %s (id, owner_id, title, file_name, file_type, storage_path, status) - values (gen_random_uuid(), %L::uuid, 'Cross-owner insert probe', 'probe.txt', 'text/plain', - %L || '/probe.txt', 'indexed')$sql$, r.rel, v_other, v_other); - elsif r.rel = 'storage.objects'::regclass then - execute format($sql$ - insert into %s (bucket_id, name, owner) - values ('clinical-documents', %L || '/cross-owner-probe-' || gen_random_uuid() || '.txt', %L::uuid)$sql$, - r.rel, v_other, v_other); - end if; - get diagnostics v_rows = row_count; - if v_rows > 0 then - raise exception 'PRIVACY LEAK: doctor % inserted % row(s) for doctor % in %', p_me, v_rows, p_other, r.rel; - end if; - exception when insufficient_privilege then null; + execute format('insert into %1$s (%2$s) select %2$s from jsonb_populate_record(null::%1$s, %3$L::jsonb)', + r.rel, v_cols, v_row); + exception + when insufficient_privilege then v_refused := true; + when others then + raise exception 'PRIVACY LEAK: doctor %''s row for doctor % got past the % insert policy (then: %)', + p_me, p_other, r.rel, sqlerrm; end; + if not v_refused then + raise exception 'PRIVACY LEAK: doctor % inserted a row for doctor % in %', p_me, p_other, r.rel; + end if; begin execute format('delete from %s where %s is distinct from %L::uuid', r.rel, v_expr, v_me); From 473e8c9c40515c216608454a010462783f3d9e26 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 15:36:34 +0000 Subject: [PATCH 07/10] test(db): allowlist browser-callable functions by exact signature A new overload reusing an allowed name is now caught. Proven by planting is_visible_document_page(integer) granted to authenticated: the test fails. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KhogQUCHgdtFBHFtpjnd1n --- tests/sql/two-user-privacy.sql | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/tests/sql/two-user-privacy.sql b/tests/sql/two-user-privacy.sql index a7e578b747..d85771e4b4 100644 --- a/tests/sql/two-user-privacy.sql +++ b/tests/sql/two-user-privacy.sql @@ -124,21 +124,26 @@ select pg_temp.assert_true(not exists ( ), 'the public role can call a public function: ' || coalesce(( select string_agg(p.oid::regprocedure::text, ', ') from pg_proc p join pg_namespace n on n.oid = p.pronamespace where n.nspname = 'public' and has_function_privilege('anon', p.oid, 'EXECUTE')), '')); +-- Exact signatures, so a new overload that reuses an allowed name is still caught. +create temp table allowed_browser_functions(signature text primary key, reason text not null); +insert into allowed_browser_functions values + ('admit_administrator_document(jsonb,jsonb,boolean)', 'administrator plus recent sign-in, checked inside'), + ('publish_site_content_record(text,uuid,text,bigint,text,text,text)', 'administrator, checked inside'), + ('record_site_content_reconciliation_plan(jsonb)', 'administrator, checked inside'), + ('retire_site_content_record(text,uuid,text,bigint,text,text,text)', 'administrator, checked inside'), + ('is_visible_document_page(uuid,uuid,text)', 'pure helper for the page policy, reads nothing'); +create temp view browser_callable_functions as + select replace(p.oid::regprocedure::text, ' ', '') as signature + from pg_proc p join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' and has_function_privilege('authenticated', p.oid, 'EXECUTE'); select pg_temp.assert_true(not exists ( - select 1 from pg_proc p join pg_namespace n on n.oid = p.pronamespace - where n.nspname = 'public' and has_function_privilege('authenticated', p.oid, 'EXECUTE') - and p.proname not in ( - 'admit_administrator_document', -- administrator plus recent sign-in, checked inside - 'publish_site_content_record', -- administrator, checked inside - 'record_site_content_reconciliation_plan', -- administrator, checked inside - 'retire_site_content_record', -- administrator, checked inside - 'is_visible_document_page' -- pure helper for the page policy, reads nothing - ) + select 1 from browser_callable_functions c + where c.signature not in (select replace('public.' || signature, ' ', '') from allowed_browser_functions) + and c.signature not in (select replace(signature, ' ', '') from allowed_browser_functions) ), 'a signed-in doctor can call a function outside the reviewed allowlist: ' || coalesce(( - select string_agg(p.oid::regprocedure::text, ', ') from pg_proc p join pg_namespace n on n.oid = p.pronamespace - where n.nspname = 'public' and has_function_privilege('authenticated', p.oid, 'EXECUTE') - and p.proname not in ('admit_administrator_document', 'publish_site_content_record', - 'record_site_content_reconciliation_plan', 'retire_site_content_record', 'is_visible_document_page')), '')); + select string_agg(c.signature, ', ') from browser_callable_functions c + where c.signature not in (select replace('public.' || signature, ' ', '') from allowed_browser_functions) + and c.signature not in (select replace(signature, ' ', '') from allowed_browser_functions)), '')); select pg_temp.assert_true(not (select prosecdef from pg_proc where oid = 'public.is_visible_document_page(uuid, uuid, text)'::regprocedure), 'is_visible_document_page must stay an ordinary (not SECURITY DEFINER) helper'); From d5d1964356b9dea43d03b652b59846e0be5d6c0f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 15:42:14 +0000 Subject: [PATCH 08/10] test(db): probe own rows attached to the other doctor's document The insert probe now also keeps the owner column as yours and moves only the parent document. Three owner policies (document_labels, document_sections, document_memory_cards) check owner_id but not the parent document's owner. Browser roles have no grant on these tables and the server routes check document ownership first, so this is unreachable today. The test records the three tables as a known gap and fails if the list stops matching in either direction. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KhogQUCHgdtFBHFtpjnd1n --- tests/sql/two-user-privacy.sql | 84 ++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 24 deletions(-) diff --git a/tests/sql/two-user-privacy.sql b/tests/sql/two-user-privacy.sql index d85771e4b4..4d32f7d181 100644 --- a/tests/sql/two-user-privacy.sql +++ b/tests/sql/two-user-privacy.sql @@ -269,6 +269,15 @@ do $$ declare r record; begin end $$; -- Totals as the owner, before either doctor acts. +-- Known gap, recorded rather than hidden: these owner policies check only owner_id, not that the +-- parent document is yours. Browser roles have no grant on these tables (part 1), so nothing can +-- reach this today. The test fails if this list stops matching what the probe finds. +create temp table known_parent_document_gap(rel regclass primary key); +insert into known_parent_document_gap values + ('public.document_labels'), ('public.document_memory_cards'), ('public.document_sections'); +create temp table parent_document_gap_seen(rel regclass primary key); +grant select on known_parent_document_gap to authenticated; +grant select, insert on parent_document_gap_seen to authenticated; create temp table owner_totals on commit drop as select rel, 0::bigint as total from owner_rule; do $$ declare r record; v bigint; begin for r in select rel from owner_rule loop @@ -280,7 +289,7 @@ end $$; create function pg_temp.check_isolation(p_me text, p_other text) returns void language plpgsql as $$ declare r record; v_me uuid := pg_temp.uid(p_me); v_other uuid := pg_temp.uid(p_other); v_own bigint; v_foreign bigint; v_rows bigint; v_expr text; - v_row jsonb; v_cols text; v_refused boolean; + v_base jsonb; v_row jsonb; v_cols text; v_refused boolean; v_variant int; v_detail text; begin for r in select rel, owner_expr, owner_col, update_col from owner_rule order by rel::text loop v_expr := replace(r.owner_expr, '$me', quote_literal(v_me) || '::uuid'); @@ -298,34 +307,52 @@ begin if v_rows > 0 then raise exception 'PRIVACY LEAK: doctor % changed % of doctor %''s rows in %', p_me, v_rows, p_other, r.rel; end if; exception when insufficient_privilege then null; end; - -- An INSERT policy must not let a doctor plant a new row in the other doctor's name. Clone one - -- of your own rows, point it at the other doctor (owner column, parent document, or storage - -- folder) and insert it. Only a row-level security refusal counts: Postgres checks the policy + -- An INSERT policy must not let a doctor plant a new row in the other doctor's name, or hang + -- their own row off the other doctor's document. Clone one of your own rows and insert it twice: + -- first with everything (owner column, parent document, storage folder) pointed at the other + -- doctor, then, where the row has both an owner column and a parent document, with only the + -- parent document moved. Only a row-level security refusal counts: Postgres checks the policy -- before constraints, so any other error or a success means the policy let the row through. + -- A successful insert is undone straight away so row totals stay exact. -- Fixture documents share their owner's id, so the other doctor's document id is v_other. - execute format('select to_jsonb(t) from %s t where %s = %L::uuid limit 1', r.rel, v_expr, v_me) into v_row; - if r.owner_col is not null then v_row := v_row || jsonb_build_object(r.owner_col, v_other); end if; - if v_row ? 'document_id' then v_row := v_row || jsonb_build_object('document_id', v_other); end if; - if r.rel = 'storage.objects'::regclass then - v_row := v_row || jsonb_build_object('name', v_other || '/planted.txt', 'owner', v_other); - end if; + execute format('select to_jsonb(t) from %s t where %s = %L::uuid limit 1', r.rel, v_expr, v_me) into v_base; select string_agg(quote_ident(a.attname), ', ' order by a.attnum) into v_cols from pg_attribute a where a.attrelid = r.rel and a.attnum > 0 and not a.attisdropped and a.attgenerated = '' and a.attidentity = '' and a.attname <> 'id'; - v_refused := false; - begin - execute format('insert into %1$s (%2$s) select %2$s from jsonb_populate_record(null::%1$s, %3$L::jsonb)', - r.rel, v_cols, v_row); - exception - when insufficient_privilege then v_refused := true; - when others then - raise exception 'PRIVACY LEAK: doctor %''s row for doctor % got past the % insert policy (then: %)', - p_me, p_other, r.rel, sqlerrm; - end; - if not v_refused then - raise exception 'PRIVACY LEAK: doctor % inserted a row for doctor % in %', p_me, p_other, r.rel; - end if; + for v_variant in 1..(case when r.owner_col is not null and v_base ? 'document_id' then 2 else 1 end) loop + v_row := v_base; + if r.owner_col is not null and v_variant = 1 then v_row := v_row || jsonb_build_object(r.owner_col, v_other); end if; + if v_row ? 'document_id' then v_row := v_row || jsonb_build_object('document_id', v_other); end if; + if r.rel = 'storage.objects'::regclass then + v_row := v_row || jsonb_build_object('name', v_other || '/planted.txt', 'owner', v_other); + end if; + v_refused := false; v_detail := null; + begin + execute format('insert into %1$s (%2$s) select %2$s from jsonb_populate_record(null::%1$s, %3$L::jsonb)', + r.rel, v_cols, v_row); + raise exception using errcode = 'PX001', message = 'probe row inserted'; + exception + when insufficient_privilege then v_refused := true; + when sqlstate 'PX001' then null; + when others then + -- A non-policy error means the policy already let the row through. + if v_variant = 2 then + v_detail := sqlerrm; + else + raise exception 'PRIVACY LEAK: doctor %''s row for doctor % got past the % insert policy (then: %)', + p_me, p_other, r.rel, sqlerrm; + end if; + end; + if not v_refused and v_variant = 2 and r.rel in (select rel from known_parent_document_gap) then + insert into parent_document_gap_seen values (r.rel) on conflict do nothing; + elsif not v_refused and v_variant = 2 then + raise exception 'PRIVACY LEAK: doctor % attached their own row to doctor %''s document in % (then: %)', + p_me, p_other, r.rel, coalesce(v_detail, 'inserted'); + elsif not v_refused then + raise exception 'PRIVACY LEAK: doctor % inserted a row for doctor % in %', p_me, p_other, r.rel; + end if; + end loop; begin execute format('delete from %s where %s is distinct from %L::uuid', r.rel, v_expr, v_me); @@ -369,6 +396,15 @@ select pg_temp.assert_true((select count(*) from public.documents where owner_id and (select count(*) from public.documents where owner_id = pg_temp.uid('b')) = 1, 'each doctor must still own exactly their own document'); +-- The parent-document gap list must match what the probe found, both ways: a new gap fails, and a +-- fixed policy fails until its table comes off the list. +select pg_temp.assert_true(not exists ( + (select rel from known_parent_document_gap except select rel from parent_document_gap_seen) + union all + (select rel from parent_document_gap_seen except select rel from known_parent_document_gap) +), 'known_parent_document_gap no longer matches the probe; found: ' || coalesce(( + select string_agg(rel::text, ', ' order by rel::text) from parent_document_gap_seen), 'none')); + select 'TWO_USER_PRIVACY_SQL_PASS: browser roles fail closed on every table, function allowlist, ' - || 'admin functions refuse doctors, owner policies isolate two doctors' as result; + || 'admin functions refuse doctors, owner policies isolate two doctors (3 known parent-document gaps)' as result; rollback; From a295ac3dba331b524281a01cb6d497a430ace38c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 15:44:06 +0000 Subject: [PATCH 09/10] test(db): probe storage inserts once per bucket A permissive insert policy scoped to one bucket is now caught; proven by planting one on clinical-images only. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KhogQUCHgdtFBHFtpjnd1n --- tests/sql/two-user-privacy.sql | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/sql/two-user-privacy.sql b/tests/sql/two-user-privacy.sql index 4d32f7d181..65b4064cb7 100644 --- a/tests/sql/two-user-privacy.sql +++ b/tests/sql/two-user-privacy.sql @@ -315,7 +315,10 @@ begin -- before constraints, so any other error or a success means the policy let the row through. -- A successful insert is undone straight away so row totals stay exact. -- Fixture documents share their owner's id, so the other doctor's document id is v_other. - execute format('select to_jsonb(t) from %s t where %s = %L::uuid limit 1', r.rel, v_expr, v_me) into v_base; + -- Storage objects are probed once per bucket, so a policy scoped to one bucket can't hide. + for v_base in execute format( + 'select distinct on (%3$s) to_jsonb(t) from %1$s t where %2$s = %4$L::uuid order by %3$s', + r.rel, v_expr, case when r.rel = 'storage.objects'::regclass then 't.bucket_id' else '1' end, v_me) loop select string_agg(quote_ident(a.attname), ', ' order by a.attnum) into v_cols from pg_attribute a where a.attrelid = r.rel and a.attnum > 0 and not a.attisdropped and a.attgenerated = '' @@ -325,7 +328,7 @@ begin if r.owner_col is not null and v_variant = 1 then v_row := v_row || jsonb_build_object(r.owner_col, v_other); end if; if v_row ? 'document_id' then v_row := v_row || jsonb_build_object('document_id', v_other); end if; if r.rel = 'storage.objects'::regclass then - v_row := v_row || jsonb_build_object('name', v_other || '/planted.txt', 'owner', v_other); + v_row := v_row || jsonb_build_object('name', v_other || '/planted-' || (v_base->>'bucket_id') || '.txt', 'owner', v_other); end if; v_refused := false; v_detail := null; begin @@ -353,6 +356,7 @@ begin raise exception 'PRIVACY LEAK: doctor % inserted a row for doctor % in %', p_me, p_other, r.rel; end if; end loop; + end loop; begin execute format('delete from %s where %s is distinct from %L::uuid', r.rel, v_expr, v_me); From 09bbbade7a7a5080514eccccc3c8c66be141359d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 16:18:17 +0000 Subject: [PATCH 10/10] test(db): wrap roster fixtures in their own transaction and probe one row per table A direct psql -f run without -1 now rolls the roster fixtures back too. distinct on uses a typed constant, so non-storage tables probe one row. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KhogQUCHgdtFBHFtpjnd1n --- tests/sql/roster-behaviour.sql | 5 +++-- tests/sql/two-user-privacy.sql | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/sql/roster-behaviour.sql b/tests/sql/roster-behaviour.sql index 5f5d1f3eae..5d198c7c15 100644 --- a/tests/sql/roster-behaviour.sql +++ b/tests/sql/roster-behaviour.sql @@ -1,6 +1,7 @@ -- Behaviour checks for the combined Roster DB PR, run against a disposable replay of files 1-5. -- Every block raises on a wrong answer, so a clean run prints only the final line. -- Run: psql -v ON_ERROR_STOP=1 -1 -f tests/sql/roster-behaviour.sql (never against a live database) +begin; create temp table ids (k text primary key, v uuid) on commit drop; grant all on ids to service_role; @@ -544,6 +545,6 @@ set local role service_role; select 'roster behaviour: all checks passed' as result; --- The local runner and CI use psql -1, but keep this explicit so selecting this file twice --- (or running it outside the runner) can never leave its synthetic fixtures behind. +-- The local runner and CI use psql -1, but the explicit begin and rollback mean a direct psql -f run +-- (without -1) also leaves no synthetic fixtures behind. rollback; diff --git a/tests/sql/two-user-privacy.sql b/tests/sql/two-user-privacy.sql index 65b4064cb7..d9ec8d7c4a 100644 --- a/tests/sql/two-user-privacy.sql +++ b/tests/sql/two-user-privacy.sql @@ -318,7 +318,7 @@ begin -- Storage objects are probed once per bucket, so a policy scoped to one bucket can't hide. for v_base in execute format( 'select distinct on (%3$s) to_jsonb(t) from %1$s t where %2$s = %4$L::uuid order by %3$s', - r.rel, v_expr, case when r.rel = 'storage.objects'::regclass then 't.bucket_id' else '1' end, v_me) loop + r.rel, v_expr, case when r.rel = 'storage.objects'::regclass then 't.bucket_id' else 'true::boolean' end, v_me) loop select string_agg(quote_ident(a.attname), ', ' order by a.attnum) into v_cols from pg_attribute a where a.attrelid = r.rel and a.attnum > 0 and not a.attisdropped and a.attgenerated = ''