fix(security): GHSA-6mf4-q26m-47pv — fail-closed on auth-probe DB errors (CVSS 9.8)

  is_auth_enabled() and auth_middleware both caught every exception during
  the auth-state probe and returned the "allow" answer instead of denying
  the request. Reporter's PoC floods /api/v1/auth/login to exhaust file
  descriptors, forcing the next SQLite connect to raise, then hits a
  protected endpoint during the fail-open window with no token — granting
  unauthenticated access to admin-account creation, API-key creation, DB
  backup download, and printer control. CWE-636 / CWE-755. Affects >= 0.1.6.

  Fix:
  - is_auth_enabled (backend/app/core/auth.py): only returns False for the
    legitimate "settings row absent" case; any actual exception propagates
    so the caller can deny the request.
  - auth_middleware (backend/app/main.py): returns 503 on any probe failure
    instead of await call_next(request).

  4 new regression tests in test_auth_fail_closed.py pin the contract
  (propagates DB exceptions, returns False for no-row, True for "true",
  False for "false"). 1 existing security test renamed and updated to
  accept either 500 or 503 (both fail-closed) and to verify the
  SQLAlchemy detail does not leak in the body.

  Codebase grep confirmed no other auth-decision predicate has the same
  fail-open shape: _validate_api_key returns None on catch (→ 401 fail-
  closed downstream), is_advanced_auth_enabled propagates correctly,
  permissions.py has no catch-alls.

  Reported by @wondercrash via private advisory.

backend/app/core/auth.py

