From 6eea61dc789a0441eebdf6895ff03ed2583f1ffc Mon Sep 17 00:00:00 2001 From: Marian Date: Fri, 31 Jul 2026 13:18:21 +0000 Subject: [PATCH] fix(oidc): survive a failing rollback in the never-raise handler too The recovery rollback after a failed commit was itself unguarded, so a rollback that raises on a wedged connection would still take the boot down -- the exact failure the never-raise contract exists to prevent. Suppressed; the caller's `async with` discards the session regardless. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_016q8EAf9Rj7ZHL92sPnXYxy --- backend/app/core/oidc_env.py | 9 ++++++-- .../tests/integration/test_oidc_env_apply.py | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/backend/app/core/oidc_env.py b/backend/app/core/oidc_env.py index 123399f75..a82b81dff 100644 --- a/backend/app/core/oidc_env.py +++ b/backend/app/core/oidc_env.py @@ -9,6 +9,7 @@ check the UI enforces. from __future__ import annotations +import contextlib import logging import os @@ -100,8 +101,12 @@ async def apply_env_oidc_provider(db: AsyncSession) -> None: # 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() + # left clean for the rest of startup. Suppressed because rollback on a + # wedged connection can itself raise -- and the whole point here is that + # nothing in this path takes the boot down. The session is discarded by + # the caller's `async with` regardless. + with contextlib.suppress(Exception): + await db.rollback() async def _apply_env_oidc_provider(db: AsyncSession) -> None: diff --git a/backend/tests/integration/test_oidc_env_apply.py b/backend/tests/integration/test_oidc_env_apply.py index 5fce40c48..60f9b913c 100644 --- a/backend/tests/integration/test_oidc_env_apply.py +++ b/backend/tests/integration/test_oidc_env_apply.py @@ -220,6 +220,29 @@ async def test_a_commit_failure_is_survivable_and_leaks_nothing(db_session, monk assert "leaked-secret" not in caplog.text # ...but nothing from the message +@pytest.mark.asyncio +async def test_a_failing_rollback_is_also_survivable(db_session, monkeypatch, caplog): + """The handler rolls back after a failed commit -- but rollback on a wedged + connection can raise too, and 'never raises' has to hold for that as well + or the boot dies on the recovery path. The rollback is suppressed.""" + + async def _raise_on_commit(): + raise RuntimeError("database is locked") + + async def _raise_on_rollback(): + raise RuntimeError("connection is closed") + + monkeypatch.setattr(db_session, "commit", _raise_on_commit) + monkeypatch.setattr(db_session, "rollback", _raise_on_rollback) + _configure(monkeypatch, BAMBUDDY_OIDC_CLIENT_SECRET="leaked-secret") + + with caplog.at_level(logging.ERROR): + await apply_env_oidc_provider(db_session) # must not raise, even here + + assert "could not be applied" in caplog.text + assert "leaked-secret" not in caplog.text + + @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."""