From 9167f9725b52953fc7592f73350f8f281d216ad5 Mon Sep 17 00:00:00 2001 From: Jason Cook Date: Thu, 23 Apr 2026 11:48:02 -0400 Subject: [PATCH] feat(oauth): refuse /start when STORE_MODEL_IN_DB is not set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without STORE_MODEL_IN_DB=True the proxy still writes OAuth credentials to LiteLLM_CredentialsTable, but ``proxy_config.get_credentials`` — the DB → ``litellm.credential_list`` reload run at startup — lives inside ``if store_model_in_db is True:`` in proxy_server.py. Result: silent data loss on restart and request-time ``api_key: oauth:`` failures because the name is no longer in the in-memory cache. Gate the /start endpoints (both ChatGPT and Copilot) behind ``get_secret_bool("STORE_MODEL_IN_DB", False)``. Admins get a clear 400 up front instead of sitting through the 15-minute device-code poll only to discover the flow silently doesn't persist. Tests: setenv STORE_MODEL_IN_DB=True in the autouse fixture so the success-path tests don't each have to opt in, plus one new test per provider asserting 400 + message when the env var is absent. 167 cases pass locally. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../chatgpt_oauth_endpoints/endpoints.py | 28 +++++++++++++++++++ .../copilot_oauth_endpoints/endpoints.py | 22 +++++++++++++++ .../test_chatgpt_oauth_endpoints.py | 23 ++++++++++++++- .../test_copilot_oauth_endpoints.py | 14 +++++++++- 4 files changed, 85 insertions(+), 2 deletions(-) diff --git a/litellm/proxy/chatgpt_oauth_endpoints/endpoints.py b/litellm/proxy/chatgpt_oauth_endpoints/endpoints.py index a61aa48b661..1071ecdf451 100644 --- a/litellm/proxy/chatgpt_oauth_endpoints/endpoints.py +++ b/litellm/proxy/chatgpt_oauth_endpoints/endpoints.py @@ -29,6 +29,7 @@ from pydantic import BaseModel from litellm._logging import verbose_proxy_logger from litellm.litellm_core_utils.credential_accessor import CredentialAccessor +from litellm.secret_managers.main import get_secret_bool from litellm.llms.chatgpt.authenticator import Authenticator from litellm.llms.chatgpt.common_utils import ( CHATGPT_DEVICE_VERIFY_URL, @@ -79,6 +80,32 @@ class RefreshResponse(BaseModel): expires_at: Optional[int] = None +def _require_store_model_in_db() -> None: + """ + Fail fast if ``STORE_MODEL_IN_DB`` is not set. + + OAuth tokens get encrypted + upserted into ``LiteLLM_CredentialsTable`` + by this flow, but the proxy only reloads that table into + ``litellm.credential_list`` on startup when ``STORE_MODEL_IN_DB=True`` + (see ``proxy_config.get_credentials`` wiring in ``proxy_server.py``). + Without the env var the write succeeds but nothing reads it back — the + credential appears to vanish on the next restart, and request-time + ``api_key: oauth:`` resolution fails because the name is no + longer in the in-memory cache. Refuse up front so admins don't sit + through the 15-minute device-code poll only to hit silent data loss. + """ + if not get_secret_bool("STORE_MODEL_IN_DB", False): + raise HTTPException( + status_code=400, + detail=( + "OAuth sign-in requires STORE_MODEL_IN_DB=True so the " + "proxy can reload credentials from the database on " + "restart. Set the env var (or equivalent in your " + "deployment config) and restart before trying again." + ), + ) + + def _require_admin(user_api_key_dict: UserAPIKeyAuth) -> None: # These endpoints write to LiteLLM_CredentialsTable (start → insert, # refresh → rotate). PROXY_ADMIN_VIEW_ONLY must not reach them. @@ -121,6 +148,7 @@ async def start_oauth( user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), ) -> StartResponse: _require_admin(user_api_key_dict) + _require_store_model_in_db() _purge_expired_sessions() # Atomically reserve a slot so concurrent callers cannot all pass the diff --git a/litellm/proxy/copilot_oauth_endpoints/endpoints.py b/litellm/proxy/copilot_oauth_endpoints/endpoints.py index 9f43376692b..1c5c8073a61 100644 --- a/litellm/proxy/copilot_oauth_endpoints/endpoints.py +++ b/litellm/proxy/copilot_oauth_endpoints/endpoints.py @@ -26,6 +26,7 @@ from pydantic import BaseModel from litellm._logging import verbose_proxy_logger from litellm.litellm_core_utils.credential_accessor import CredentialAccessor +from litellm.secret_managers.main import get_secret_bool from litellm.llms.github_copilot.authenticator import Authenticator from litellm.llms.github_copilot.common_utils import GithubCopilotError from litellm.llms.github_copilot.db_authenticator import ( @@ -73,6 +74,26 @@ class RefreshResponse(BaseModel): api_key_expires_at: Optional[int] = None +def _require_store_model_in_db() -> None: + """ + Fail fast if ``STORE_MODEL_IN_DB`` is not set — see the chatgpt + endpoints for the full rationale. Without this env var the OAuth + credential write succeeds but the proxy won't reload it on restart, + causing silent data loss after the user sits through the device-code + poll. + """ + if not get_secret_bool("STORE_MODEL_IN_DB", False): + raise HTTPException( + status_code=400, + detail=( + "OAuth sign-in requires STORE_MODEL_IN_DB=True so the " + "proxy can reload credentials from the database on " + "restart. Set the env var (or equivalent in your " + "deployment config) and restart before trying again." + ), + ) + + def _require_admin(user_api_key_dict: UserAPIKeyAuth) -> None: # These endpoints write to LiteLLM_CredentialsTable (start → insert, # refresh → rotate). PROXY_ADMIN_VIEW_ONLY must not reach them. @@ -115,6 +136,7 @@ async def start_oauth( user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), ) -> StartResponse: _require_admin(user_api_key_dict) + _require_store_model_in_db() _purge_expired_sessions() # Atomically reserve a slot so concurrent callers cannot all pass the diff --git a/tests/test_litellm/proxy/chatgpt_oauth_endpoints/test_chatgpt_oauth_endpoints.py b/tests/test_litellm/proxy/chatgpt_oauth_endpoints/test_chatgpt_oauth_endpoints.py index 6220160d70e..0d00529cfd5 100644 --- a/tests/test_litellm/proxy/chatgpt_oauth_endpoints/test_chatgpt_oauth_endpoints.py +++ b/tests/test_litellm/proxy/chatgpt_oauth_endpoints/test_chatgpt_oauth_endpoints.py @@ -42,7 +42,11 @@ def _view_only_admin() -> UserAPIKeyAuth: @pytest.fixture(autouse=True) -def _clear_sessions(): +def _clear_sessions(monkeypatch): + # The /start guard refuses to run without STORE_MODEL_IN_DB — set it + # here so the success-path tests in TestStartOAuth don't all have to + # opt in individually. The dedicated guard test overrides it. + monkeypatch.setenv("STORE_MODEL_IN_DB", "True") with _sessions_lock: _sessions.clear() yield @@ -70,6 +74,23 @@ class TestStartOAuth: ) assert exc_info.value.status_code == 403 + @pytest.mark.asyncio + async def test_rejects_when_store_model_in_db_unset(self, monkeypatch): + """ + Without STORE_MODEL_IN_DB the proxy won't reload credentials from + LiteLLM_CredentialsTable on restart, so the OAuth write would be + silently lost. Refuse up front rather than have the admin sit + through the 15-minute device-code poll. + """ + monkeypatch.delenv("STORE_MODEL_IN_DB", raising=False) + with pytest.raises(HTTPException) as exc_info: + await start_oauth( + StartRequest(credential_name="c"), + user_api_key_dict=_admin(), + ) + assert exc_info.value.status_code == 400 + assert "STORE_MODEL_IN_DB" in exc_info.value.detail + @pytest.mark.asyncio async def test_rejects_when_session_cap_reached(self): from litellm.proxy.chatgpt_oauth_endpoints.endpoints import SESSIONS_MAX_SIZE diff --git a/tests/test_litellm/proxy/copilot_oauth_endpoints/test_copilot_oauth_endpoints.py b/tests/test_litellm/proxy/copilot_oauth_endpoints/test_copilot_oauth_endpoints.py index bb3b1b7782c..b8100b76dbb 100644 --- a/tests/test_litellm/proxy/copilot_oauth_endpoints/test_copilot_oauth_endpoints.py +++ b/tests/test_litellm/proxy/copilot_oauth_endpoints/test_copilot_oauth_endpoints.py @@ -40,7 +40,8 @@ def _view_only_admin() -> UserAPIKeyAuth: @pytest.fixture(autouse=True) -def _clear_sessions(): +def _clear_sessions(monkeypatch): + monkeypatch.setenv("STORE_MODEL_IN_DB", "True") with _sessions_lock: _sessions.clear() DBAuthenticator._api_key_cache.clear() @@ -68,6 +69,17 @@ class TestStartOAuth: ) assert exc_info.value.status_code == 403 + @pytest.mark.asyncio + async def test_rejects_when_store_model_in_db_unset(self, monkeypatch): + monkeypatch.delenv("STORE_MODEL_IN_DB", raising=False) + with pytest.raises(HTTPException) as exc_info: + await start_oauth( + StartRequest(credential_name="c"), + user_api_key_dict=_admin(), + ) + assert exc_info.value.status_code == 400 + assert "STORE_MODEL_IN_DB" in exc_info.value.detail + @pytest.mark.asyncio async def test_rejects_when_session_cap_reached(self): from litellm.proxy.copilot_oauth_endpoints.endpoints import SESSIONS_MAX_SIZE