fix(security): fail closed in /api/models auth gate on unexpected errors (#3489)

GET /api/models swallowed any non-HTTPException raised while checking
whether the caller is authenticated (bare except Exception: pass), so a
broken auth_manager or an exception from get_current_user silently
granted the full model list to an anonymous caller instead of rejecting
the request. Now any unexpected exception logs and returns HTTP 500.

Split out of #2360 per reviewer request to keep the deny-list and the
auth-gate fix as separate, single-purpose PRs.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Giuseppe Castelluccio
2026-06-08 20:23:39 +02:00
committed by GitHub
parent 34a3f8637a
commit 095c74b985
2 changed files with 22 additions and 2 deletions
+3 -2
View File
@@ -1232,8 +1232,9 @@ def setup_model_routes(model_discovery):
raise HTTPException(401, "Not authenticated") raise HTTPException(401, "Not authenticated")
except HTTPException: except HTTPException:
raise raise
except Exception: except Exception as e:
pass logger.error('Auth gate error in GET /api/models, failing closed: %s', e)
raise HTTPException(status_code=500, detail='Internal error')
# Admins see every endpoint (they manage the global pool); regular # Admins see every endpoint (they manage the global pool); regular
# users get the owner-scoped view. # users get the owner-scoped view.
_is_admin = False _is_admin = False
+19
View File
@@ -10,6 +10,7 @@ from types import SimpleNamespace
import httpx import httpx
import pytest import pytest
from fastapi import HTTPException
from tests.helpers.import_state import clear_fake_endpoint_resolver_modules, preserve_import_state from tests.helpers.import_state import clear_fake_endpoint_resolver_modules, preserve_import_state
@@ -1273,6 +1274,24 @@ def test_background_refresh_failure_keeps_existing_cached_models(monkeypatch):
assert json.loads(ep.cached_models) == ["cached-model"] assert json.loads(ep.cached_models) == ["cached-model"]
def test_api_models_auth_gate_fails_closed_on_unexpected_error(monkeypatch):
"""A non-HTTPException raised while checking auth must yield 500, not a
silent pass-through that leaks the model list to an unauthenticated caller."""
router = model_routes.setup_model_routes(model_discovery=None)
monkeypatch.setattr(model_routes, "_auth_disabled", lambda: (_ for _ in ()).throw(RuntimeError("boom")))
request = SimpleNamespace(
state=SimpleNamespace(current_user=None),
app=SimpleNamespace(state=SimpleNamespace(auth_manager=SimpleNamespace(is_configured=True))),
)
with pytest.raises(HTTPException) as exc:
_route_endpoint(router, "/api/models")(request)
assert exc.value.status_code == 500
def test_llm_core_list_model_ids_uses_cached_configured_proxy(monkeypatch): def test_llm_core_list_model_ids_uses_cached_configured_proxy(monkeypatch):
ep = _route_ep( ep = _route_ep(
"proxy", "proxy",