From dab740ddf781a8a0f635dd3b9d010f88e3bba351 Mon Sep 17 00:00:00 2001 From: Michael Chihlas Date: Fri, 24 Apr 2026 13:14:08 -0400 Subject: [PATCH] fix(tests): isolate test DB from dev DB and plug admin-db override gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the 06:32 AM outage: running 'pytest tests/' inside the resolutionflow_backend container silently dropped the public schema on the DEV database. Two layered bugs made this possible; both are fixed. Bug 1 — env-var lookup in conftest.TEST_DATABASE_URL put DATABASE_URL (which normally points at the dev/prod DB) ahead of DATABASE_TEST_URL. When DATABASE_URL is set, pytest used the dev DB as the 'test' DB and the test_db fixture's DROP SCHEMA public CASCADE wiped it. Fixed: - Honor only DATABASE_TEST_URL (or the localhost fallback). - Assert at module load that the DB name contains 'test' — refuses to run otherwise. Makes future misconfiguration impossible. Bug 2 — conftest overrode app.dependency_overrides[get_db] but not get_admin_db. Endpoints using get_admin_db (register, admin routes) bypassed the test session and hit the real admin DB. Before Bug 1 was fixed this was hidden because both engines pointed at the same dev DB. With isolation in place, register started failing 'Email already registered' because of stale users in the dev DB. Fixed: - Also override get_admin_db to yield the same test session. RLS is not enabled in the create_all-managed test schema, so sharing is safe. Also adds DATABASE_TEST_URL=resolutionflow_test to docker-compose.dev.yml so pytest in the container works out of the box. Verified: 49/50 Phase 8 + 9 tests pass against resolutionflow_test; the 1 failure is the pre-existing Phase 8 Issue #4 (test_record_decision_persists_and_bumps_state_version). Refs gitea #145 (will update that issue with this as the primary fix). Co-Authored-By: Claude Opus 4.7 (1M context) --- backend/tests/conftest.py | 33 +++++++++++++++++++++++++-------- docker-compose.dev.yml | 3 +++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 9c1f60e6..f7cc56cb 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -14,21 +14,33 @@ from sqlalchemy.pool import NullPool from app.main import app from app.core.database import Base, get_db +from app.core.admin_database import get_admin_db from app.core.config import settings # Disable invite code requirement for tests settings.REQUIRE_INVITE_CODE = False -# Test database URL (separate from production) -# Use DATABASE_TEST_URL env var if set (e.g. inside Docker where host is 'db'), -# otherwise fall back to localhost for local development. +# Test database URL — NEVER reuse DATABASE_URL. The test_db fixture does +# `DROP SCHEMA public CASCADE` on every test; if DATABASE_URL (which normally +# points at the dev/prod DB) leaked into this value, running `pytest tests/` +# would silently nuke the dev database. Only DATABASE_TEST_URL is honored, +# and the safety assertion below refuses to run against a DB whose name +# doesn't contain "test". import os TEST_DATABASE_URL = os.environ.get( - "DATABASE_URL", - os.environ.get( - "DATABASE_TEST_URL", - "postgresql+asyncpg://postgres:postgres@localhost:5432/patherly_test", - ), + "DATABASE_TEST_URL", + "postgresql+asyncpg://postgres:postgres@localhost:5432/resolutionflow_test", +) + +# Belt-and-suspenders: refuse to run tests against a DB whose name doesn't +# contain "test". Parses the last path segment of the URL (everything after +# the final '/', with query string stripped) so credentials / hosts that +# happen to contain "test" can't bypass the check. +_test_db_name = TEST_DATABASE_URL.rsplit("/", 1)[-1].split("?", 1)[0].lower() +assert "test" in _test_db_name, ( + f"Refusing to run tests against database {_test_db_name!r} — " + f"the DB name must contain 'test'. Set DATABASE_TEST_URL to a dedicated " + f"test database (e.g. resolutionflow_test)." ) @@ -131,6 +143,11 @@ async def client(test_db: AsyncSession): yield test_db app.dependency_overrides[get_db] = override_get_db + # Endpoints that use get_admin_db (register, admin routes, service accounts) + # must also hit the test DB; otherwise they leak into the real admin DB. + # RLS is not enabled in the test schema (create_all, not alembic), so sharing + # the same session is safe. + app.dependency_overrides[get_admin_db] = override_get_db transport = ASGITransport(app=app) async with AsyncClient(transport=transport, base_url="http://test") as ac: diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index b4356197..5d6454db 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -33,6 +33,9 @@ services: - DEBUG=true - DATABASE_URL=postgresql+asyncpg://postgres:postgres@db:5432/resolutionflow - DATABASE_URL_SYNC=postgresql://postgres:postgres@db:5432/resolutionflow + # Dedicated test database — pytest will refuse to run against any DB + # whose name doesn't contain 'test' (conftest.py safety assertion). + - DATABASE_TEST_URL=postgresql+asyncpg://postgres:postgres@db:5432/resolutionflow_test - SECRET_KEY=${SECRET_KEY} - ALGORITHM=HS256 - ACCESS_TOKEN_EXPIRE_MINUTES=15