From 095c74b98520266a4d3511ff5283f822207ef3a0 Mon Sep 17 00:00:00 2001 From: Giuseppe Castelluccio Date: Mon, 8 Jun 2026 20:23:39 +0200 Subject: [PATCH] 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 --- routes/model_routes.py | 5 +++-- tests/test_model_routes.py | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/routes/model_routes.py b/routes/model_routes.py index 15f1543c3..a54f4d302 100644 --- a/routes/model_routes.py +++ b/routes/model_routes.py @@ -1232,8 +1232,9 @@ def setup_model_routes(model_discovery): raise HTTPException(401, "Not authenticated") except HTTPException: raise - except Exception: - pass + except Exception as e: + 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 # users get the owner-scoped view. _is_admin = False diff --git a/tests/test_model_routes.py b/tests/test_model_routes.py index a39b3e7ae..f0f36ee7f 100644 --- a/tests/test_model_routes.py +++ b/tests/test_model_routes.py @@ -10,6 +10,7 @@ from types import SimpleNamespace import httpx import pytest +from fastapi import HTTPException 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"] +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): ep = _route_ep( "proxy",