SEL-292: Ordinary test cleanup can erase an inherited database

.github/workflows/ci.yml

@@ -174,6 +174,8 @@ jobs:
     timeout-minutes: 20
     permissions:
       contents: read
+    env:
+      AMW_TEST_DB_DISPOSABLE: "1"
     services:
       postgres:
         image: postgres:16
@@ -261,6 +263,7 @@ jobs:
       run:
         working-directory: gateway
     env:
+      AMW_TEST_DB_DISPOSABLE: "1"
       CI_STRICT: "1"
       ENVIRONMENT: test
     steps:

gateway/Makefile

@@ -58,9 +58,10 @@ prove-trust-plane-postgres: prove-crash-recovery
 # recovery, and fail-closed manual review after an ambiguous side effect
 # (never an automatic redispatch).
 #
-# Requires DATABASE_URL=postgresql+asyncpg://... pointing at a DEDICATED,
-# EMPTY database already migrated to head, STATE_BACKEND=postgres, a local/test
-# ENVIRONMENT, and explicit MCP_STRESS_DB_ISOLATED=1 acknowledgment. Provision
+# Requires DATABASE_URL=postgresql+asyncpg://sellers@127.0.0.1:55439/amw_action_*
+# pointing at a DEDICATED, EMPTY database already migrated to head,
+# STATE_BACKEND=postgres, ENVIRONMENT=test, and explicit
+# MCP_STRESS_DB_ISOLATED=1 acknowledgment. Provision
 # and migrate only the disposable database before invoking this target; this
 # target never migrates or overrides the caller's isolation settings.
 # The harness refuses to run otherwise: it fails closed on a

gateway/docs/TESTING_TIERS.md

@@ -32,12 +32,13 @@ version bump and a CHANGELOG entry (see the test module docstrings).
 Concurrency and row-lock proofs skip by default because SQLite treats
 `SELECT ... FOR UPDATE` as a silent no-op with different semantics from
 Postgres. To run them, point at a throwaway database you create and drop
