mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-17 19:36:28 +02:00
fix(oidc): make apply_env_oidc_provider never raise on DB errors
The db.execute/db.commit calls in the upsert and release paths sat outside the try/except that only wrapped OIDCProviderCreate, so a commit failure at startup (connection blip, WAL lock) propagated out of the lifespan and took the instance down -- the exact outcome this module exists to avoid. The body now runs inside a private _apply_env_oidc_provider(), with the public entry point catching, logging and rolling back on any exception.
This commit is contained in:
@@ -90,9 +90,21 @@ _APPLIED_FIELDS = (
|
||||
async def apply_env_oidc_provider(db: AsyncSession) -> None:
|
||||
"""Upsert the env-managed provider, or release it when the config is gone.
|
||||
|
||||
Never raises: this runs during startup, and a typo in one variable must not
|
||||
stop the app from booting. A rejected config is logged and skipped.
|
||||
Never raises: this runs during startup, and a typo in one variable -- or a
|
||||
DB error on commit -- must not stop the app from booting. A rejected
|
||||
config is logged and skipped.
|
||||
"""
|
||||
try:
|
||||
await _apply_env_oidc_provider(db)
|
||||
except Exception as exc: # noqa: BLE001 -- startup must survive any failure here
|
||||
# Never str(exc): a DB error message can echo a configured value. Class only.
|
||||
logger.error("BAMBUDDY_OIDC_* could not be applied: %s", type(exc).__name__)
|
||||
# A commit may have half-applied; roll back so the shared session is
|
||||
# left clean for the rest of startup.
|
||||
await db.rollback()
|
||||
|
||||
|
||||
async def _apply_env_oidc_provider(db: AsyncSession) -> None:
|
||||
# Imported here rather than at module scope: app.core is imported by the
|
||||
# models themselves, so a top-level import would be a cycle.
|
||||
from backend.app.models.group import Group
|
||||
|
||||
@@ -198,6 +198,28 @@ async def test_a_non_validation_error_is_survivable_and_leaks_nothing(db_session
|
||||
assert "leaked-secret" not in caplog.text # ...but nothing from the message
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_commit_failure_is_survivable_and_leaks_nothing(db_session, monkeypatch, caplog):
|
||||
"""The upsert's db.execute/db.commit calls sit outside the inner
|
||||
ValidationError guard -- a Postgres blip or a SQLite WAL lock at startup
|
||||
must not propagate out of the lifespan either. Only the exception class
|
||||
may be logged, never str(exc), since a DB error message can echo a
|
||||
configured value."""
|
||||
|
||||
async def _raise_on_commit():
|
||||
raise RuntimeError("database is locked")
|
||||
|
||||
monkeypatch.setattr(db_session, "commit", _raise_on_commit)
|
||||
_configure(monkeypatch, BAMBUDDY_OIDC_CLIENT_SECRET="leaked-secret")
|
||||
|
||||
with caplog.at_level(logging.ERROR):
|
||||
await apply_env_oidc_provider(db_session) # must not raise
|
||||
|
||||
assert "could not be applied" in caplog.text
|
||||
assert "RuntimeError" in caplog.text # class is logged...
|
||||
assert "leaked-secret" not in caplog.text # ...but nothing from the message
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_applying_twice_without_changes_is_a_no_op(db_session, monkeypatch):
|
||||
"""Every boot re-applies; the second run must not create a second row."""
|
||||
|
||||
Reference in New Issue
Block a user