From 7691dd722259bae79bed37e22527fab0bd3a0235 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Wed, 28 Oct 2026 13:37:00 -0700 Subject: [PATCH 1/2] fix(db): grant regengine SELECT (status) on tenants (v139 / SEL-616) Auth for Compliance Dashboard readiness fails in prod because api_key_store selects tenants.status while v111 only granted SELECT (id). Add alembic + supabase v139 with column-level SELECT (status) only, extend the v111 REGENGINE_COLUMN_GRANTS inventory so cutover keeps it, and test that regengine never gets table-level SELECT on tenants. Do not apply to prod without C.Lee approval via DevOps. Pairs with #3740. --- ...3_b71d9e4c2a60_v111_runtime_role_grants.py | 7 +- ...d2f4b0_v139_tenants_status_select_grant.py | 58 ++++ ...20000_v139_tenants_status_select_grant.sql | 30 ++ .../test_v139_tenants_status_select_grant.py | 328 ++++++++++++++++++ 4 files changed, 421 insertions(+), 2 deletions(-) create mode 100644 alembic/versions/20261028_e7a1c9d2f4b0_v139_tenants_status_select_grant.py create mode 100644 supabase/migrations/20261028120000_v139_tenants_status_select_grant.sql create mode 100644 tests/migrations/test_v139_tenants_status_select_grant.py diff --git a/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py b/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py index 48d3f91c5..5d08e5ecf 100644 --- a/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py +++ b/alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py @@ -649,8 +649,11 @@ def _build_migration_owner_posture() -> str: "public.api_keys": {"UPDATE": ("last_used_at", "total_requests")}, # Runtime validation/background work may test one credential-bound UUID, # but tenant names, settings, and webhook configuration remain control- - # plane data. - "public.tenants": {"SELECT": ("id",)}, + # plane data. ``status`` lets api_key_store.get_tenant_status (and the + # Compliance Dashboard readiness path) fail closed on suspended tenants + # without exposing names/settings. v139 applies the same column grant to + # databases already past v111; this inventory keeps it across cutover. + "public.tenants": {"SELECT": ("id", "status")}, # 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 diff --git a/alembic/versions/20261028_e7a1c9d2f4b0_v139_tenants_status_select_grant.py b/alembic/versions/20261028_e7a1c9d2f4b0_v139_tenants_status_select_grant.py new file mode 100644 index 000000000..691f1ddda --- /dev/null +++ b/alembic/versions/20261028_e7a1c9d2f4b0_v139_tenants_status_select_grant.py @@ -0,0 +1,58 @@ +"""Grant regengine column-level SELECT on tenants.status (v139 / SEL-616). + +Prod auth for the Compliance Dashboard readiness path fails because +``shared.api_key_store`` runs ``SELECT status FROM tenants WHERE id = ...`` +and v111 only granted ``SELECT (id)`` to ``regengine``. This revision adds +the missing column privilege only — never a table-level SELECT — and the +matching v111 ``REGENGINE_COLUMN_GRANTS`` inventory entry keeps the grant +across ``apply_runtime_role_cutover``. + +Mirrors ``supabase/migrations/20261028120000_v139_tenants_status_select_grant.sql``. +Do not apply to prod without C.Lee's approval via DevOps. Pairs with the +UI degrade in #3740; this PR is the DB half of SEL-616. + +Revision ID: e7a1c9d2f4b0 +Revises: a3f7ce361b0e +Create Date: 2026-10-28 +""" + +from typing import Sequence, Union + +from alembic import op + +revision: str = "e7a1c9d2f4b0" # pragma: allowlist secret +down_revision: Union[str, Sequence[str], None] = "a3f7ce361b0e" # pragma: allowlist secret +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.execute( + """ + DO $grant$ + BEGIN + IF to_regclass('public.tenants') IS NOT NULL + AND EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') THEN + -- Column-level only. Do not GRANT SELECT ON public.tenants + -- (table-level); names/settings stay control-plane. + GRANT SELECT (status) ON TABLE public.tenants TO regengine; + END IF; + END + $grant$; + """ + ) + + +def downgrade() -> None: + op.execute( + """ + DO $revoke$ + BEGIN + IF to_regclass('public.tenants') IS NOT NULL + AND EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') THEN + REVOKE SELECT (status) ON TABLE public.tenants FROM regengine; + END IF; + END + $revoke$; + """ + ) diff --git a/supabase/migrations/20261028120000_v139_tenants_status_select_grant.sql b/supabase/migrations/20261028120000_v139_tenants_status_select_grant.sql new file mode 100644 index 000000000..3b08c80e1 --- /dev/null +++ b/supabase/migrations/20261028120000_v139_tenants_status_select_grant.sql @@ -0,0 +1,30 @@ +-- v139 / SEL-616: column-level SELECT on public.tenants(status) for regengine. +-- +-- Mirrors alembic/versions/20261028_e7a1c9d2f4b0_v139_tenants_status_select_grant.py. +-- Auth (api_key_store.get_tenant_status) needs status to fail closed on +-- suspended tenants. v111 only granted SELECT (id). Do NOT grant table-level +-- SELECT — names, settings, and webhook config stay control-plane. +-- +-- Idempotent. Do not apply to prod without C.Lee's approval via DevOps. +-- Pairs with the Compliance Dashboard degrade in #3740. + +DO $grant$ +BEGIN + IF to_regclass('public.tenants') IS NOT NULL + AND EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') THEN + -- Column-level only. Never GRANT SELECT ON public.tenants (table-level). + GRANT SELECT (status) ON TABLE public.tenants TO regengine; + END IF; +END +$grant$; + +-- Downgrade (run by hand only): +-- +-- DO $revoke$ +-- BEGIN +-- IF to_regclass('public.tenants') IS NOT NULL +-- AND EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') THEN +-- REVOKE SELECT (status) ON TABLE public.tenants FROM regengine; +-- END IF; +-- END +-- $revoke$; diff --git a/tests/migrations/test_v139_tenants_status_select_grant.py b/tests/migrations/test_v139_tenants_status_select_grant.py new file mode 100644 index 000000000..9786ff2a9 --- /dev/null +++ b/tests/migrations/test_v139_tenants_status_select_grant.py @@ -0,0 +1,328 @@ +"""v139 / SEL-616: column-level SELECT (status) on public.tenants for regengine. + +Structural checks cover the alembic + supabase pair and the v111 inventory. +The live scratch suite (when Postgres is available) proves regengine can read +``status``, cannot read other tenant columns, and has no table-level SELECT. +""" + +from __future__ import annotations + +import importlib.util +import re +from pathlib import Path +from types import SimpleNamespace +from uuid import uuid4 + +import pytest +from sqlalchemy import text + +from tests.migrations.scratch_db import scratch_database_engine + +REPO_ROOT = Path(__file__).resolve().parents[2] +V139 = ( + REPO_ROOT + / "alembic/versions/20261028_e7a1c9d2f4b0_v139_tenants_status_select_grant.py" +) +V111 = REPO_ROOT / "alembic/versions/20260803_b71d9e4c2a60_v111_runtime_role_grants.py" +SUPABASE = ( + REPO_ROOT / "supabase/migrations/20261028120000_v139_tenants_status_select_grant.sql" +) + + +def _load(path: Path, name: str | None = None): + spec = importlib.util.spec_from_file_location(name or path.stem, 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 test_upgrade_grants_only_status_column_to_regengine(monkeypatch) -> None: + migration = _load(V139) + assert migration.down_revision == "a3f7ce361b0e" # pragma: allowlist secret + assert migration.revision == "e7a1c9d2f4b0" # pragma: allowlist secret + + statements: list[str] = [] + monkeypatch.setattr(migration, "op", SimpleNamespace(execute=statements.append)) + migration.upgrade() + + assert len(statements) == 1 + body = statements[0] + assert "GRANT SELECT (status) ON TABLE public.tenants TO regengine" in body + # Column-level only — no table-level SELECT for regengine on tenants. + assert not re.search( + r"GRANT\s+SELECT\s+ON\s+(TABLE\s+)?public\.tenants\s+TO\s+regengine", + body, + flags=re.IGNORECASE, + ) + assert "GRANT SELECT ON TABLE public.tenants TO regengine" not in body + assert "regengine_sysadmin" not in body + + +def test_downgrade_revokes_the_same_column_grant(monkeypatch) -> None: + migration = _load(V139) + statements: list[str] = [] + monkeypatch.setattr(migration, "op", SimpleNamespace(execute=statements.append)) + migration.downgrade() + + assert len(statements) == 1 + body = statements[0] + assert "REVOKE SELECT (status) ON TABLE public.tenants FROM regengine" in body + assert not re.search( + r"REVOKE\s+SELECT\s+ON\s+(TABLE\s+)?public\.tenants\s+FROM\s+regengine", + body, + flags=re.IGNORECASE, + ) + + +def test_supabase_pair_matches_column_grant() -> None: + sql = SUPABASE.read_text() + assert "GRANT SELECT (status) ON TABLE public.tenants TO regengine" in sql + assert not re.search( + r"GRANT\s+SELECT\s+ON\s+(TABLE\s+)?public\.tenants\s+TO\s+regengine", + sql, + flags=re.IGNORECASE, + ) + assert "SEL-616" in sql + assert "e7a1c9d2f4b0" in sql or "v139" in sql.lower() + + +def test_v111_inventory_lists_id_and_status_only_no_table_select() -> None: + """Cutover must keep the column grant and must not promote table SELECT.""" + # Load without executing alembic.op side effects: read source + ast/exec of + # the grant dicts only is fragile; import via exec after stubbing op. + import types + import sys + + stub = types.ModuleType("alembic") + stub_op = types.ModuleType("alembic.op") + stub.op = stub_op + sys.modules["alembic"] = stub + sys.modules["alembic.op"] = stub_op + try: + mod = _load(V111, f"v111_inventory_{uuid4().hex}") + finally: + sys.modules.pop("alembic.op", None) + sys.modules.pop("alembic", None) + + assert mod.REGENGINE_COLUMN_GRANTS["public.tenants"]["SELECT"] == ("id", "status") + # Runtime must not have table-level SELECT/INSERT/UPDATE/DELETE on tenants. + assert "public.tenants" not in mod.REGENGINE_TABLE_GRANTS + # Sysadmin still owns table-level SELECT+INSERT; that is intentional. + assert mod.REGENGINE_SYSADMIN_TABLE_GRANTS["public.tenants"] == ("SELECT", "INSERT") + + +def _bootstrap_tenants_and_role(engine) -> None: + """Scratch-DB only. Never DROP the cluster-wide ``regengine`` role.""" + with engine.begin() as conn: + conn.execute(text("CREATE EXTENSION IF NOT EXISTS pgcrypto")) + conn.execute( + text( + """ + CREATE TABLE public.tenants ( + id uuid primary key default gen_random_uuid(), + name text not null, + slug text not null unique, + status text not null default 'active', + settings jsonb not null default '{}'::jsonb + ) + """ + ) + ) + # Role is cluster-wide and may own other local DBs — create if missing + # only; never DROP. Privileges below are scoped to this scratch table. + conn.execute( + text( + """ + DO $role$ + BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'regengine') THEN + CREATE ROLE regengine NOINHERIT LOGIN; + END IF; + END + $role$; + """ + ) + ) + # Start from a clean ACL on this scratch table, then simulate post-v111: + # column SELECT (id) only, no table-level SELECT. + conn.execute(text("REVOKE ALL ON TABLE public.tenants FROM PUBLIC, regengine")) + conn.execute( + text( + """ + DO $col$ + DECLARE + col_name text; + BEGIN + FOR col_name IN + SELECT a.attname + FROM pg_attribute AS a + JOIN pg_class AS c ON c.oid = a.attrelid + JOIN pg_namespace AS n ON n.oid = c.relnamespace + WHERE n.nspname = 'public' + AND c.relname = 'tenants' + AND a.attnum > 0 + AND NOT a.attisdropped + LOOP + EXECUTE format( + 'REVOKE ALL (%I) ON TABLE public.tenants FROM PUBLIC, regengine', + col_name + ); + END LOOP; + END + $col$; + """ + ) + ) + conn.execute(text("GRANT USAGE ON SCHEMA public TO regengine")) + conn.execute(text("GRANT SELECT (id) ON TABLE public.tenants TO regengine")) + + +def test_live_scratch_regengine_reads_status_only_no_table_select() -> None: + """Apply v139 on a disposable DB and prove the column-only contract.""" + db_name = "inc2_scratch_v139_" + uuid4().hex[:12] + migration = _load(V139, f"v139_live_{uuid4().hex}") + + with scratch_database_engine(db_name) as engine: + _bootstrap_tenants_and_role(engine) + + # Apply upgrade through the migration's op.execute path. + with engine.begin() as conn: + + class _Op: + @staticmethod + def execute(statement: str) -> None: + conn.execute(text(statement)) + + migration.op = _Op # type: ignore[attr-defined] + migration.upgrade() + + tenant_id = uuid4() + with engine.begin() as conn: + conn.execute( + text( + """ + INSERT INTO public.tenants (id, name, slug, status, settings) + VALUES ( + CAST(:id AS uuid), + 'Scratch tenant', + :slug, + 'active', + CAST(:settings AS jsonb) + ) + """ + ), + { + "id": str(tenant_id), + "slug": f"scratch-{tenant_id.hex[:8]}", + "settings": "{}", + }, + ) + + with engine.connect() as conn: + # Column privileges: id + status only. + cols = { + row.column_name + for row in conn.execute( + text( + """ + SELECT column_name + FROM information_schema.column_privileges + WHERE grantee = 'regengine' + AND table_schema = 'public' + AND table_name = 'tenants' + AND privilege_type = 'SELECT' + """ + ) + ) + } + assert cols == {"id", "status"} + + # No table-level SELECT (or any other table privilege) for regengine. + table_privs = { + row.privilege_type + for row in conn.execute( + text( + """ + SELECT privilege_type + FROM information_schema.table_privileges + WHERE grantee = 'regengine' + AND table_schema = 'public' + AND table_name = 'tenants' + """ + ) + ) + } + assert table_privs == set() + assert "SELECT" not in table_privs + + # relacl must not carry a table-level SELECT for regengine either. + relacl_select = conn.execute( + text( + """ + SELECT EXISTS ( + SELECT 1 + FROM pg_class AS c + JOIN pg_namespace AS n ON n.oid = c.relnamespace + CROSS JOIN LATERAL aclexplode(c.relacl) AS acl + JOIN pg_roles AS r ON r.oid = acl.grantee + WHERE n.nspname = 'public' + AND c.relname = 'tenants' + AND r.rolname = 'regengine' + AND acl.privilege_type = 'SELECT' + ) + """ + ) + ).scalar_one() + assert relacl_select is False + + # Live SET ROLE: status readable; name/settings denied. + # Each denied statement gets its own connection so a failed query + # does not poison the next assertion with InFailedSqlTransaction. + with engine.connect() as conn: + conn.execute(text("SET ROLE regengine")) + status = conn.execute( + text("SELECT status FROM public.tenants WHERE id = :id"), + {"id": str(tenant_id)}, + ).scalar_one() + assert status == "active" + conn.execute(text("RESET ROLE")) + + for denied_sql in ( + "SELECT name FROM public.tenants", + "SELECT settings FROM public.tenants", + "SELECT * FROM public.tenants", + ): + with engine.connect() as conn: + conn.execute(text("SET ROLE regengine")) + with pytest.raises(Exception, match="permission denied"): + conn.execute(text(denied_sql)) + + # Downgrade removes status but leaves the pre-existing id grant. + with engine.begin() as conn: + + class _OpDown: + @staticmethod + def execute(statement: str) -> None: + conn.execute(text(statement)) + + migration.op = _OpDown # type: ignore[attr-defined] + migration.downgrade() + + with engine.connect() as conn: + cols_after = { + row.column_name + for row in conn.execute( + text( + """ + SELECT column_name + FROM information_schema.column_privileges + WHERE grantee = 'regengine' + AND table_schema = 'public' + AND table_name = 'tenants' + AND privilege_type = 'SELECT' + """ + ) + ) + } + assert cols_after == {"id"} From a3978a6aad67bd8135d5c23e8a7e06d47c6f7e7b Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:14:54 -0700 Subject: [PATCH 2/2] fix(sel-616): allowlist v139 revision hex in secrets scan detect-secrets Hex High Entropy String on the alembic revision id assertion is a false positive; same pragma already used on the sibling revision asserts in this file. --- tests/migrations/test_v139_tenants_status_select_grant.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/migrations/test_v139_tenants_status_select_grant.py b/tests/migrations/test_v139_tenants_status_select_grant.py index 9786ff2a9..1bed63092 100644 --- a/tests/migrations/test_v139_tenants_status_select_grant.py +++ b/tests/migrations/test_v139_tenants_status_select_grant.py @@ -84,7 +84,7 @@ def test_supabase_pair_matches_column_grant() -> None: flags=re.IGNORECASE, ) assert "SEL-616" in sql - assert "e7a1c9d2f4b0" in sql or "v139" in sql.lower() + assert "e7a1c9d2f4b0" in sql or "v139" in sql.lower() # pragma: allowlist secret def test_v111_inventory_lists_id_and_status_only_no_table_select() -> None: