From ec956ff6a65d8dcd18f56b3abdcd63c7351504b0 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Wed, 28 Oct 2026 08:37:36 -0700 Subject: [PATCH 1/7] feat(storage): partner file metadata + 30-day retention schema (v138) Add partner_files / partner_file_derived / partner_file_deletion_log with CASE RLS, trigger-maintained delete_after (+30d), and private partner-uploads bucket policies. Mirror on Alembic; nullable raw_bytes + payload check on fsma.manifest_blobs for later hash/pointer cutover. Migration-only; do not apply without C.Lee yes. Synthetic fixtures only until SEL-590. Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> --- ...901_v138_partner_file_storage_retention.py | 369 ++++++++++++++++++ ...100000_partner_files_storage_retention.sql | 256 ++++++++++++ .../20261028100010_partner_uploads_bucket.sql | 58 +++ 3 files changed, 683 insertions(+) create mode 100644 alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py create mode 100644 supabase/migrations/20261028100000_partner_files_storage_retention.sql create mode 100644 supabase/migrations/20261028100010_partner_uploads_bucket.sql diff --git a/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py b/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py new file mode 100644 index 000000000..bf0a7887a --- /dev/null +++ b/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py @@ -0,0 +1,369 @@ +"""Partner file storage metadata + 30-day retention (v138). + +Mirrors supabase/migrations/20261028100000_partner_files_storage_retention.sql +exactly for partner_files / partner_file_derived / partner_file_deletion_log +(tables, indexes, CASE RLS, grants). Bucket creation lives only in +supabase/migrations/20261028100010_partner_uploads_bucket.sql (storage schema +is Supabase-only). + +Also prepares Railway fsma.manifest_blobs for hash/pointer cutover: raw_bytes +becomes nullable with a check that at least one of raw_bytes or storage_url is +present (storage_url already exists from v095). Does not move bytes; does not +apply envelope encryption. + +Additive and idempotent. Downgrade drops only what this revision created and +restores raw_bytes NOT NULL when safe. + +Revision ID: b8c1e4f2a901 +Revises: a3f7ce361b0e +Create Date: 2026-10-28 +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + +revision: str = "b8c1e4f2a901" # pragma: allowlist secret +down_revision: Union[str, Sequence[str], None] = "a3f7ce361b0e" # pragma: allowlist secret +branch_labels = None +depends_on = None + +# Mirrored from the supabase SQL migration (minus the hand-only downgrade notes). +_PARTNER_FILES_SQL = """ +-- Partner file storage metadata + 30-day retention clock (SEL-549 / first-file). +-- +-- Metadata only. Partner file bytes live in the private Supabase Storage bucket +-- `partner-uploads` (see sibling migration 20261028100010). Never store partner +-- file contents in these tables or in the deletion log. +-- +-- Clock: delete_after = report_delivered_at + 30 days (trigger-maintained; UTC). +-- Envelope encryption columns (kek_key_id / wrapped_dek) are nullable; v1 may +-- rely on provider at-rest encryption only (design open Q1 deferred). +-- +-- RLS uses CASE + (select public.get_tenant_context()), matching epcis / +-- data_gap (not the JWT sketch). Idempotent. Do not apply to prod without +-- C.Lee yes. Synthetic fixtures only until SEL-590 clears. + +CREATE TABLE IF NOT EXISTS public.partner_files ( + id uuid primary key default gen_random_uuid(), + tenant_id uuid not null references public.tenants(id), + engagement_id uuid not null, + storage_bucket text not null default 'partner-uploads', + storage_path text not null, + content_sha256 text not null, + ciphertext_sha256 text, + byte_size bigint not null, + content_type text not null, + original_filename text not null, + kek_key_id text, + wrapped_dek bytea, + uploaded_at timestamptz not null default now(), + report_delivered_at timestamptz, + -- Maintained by partner_files_set_delete_after trigger (timestamptz + interval + -- is not IMMUTABLE, so a generated column is not allowed). + delete_after timestamptz, + deleted_at timestamptz, + status text not null default 'uploaded', + constraint partner_files_byte_size_check + check (byte_size > 0), + constraint partner_files_content_sha256_check + check (content_sha256 ~ '^[0-9a-f]{64}$'), + constraint partner_files_ciphertext_sha256_check + check (ciphertext_sha256 is null or ciphertext_sha256 ~ '^[0-9a-f]{64}$'), + constraint partner_files_status_check + check (status in ( + 'uploaded', + 'processing', + 'report_ready', + 'delivered', + 'pending_delete', + 'deleted' + )), + constraint partner_files_storage_bucket_check + check (storage_bucket = 'partner-uploads'), + constraint partner_files_storage_path_unique + unique (storage_bucket, storage_path) +); + +CREATE INDEX IF NOT EXISTS partner_files_due_delete_idx + ON public.partner_files (delete_after) + WHERE deleted_at IS NULL AND report_delivered_at IS NOT NULL; + +CREATE INDEX IF NOT EXISTS partner_files_tenant_uploaded_idx + ON public.partner_files (tenant_id, uploaded_at desc); + +-- delete_after = report_delivered_at + 30 days (trigger; generated col not allowed) +CREATE OR REPLACE FUNCTION public.partner_files_set_delete_after() +RETURNS trigger +LANGUAGE plpgsql +AS $$ +BEGIN + IF NEW.report_delivered_at IS NULL THEN + NEW.delete_after := NULL; + ELSE + NEW.delete_after := NEW.report_delivered_at + interval '30 days'; + END IF; + RETURN NEW; +END; +$$; + +DROP TRIGGER IF EXISTS partner_files_set_delete_after ON public.partner_files; +CREATE TRIGGER partner_files_set_delete_after + BEFORE INSERT OR UPDATE OF report_delivered_at + ON public.partner_files + FOR EACH ROW + EXECUTE FUNCTION public.partner_files_set_delete_after(); + +CREATE TABLE IF NOT EXISTS public.partner_file_derived ( + id uuid primary key default gen_random_uuid(), + partner_file_id uuid not null references public.partner_files(id) on delete cascade, + tenant_id uuid not null references public.tenants(id), + kind text not null, + storage_bucket text, + storage_path text, + row_ref text, + created_at timestamptz not null default now(), + deleted_at timestamptz, + constraint partner_file_derived_kind_check + check (char_length(kind) between 1 and 64) +); + +CREATE INDEX IF NOT EXISTS partner_file_derived_file_idx + ON public.partner_file_derived (partner_file_id) + WHERE deleted_at IS NULL; + +CREATE TABLE IF NOT EXISTS public.partner_file_deletion_log ( + id uuid primary key default gen_random_uuid(), + partner_file_id uuid not null, + tenant_id uuid not null, + engagement_id uuid, + storage_bucket text not null, + storage_path text not null, + content_sha256 text not null, + ciphertext_sha256 text, + byte_size bigint, + report_delivered_at timestamptz, + deleted_at timestamptz not null default now(), + actor text not null, + reason text not null, + entry_hash text not null, + prev_entry_hash text, + constraint partner_file_deletion_log_content_sha256_check + check (content_sha256 ~ '^[0-9a-f]{64}$'), + constraint partner_file_deletion_log_ciphertext_sha256_check + check (ciphertext_sha256 is null or ciphertext_sha256 ~ '^[0-9a-f]{64}$'), + constraint partner_file_deletion_log_entry_hash_check + check (entry_hash ~ '^[0-9a-f]{64}$'), + constraint partner_file_deletion_log_prev_entry_hash_check + check (prev_entry_hash is null or prev_entry_hash ~ '^[0-9a-f]{64}$'), + constraint partner_file_deletion_log_actor_check + check ( + actor = 'retention_job' + or actor = 'partner_request' + or actor like 'ops:%' + ), + constraint partner_file_deletion_log_reason_check + check (reason in ('retention_30d', 'partner_request_15d', 'terms_end')) +); + +CREATE INDEX IF NOT EXISTS partner_file_deletion_log_tenant_deleted_idx + ON public.partner_file_deletion_log (tenant_id, deleted_at desc); + +ALTER TABLE public.partner_files ENABLE ROW LEVEL SECURITY; +ALTER TABLE public.partner_files FORCE ROW LEVEL SECURITY; +ALTER TABLE public.partner_file_derived ENABLE ROW LEVEL SECURITY; +ALTER TABLE public.partner_file_derived FORCE ROW LEVEL SECURITY; +ALTER TABLE public.partner_file_deletion_log ENABLE ROW LEVEL SECURITY; +ALTER TABLE public.partner_file_deletion_log FORCE ROW LEVEL SECURITY; + +-- CASE form, not OR: public.get_tenant_context() raises when app.tenant_id is +-- unset and Postgres does not order OR operands, so the sysadmin arm is tested +-- first (same predicate shape as alembic v096 / v134 / v135 / v137). +DROP POLICY IF EXISTS tenant_isolation_partner_files ON public.partner_files; +CREATE POLICY tenant_isolation_partner_files ON public.partner_files + FOR ALL + USING ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ) + WITH CHECK ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ); + +DROP POLICY IF EXISTS tenant_isolation_partner_file_derived ON public.partner_file_derived; +CREATE POLICY tenant_isolation_partner_file_derived ON public.partner_file_derived + FOR ALL + USING ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ) + WITH CHECK ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ); + +DROP POLICY IF EXISTS tenant_isolation_partner_file_deletion_log ON public.partner_file_deletion_log; +CREATE POLICY tenant_isolation_partner_file_deletion_log ON public.partner_file_deletion_log + FOR SELECT + USING ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ); + +-- Inserts into the deletion log go through the retention worker (regengine / +-- SECURITY DEFINER RPC). No authenticated UPDATE/DELETE on the log. +DROP POLICY IF EXISTS partner_file_deletion_log_insert_worker ON public.partner_file_deletion_log; +CREATE POLICY partner_file_deletion_log_insert_worker ON public.partner_file_deletion_log + FOR INSERT + WITH CHECK ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ); + +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'anon') THEN + REVOKE ALL ON public.partner_files FROM anon; + REVOKE ALL ON public.partner_file_derived FROM anon; + REVOKE ALL ON public.partner_file_deletion_log FROM anon; + END IF; + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'authenticated') THEN + -- Metadata reads/writes for tenant users go through the app role / + -- signed upload path, not broad authenticated table DML. + REVOKE ALL ON public.partner_files FROM authenticated; + REVOKE ALL ON public.partner_file_derived FROM authenticated; + REVOKE ALL ON public.partner_file_deletion_log FROM authenticated; + END IF; + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') THEN + GRANT SELECT, INSERT, UPDATE ON public.partner_files TO regengine; + GRANT SELECT, INSERT, UPDATE ON public.partner_file_derived TO regengine; + -- Append-only audit: INSERT + SELECT only. Never UPDATE/DELETE. + GRANT SELECT, INSERT ON public.partner_file_deletion_log TO regengine; + REVOKE UPDATE, DELETE ON public.partner_file_deletion_log FROM regengine; + END IF; + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine_sysadmin') THEN + GRANT SELECT ON public.partner_files TO regengine_sysadmin; + GRANT SELECT ON public.partner_file_derived TO regengine_sysadmin; + GRANT SELECT ON public.partner_file_deletion_log TO regengine_sysadmin; + END IF; + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'service_role') THEN + -- Prefer SECURITY DEFINER RPCs; keep service_role from mutating the log. + REVOKE UPDATE, DELETE ON public.partner_file_deletion_log FROM service_role; + END IF; +END $$; + +""" + +_MANIFEST_BLOBS_SQL = """ +-- Railway cutover helper: allow hash/pointer-only manifest_blobs rows. +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_schema = 'fsma' + AND table_name = 'manifest_blobs' + AND column_name = 'raw_bytes' + ) THEN + ALTER TABLE fsma.manifest_blobs ALTER COLUMN raw_bytes DROP NOT NULL; + END IF; +END $$; + +ALTER TABLE fsma.manifest_blobs DROP CONSTRAINT IF EXISTS manifest_blobs_payload_present_check; +ALTER TABLE fsma.manifest_blobs + ADD CONSTRAINT manifest_blobs_payload_present_check + CHECK ( + raw_bytes IS NOT NULL + OR (storage_url IS NOT NULL AND length(btrim(storage_url)) > 0) + ); +""" + + +def upgrade() -> None: + # Split on semicolons outside dollar-quoted blocks so DO $$ ... $$; stays intact. + def _statements(sql: str) -> list[str]: + out: list[str] = [] + buf: list[str] = [] + in_dollar = False + i = 0 + while i < len(sql): + if sql.startswith("$$", i): + in_dollar = not in_dollar + buf.append("$$") + i += 2 + continue + ch = sql[i] + if ch == ";" and not in_dollar: + stmt = "".join(buf).strip() + if stmt and not all( + line.strip().startswith("--") or not line.strip() + for line in stmt.splitlines() + ): + out.append(stmt) + buf = [] + i += 1 + continue + buf.append(ch) + i += 1 + tail = "".join(buf).strip() + if tail: + out.append(tail) + return out + + for statement in _statements(_PARTNER_FILES_SQL) + _statements(_MANIFEST_BLOBS_SQL): + op.execute(sa.text(statement)) + + +def downgrade() -> None: + op.execute( + sa.text( + """ +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 FROM information_schema.tables + WHERE table_schema = 'fsma' AND table_name = 'manifest_blobs' + ) THEN + ALTER TABLE fsma.manifest_blobs DROP CONSTRAINT IF EXISTS manifest_blobs_payload_present_check; + IF NOT EXISTS ( + SELECT 1 FROM fsma.manifest_blobs WHERE raw_bytes IS NULL + ) THEN + ALTER TABLE fsma.manifest_blobs ALTER COLUMN raw_bytes SET NOT NULL; + END IF; + END IF; +END $$; +""" + ) + ) + op.execute(sa.text("DROP POLICY IF EXISTS partner_file_deletion_log_insert_worker ON public.partner_file_deletion_log")) + op.execute(sa.text("DROP POLICY IF EXISTS tenant_isolation_partner_file_deletion_log ON public.partner_file_deletion_log")) + op.execute(sa.text("DROP POLICY IF EXISTS tenant_isolation_partner_file_derived ON public.partner_file_derived")) + op.execute(sa.text("DROP POLICY IF EXISTS tenant_isolation_partner_files ON public.partner_files")) + op.execute(sa.text("DROP TRIGGER IF EXISTS partner_files_set_delete_after ON public.partner_files")) + op.execute(sa.text("DROP FUNCTION IF EXISTS public.partner_files_set_delete_after()")) + op.execute(sa.text("DROP INDEX IF EXISTS public.partner_file_deletion_log_tenant_deleted_idx")) + op.execute(sa.text("DROP INDEX IF EXISTS public.partner_file_derived_file_idx")) + op.execute(sa.text("DROP INDEX IF EXISTS public.partner_files_tenant_uploaded_idx")) + op.execute(sa.text("DROP INDEX IF EXISTS public.partner_files_due_delete_idx")) + op.execute(sa.text("DROP TABLE IF EXISTS public.partner_file_deletion_log")) + op.execute(sa.text("DROP TABLE IF EXISTS public.partner_file_derived")) + op.execute(sa.text("DROP TABLE IF EXISTS public.partner_files")) diff --git a/supabase/migrations/20261028100000_partner_files_storage_retention.sql b/supabase/migrations/20261028100000_partner_files_storage_retention.sql new file mode 100644 index 000000000..6db6e2433 --- /dev/null +++ b/supabase/migrations/20261028100000_partner_files_storage_retention.sql @@ -0,0 +1,256 @@ +-- Partner file storage metadata + 30-day retention clock (SEL-549 / first-file). +-- +-- Metadata only. Partner file bytes live in the private Supabase Storage bucket +-- `partner-uploads` (see sibling migration 20261028100010). Never store partner +-- file contents in these tables or in the deletion log. +-- +-- Clock: delete_after = report_delivered_at + 30 days (trigger-maintained; UTC). +-- Envelope encryption columns (kek_key_id / wrapped_dek) are nullable; v1 may +-- rely on provider at-rest encryption only (design open Q1 deferred). +-- +-- RLS uses CASE + (select public.get_tenant_context()), matching epcis / +-- data_gap (not the JWT sketch). Idempotent. Do not apply to prod without +-- C.Lee yes. Synthetic fixtures only until SEL-590 clears. + +CREATE TABLE IF NOT EXISTS public.partner_files ( + id uuid primary key default gen_random_uuid(), + tenant_id uuid not null references public.tenants(id), + engagement_id uuid not null, + storage_bucket text not null default 'partner-uploads', + storage_path text not null, + content_sha256 text not null, + ciphertext_sha256 text, + byte_size bigint not null, + content_type text not null, + original_filename text not null, + kek_key_id text, + wrapped_dek bytea, + uploaded_at timestamptz not null default now(), + report_delivered_at timestamptz, + -- Maintained by partner_files_set_delete_after trigger (timestamptz + interval + -- is not IMMUTABLE, so a generated column is not allowed). + delete_after timestamptz, + deleted_at timestamptz, + status text not null default 'uploaded', + constraint partner_files_byte_size_check + check (byte_size > 0), + constraint partner_files_content_sha256_check + check (content_sha256 ~ '^[0-9a-f]{64}$'), + constraint partner_files_ciphertext_sha256_check + check (ciphertext_sha256 is null or ciphertext_sha256 ~ '^[0-9a-f]{64}$'), + constraint partner_files_status_check + check (status in ( + 'uploaded', + 'processing', + 'report_ready', + 'delivered', + 'pending_delete', + 'deleted' + )), + constraint partner_files_storage_bucket_check + check (storage_bucket = 'partner-uploads'), + constraint partner_files_storage_path_unique + unique (storage_bucket, storage_path) +); + +CREATE INDEX IF NOT EXISTS partner_files_due_delete_idx + ON public.partner_files (delete_after) + WHERE deleted_at IS NULL AND report_delivered_at IS NOT NULL; + +CREATE INDEX IF NOT EXISTS partner_files_tenant_uploaded_idx + ON public.partner_files (tenant_id, uploaded_at desc); + +-- delete_after = report_delivered_at + 30 days (trigger; generated col not allowed) +CREATE OR REPLACE FUNCTION public.partner_files_set_delete_after() +RETURNS trigger +LANGUAGE plpgsql +AS $$ +BEGIN + IF NEW.report_delivered_at IS NULL THEN + NEW.delete_after := NULL; + ELSE + NEW.delete_after := NEW.report_delivered_at + interval '30 days'; + END IF; + RETURN NEW; +END; +$$; + +DROP TRIGGER IF EXISTS partner_files_set_delete_after ON public.partner_files; +CREATE TRIGGER partner_files_set_delete_after + BEFORE INSERT OR UPDATE OF report_delivered_at + ON public.partner_files + FOR EACH ROW + EXECUTE FUNCTION public.partner_files_set_delete_after(); + +CREATE TABLE IF NOT EXISTS public.partner_file_derived ( + id uuid primary key default gen_random_uuid(), + partner_file_id uuid not null references public.partner_files(id) on delete cascade, + tenant_id uuid not null references public.tenants(id), + kind text not null, + storage_bucket text, + storage_path text, + row_ref text, + created_at timestamptz not null default now(), + deleted_at timestamptz, + constraint partner_file_derived_kind_check + check (char_length(kind) between 1 and 64) +); + +CREATE INDEX IF NOT EXISTS partner_file_derived_file_idx + ON public.partner_file_derived (partner_file_id) + WHERE deleted_at IS NULL; + +CREATE TABLE IF NOT EXISTS public.partner_file_deletion_log ( + id uuid primary key default gen_random_uuid(), + partner_file_id uuid not null, + tenant_id uuid not null, + engagement_id uuid, + storage_bucket text not null, + storage_path text not null, + content_sha256 text not null, + ciphertext_sha256 text, + byte_size bigint, + report_delivered_at timestamptz, + deleted_at timestamptz not null default now(), + actor text not null, + reason text not null, + entry_hash text not null, + prev_entry_hash text, + constraint partner_file_deletion_log_content_sha256_check + check (content_sha256 ~ '^[0-9a-f]{64}$'), + constraint partner_file_deletion_log_ciphertext_sha256_check + check (ciphertext_sha256 is null or ciphertext_sha256 ~ '^[0-9a-f]{64}$'), + constraint partner_file_deletion_log_entry_hash_check + check (entry_hash ~ '^[0-9a-f]{64}$'), + constraint partner_file_deletion_log_prev_entry_hash_check + check (prev_entry_hash is null or prev_entry_hash ~ '^[0-9a-f]{64}$'), + constraint partner_file_deletion_log_actor_check + check ( + actor = 'retention_job' + or actor = 'partner_request' + or actor like 'ops:%' + ), + constraint partner_file_deletion_log_reason_check + check (reason in ('retention_30d', 'partner_request_15d', 'terms_end')) +); + +CREATE INDEX IF NOT EXISTS partner_file_deletion_log_tenant_deleted_idx + ON public.partner_file_deletion_log (tenant_id, deleted_at desc); + +ALTER TABLE public.partner_files ENABLE ROW LEVEL SECURITY; +ALTER TABLE public.partner_files FORCE ROW LEVEL SECURITY; +ALTER TABLE public.partner_file_derived ENABLE ROW LEVEL SECURITY; +ALTER TABLE public.partner_file_derived FORCE ROW LEVEL SECURITY; +ALTER TABLE public.partner_file_deletion_log ENABLE ROW LEVEL SECURITY; +ALTER TABLE public.partner_file_deletion_log FORCE ROW LEVEL SECURITY; + +-- CASE form, not OR: public.get_tenant_context() raises when app.tenant_id is +-- unset and Postgres does not order OR operands, so the sysadmin arm is tested +-- first (same predicate shape as alembic v096 / v134 / v135 / v137). +DROP POLICY IF EXISTS tenant_isolation_partner_files ON public.partner_files; +CREATE POLICY tenant_isolation_partner_files ON public.partner_files + FOR ALL + USING ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ) + WITH CHECK ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ); + +DROP POLICY IF EXISTS tenant_isolation_partner_file_derived ON public.partner_file_derived; +CREATE POLICY tenant_isolation_partner_file_derived ON public.partner_file_derived + FOR ALL + USING ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ) + WITH CHECK ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ); + +DROP POLICY IF EXISTS tenant_isolation_partner_file_deletion_log ON public.partner_file_deletion_log; +CREATE POLICY tenant_isolation_partner_file_deletion_log ON public.partner_file_deletion_log + FOR SELECT + USING ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ); + +-- Inserts into the deletion log go through the retention worker (regengine / +-- SECURITY DEFINER RPC). No authenticated UPDATE/DELETE on the log. +DROP POLICY IF EXISTS partner_file_deletion_log_insert_worker ON public.partner_file_deletion_log; +CREATE POLICY partner_file_deletion_log_insert_worker ON public.partner_file_deletion_log + FOR INSERT + WITH CHECK ( + CASE + WHEN (select current_setting('regengine.is_sysadmin', true)) = 'true' + AND current_user = 'regengine_sysadmin' THEN true + ELSE tenant_id = (select public.get_tenant_context()) + END + ); + +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'anon') THEN + REVOKE ALL ON public.partner_files FROM anon; + REVOKE ALL ON public.partner_file_derived FROM anon; + REVOKE ALL ON public.partner_file_deletion_log FROM anon; + END IF; + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'authenticated') THEN + -- Metadata reads/writes for tenant users go through the app role / + -- signed upload path, not broad authenticated table DML. + REVOKE ALL ON public.partner_files FROM authenticated; + REVOKE ALL ON public.partner_file_derived FROM authenticated; + REVOKE ALL ON public.partner_file_deletion_log FROM authenticated; + END IF; + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') THEN + GRANT SELECT, INSERT, UPDATE ON public.partner_files TO regengine; + GRANT SELECT, INSERT, UPDATE ON public.partner_file_derived TO regengine; + -- Append-only audit: INSERT + SELECT only. Never UPDATE/DELETE. + GRANT SELECT, INSERT ON public.partner_file_deletion_log TO regengine; + REVOKE UPDATE, DELETE ON public.partner_file_deletion_log FROM regengine; + END IF; + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine_sysadmin') THEN + GRANT SELECT ON public.partner_files TO regengine_sysadmin; + GRANT SELECT ON public.partner_file_derived TO regengine_sysadmin; + GRANT SELECT ON public.partner_file_deletion_log TO regengine_sysadmin; + END IF; + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'service_role') THEN + -- Prefer SECURITY DEFINER RPCs; keep service_role from mutating the log. + REVOKE UPDATE, DELETE ON public.partner_file_deletion_log FROM service_role; + END IF; +END $$; + +-- Downgrade (run by hand only): +-- +-- DROP POLICY IF EXISTS partner_file_deletion_log_insert_worker ON public.partner_file_deletion_log; +-- DROP POLICY IF EXISTS tenant_isolation_partner_file_deletion_log ON public.partner_file_deletion_log; +-- DROP POLICY IF EXISTS tenant_isolation_partner_file_derived ON public.partner_file_derived; +-- DROP POLICY IF EXISTS tenant_isolation_partner_files ON public.partner_files; +-- DROP TRIGGER IF EXISTS partner_files_set_delete_after ON public.partner_files; +-- DROP FUNCTION IF EXISTS public.partner_files_set_delete_after(); +-- DROP INDEX IF EXISTS public.partner_file_deletion_log_tenant_deleted_idx; +-- DROP INDEX IF EXISTS public.partner_file_derived_file_idx; +-- DROP INDEX IF EXISTS public.partner_files_tenant_uploaded_idx; +-- DROP INDEX IF EXISTS public.partner_files_due_delete_idx; +-- DROP TABLE IF EXISTS public.partner_file_deletion_log; +-- DROP TABLE IF EXISTS public.partner_file_derived; +-- DROP TABLE IF EXISTS public.partner_files; diff --git a/supabase/migrations/20261028100010_partner_uploads_bucket.sql b/supabase/migrations/20261028100010_partner_uploads_bucket.sql new file mode 100644 index 000000000..b195fe80f --- /dev/null +++ b/supabase/migrations/20261028100010_partner_uploads_bucket.sql @@ -0,0 +1,58 @@ +-- Private Supabase Storage bucket for partner Beta uploads. +-- +-- Layout: partner-uploads/{tenant_id}/{engagement_id}/{partner_file_id} +-- Private (not public). No anonymous list/read. MIME allowlist CSV-oriented. +-- Client UPDATE/DELETE on storage.objects is denied; retention uses service +-- role / SECURITY DEFINER. Folder[1] must equal live tenant context. +-- +-- Supabase-only (storage schema). Not mirrored on Railway Postgres. +-- Idempotent. Do not apply to prod without C.Lee yes. + +INSERT INTO storage.buckets (id, name, public, file_size_limit, allowed_mime_types) +VALUES ( + 'partner-uploads', + 'partner-uploads', + false, + 26214400, -- 25 MiB Beta cap + ARRAY[ + 'text/csv', + 'application/csv', + 'text/plain', + 'application/vnd.ms-excel' + ]::text[] +) +ON CONFLICT (id) DO UPDATE +SET + public = EXCLUDED.public, + file_size_limit = EXCLUDED.file_size_limit, + allowed_mime_types = EXCLUDED.allowed_mime_types; + +-- storage.objects RLS for this bucket. CASE + get_tenant_context (not JWT). +DROP POLICY IF EXISTS partner_uploads_select_own ON storage.objects; +CREATE POLICY partner_uploads_select_own ON storage.objects + FOR SELECT + TO authenticated + USING ( + bucket_id = 'partner-uploads' + AND (storage.foldername(name))[1] = (select public.get_tenant_context())::text + ); + +DROP POLICY IF EXISTS partner_uploads_insert_own ON storage.objects; +CREATE POLICY partner_uploads_insert_own ON storage.objects + FOR INSERT + TO authenticated + WITH CHECK ( + bucket_id = 'partner-uploads' + AND (storage.foldername(name))[1] = (select public.get_tenant_context())::text + ); + +-- Explicitly no UPDATE/DELETE policies for authenticated on this bucket. +DROP POLICY IF EXISTS partner_uploads_update_own ON storage.objects; +DROP POLICY IF EXISTS partner_uploads_delete_own ON storage.objects; + +-- Downgrade (run by hand only): +-- +-- DROP POLICY IF EXISTS partner_uploads_insert_own ON storage.objects; +-- DROP POLICY IF EXISTS partner_uploads_select_own ON storage.objects; +-- DELETE FROM storage.objects WHERE bucket_id = 'partner-uploads'; +-- DELETE FROM storage.buckets WHERE id = 'partner-uploads'; From 290a5a04aea3c8f18c0a2e843d3c863b0e594a1a Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Wed, 28 Oct 2026 08:50:15 -0700 Subject: [PATCH 2/7] fix(storage): v138 alembic statement tuple + always-recompute delete_after Codex FAIL on #3729: drop the runtime semicolon splitter (comment `;` broke upgrade) in favor of an explicit _UPGRADE_STATEMENTS tuple like v137, and fire partner_files_set_delete_after BEFORE INSERT OR UPDATE so a direct UPDATE of delete_after cannot desync the retention clock. Stub PG16 tests cover idempotent apply and the overwrite behavior. Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> --- ...901_v138_partner_file_storage_retention.py | 228 +++++++-------- ...100000_partner_files_storage_retention.sql | 2 +- ...est_v138_partner_file_storage_retention.py | 269 ++++++++++++++++++ 3 files changed, 387 insertions(+), 112 deletions(-) create mode 100644 tests/migrations/test_v138_partner_file_storage_retention.py diff --git a/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py b/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py index bf0a7887a..01b09b86d 100644 --- a/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py +++ b/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py @@ -1,18 +1,20 @@ -"""Partner file storage metadata + 30-day retention (v138). +"""Partner file storage metadata + 30-day retention (v138 / SEL-549 first-file). Mirrors supabase/migrations/20261028100000_partner_files_storage_retention.sql -exactly for partner_files / partner_file_derived / partner_file_deletion_log -(tables, indexes, CASE RLS, grants). Bucket creation lives only in -supabase/migrations/20261028100010_partner_uploads_bucket.sql (storage schema -is Supabase-only). +(metadata + RLS + delete_after trigger) and the manifest_blobs payload-pointer +changes from that same design. Bucket DDL stays in the Supabase-only sibling +20261028100010_partner_uploads_bucket.sql (storage.buckets is not on Railway). -Also prepares Railway fsma.manifest_blobs for hash/pointer cutover: raw_bytes -becomes nullable with a check that at least one of raw_bytes or storage_url is -present (storage_url already exists from v095). Does not move bytes; does not -apply envelope encryption. +Statements are an explicit tuple (same shape as v137) — no runtime semicolon +splitter. Comments may contain `;`; dollar-quoted function / DO bodies stay +inside a single statement string. -Additive and idempotent. Downgrade drops only what this revision created and -restores raw_bytes NOT NULL when safe. +delete_after is maintained by a BEFORE INSERT OR UPDATE trigger that always +recomputes from report_delivered_at (+ 30 days, or NULL), so a direct UPDATE of +delete_after cannot desync the retention clock. + +Additive and idempotent. Do not apply to prod without C.Lee yes. +Synthetic fixtures only until SEL-590 clears. Revision ID: b8c1e4f2a901 Revises: a3f7ce361b0e @@ -22,15 +24,14 @@ from typing import Sequence, Union from alembic import op -import sqlalchemy as sa revision: str = "b8c1e4f2a901" # pragma: allowlist secret down_revision: Union[str, Sequence[str], None] = "a3f7ce361b0e" # pragma: allowlist secret branch_labels = None depends_on = None -# Mirrored from the supabase SQL migration (minus the hand-only downgrade notes). -_PARTNER_FILES_SQL = """ +_UPGRADE_STATEMENTS: tuple[str, ...] = ( + """ -- Partner file storage metadata + 30-day retention clock (SEL-549 / first-file). -- -- Metadata only. Partner file bytes live in the private Supabase Storage bucket @@ -84,15 +85,18 @@ check (storage_bucket = 'partner-uploads'), constraint partner_files_storage_path_unique unique (storage_bucket, storage_path) -); - +) +""", + """ CREATE INDEX IF NOT EXISTS partner_files_due_delete_idx ON public.partner_files (delete_after) - WHERE deleted_at IS NULL AND report_delivered_at IS NOT NULL; - + WHERE deleted_at IS NULL AND report_delivered_at IS NOT NULL +""", + """ CREATE INDEX IF NOT EXISTS partner_files_tenant_uploaded_idx - ON public.partner_files (tenant_id, uploaded_at desc); - + ON public.partner_files (tenant_id, uploaded_at desc) +""", + """ -- delete_after = report_delivered_at + 30 days (trigger; generated col not allowed) CREATE OR REPLACE FUNCTION public.partner_files_set_delete_after() RETURNS trigger @@ -106,15 +110,19 @@ END IF; RETURN NEW; END; -$$; - -DROP TRIGGER IF EXISTS partner_files_set_delete_after ON public.partner_files; +$$ +""", + """ +DROP TRIGGER IF EXISTS partner_files_set_delete_after ON public.partner_files +""", + """ CREATE TRIGGER partner_files_set_delete_after - BEFORE INSERT OR UPDATE OF report_delivered_at + BEFORE INSERT OR UPDATE ON public.partner_files FOR EACH ROW - EXECUTE FUNCTION public.partner_files_set_delete_after(); - + EXECUTE FUNCTION public.partner_files_set_delete_after() +""", + """ CREATE TABLE IF NOT EXISTS public.partner_file_derived ( id uuid primary key default gen_random_uuid(), partner_file_id uuid not null references public.partner_files(id) on delete cascade, @@ -127,12 +135,14 @@ deleted_at timestamptz, constraint partner_file_derived_kind_check check (char_length(kind) between 1 and 64) -); - +) +""", + """ CREATE INDEX IF NOT EXISTS partner_file_derived_file_idx ON public.partner_file_derived (partner_file_id) - WHERE deleted_at IS NULL; - + WHERE deleted_at IS NULL +""", + """ CREATE TABLE IF NOT EXISTS public.partner_file_deletion_log ( id uuid primary key default gen_random_uuid(), partner_file_id uuid not null, @@ -165,22 +175,37 @@ ), constraint partner_file_deletion_log_reason_check check (reason in ('retention_30d', 'partner_request_15d', 'terms_end')) -); - +) +""", + """ CREATE INDEX IF NOT EXISTS partner_file_deletion_log_tenant_deleted_idx - ON public.partner_file_deletion_log (tenant_id, deleted_at desc); - -ALTER TABLE public.partner_files ENABLE ROW LEVEL SECURITY; -ALTER TABLE public.partner_files FORCE ROW LEVEL SECURITY; -ALTER TABLE public.partner_file_derived ENABLE ROW LEVEL SECURITY; -ALTER TABLE public.partner_file_derived FORCE ROW LEVEL SECURITY; -ALTER TABLE public.partner_file_deletion_log ENABLE ROW LEVEL SECURITY; -ALTER TABLE public.partner_file_deletion_log FORCE ROW LEVEL SECURITY; - + ON public.partner_file_deletion_log (tenant_id, deleted_at desc) +""", + """ +ALTER TABLE public.partner_files ENABLE ROW LEVEL SECURITY +""", + """ +ALTER TABLE public.partner_files FORCE ROW LEVEL SECURITY +""", + """ +ALTER TABLE public.partner_file_derived ENABLE ROW LEVEL SECURITY +""", + """ +ALTER TABLE public.partner_file_derived FORCE ROW LEVEL SECURITY +""", + """ +ALTER TABLE public.partner_file_deletion_log ENABLE ROW LEVEL SECURITY +""", + """ +ALTER TABLE public.partner_file_deletion_log FORCE ROW LEVEL SECURITY +""", + """ -- CASE form, not OR: public.get_tenant_context() raises when app.tenant_id is -- unset and Postgres does not order OR operands, so the sysadmin arm is tested -- first (same predicate shape as alembic v096 / v134 / v135 / v137). -DROP POLICY IF EXISTS tenant_isolation_partner_files ON public.partner_files; +DROP POLICY IF EXISTS tenant_isolation_partner_files ON public.partner_files +""", + """ CREATE POLICY tenant_isolation_partner_files ON public.partner_files FOR ALL USING ( @@ -196,9 +221,12 @@ AND current_user = 'regengine_sysadmin' THEN true ELSE tenant_id = (select public.get_tenant_context()) END - ); - -DROP POLICY IF EXISTS tenant_isolation_partner_file_derived ON public.partner_file_derived; + ) +""", + """ +DROP POLICY IF EXISTS tenant_isolation_partner_file_derived ON public.partner_file_derived +""", + """ CREATE POLICY tenant_isolation_partner_file_derived ON public.partner_file_derived FOR ALL USING ( @@ -214,9 +242,12 @@ AND current_user = 'regengine_sysadmin' THEN true ELSE tenant_id = (select public.get_tenant_context()) END - ); - -DROP POLICY IF EXISTS tenant_isolation_partner_file_deletion_log ON public.partner_file_deletion_log; + ) +""", + """ +DROP POLICY IF EXISTS tenant_isolation_partner_file_deletion_log ON public.partner_file_deletion_log +""", + """ CREATE POLICY tenant_isolation_partner_file_deletion_log ON public.partner_file_deletion_log FOR SELECT USING ( @@ -225,11 +256,14 @@ AND current_user = 'regengine_sysadmin' THEN true ELSE tenant_id = (select public.get_tenant_context()) END - ); - + ) +""", + """ -- Inserts into the deletion log go through the retention worker (regengine / -- SECURITY DEFINER RPC). No authenticated UPDATE/DELETE on the log. -DROP POLICY IF EXISTS partner_file_deletion_log_insert_worker ON public.partner_file_deletion_log; +DROP POLICY IF EXISTS partner_file_deletion_log_insert_worker ON public.partner_file_deletion_log +""", + """ CREATE POLICY partner_file_deletion_log_insert_worker ON public.partner_file_deletion_log FOR INSERT WITH CHECK ( @@ -238,8 +272,9 @@ AND current_user = 'regengine_sysadmin' THEN true ELSE tenant_id = (select public.get_tenant_context()) END - ); - + ) +""", + """ DO $$ BEGIN IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'anon') THEN @@ -270,11 +305,9 @@ -- Prefer SECURITY DEFINER RPCs; keep service_role from mutating the log. REVOKE UPDATE, DELETE ON public.partner_file_deletion_log FROM service_role; END IF; -END $$; - -""" - -_MANIFEST_BLOBS_SQL = """ +END $$ +""", + """ -- Railway cutover helper: allow hash/pointer-only manifest_blobs rows. DO $$ BEGIN @@ -286,57 +319,31 @@ ) THEN ALTER TABLE fsma.manifest_blobs ALTER COLUMN raw_bytes DROP NOT NULL; END IF; -END $$; - -ALTER TABLE fsma.manifest_blobs DROP CONSTRAINT IF EXISTS manifest_blobs_payload_present_check; +END $$ +""", + """ +ALTER TABLE fsma.manifest_blobs DROP CONSTRAINT IF EXISTS manifest_blobs_payload_present_check +""", + """ ALTER TABLE fsma.manifest_blobs ADD CONSTRAINT manifest_blobs_payload_present_check CHECK ( raw_bytes IS NOT NULL OR (storage_url IS NOT NULL AND length(btrim(storage_url)) > 0) - ); -""" + ) +""", +) def upgrade() -> None: - # Split on semicolons outside dollar-quoted blocks so DO $$ ... $$; stays intact. - def _statements(sql: str) -> list[str]: - out: list[str] = [] - buf: list[str] = [] - in_dollar = False - i = 0 - while i < len(sql): - if sql.startswith("$$", i): - in_dollar = not in_dollar - buf.append("$$") - i += 2 - continue - ch = sql[i] - if ch == ";" and not in_dollar: - stmt = "".join(buf).strip() - if stmt and not all( - line.strip().startswith("--") or not line.strip() - for line in stmt.splitlines() - ): - out.append(stmt) - buf = [] - i += 1 - continue - buf.append(ch) - i += 1 - tail = "".join(buf).strip() - if tail: - out.append(tail) - return out - - for statement in _statements(_PARTNER_FILES_SQL) + _statements(_MANIFEST_BLOBS_SQL): - op.execute(sa.text(statement)) + for statement in _UPGRADE_STATEMENTS: + op.execute(statement) def downgrade() -> None: + op.execute( - sa.text( - """ + """ DO $$ BEGIN IF EXISTS ( @@ -352,18 +359,17 @@ def downgrade() -> None: END IF; END $$; """ - ) ) - op.execute(sa.text("DROP POLICY IF EXISTS partner_file_deletion_log_insert_worker ON public.partner_file_deletion_log")) - op.execute(sa.text("DROP POLICY IF EXISTS tenant_isolation_partner_file_deletion_log ON public.partner_file_deletion_log")) - op.execute(sa.text("DROP POLICY IF EXISTS tenant_isolation_partner_file_derived ON public.partner_file_derived")) - op.execute(sa.text("DROP POLICY IF EXISTS tenant_isolation_partner_files ON public.partner_files")) - op.execute(sa.text("DROP TRIGGER IF EXISTS partner_files_set_delete_after ON public.partner_files")) - op.execute(sa.text("DROP FUNCTION IF EXISTS public.partner_files_set_delete_after()")) - op.execute(sa.text("DROP INDEX IF EXISTS public.partner_file_deletion_log_tenant_deleted_idx")) - op.execute(sa.text("DROP INDEX IF EXISTS public.partner_file_derived_file_idx")) - op.execute(sa.text("DROP INDEX IF EXISTS public.partner_files_tenant_uploaded_idx")) - op.execute(sa.text("DROP INDEX IF EXISTS public.partner_files_due_delete_idx")) - op.execute(sa.text("DROP TABLE IF EXISTS public.partner_file_deletion_log")) - op.execute(sa.text("DROP TABLE IF EXISTS public.partner_file_derived")) - op.execute(sa.text("DROP TABLE IF EXISTS public.partner_files")) + op.execute("DROP POLICY IF EXISTS partner_file_deletion_log_insert_worker ON public.partner_file_deletion_log") + op.execute("DROP POLICY IF EXISTS tenant_isolation_partner_file_deletion_log ON public.partner_file_deletion_log") + op.execute("DROP POLICY IF EXISTS tenant_isolation_partner_file_derived ON public.partner_file_derived") + op.execute("DROP POLICY IF EXISTS tenant_isolation_partner_files ON public.partner_files") + op.execute("DROP TRIGGER IF EXISTS partner_files_set_delete_after ON public.partner_files") + op.execute("DROP FUNCTION IF EXISTS public.partner_files_set_delete_after()") + op.execute("DROP INDEX IF EXISTS public.partner_file_deletion_log_tenant_deleted_idx") + op.execute("DROP INDEX IF EXISTS public.partner_file_derived_file_idx") + op.execute("DROP INDEX IF EXISTS public.partner_files_tenant_uploaded_idx") + op.execute("DROP INDEX IF EXISTS public.partner_files_due_delete_idx") + op.execute("DROP TABLE IF EXISTS public.partner_file_deletion_log") + op.execute("DROP TABLE IF EXISTS public.partner_file_derived") + op.execute("DROP TABLE IF EXISTS public.partner_files") diff --git a/supabase/migrations/20261028100000_partner_files_storage_retention.sql b/supabase/migrations/20261028100000_partner_files_storage_retention.sql index 6db6e2433..426ec2c41 100644 --- a/supabase/migrations/20261028100000_partner_files_storage_retention.sql +++ b/supabase/migrations/20261028100000_partner_files_storage_retention.sql @@ -77,7 +77,7 @@ $$; DROP TRIGGER IF EXISTS partner_files_set_delete_after ON public.partner_files; CREATE TRIGGER partner_files_set_delete_after - BEFORE INSERT OR UPDATE OF report_delivered_at + BEFORE INSERT OR UPDATE ON public.partner_files FOR EACH ROW EXECUTE FUNCTION public.partner_files_set_delete_after(); diff --git a/tests/migrations/test_v138_partner_file_storage_retention.py b/tests/migrations/test_v138_partner_file_storage_retention.py new file mode 100644 index 000000000..16495bdde --- /dev/null +++ b/tests/migrations/test_v138_partner_file_storage_retention.py @@ -0,0 +1,269 @@ +"""v138 partner file storage retention — structure + stub PG16 behavior. + +Covers the Codex FAIL fixes on #3729: + +1. No runtime semicolon splitter — statements are an explicit tuple (v137 + shape). Structure tests assert a commented ``;`` and a ``$$`` function + body both live inside single statement strings. +2. ``delete_after`` trigger is ``BEFORE INSERT OR UPDATE`` (no ``OF`` list) + and always recomputes from ``report_delivered_at``, so a direct UPDATE of + ``delete_after`` is overwritten. +3. Stub PG16: apply ``upgrade()`` twice (idempotent) against a minimal + schema, then exercise the retention clock. + +Uses a disposable local Postgres database (Homebrew / CI service) via +``scratch_database_engine``. Skips when no Postgres is available; fails under +``REGENGINE_REQUIRE_DB=1``. +""" + +from __future__ import annotations + +import ast +import importlib.util +import os +from pathlib import Path +from uuid import uuid4 + +import pytest +from sqlalchemy import create_engine, text + +from tests.migrations.scratch_db import scratch_database_engine + +REPO_ROOT = Path(__file__).resolve().parents[2] +MIGRATION_PATH = ( + REPO_ROOT + / "alembic" + / "versions" + / "20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py" +) + + +def _load_migration(): + spec = importlib.util.spec_from_file_location( + f"v138_partner_{uuid4().hex}", MIGRATION_PATH + ) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _statement_strings() -> list[str]: + tree = ast.parse(MIGRATION_PATH.read_text()) + for node in tree.body: + if ( + isinstance(node, ast.AnnAssign) + and isinstance(node.target, ast.Name) + and node.target.id == "_UPGRADE_STATEMENTS" + ): + assert isinstance(node.value, ast.Tuple) + out: list[str] = [] + for elt in node.value.elts: + assert isinstance(elt, ast.Constant) and isinstance(elt.value, str) + out.append(elt.value) + return out + raise AssertionError("_UPGRADE_STATEMENTS not found") + + +def test_no_runtime_semicolon_splitter() -> None: + src = MIGRATION_PATH.read_text() + assert "def _statements" not in src + assert "_UPGRADE_STATEMENTS" in src + tree = ast.parse(src) + names = { + node.name + for node in tree.body + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + } + assert "upgrade" in names and "downgrade" in names + assert "_statements" not in names + + +def test_commented_semicolon_and_dollar_body_are_single_statements() -> None: + """Authoring regression: comment `;` and `$$` bodies must not be split.""" + stmts = _statement_strings() + # Header comment with an intentional semicolon survives inside a statement. + assert any("trigger-maintained; UTC" in s for s in stmts) + # Function body is one statement containing the dollar-quoted block. + fn = [ + s + for s in stmts + if "CREATE OR REPLACE FUNCTION public.partner_files_set_delete_after" in s + ] + assert len(fn) == 1 + assert "AS $$" in fn[0] + assert "interval '30 days'" in fn[0] + assert fn[0].count("$$") >= 2 + # DO $$ role grants stay intact as one statement. + dos = [s for s in stmts if s.lstrip().startswith("DO $$")] + assert dos + assert all("END $$" in s for s in dos) + + +def test_delete_after_trigger_is_before_any_update() -> None: + stmts = _statement_strings() + trig = next(s for s in stmts if "CREATE TRIGGER partner_files_set_delete_after" in s) + assert "BEFORE INSERT OR UPDATE" in trig + assert "OF report_delivered_at" not in trig + + +def _bootstrap_minimal_schema(engine) -> None: + with engine.begin() as conn: + conn.execute( + text( + """ + CREATE TABLE public.tenants ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid() + ); + CREATE OR REPLACE FUNCTION public.get_tenant_context() + RETURNS uuid + LANGUAGE sql + STABLE + AS $$ + SELECT NULLIF(current_setting('app.tenant_id', true), '')::uuid + $$; + CREATE SCHEMA IF NOT EXISTS fsma; + CREATE TABLE fsma.manifest_blobs ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + tenant_id uuid NOT NULL, + raw_bytes bytea NOT NULL, + storage_url text + ); + """ + ) + ) + + +def _apply_upgrade(engine, monkeypatch) -> None: + module = _load_migration() + + class _Operations: + @staticmethod + def execute(statement: str) -> None: + with engine.begin() as conn: + conn.execute(text(statement)) + + monkeypatch.setattr(module, "op", _Operations()) + module.upgrade() + + +@pytest.fixture(scope="module") +def pg_engine(): + """Disposable PG16 database with v138 minimal deps, or skip/fail.""" + # Prefer an explicit scratch URL; otherwise peer-auth local Homebrew PG. + if not os.environ.get("REGENGINE_MIGRATION_TEST_DB"): + os.environ["REGENGINE_MIGRATION_TEST_DB"] = "postgresql://sellers@/postgres?host=/tmp" + db_name = "inc2_scratch_v138_" + uuid4().hex[:12] + with scratch_database_engine(db_name) as engine: + _bootstrap_minimal_schema(engine) + yield engine + + +def test_upgrade_idempotent_on_stub_pg16(pg_engine, monkeypatch) -> None: + _apply_upgrade(pg_engine, monkeypatch) + _apply_upgrade(pg_engine, monkeypatch) # second apply must be a no-op + with pg_engine.connect() as conn: + exists = conn.execute( + text( + """ + SELECT COUNT(*) FROM information_schema.tables + WHERE table_schema = 'public' + AND table_name IN ( + 'partner_files', + 'partner_file_derived', + 'partner_file_deletion_log' + ) + """ + ) + ).scalar_one() + assert exists == 3 + version = conn.execute(text("SHOW server_version")).scalar_one() + assert version.startswith("16."), version + tgtype = conn.execute( + text( + """ + SELECT pg_get_triggerdef(t.oid) + FROM pg_trigger t + JOIN pg_class c ON c.oid = t.tgrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = 'public' + AND c.relname = 'partner_files' + AND t.tgname = 'partner_files_set_delete_after' + AND NOT t.tgisinternal + """ + ) + ).scalar_one() + assert "BEFORE INSERT OR UPDATE" in tgtype + assert "OF report_delivered_at" not in tgtype + + +def test_delete_after_recomputed_on_direct_update(pg_engine, monkeypatch) -> None: + # Ensure schema is present (idempotent with the previous test). + _apply_upgrade(pg_engine, monkeypatch) + tenant_id = str(uuid4()) + file_id = str(uuid4()) + engagement_id = str(uuid4()) + with pg_engine.begin() as conn: + conn.execute( + text("INSERT INTO public.tenants (id) VALUES (CAST(:id AS uuid))"), + {"id": tenant_id}, + ) + # Bypass RLS for the owning role on a fresh stub. + conn.execute(text("ALTER TABLE public.partner_files DISABLE ROW LEVEL SECURITY")) + row = conn.execute( + text( + """ + INSERT INTO public.partner_files ( + id, tenant_id, engagement_id, storage_path, content_sha256, + original_filename, byte_size, content_type, status, + report_delivered_at + ) VALUES ( + CAST(:id AS uuid), + CAST(:tenant_id AS uuid), + CAST(:engagement_id AS uuid), + 'tenants/' || :tenant_id || '/synthetic.csv', + repeat('a', 64), + 'synthetic.csv', + 12, + 'text/csv', + 'delivered', + TIMESTAMPTZ '2026-10-01 00:00:00+00' + ) + RETURNING delete_after + """ + ), + { + "id": file_id, + "tenant_id": tenant_id, + "engagement_id": engagement_id, + }, + ).scalar_one() + assert row.utctimetuple()[:3] == (2026, 10, 31) + + # Direct UPDATE of delete_after must be overwritten by the trigger. + overwritten = conn.execute( + text( + """ + UPDATE public.partner_files + SET delete_after = TIMESTAMPTZ '2099-01-01 00:00:00+00' + WHERE id = CAST(:id AS uuid) + RETURNING delete_after AT TIME ZONE 'UTC' + """ + ), + {"id": file_id}, + ).scalar_one() + assert str(overwritten)[:10] == "2026-10-31" + + # Clearing delivery clears the clock. + cleared = conn.execute( + text( + """ + UPDATE public.partner_files + SET report_delivered_at = NULL + WHERE id = CAST(:id AS uuid) + RETURNING delete_after + """ + ), + {"id": file_id}, + ).scalar_one() + assert cleared is None From 514ad7c75ca7acdbfba7c17909afd53f7df8abb9 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Wed, 28 Oct 2026 09:04:51 -0700 Subject: [PATCH 3/7] fix(storage): grant runtime INSERT on manifest_blobs.storage_url Claude FAIL on #3732: partner-uploads cutover writes storage_url, but v126 column INSERT excluded it. Extend v138 + v111 inventory so the runtime role can insert pointer-only rows; keep created_at ungated. Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> --- ...3_b71d9e4c2a60_v111_runtime_role_grants.py | 9 ++--- ...901_v138_partner_file_storage_retention.py | 34 +++++++++++++++++++ ...100000_partner_files_storage_retention.sql | 17 ++++++++++ ...est_v138_partner_file_storage_retention.py | 23 +++++++++++++ .../test_remediation_disabled_contract_pg.py | 8 ++--- .../test_postgres_evidence_workflow_pg.py | 18 +++++----- 6 files changed, 92 insertions(+), 17 deletions(-) diff --git a/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py b/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py index 48d3f91c5..960778696 100644 --- a/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py +++ b/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py @@ -652,10 +652,10 @@ def _build_migration_owner_posture() -> str: # plane data. "public.tenants": {"SELECT": ("id",)}, # v126 -- POST /api/v1/ingest/csv (#3122) retains each upload verbatim - # in the ingest transaction (webhook_compat._open_ingestion_run). INSERT - # is limited to the columns that statement writes, so storage_url and - # created_at cannot be forged; reads, updates and deletes stay denied - # (the disabled remediation surface keeps its contract). The composite + # in the ingest transaction (webhook_compat._open_ingestion_run). + # v138 extends INSERT with storage_url for partner-uploads pointer rows; + # created_at stays ungated so timestamps cannot be forged. Reads, updates + # and deletes stay denied (disabled remediation surface). The composite # (ingestion_run_id, tenant_id) foreign key pins a blob to its own # tenant's run. "fsma.manifest_blobs": { @@ -667,6 +667,7 @@ def _build_migration_owner_posture() -> str: "original_filename", "content_type", "uploaded_by", + "storage_url", ) }, # v126 -- the agent-artifact worker (shared.agent_task_bridge) reads only diff --git a/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py b/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py index 01b09b86d..532bed829 100644 --- a/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py +++ b/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py @@ -331,6 +331,23 @@ raw_bytes IS NOT NULL OR (storage_url IS NOT NULL AND length(btrim(storage_url)) > 0) ) +""", + """ +DO $$ +BEGIN + -- v138: runtime role must INSERT storage_url for partner-uploads pointer rows. + -- Inventory (v111 REGENGINE_COLUMN_GRANTS) also lists storage_url so + -- apply_runtime_role_cutover.py keeps the grant across deploys. + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') + AND EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_schema = 'fsma' + AND table_name = 'manifest_blobs' + AND column_name = 'storage_url' + ) THEN + GRANT INSERT (storage_url) ON TABLE fsma.manifest_blobs TO regengine; + END IF; +END $$ """, ) @@ -345,6 +362,23 @@ def downgrade() -> None: op.execute( """ DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') + AND EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_schema = 'fsma' + AND table_name = 'manifest_blobs' + AND column_name = 'storage_url' + ) THEN + REVOKE INSERT (storage_url) ON TABLE fsma.manifest_blobs FROM regengine; + END IF; +END $$; +""" + ) + + op.execute( + """ +DO $$ BEGIN IF EXISTS ( SELECT 1 FROM information_schema.tables diff --git a/supabase/migrations/20261028100000_partner_files_storage_retention.sql b/supabase/migrations/20261028100000_partner_files_storage_retention.sql index 426ec2c41..1b006d6b1 100644 --- a/supabase/migrations/20261028100000_partner_files_storage_retention.sql +++ b/supabase/migrations/20261028100000_partner_files_storage_retention.sql @@ -254,3 +254,20 @@ END $$; -- DROP TABLE IF EXISTS public.partner_file_deletion_log; -- DROP TABLE IF EXISTS public.partner_file_derived; -- DROP TABLE IF EXISTS public.partner_files; + +-- Runtime role: allow pointer-only manifest_blobs inserts (partner-uploads). +DO $$ +BEGIN + -- v138: runtime role must INSERT storage_url for partner-uploads pointer rows. + -- Inventory (v111 REGENGINE_COLUMN_GRANTS) also lists storage_url so + -- apply_runtime_role_cutover.py keeps the grant across deploys. + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') + AND EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_schema = 'fsma' + AND table_name = 'manifest_blobs' + AND column_name = 'storage_url' + ) THEN + GRANT INSERT (storage_url) ON TABLE fsma.manifest_blobs TO regengine; + END IF; +END $$; diff --git a/tests/migrations/test_v138_partner_file_storage_retention.py b/tests/migrations/test_v138_partner_file_storage_retention.py index 16495bdde..2a5bf12e3 100644 --- a/tests/migrations/test_v138_partner_file_storage_retention.py +++ b/tests/migrations/test_v138_partner_file_storage_retention.py @@ -267,3 +267,26 @@ def test_delete_after_recomputed_on_direct_update(pg_engine, monkeypatch) -> Non {"id": file_id}, ).scalar_one() assert cleared is None + +def test_runtime_grant_includes_storage_url_insert() -> None: + """v138 must GRANT INSERT (storage_url) and inventory must list it. + + Structural only — no live DB / no partner payloads. + """ + from pathlib import Path + import importlib.util + + alembic = Path("alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py") + text = alembic.read_text(encoding="utf-8") + assert "GRANT INSERT (storage_url) ON TABLE fsma.manifest_blobs TO regengine" in text + assert "REVOKE INSERT (storage_url) ON TABLE fsma.manifest_blobs FROM regengine" in text + + inv_path = Path("alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py") + spec = importlib.util.spec_from_file_location("v111_grants", inv_path) + mod = importlib.util.module_from_spec(spec) + assert spec.loader is not None + spec.loader.exec_module(mod) + cols = mod.REGENGINE_COLUMN_GRANTS["fsma.manifest_blobs"]["INSERT"] + assert "storage_url" in cols + assert "created_at" not in cols + diff --git a/tests/security/test_remediation_disabled_contract_pg.py b/tests/security/test_remediation_disabled_contract_pg.py index 06e18311e..4ad14760d 100644 --- a/tests/security/test_remediation_disabled_contract_pg.py +++ b/tests/security/test_remediation_disabled_contract_pg.py @@ -17,10 +17,10 @@ from tests.migrations.rls_db import guarded_live_database_url PROHIBITED_TABLE_PRIVILEGES = { - # The enabled CSV upload route retains each file here through a v126 - # column-scoped INSERT that excludes storage_url and created_at. No - # table-wide privilege exists: reading, re-pointing or deleting a - # retained manifest is remediation work and stays denied. + # The enabled CSV upload route retains each file here through a + # column-scoped INSERT (v126 columns + v138 storage_url). created_at stays + # excluded. No table-wide privilege exists: reading, re-pointing (UPDATE) + # or deleting a retained manifest is remediation work and stays denied. "fsma.manifest_blobs": ("SELECT", "INSERT", "UPDATE", "DELETE"), "fsma.manifest_records": ("SELECT", "INSERT", "UPDATE", "DELETE"), "fsma.remediation_cases": ("SELECT", "INSERT", "UPDATE", "DELETE"), diff --git a/tests/shared/test_postgres_evidence_workflow_pg.py b/tests/shared/test_postgres_evidence_workflow_pg.py index c851a77bb..963720050 100644 --- a/tests/shared/test_postgres_evidence_workflow_pg.py +++ b/tests/shared/test_postgres_evidence_workflow_pg.py @@ -556,15 +556,15 @@ def rejected(match, statement, params=None, call=None): blob_insert, {"run": other_blobless_run, "tenant": w.tenant, "sha": "0" * 64}, ) - # INSERT is column-scoped: retention metadata cannot be forged. - for forged_column, forged_value in (("storage_url", "https://attacker.example/x"), ("created_at", "1990-01-01")): - rejected( - "permission denied", - f"""INSERT INTO fsma.manifest_blobs - (ingestion_run_id, tenant_id, raw_bytes, sha256, original_filename, {forged_column}) - VALUES (CAST(:run AS uuid), CAST(:tenant AS uuid), '\\x00', :sha, 'forged.csv', :value)""", - {"run": own_run, "tenant": w.tenant, "sha": "0" * 64, "value": forged_value}, - ) + # INSERT is column-scoped: created_at still cannot be forged. v138 grants + # storage_url INSERT for partner-uploads pointer-only rows. + rejected( + "permission denied", + """INSERT INTO fsma.manifest_blobs + (ingestion_run_id, tenant_id, raw_bytes, sha256, original_filename, created_at) + VALUES (CAST(:run AS uuid), CAST(:tenant AS uuid), '\\x00', :sha, 'forged.csv', :value)""", + {"run": own_run, "tenant": w.tenant, "sha": "0" * 64, "value": "1990-01-01"}, + ) rejected("row-level security", None, call=lambda db: persist_agent_artifact(db, other_payload)) # Reusing another tenant's artifact id under the own tenant is a loud # primary-key violation, never a silent ON CONFLICT no-op. From 481e5fce67f13266f4ec9ccc9d29723580607fae Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Wed, 28 Oct 2026 10:23:09 -0700 Subject: [PATCH 4/7] fix(runbook): allow storage_url INSERT in v126 grant verify v138 grants INSERT on fsma.manifest_blobs.storage_url for partner-uploads pointer rows. The verify runbook treated that grant as NOT READY; require storage_url INSERT and keep created_at INSERT denied. --- runbooks/verify-v126-runtime-grants.sql | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/runbooks/verify-v126-runtime-grants.sql b/runbooks/verify-v126-runtime-grants.sql index 2669e3fbd..ba01b4ff6 100644 --- a/runbooks/verify-v126-runtime-grants.sql +++ b/runbooks/verify-v126-runtime-grants.sql @@ -5,8 +5,9 @@ -- -- WHAT IT CHECKS (for the runtime role `regengine`, from the catalog only): -- * POST /api/v1/ingest/csv can retain the upload: INSERT on exactly the --- seven columns the route writes (not storage_url/created_at), nothing --- else on fsma.manifest_blobs, and a validated (ingestion_run_id, +-- seven columns the route writes, plus storage_url (v138 partner-uploads +-- pointer rows). created_at stays non-insertable. Nothing else on +-- fsma.manifest_blobs, and a validated (ingestion_run_id, -- tenant_id) foreign key so a blob can only attach to its own tenant's run. -- * The agent-artifact worker can write fsma.agent_artifacts and its -- ingestion-run receipt: the narrow column SELECTs (including the @@ -68,8 +69,8 @@ checks(sort_key, check_name, expected, actual) AS ( SELECT 1, 'regengine is NOSUPERUSER', false, rolsuper FROM pg_roles WHERE rolname = 'regengine' UNION ALL SELECT 2, 'regengine is NOBYPASSRLS', false, rolbypassrls FROM pg_roles WHERE rolname = 'regengine' - -- CSV upload retention (#3122) - UNION ALL SELECT 10, 'manifest_blobs INSERT on the 7 columns the route writes', true, + -- CSV upload retention (#3122); v138 also grants INSERT (storage_url). + UNION ALL SELECT 10, 'manifest_blobs INSERT on the 7 route columns and storage_url', true, has_column_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'ingestion_run_id', 'INSERT') AND has_column_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'tenant_id', 'INSERT') AND has_column_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'raw_bytes', 'INSERT') @@ -77,6 +78,7 @@ checks(sort_key, check_name, expected, actual) AS ( AND has_column_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'original_filename', 'INSERT') AND has_column_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'content_type', 'INSERT') AND has_column_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'uploaded_by', 'INSERT') + AND has_column_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'storage_url', 'INSERT') UNION ALL SELECT 11, 'manifest_blobs SELECT (any column) stays denied', false, has_any_column_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'SELECT') UNION ALL SELECT 12, 'manifest_blobs UPDATE (any column) stays denied', false, @@ -88,9 +90,8 @@ checks(sort_key, check_name, expected, actual) AS ( AND has_table_privilege('regengine', to_regclass('fsma.ingestion_runs'), 'UPDATE') UNION ALL SELECT 15, 'manifest_blobs table-wide INSERT stays denied', false, has_table_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'INSERT') - UNION ALL SELECT 16, 'manifest_blobs storage_url/created_at not insertable', false, - has_column_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'storage_url', 'INSERT') - OR has_column_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'created_at', 'INSERT') + UNION ALL SELECT 16, 'manifest_blobs created_at not insertable', false, + has_column_privilege('regengine', to_regclass('fsma.manifest_blobs'), 'created_at', 'INSERT') -- Compared through the catalog, not pg_get_constraintdef() text, whose -- schema prefix depends on the session search_path. UNION ALL SELECT 17, 'manifest_blobs FK is (ingestion_run_id, tenant_id) -> ingestion_runs', true, From 830f2d6945104d0b543db3e617d6c11fa0e4a997 Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Wed, 28 Oct 2026 12:29:18 -0700 Subject: [PATCH 5/7] fix(retention,schema): wire v138 partner deleted_at + regenerate snapshot Live DB retention gate failed: partner_files / partner_file_derived / partner_file_deletion_log carry deleted_at but _RESOURCE_RELATIONS left every policy unwired (#2900). Add RetentionPolicy entries, map them to the public relations, and route soft/hard/cleanup SQL through _sql_target. Skip deletion_log in the cleanup sweep (append-only audit). WS-2c schema parity: regenerate docs/schema/expected_schema.json for the three new tables; raw_bytes nullable on fsma.manifest_blobs is intentional for pointer-only storage_url cutover (v138 payload check). --- docs/schema/expected_schema.json | 216 +++++++++++++++++- services/shared/data_retention.py | 87 +++++-- .../test_data_retention_failure_modes_2900.py | 14 +- 3 files changed, 296 insertions(+), 21 deletions(-) diff --git a/docs/schema/expected_schema.json b/docs/schema/expected_schema.json index 73e13227f..7803847ca 100644 --- a/docs/schema/expected_schema.json +++ b/docs/schema/expected_schema.json @@ -2824,7 +2824,7 @@ "type": "text" }, "raw_bytes": { - "nullable": false, + "nullable": true, "type": "bytea" }, "sha256": { @@ -6063,6 +6063,220 @@ }, "foreign_keys": [] }, + "public.partner_file_deletion_log": { + "columns": { + "actor": { + "nullable": false, + "type": "text" + }, + "byte_size": { + "nullable": true, + "type": "bigint" + }, + "ciphertext_sha256": { + "nullable": true, + "type": "text" + }, + "content_sha256": { + "nullable": false, + "type": "text" + }, + "deleted_at": { + "nullable": false, + "type": "timestamp" + }, + "engagement_id": { + "nullable": true, + "type": "uuid" + }, + "entry_hash": { + "nullable": false, + "type": "text" + }, + "id": { + "nullable": false, + "type": "uuid" + }, + "partner_file_id": { + "nullable": false, + "type": "uuid" + }, + "prev_entry_hash": { + "nullable": true, + "type": "text" + }, + "reason": { + "nullable": false, + "type": "text" + }, + "report_delivered_at": { + "nullable": true, + "type": "timestamp" + }, + "storage_bucket": { + "nullable": false, + "type": "text" + }, + "storage_path": { + "nullable": false, + "type": "text" + }, + "tenant_id": { + "nullable": false, + "type": "uuid" + } + }, + "foreign_keys": [] + }, + "public.partner_file_derived": { + "columns": { + "created_at": { + "nullable": false, + "type": "timestamp" + }, + "deleted_at": { + "nullable": true, + "type": "timestamp" + }, + "id": { + "nullable": false, + "type": "uuid" + }, + "kind": { + "nullable": false, + "type": "text" + }, + "partner_file_id": { + "nullable": false, + "type": "uuid" + }, + "row_ref": { + "nullable": true, + "type": "text" + }, + "storage_bucket": { + "nullable": true, + "type": "text" + }, + "storage_path": { + "nullable": true, + "type": "text" + }, + "tenant_id": { + "nullable": false, + "type": "uuid" + } + }, + "foreign_keys": [ + { + "constrained_columns": [ + "partner_file_id" + ], + "name": "partner_file_derived_partner_file_id_fkey", + "referred_columns": [ + "id" + ], + "referred_schema": "public", + "referred_table": "partner_files" + }, + { + "constrained_columns": [ + "tenant_id" + ], + "name": "partner_file_derived_tenant_id_fkey", + "referred_columns": [ + "id" + ], + "referred_schema": "public", + "referred_table": "tenants" + } + ] + }, + "public.partner_files": { + "columns": { + "byte_size": { + "nullable": false, + "type": "bigint" + }, + "ciphertext_sha256": { + "nullable": true, + "type": "text" + }, + "content_sha256": { + "nullable": false, + "type": "text" + }, + "content_type": { + "nullable": false, + "type": "text" + }, + "delete_after": { + "nullable": true, + "type": "timestamp" + }, + "deleted_at": { + "nullable": true, + "type": "timestamp" + }, + "engagement_id": { + "nullable": false, + "type": "uuid" + }, + "id": { + "nullable": false, + "type": "uuid" + }, + "kek_key_id": { + "nullable": true, + "type": "text" + }, + "original_filename": { + "nullable": false, + "type": "text" + }, + "report_delivered_at": { + "nullable": true, + "type": "timestamp" + }, + "status": { + "nullable": false, + "type": "text" + }, + "storage_bucket": { + "nullable": false, + "type": "text" + }, + "storage_path": { + "nullable": false, + "type": "text" + }, + "tenant_id": { + "nullable": false, + "type": "uuid" + }, + "uploaded_at": { + "nullable": false, + "type": "timestamp" + }, + "wrapped_dek": { + "nullable": true, + "type": "bytea" + } + }, + "foreign_keys": [ + { + "constrained_columns": [ + "tenant_id" + ], + "name": "partner_files_tenant_id_fkey", + "referred_columns": [ + "id" + ], + "referred_schema": "public", + "referred_table": "tenants" + } + ] + }, "public.pcos_authority_documents": { "columns": { "content_type": { diff --git a/services/shared/data_retention.py b/services/shared/data_retention.py index a0b25eb20..3529065f7 100644 --- a/services/shared/data_retention.py +++ b/services/shared/data_retention.py @@ -47,6 +47,10 @@ class RetentionPolicy(str, Enum): AUDIT_LOG = "audit_log" # Keep for 24 months, then anonymize TRANSACTION_LOG = "transaction_log" # Keep for 7 years per FDA requirements TEMPORARY_PII = "temporary_pii" # Soft-delete after 0 days, hard-delete after 14 days + # v138 partner first-file storage (SEL-549): soft-delete column is real. + PARTNER_FILES = "partner_files" # Soft-delete after 0 days, hard-delete after 30 days + PARTNER_FILE_DERIVED = "partner_file_derived" # Soft-delete after 0 days, hard-delete after 30 days + PARTNER_FILE_DELETION_LOG = "partner_file_deletion_log" # Append-only deletion audit; not swept @dataclass @@ -107,6 +111,30 @@ def anonymization_threshold(self) -> datetime: audit_retention_months=12, pii_fields=["email", "phone", "name"], ), + RetentionPolicy.PARTNER_FILES: RetentionConfig( + policy=RetentionPolicy.PARTNER_FILES, + soft_delete_days=0, + hard_delete_days=30, + audit_retention_months=24, + pii_fields=[], + ), + RetentionPolicy.PARTNER_FILE_DERIVED: RetentionConfig( + policy=RetentionPolicy.PARTNER_FILE_DERIVED, + soft_delete_days=0, + hard_delete_days=30, + audit_retention_months=24, + pii_fields=[], + ), + # Append-only deletion audit (deleted_at is the event timestamp, not a + # soft-delete marker). Kept in RETENTION_CONFIGS so the policy is complete; + # cleanup_deleted_records_job skips it the same way it skips AUDIT_LOG. + RetentionPolicy.PARTNER_FILE_DELETION_LOG: RetentionConfig( + policy=RetentionPolicy.PARTNER_FILE_DELETION_LOG, + soft_delete_days=0, + hard_delete_days=0, + audit_retention_months=84, + pii_fields=[], + ), } @@ -122,15 +150,15 @@ class RetentionSchemaError(RuntimeError): # Every retention resource name -> the relation that actually holds that data, # or ``None`` when no such relation exists in the alembic-managed schema. # -# #2900: these names were interpolated straight into SQL as table names. On a -# database at `alembic upgrade head`, `to_regclass()` resolves NONE of them, -# and — decisively — the migrated schema contains ZERO columns named -# `deleted_at`, the column this entire soft-delete layer reads and writes. -# There is nothing to repoint to: this is not a rename, the schema these -# statements were written for has never existed in this repository. The -# nearest real relations are recorded below for whoever designs the migration -# that fixes this properly; they are deliberately NOT wired up, because doing -# so would change what erasure and retention actually delete. +# #2900: historical RetentionPolicy values were interpolated straight into SQL +# as table names. Most still have no backing relation with a ``deleted_at`` +# column (nearest candidates are documented inline and deliberately left +# unwired so erasure does not suddenly target the wrong rows). +# +# v138 (partner first-file storage) introduced real ``deleted_at`` columns on +# ``public.partner_files``, ``public.partner_file_derived``, and +# ``public.partner_file_deletion_log``. Those are wired below; retention SQL +# uses ``_sql_target`` so statements hit the schema-qualified relation. # # Keep every RetentionPolicy value present here. tests/shared/ # test_retention_relations_live.py checks both directions of this map against @@ -158,6 +186,11 @@ class RetentionSchemaError(RuntimeError): "transaction_log": None, # No relation and no candidate anywhere in the schema. "temporary_pii": None, + # v138 partner first-file storage + 30-day retention (SEL-549). + "partner_files": "public.partner_files", + "partner_file_derived": "public.partner_file_derived", + # Append-only deletion audit; deleted_at is the event timestamp. + "partner_file_deletion_log": "public.partner_file_deletion_log", } @@ -167,6 +200,20 @@ class RetentionSchemaError(RuntimeError): _ALLOWED_RESOURCE_TABLES: frozenset[str] = frozenset(policy.value.replace("-", "_") for policy in RetentionPolicy) +def _sql_target(resource_type: str) -> str: + """Relation name used in retention SQL (#2900 / v138). + + Prefer the schema-qualified entry from ``_RESOURCE_RELATIONS`` when a + real table is wired. Fall back to the bare resource name so historical + phantom policies still fail loudly at execute time as ``UndefinedTable`` + (covered by ``test_data_retention_failure_modes_2900``). + """ + relation = _RESOURCE_RELATIONS.get(resource_type) + if relation is not None: + return relation + return resource_type + + # Postgres SQLSTATEs for "you named something that does not exist": undefined # table / column / object / function. A retention statement that trips one of # these is a schema fault, never a data outcome. @@ -421,9 +468,10 @@ async def soft_delete_record( "deleted_at": deleted_at, "record_id": record_id, } + target = _sql_target(resource_type) if tenant_scoped: update_query = text(f""" - UPDATE {resource_type} + UPDATE {target} SET deleted_at = :deleted_at WHERE id = :record_id AND tenant_id = :tenant_id @@ -432,7 +480,7 @@ async def soft_delete_record( params["tenant_id"] = str(tenant_id) else: update_query = text(f""" - UPDATE {resource_type} + UPDATE {target} SET deleted_at = :deleted_at WHERE id = :record_id AND deleted_at IS NULL """) @@ -523,9 +571,10 @@ async def hard_delete_record( """ _validate_resource_type(resource_type) try: + target = _sql_target(resource_type) # Verify record was soft-deleted check_query = text(f""" - SELECT deleted_at FROM {resource_type} + SELECT deleted_at FROM {target} WHERE id = :record_id AND deleted_at IS NOT NULL """) @@ -543,7 +592,7 @@ async def hard_delete_record( # Delete the record delete_query = text(f""" - DELETE FROM {resource_type} + DELETE FROM {target} WHERE id = :record_id """) @@ -975,15 +1024,21 @@ async def cleanup_deleted_records_job(): # Process hard-deletes for each retention policy for policy, config in RETENTION_CONFIGS.items(): - if policy == RetentionPolicy.AUDIT_LOG: - # Skip — handled separately + if policy in ( + RetentionPolicy.AUDIT_LOG, + RetentionPolicy.PARTNER_FILE_DELETION_LOG, + ): + # AUDIT_LOG anonymised separately; deletion_log is + # append-only (deleted_at is the event timestamp). continue threshold = config.hard_delete_threshold() + resource_type = policy.value.replace("-", "_") + target = _sql_target(resource_type) # Find soft-deleted records eligible for hard-delete query = text(f""" - SELECT id FROM {policy.value.replace("-", "_")} + SELECT id FROM {target} WHERE deleted_at IS NOT NULL AND deleted_at < :threshold LIMIT 100 """) diff --git a/tests/shared/test_data_retention_failure_modes_2900.py b/tests/shared/test_data_retention_failure_modes_2900.py index 64fc76dcc..14b37958e 100644 --- a/tests/shared/test_data_retention_failure_modes_2900.py +++ b/tests/shared/test_data_retention_failure_modes_2900.py @@ -191,7 +191,15 @@ def __exit__(self, *_a): stats = _run(cleanup_deleted_records_job()) # One rollback per policy that failed (AUDIT_LOG is handled separately). - failing_policies = [p for p in RETENTION_CONFIGS if p is not RetentionPolicy.AUDIT_LOG] + failing_policies = [ + p + for p in RETENTION_CONFIGS + if p + not in ( + RetentionPolicy.AUDIT_LOG, + RetentionPolicy.PARTNER_FILE_DELETION_LOG, + ) + ] assert db.rollback.call_count >= len(failing_policies), ( f"expected a rollback after each failed policy sweep, got {db.rollback.call_count}" ) @@ -312,9 +320,7 @@ def test_process_deletion_request_reports_failed_schedule_audit(): """Delete and its audit both succeed, but the follow-up schedule audit fails: the delete must still read as done, with the error attached.""" mgr = _manager() - mgr.audit_logger.log = AsyncMock( - side_effect=AuditStorageError("audit event evt-2 was not persisted") - ) + mgr.audit_logger.log = AsyncMock(side_effect=AuditStorageError("audit event evt-2 was not persisted")) db = _ok_update(MagicMock()) result = _run( From a8e8e9aff66c67b48754465872e580cfc6744ac6 Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Wed, 28 Oct 2026 12:44:41 -0700 Subject: [PATCH 6/7] fix(v138): grant DELETE on partner_files for retention sweep Hard-delete cleanup needs DELETE on partner_files and partner_file_derived as regengine; keep deletion_log append-only. Update v111 cutover inventory so re-apply keeps the grants, and prove the runtime role can purge an expired structural row. --- ...3_b71d9e4c2a60_v111_runtime_role_grants.py | 9 ++ ...901_v138_partner_file_storage_retention.py | 23 ++- ...100000_partner_files_storage_retention.sql | 4 +- ...est_v138_partner_file_storage_retention.py | 70 ++++++--- tests/shared/test_retention_relations_live.py | 140 ++++++++++++++++++ 5 files changed, 222 insertions(+), 24 deletions(-) diff --git a/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py b/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py index 960778696..d05123802 100644 --- a/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py +++ b/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py @@ -577,6 +577,11 @@ def _build_migration_owner_posture() -> str: "public.scheduler_seen_items": ("SELECT", "INSERT", "UPDATE", "DELETE"), "public.tenant_compliance_status": ("SELECT", "INSERT", "UPDATE"), "public.tenant_product_profile": ("SELECT",), + # v138 partner-file storage. Hard-delete sweep needs DELETE on the two + # soft-delete targets; partner_file_deletion_log stays append-only. + "public.partner_files": ("SELECT", "INSERT", "UPDATE", "DELETE"), + "public.partner_file_derived": ("SELECT", "INSERT", "UPDATE", "DELETE"), + "public.partner_file_deletion_log": ("SELECT", "INSERT"), } # Explicit mutable data-plane inventory for the enabled ingestion/workflow @@ -752,6 +757,10 @@ def _build_migration_owner_posture() -> str: "public.webauthn_challenges": ("SELECT", "INSERT", "UPDATE", "DELETE"), "public.webauthn_credentials": ("SELECT", "INSERT", "UPDATE", "DELETE"), "public.webhook_outbox": ("SELECT", "INSERT", "UPDATE"), + # v138 partner-file storage — control-plane read only. + "public.partner_files": ("SELECT",), + "public.partner_file_derived": ("SELECT",), + "public.partner_file_deletion_log": ("SELECT",), } REGENGINE_SYSADMIN_COLUMN_GRANTS = { diff --git a/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py b/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py index 532bed829..7aa1b509b 100644 --- a/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py +++ b/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py @@ -290,8 +290,8 @@ REVOKE ALL ON public.partner_file_deletion_log FROM authenticated; END IF; IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') THEN - GRANT SELECT, INSERT, UPDATE ON public.partner_files TO regengine; - GRANT SELECT, INSERT, UPDATE ON public.partner_file_derived TO regengine; + GRANT SELECT, INSERT, UPDATE, DELETE ON public.partner_files TO regengine; + GRANT SELECT, INSERT, UPDATE, DELETE ON public.partner_file_derived TO regengine; -- Append-only audit: INSERT + SELECT only. Never UPDATE/DELETE. GRANT SELECT, INSERT ON public.partner_file_deletion_log TO regengine; REVOKE UPDATE, DELETE ON public.partner_file_deletion_log FROM regengine; @@ -358,6 +358,25 @@ def upgrade() -> None: def downgrade() -> None: + # Retention hard-delete (cleanup_deleted_records_job) needs DELETE on the + # two sweep targets. Revoke that privilege before DROP so a partial + # downgrade cannot leave a dangling DELETE grant if DROP is interrupted. + # partner_file_deletion_log never received DELETE (append-only). + op.execute( + """ +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') THEN + IF to_regclass('public.partner_files') IS NOT NULL THEN + REVOKE DELETE ON TABLE public.partner_files FROM regengine; + END IF; + IF to_regclass('public.partner_file_derived') IS NOT NULL THEN + REVOKE DELETE ON TABLE public.partner_file_derived FROM regengine; + END IF; + END IF; +END $$; +""" + ) op.execute( """ diff --git a/supabase/migrations/20261028100000_partner_files_storage_retention.sql b/supabase/migrations/20261028100000_partner_files_storage_retention.sql index 1b006d6b1..003b65773 100644 --- a/supabase/migrations/20261028100000_partner_files_storage_retention.sql +++ b/supabase/migrations/20261028100000_partner_files_storage_retention.sql @@ -222,8 +222,8 @@ BEGIN REVOKE ALL ON public.partner_file_deletion_log FROM authenticated; END IF; IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') THEN - GRANT SELECT, INSERT, UPDATE ON public.partner_files TO regengine; - GRANT SELECT, INSERT, UPDATE ON public.partner_file_derived TO regengine; + GRANT SELECT, INSERT, UPDATE, DELETE ON public.partner_files TO regengine; + GRANT SELECT, INSERT, UPDATE, DELETE ON public.partner_file_derived TO regengine; -- Append-only audit: INSERT + SELECT only. Never UPDATE/DELETE. GRANT SELECT, INSERT ON public.partner_file_deletion_log TO regengine; REVOKE UPDATE, DELETE ON public.partner_file_deletion_log FROM regengine; diff --git a/tests/migrations/test_v138_partner_file_storage_retention.py b/tests/migrations/test_v138_partner_file_storage_retention.py index 2a5bf12e3..4005fc44f 100644 --- a/tests/migrations/test_v138_partner_file_storage_retention.py +++ b/tests/migrations/test_v138_partner_file_storage_retention.py @@ -30,18 +30,11 @@ from tests.migrations.scratch_db import scratch_database_engine REPO_ROOT = Path(__file__).resolve().parents[2] -MIGRATION_PATH = ( - REPO_ROOT - / "alembic" - / "versions" - / "20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py" -) +MIGRATION_PATH = REPO_ROOT / "alembic" / "versions" / "20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py" def _load_migration(): - spec = importlib.util.spec_from_file_location( - f"v138_partner_{uuid4().hex}", MIGRATION_PATH - ) + spec = importlib.util.spec_from_file_location(f"v138_partner_{uuid4().hex}", MIGRATION_PATH) assert spec is not None and spec.loader is not None module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) @@ -70,11 +63,7 @@ def test_no_runtime_semicolon_splitter() -> None: assert "def _statements" not in src assert "_UPGRADE_STATEMENTS" in src tree = ast.parse(src) - names = { - node.name - for node in tree.body - if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) - } + names = {node.name for node in tree.body if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))} assert "upgrade" in names and "downgrade" in names assert "_statements" not in names @@ -85,11 +74,7 @@ def test_commented_semicolon_and_dollar_body_are_single_statements() -> None: # Header comment with an intentional semicolon survives inside a statement. assert any("trigger-maintained; UTC" in s for s in stmts) # Function body is one statement containing the dollar-quoted block. - fn = [ - s - for s in stmts - if "CREATE OR REPLACE FUNCTION public.partner_files_set_delete_after" in s - ] + fn = [s for s in stmts if "CREATE OR REPLACE FUNCTION public.partner_files_set_delete_after" in s] assert len(fn) == 1 assert "AS $$" in fn[0] assert "interval '30 days'" in fn[0] @@ -268,13 +253,14 @@ def test_delete_after_recomputed_on_direct_update(pg_engine, monkeypatch) -> Non ).scalar_one() assert cleared is None + def test_runtime_grant_includes_storage_url_insert() -> None: """v138 must GRANT INSERT (storage_url) and inventory must list it. Structural only — no live DB / no partner payloads. """ - from pathlib import Path import importlib.util + from pathlib import Path alembic = Path("alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py") text = alembic.read_text(encoding="utf-8") @@ -290,3 +276,47 @@ def test_runtime_grant_includes_storage_url_insert() -> None: assert "storage_url" in cols assert "created_at" not in cols + +def test_runtime_grant_includes_partner_files_delete_for_retention_sweep() -> None: + """v138 + cutover inventory must grant DELETE on the two sweep targets. + + cleanup_deleted_records_job hard-deletes expired partner_files / + partner_file_derived rows as the regengine role. Without DELETE the sweep + fails on privileges and retries forever (Claude FAIL on #3729). + partner_file_deletion_log stays append-only (no DELETE). + Structural only — no live DB / no partner payloads. + """ + import importlib.util + from pathlib import Path + + alembic = Path("alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py") + text = alembic.read_text(encoding="utf-8") + assert "GRANT SELECT, INSERT, UPDATE, DELETE ON public.partner_files TO regengine" in text + assert "GRANT SELECT, INSERT, UPDATE, DELETE ON public.partner_file_derived TO regengine" in text + assert "GRANT SELECT, INSERT ON public.partner_file_deletion_log TO regengine" in text + assert "REVOKE UPDATE, DELETE ON public.partner_file_deletion_log FROM regengine" in text + assert "REVOKE DELETE ON TABLE public.partner_files FROM regengine" in text + assert "REVOKE DELETE ON TABLE public.partner_file_derived FROM regengine" in text + + inv_path = Path("alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py") + spec = importlib.util.spec_from_file_location("v111_grants_partner_delete", inv_path) + mod = importlib.util.module_from_spec(spec) + assert spec.loader is not None + spec.loader.exec_module(mod) + assert mod.REGENGINE_TABLE_GRANTS["public.partner_files"] == ( + "SELECT", + "INSERT", + "UPDATE", + "DELETE", + ) + assert mod.REGENGINE_TABLE_GRANTS["public.partner_file_derived"] == ( + "SELECT", + "INSERT", + "UPDATE", + "DELETE", + ) + assert mod.REGENGINE_TABLE_GRANTS["public.partner_file_deletion_log"] == ( + "SELECT", + "INSERT", + ) + assert "DELETE" not in mod.REGENGINE_TABLE_GRANTS["public.partner_file_deletion_log"] diff --git a/tests/shared/test_retention_relations_live.py b/tests/shared/test_retention_relations_live.py index bdb0da77e..6440827fe 100644 --- a/tests/shared/test_retention_relations_live.py +++ b/tests/shared/test_retention_relations_live.py @@ -138,3 +138,143 @@ def test_soft_delete_column_state_matches_what_the_module_declares() -> None: ) finally: engine.dispose() + + +def test_runtime_role_can_hard_delete_expired_partner_files() -> None: + """regengine must be able to hard-delete an expired partner_files row. + + The retention sweep runs as the ordinary runtime role (get_db → + DATABASE_URL). v138 puts partner_files in the hard-delete path; without + DELETE the job fails on privileges and retries forever. This proves the + grant + sweep path on a migrated scratch DB using only structural rows + (no realistic partner payloads). + """ + import asyncio + from datetime import datetime, timedelta, timezone + from unittest.mock import AsyncMock, MagicMock + from uuid import uuid4 + + from shared.data_retention import ( + AuditActor, + DataRetentionManager, + RetentionPolicy, + ) + from sqlalchemy import create_engine, text + from sqlalchemy.orm import sessionmaker + + engine = create_engine(sync_postgres_url(DATABASE_URL)) + tenant_id = str(uuid4()) + file_id = str(uuid4()) + engagement_id = str(uuid4()) + expired_at = datetime.now(timezone.utc) - timedelta(days=31) + + try: + # Privilege gate (independent of session role): cutover + v138. + with engine.connect() as conn: + has_delete_files = conn.execute( + text("SELECT has_table_privilege('regengine', to_regclass('public.partner_files'), 'DELETE')") + ).scalar() + has_delete_derived = conn.execute( + text("SELECT has_table_privilege('regengine', to_regclass('public.partner_file_derived'), 'DELETE')") + ).scalar() + has_delete_log = conn.execute( + text( + "SELECT has_table_privilege('regengine', to_regclass('public.partner_file_deletion_log'), 'DELETE')" + ) + ).scalar() + conn.commit() + assert has_delete_files is True, ( + "regengine lacks DELETE on public.partner_files — retention " + "hard-delete will fail once rows age past 30 days" + ) + assert has_delete_derived is True, "regengine lacks DELETE on public.partner_file_derived" + assert has_delete_log is False, "partner_file_deletion_log must stay append-only (no DELETE)" + + # Seed as the live-suite provider (postgres / superuser). Minimal + # structural columns only — synthetic path/hash, not a real file. + with engine.begin() as conn: + conn.execute( + text( + "INSERT INTO public.tenants (id, name, slug) VALUES (CAST(:id AS uuid), 'retention sweep', :slug)" + ), + {"id": tenant_id, "slug": f"ret-{tenant_id[:8]}"}, + ) + conn.execute(text("ALTER TABLE public.partner_files DISABLE ROW LEVEL SECURITY")) + conn.execute( + text( + """ + INSERT INTO public.partner_files ( + id, tenant_id, engagement_id, storage_path, + content_sha256, original_filename, byte_size, + content_type, status, deleted_at + ) VALUES ( + CAST(:id AS uuid), + CAST(:tenant_id AS uuid), + CAST(:engagement_id AS uuid), + 'tenants/' || :tenant_id || '/structural.csv', + repeat('a', 64), + 'structural.csv', + 12, + 'text/csv', + 'deleted', + :deleted_at + ) + """ + ), + { + "id": file_id, + "tenant_id": tenant_id, + "engagement_id": engagement_id, + "deleted_at": expired_at, + }, + ) + conn.execute(text("ALTER TABLE public.partner_files ENABLE ROW LEVEL SECURITY")) + + # Prove the DELETE as the runtime role under tenant RLS. + Session = sessionmaker(bind=engine) + session = Session() + try: + session.execute(text("SET LOCAL ROLE regengine")) + session.execute( + text("SELECT set_config('app.tenant_id', :tid, true)"), + {"tid": tenant_id}, + ) + audit = MagicMock() + audit.log_data_modification = AsyncMock(return_value=None) + mgr = DataRetentionManager(audit_logger=audit) + actor = AuditActor( + actor_id="system:retention_cleanup", + actor_type="system", + username="system", + ) + deleted_ok = asyncio.run( + mgr.hard_delete_record( + db=session, + resource_type=RetentionPolicy.PARTNER_FILES.value, + record_id=file_id, + actor=actor, + ) + ) + assert deleted_ok is True + finally: + session.close() + + # Row must be gone even when inspected as the provider role. + with engine.connect() as conn: + still_there = conn.execute( + text("SELECT 1 FROM public.partner_files WHERE id = CAST(:id AS uuid)"), + {"id": file_id}, + ).scalar() + conn.commit() + assert still_there is None, "expired partner_files row survived hard-delete as regengine" + finally: + with engine.begin() as conn: + conn.execute( + text("DELETE FROM public.partner_files WHERE id = CAST(:id AS uuid)"), + {"id": file_id}, + ) + conn.execute( + text("DELETE FROM public.tenants WHERE id = CAST(:id AS uuid)"), + {"id": tenant_id}, + ) + engine.dispose() From e933be9f45dafe48e20e91528ec7d84e7186dcb0 Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Wed, 28 Oct 2026 18:36:04 -0700 Subject: [PATCH 7/7] fix(v140): reparent partner-file storage onto v139 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge main (v139 / e7a1c9d2f4b0 from #3741) and point the partner-file storage retention migration at that head so alembic has a single head. Renumber the migration label v138 → v140 to follow the sequence; keep revision id b8c1e4f2a901. Signed-off-by: Christopher Sellers <1@christophersellers.com> --- ...901_v140_partner_file_storage_retention.py} | 8 ++++---- ...8100000_partner_files_storage_retention.sql | 2 +- ...est_v140_partner_file_storage_retention.py} | 18 +++++++++--------- 3 files changed, 14 insertions(+), 14 deletions(-) rename alembic/versions/{20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py => 20261028_b8c1e4f2a901_v140_partner_file_storage_retention.py} (98%) rename tests/migrations/{test_v138_partner_file_storage_retention.py => test_v140_partner_file_storage_retention.py} (95%) diff --git a/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py b/alembic/versions/20261028_b8c1e4f2a901_v140_partner_file_storage_retention.py similarity index 98% rename from alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py rename to alembic/versions/20261028_b8c1e4f2a901_v140_partner_file_storage_retention.py index 7aa1b509b..f7bbadcfc 100644 --- a/alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py +++ b/alembic/versions/20261028_b8c1e4f2a901_v140_partner_file_storage_retention.py @@ -1,4 +1,4 @@ -"""Partner file storage metadata + 30-day retention (v138 / SEL-549 first-file). +"""Partner file storage metadata + 30-day retention (v140 / SEL-549 first-file). Mirrors supabase/migrations/20261028100000_partner_files_storage_retention.sql (metadata + RLS + delete_after trigger) and the manifest_blobs payload-pointer @@ -17,7 +17,7 @@ Synthetic fixtures only until SEL-590 clears. Revision ID: b8c1e4f2a901 -Revises: a3f7ce361b0e +Revises: e7a1c9d2f4b0 Create Date: 2026-10-28 """ @@ -26,7 +26,7 @@ from alembic import op revision: str = "b8c1e4f2a901" # pragma: allowlist secret -down_revision: Union[str, Sequence[str], None] = "a3f7ce361b0e" # pragma: allowlist secret +down_revision: Union[str, Sequence[str], None] = "e7a1c9d2f4b0" # pragma: allowlist secret branch_labels = None depends_on = None @@ -335,7 +335,7 @@ """ DO $$ BEGIN - -- v138: runtime role must INSERT storage_url for partner-uploads pointer rows. + -- v140: runtime role must INSERT storage_url for partner-uploads pointer rows. -- Inventory (v111 REGENGINE_COLUMN_GRANTS) also lists storage_url so -- apply_runtime_role_cutover.py keeps the grant across deploys. IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') diff --git a/supabase/migrations/20261028100000_partner_files_storage_retention.sql b/supabase/migrations/20261028100000_partner_files_storage_retention.sql index 003b65773..2fdcd63cf 100644 --- a/supabase/migrations/20261028100000_partner_files_storage_retention.sql +++ b/supabase/migrations/20261028100000_partner_files_storage_retention.sql @@ -258,7 +258,7 @@ END $$; -- Runtime role: allow pointer-only manifest_blobs inserts (partner-uploads). DO $$ BEGIN - -- v138: runtime role must INSERT storage_url for partner-uploads pointer rows. + -- v140: runtime role must INSERT storage_url for partner-uploads pointer rows. -- Inventory (v111 REGENGINE_COLUMN_GRANTS) also lists storage_url so -- apply_runtime_role_cutover.py keeps the grant across deploys. IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') diff --git a/tests/migrations/test_v138_partner_file_storage_retention.py b/tests/migrations/test_v140_partner_file_storage_retention.py similarity index 95% rename from tests/migrations/test_v138_partner_file_storage_retention.py rename to tests/migrations/test_v140_partner_file_storage_retention.py index 4005fc44f..e389a8ac1 100644 --- a/tests/migrations/test_v138_partner_file_storage_retention.py +++ b/tests/migrations/test_v140_partner_file_storage_retention.py @@ -1,4 +1,4 @@ -"""v138 partner file storage retention — structure + stub PG16 behavior. +"""v140 partner file storage retention — structure + stub PG16 behavior. Covers the Codex FAIL fixes on #3729: @@ -30,11 +30,11 @@ from tests.migrations.scratch_db import scratch_database_engine REPO_ROOT = Path(__file__).resolve().parents[2] -MIGRATION_PATH = REPO_ROOT / "alembic" / "versions" / "20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py" +MIGRATION_PATH = REPO_ROOT / "alembic" / "versions" / "20261028_b8c1e4f2a901_v140_partner_file_storage_retention.py" def _load_migration(): - spec = importlib.util.spec_from_file_location(f"v138_partner_{uuid4().hex}", MIGRATION_PATH) + spec = importlib.util.spec_from_file_location(f"v140_partner_{uuid4().hex}", MIGRATION_PATH) assert spec is not None and spec.loader is not None module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) @@ -134,11 +134,11 @@ def execute(statement: str) -> None: @pytest.fixture(scope="module") def pg_engine(): - """Disposable PG16 database with v138 minimal deps, or skip/fail.""" + """Disposable PG16 database with v140 minimal deps, or skip/fail.""" # Prefer an explicit scratch URL; otherwise peer-auth local Homebrew PG. if not os.environ.get("REGENGINE_MIGRATION_TEST_DB"): os.environ["REGENGINE_MIGRATION_TEST_DB"] = "postgresql://sellers@/postgres?host=/tmp" - db_name = "inc2_scratch_v138_" + uuid4().hex[:12] + db_name = "inc2_scratch_v140_" + uuid4().hex[:12] with scratch_database_engine(db_name) as engine: _bootstrap_minimal_schema(engine) yield engine @@ -255,14 +255,14 @@ def test_delete_after_recomputed_on_direct_update(pg_engine, monkeypatch) -> Non def test_runtime_grant_includes_storage_url_insert() -> None: - """v138 must GRANT INSERT (storage_url) and inventory must list it. + """v140 must GRANT INSERT (storage_url) and inventory must list it. Structural only — no live DB / no partner payloads. """ import importlib.util from pathlib import Path - alembic = Path("alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py") + alembic = Path("alembic/versions/20261028_b8c1e4f2a901_v140_partner_file_storage_retention.py") text = alembic.read_text(encoding="utf-8") assert "GRANT INSERT (storage_url) ON TABLE fsma.manifest_blobs TO regengine" in text assert "REVOKE INSERT (storage_url) ON TABLE fsma.manifest_blobs FROM regengine" in text @@ -278,7 +278,7 @@ def test_runtime_grant_includes_storage_url_insert() -> None: def test_runtime_grant_includes_partner_files_delete_for_retention_sweep() -> None: - """v138 + cutover inventory must grant DELETE on the two sweep targets. + """v140 + cutover inventory must grant DELETE on the two sweep targets. cleanup_deleted_records_job hard-deletes expired partner_files / partner_file_derived rows as the regengine role. Without DELETE the sweep @@ -289,7 +289,7 @@ def test_runtime_grant_includes_partner_files_delete_for_retention_sweep() -> No import importlib.util from pathlib import Path - alembic = Path("alembic/versions/20261028_b8c1e4f2a901_v138_partner_file_storage_retention.py") + alembic = Path("alembic/versions/20261028_b8c1e4f2a901_v140_partner_file_storage_retention.py") text = alembic.read_text(encoding="utf-8") assert "GRANT SELECT, INSERT, UPDATE, DELETE ON public.partner_files TO regengine" in text assert "GRANT SELECT, INSERT, UPDATE, DELETE ON public.partner_file_derived TO regengine" in text