@@ -471,16 +471,27 @@ async def authenticate_user_by_email(db: AsyncSession, email: str, password: str
 
 
 async def is_auth_enabled(db: AsyncSession) -> bool:
-    """Check if authentication is enabled."""
-    try:
-        result = await db.execute(select(Settings).where(Settings.key == "auth_enabled"))
-        setting = result.scalar_one_or_none()
-        if setting is None:
-            return False
-        return setting.value.lower() == "true"
-    except Exception:
-        # If settings table doesn't exist or query fails, assume auth is disabled
+    """Check if authentication is enabled.
+
+    Fails CLOSED on database errors. A previous version of this function
+    caught every exception and returned False — silently treating an
+    unavailable database as "auth is disabled" and granting unauthenticated
+    access to every endpoint that called it (GHSA-6mf4-q26m-47pv, CVSS 9.8).
+    An attacker could trigger that fail-open by flooding /api/v1/auth/login
+    to exhaust the process's file-descriptor budget, then hit a protected
+    endpoint during the window where the next DB op raised.
+
+    Legitimate "auth was never configured" still returns False — the
+    settings row is simply absent, ``scalar_one_or_none`` returns None,
+    no exception. Any OTHER failure (connection error, fd exhaustion,
+    schema mismatch, …) propagates so the caller can deny the request
+    (503 / 500). Fail-closed is the only safe default for an auth probe.
+    """
+    result = await db.execute(select(Settings).where(Settings.key == "auth_enabled"))
+    setting = result.scalar_one_or_none()
+    if setting is None:
         return False
+    return setting.value.lower() == "true"
 
 
 async def _user_from_api_key(db: AsyncSession, api_key: APIKey) -> User | None:

backend/app/main.py

@@ -5301,7 +5301,10 @@ async def auth_middleware(request, call_next):
         if pattern in path:
             return await call_next(request)
 
-    # Check if auth is enabled
+    # Check if auth is enabled. Fail CLOSED on any exception during the
+    # probe — GHSA-6mf4-q26m-47pv: the previous fail-open path here let
+    # an attacker who could force a DB exception (e.g. file-descriptor
+    # exhaustion via login flood) bypass auth on every protected endpoint.
     try:
         async with async_session() as db:
             from backend.app.core.auth import is_auth_enabled
@@ -5312,8 +5315,11 @@ async def auth_middleware(request, call_next):
             # Auth disabled, allow all requests
             return await call_next(request)
     except Exception:
-        # If we can't check auth status, allow request (fail open for DB issues)
-        return await call_next(request)
+        logging.getLogger(__name__).exception("auth_middleware: failing closed on auth-probe error from %s", path)
+        return JSONResponse(
+            status_code=503,
+            content={"detail": "Authentication service temporarily unavailable"},
+        )
 
     # Auth is enabled - require valid token
     auth_header = request.headers.get("Authorization")

backend/tests/integration/test_security.py

@@ -1594,8 +1594,19 @@ class TestEncryptionStatusEndpoint:
 
     @pytest.mark.asyncio
     @pytest.mark.integration
-    async def test_status_returns_500_on_db_error(self, async_client, monkeypatch):
-        """A8: SQLAlchemyError during count queries → 500 with static message."""
+    async def test_status_returns_503_on_db_error(self, async_client, monkeypatch):
+        """A8: a DB failure during the request must NOT leak internal detail
+        and must NOT silently succeed.
+
+        Post-GHSA-6mf4-q26m-47pv: the auth middleware's ``is_auth_enabled``
+        probe runs its own DB query before the route is dispatched. Patching
+        ``AsyncSession.execute`` to raise now trips the middleware first and
+        the request fails closed with 503 — a stronger guarantee than the
+        previous route-level 500, because under the old fail-open the
+        middleware would have proceeded to dispatch the route unauthenticated.
+        Either status would be acceptable; the assertion here pins the
+        defense-in-depth posture (request denied, no leak).
+        """
         from unittest.mock import AsyncMock
 
         from sqlalchemy.exc import SQLAlchemyError
@@ -1608,8 +1619,11 @@ class TestEncryptionStatusEndpoint:
         monkeypatch.setattr("sqlalchemy.ext.asyncio.AsyncSession.execute", AsyncMock(side_effect=_raise))
 
         resp = await async_client.get(self.STATUS_URL, headers={"Authorization": f"Bearer {token}"})
-        assert resp.status_code == 500
-        assert "encryption status" in resp.json().get("detail", "").lower()
+        assert resp.status_code in (500, 503)
+        # Either layer's error message must not leak the SQLAlchemy details.
+        body = resp.json().get("detail", "").lower()
+        assert "simulated" not in body
+        assert "sqlalchemy" not in body
 
     @pytest.mark.asyncio
     @pytest.mark.integration

backend/tests/unit/test_auth_fail_closed.py

@@ -0,0 +1,81 @@
+"""Regression tests for the GHSA-6mf4-q26m-47pv fail-open auth bypass.
+
+The previous version of ``is_auth_enabled`` caught every exception and
+returned False (auth disabled). An attacker could trigger a DB-side
+exception — the documented PoC exhausts file descriptors via a flood on
+``/api/v1/auth/login`` until the next SQLite ``connect`` raises — and then
+hit any protected endpoint during that fail-open window with no token at
+all. Severity CVSS 9.8.
+
+These tests pin the fail-closed contract:
+
+1. ``is_auth_enabled`` propagates any DB exception (instead of swallowing
+   it and returning False).
+2. The "no settings row" path still returns False (auth was legitimately
+   never configured).
+3. ``setting.value == "true"`` still returns True.
+"""
+
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+
+from backend.app.core.auth import is_auth_enabled
+
+
+@pytest.mark.asyncio
+async def test_is_auth_enabled_propagates_db_exception_instead_of_failing_open():
+    """The core regression for GHSA-6mf4-q26m-47pv. A DB error during the
+    auth-enabled probe must propagate — fail closed — instead of returning
+    False and treating the system as auth-disabled."""
+
+    db = AsyncMock()
+    db.execute = AsyncMock(side_effect=OSError("simulated file-descriptor exhaustion"))
+
+    with pytest.raises(OSError, match="simulated file-descriptor exhaustion"):
+        await is_auth_enabled(db)
+
+
+@pytest.mark.asyncio
+async def test_is_auth_enabled_returns_false_when_settings_row_absent():
+    """Legitimate 'auth was never configured' path: the settings row simply
+    does not exist. ``scalar_one_or_none`` returns None, no exception, and
+    the function returns False — system is auth-disabled by configuration,
+    not because the DB blew up."""
+
+    result = MagicMock()
+    result.scalar_one_or_none = MagicMock(return_value=None)
+    db = AsyncMock()
+    db.execute = AsyncMock(return_value=result)
+
+    assert await is_auth_enabled(db) is False
+
+
+@pytest.mark.asyncio
+async def test_is_auth_enabled_returns_true_when_setting_value_is_true():
+    """Happy path: the settings row exists and its value is "true" → auth
+    is enabled and the caller must require credentials."""
+
+    setting = MagicMock()
+    setting.value = "true"
+    result = MagicMock()
+    result.scalar_one_or_none = MagicMock(return_value=setting)
+    db = AsyncMock()
+    db.execute = AsyncMock(return_value=result)
+
+    assert await is_auth_enabled(db) is True
+
+
+@pytest.mark.asyncio
+async def test_is_auth_enabled_returns_false_when_setting_value_is_false():
+    """Happy path: the settings row exists and its value is "false" → auth
+    is disabled by configuration (legitimate, not exception)."""
+
+    setting = MagicMock()
+    setting.value = "false"
+    result = MagicMock()
+    result.scalar_one_or_none = MagicMock(return_value=setting)
+    db = AsyncMock()
+    db.execute = AsyncMock(return_value=result)
+
+    assert await is_auth_enabled(db) is False