-yourself. Never use a shared or production database: the guards below
-refuse anything that does not look disposable.
+yourself. Never use a shared or production database. Ordinary local Postgres runs
+require the fixed disposable action-test profile below.
 
 ```bash
-# The guard only accepts postgresql+asyncpg://sellers@127.0.0.1:55439/amw_action_*
-# with ENVIRONMENT=test. createdb, Alembic, and pytest must all use port 55439.
+# Ordinary local runs only accept postgresql+asyncpg://sellers@127.0.0.1:55439/amw_action_*
+# with ENVIRONMENT=test and an explicit Postgres test flag. createdb, Alembic,
+# and pytest must all use port 55439.
 # If the server listens on another port, forward that port onto 127.0.0.1:55439
 # before these commands. There is no forward example in this file.
 createdb -h 127.0.0.1 -p 55439 -U sellers amw_action_local
@@ -62,7 +63,9 @@ two-process crash harness (`make prove-crash-recovery`,
 
 CI runs tier 2 on every pull request in `postgres-concurrency` and
 `postgres-proofs` (`.github/workflows/ci.yml`). Both jobs use disposable
-Postgres service databases. `postgres-proofs` runs row-lock, action
+Postgres service databases and set `AMW_TEST_DB_DISPOSABLE=1` in their job
+environment; the shared fixture then accepts PostgreSQL URLs with
+`ENVIRONMENT=test`. `postgres-proofs` runs row-lock, action
 accounting, migration, asyncpg driver, and both multiprocess crash suites
 (`tests/test_mcp_postgres_multiprocess.py` and
 `tests/test_action_multiprocess.py`) with `CI_STRICT=1`; its final step

gateway/tests/conftest.py

@@ -11,7 +11,8 @@
 import pytest
 import pytest_asyncio
 from sqlalchemy import DateTime, event, inspect as sa_inspect, text
-from sqlalchemy.engine import Engine
+from sqlalchemy.engine import Engine, make_url
+from sqlalchemy.exc import ArgumentError
 from sqlalchemy.orm import Session
 
 # --- Proof-surface test classification -------------------------------------
@@ -137,6 +138,45 @@ def iter_routes(routes):
 os.environ.setdefault("DATABASE_URL", _DEFAULT_TEST_DB_URL)
 if _XDIST_WORKER and os.environ["DATABASE_URL"] == _DEFAULT_TEST_DB_URL:
     os.environ["DATABASE_URL"] = f"sqlite+aiosqlite:///./{TEST_DB_FILE}"
+
+
+def require_disposable_test_database() -> None:
+    """Refuse inherited databases before test setup or destructive cleanup."""
+    url = os.environ.get("DATABASE_URL", "")
+    if url == f"sqlite+aiosqlite:///./{TEST_DB_FILE}":
+        return
+    if (
+        os.environ.get("AMW_TEST_DB_DISPOSABLE") == "1"
+        and os.environ.get("ENVIRONMENT") == "test"
+    ):
+        try:
+            marked_url = make_url(url)
+        except (ArgumentError, ValueError):
+            pass
+        else:
+            if (
+                marked_url.drivername == "postgresql+asyncpg"
+                and marked_url.host
+                and marked_url.database
+            ):
+                return
+    if (
+        os.environ.get("RUN_POSTGRES_CONCURRENCY_TESTS") == "1"
+        or os.environ.get("RUN_MCP_MULTIPROCESS_TESTS") == "1"
+    ):
+        from tests.support.action_database_guard import (
+            require_action_database_url,
+            require_action_test_environment,
+        )
+
+        require_action_test_environment(os.environ.get("ENVIRONMENT"))
+        require_action_database_url(url)
+        return
+    # An inherited URL may contain credentials; never include it in the error.
+    raise ValueError("unsafe_test_database_url")
+
+
+require_disposable_test_database()
 # Durable state stays in memory for the suite. State this explicitly: the tests
 # always ran on memory state, but only as an accident of auto-resolution
 # choosing postgres and asyncpg then failing on the SQLite DSN above. Once a
@@ -345,6 +385,7 @@ def mount_dormant_surfaces_for_marked_tests(request):
 @pytest_asyncio.fixture(scope="session", autouse=True)
 async def setup_database():
     """Set up database tables before any tests run."""
+    require_disposable_test_database()
     from app.core.durable_state import close_durable_state
     from app.db.database import init_db, close_db
 
@@ -410,6 +451,7 @@ async def setup_database():
 @pytest_asyncio.fixture(scope="function")
 async def clean_database():
     """Clean database tables between tests."""
+    require_disposable_test_database()
     from app.db.database import get_session_factory
 
     factory = get_session_factory()

gateway/tests/test_action_database_guard.py

@@ -79,6 +79,7 @@ async def setup_sentinel():
         ):
             env.pop(key, None)
     for key in (
+        "AMW_TEST_DB_DISPOSABLE",
         "RUN_MCP_MULTIPROCESS_TESTS",
         "RUN_POSTGRES_CONCURRENCY_TESTS",
         "ACTION_MIGRATION_DATABASE_URL",
@@ -87,7 +88,7 @@ async def setup_sentinel():
     env["DATABASE_URL"] = VALID_URL + query
     if flag == "ACTION_MIGRATION_DATABASE_URL":
         env[flag] = VALID_URL + query
-        env["DATABASE_URL"] = f"sqlite+aiosqlite:///{tmp_path / 'runner.db'}"
+        env["DATABASE_URL"] = "sqlite+aiosqlite:///./test.db"
     else:
         env[flag] = "1"
     result = subprocess.run(

gateway/tests/test_sqlite_backend.py

@@ -4,6 +4,7 @@
 
 import os
 import tempfile
+from contextlib import contextmanager
 
 import pytest
 
@@ -27,6 +28,25 @@ def reset_settings():
     get_settings.cache_clear()
 
 
+@contextmanager
+def _cleared_database_url():
+    """Hide ``DATABASE_URL`` for one store, then put the suite URL back.
+
+    The session database fixture re-checks this variable before every later
+    test. Leaving it cleared looks like an inherited database and aborts the
+    worker.
+    """
+    previous = os.environ.get("DATABASE_URL")
+    os.environ["DATABASE_URL"] = ""
+    try:
+        yield
+    finally:
+        if previous is None:
+            os.environ.pop("DATABASE_URL", None)
+        else:
+            os.environ["DATABASE_URL"] = previous
+
+
 @pytest.fixture
 def sqlite_store():
     """Create a SQLite-based DurableStateStore for testing."""
@@ -35,20 +55,26 @@ def sqlite_store():
 
     os.environ["SQLITE_URL"] = db_path
     os.environ["STATE_BACKEND"] = "sqlite"
-    os.environ["DATABASE_URL"] = ""
     os.environ["REDIS_URL"] = ""
-
-    store = DurableStateStore()
-    yield store
-
-    import asyncio
-
-    asyncio.run(store.close())
-
-    try:
-        os.unlink(db_path)
-    except FileNotFoundError:
-        pass
+    with _cleared_database_url():
+        store = DurableStateStore()
+        try:
+            yield store
+        finally:
+            import asyncio
+
+            asyncio.run(store.close())
+            try:
+                os.unlink(db_path)
+            except FileNotFoundError:
+                pass
+
+
+def test_cleared_database_url_is_restored_before_later_tests():
+    original = os.environ["DATABASE_URL"]
+    with _cleared_database_url():
+        assert os.environ["DATABASE_URL"] == ""
+    assert os.environ["DATABASE_URL"] == original
 
 
 @pytest.mark.asyncio