diff --git a/helm/litellm-helm/values.yaml b/helm/litellm-helm/values.yaml index 637be2322e3..5720f51d471 100644 --- a/helm/litellm-helm/values.yaml +++ b/helm/litellm-helm/values.yaml @@ -331,11 +331,11 @@ db: # # /health/drain is off by default; enable it with # general_settings.enable_drain_endpoint: true. The kubelet calls preStop -# hooks without proxy credentials, so when the health port is reachable from -# other pods (the common case) also set -# general_settings.drain_endpoint_token (or the DRAIN_ENDPOINT_TOKEN env +# hooks without proxy credentials, so the endpoint requires a shared secret: +# set general_settings.drain_endpoint_token (or the DRAIN_ENDPOINT_TOKEN env # var) and send the same value on the X-Drain-Token header from the hook. -# Calls missing/wrong the token get a 401 and have no side effect. +# Calls missing/wrong the token get a 401 and have no side effect; when no +# token is configured at all, every call is rejected with 401 (fail closed). # Example: # lifecycle: # preStop: diff --git a/litellm/proxy/health_endpoints/_health_endpoints.py b/litellm/proxy/health_endpoints/_health_endpoints.py index 8b57bdca2fe..566039beda2 100644 --- a/litellm/proxy/health_endpoints/_health_endpoints.py +++ b/litellm/proxy/health_endpoints/_health_endpoints.py @@ -1675,13 +1675,20 @@ def _authorize_drain_request(request: Request) -> None: """ Reject /health/drain calls that don't carry the configured X-Drain-Token. - When no token is configured the endpoint is treated as already opted-in - (the ``enable_drain_endpoint`` flag is the only gate). Comparison uses - ``secrets.compare_digest`` to avoid timing leaks. + Fails closed: an enabled drain endpoint with no token configured rejects + every call with 401 instead of accepting them, otherwise anything able to + reach the health port could trigger a process-wide shutdown. Comparison + uses ``secrets.compare_digest`` to avoid timing leaks. """ expected: Final = _drain_endpoint_token() if expected is None: - return + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail=( + "Drain endpoint requires a token: set general_settings." + "drain_endpoint_token or the DRAIN_ENDPOINT_TOKEN environment variable" + ), + ) supplied: Final = request.headers.get("x-drain-token") or "" if not secrets.compare_digest(supplied, expected): raise HTTPException( @@ -1777,11 +1784,12 @@ async def health_drain(request: Request): Because the kubelet calls preStop hooks without proxy credentials, the endpoint does not require ``user_api_key_auth``. To prevent any - pod-reachable caller from triggering shutdown, set + pod-reachable caller from triggering shutdown, a token is required: set ``general_settings.drain_endpoint_token`` (or the ``DRAIN_ENDPOINT_TOKEN`` env var) and supply the same value on the ``X-Drain-Token`` header from the preStop hook. Calls without the header (or with a wrong value) get a - 401 and have no side effect. + 401 and have no side effect. The endpoint fails closed: when enabled with + no token configured, every call is rejected with 401. When enabled, it marks the worker as shutting down (so /health/readiness and /health/liveliness immediately start returning 503, removing the pod diff --git a/tests/test_litellm/proxy/health_endpoints/test_graceful_shutdown_endpoints.py b/tests/test_litellm/proxy/health_endpoints/test_graceful_shutdown_endpoints.py index e20b54f28f5..08ae45f9a76 100644 --- a/tests/test_litellm/proxy/health_endpoints/test_graceful_shutdown_endpoints.py +++ b/tests/test_litellm/proxy/health_endpoints/test_graceful_shutdown_endpoints.py @@ -38,9 +38,8 @@ def client(): def enable_drain(monkeypatch): from litellm.proxy import proxy_server - monkeypatch.setattr( - proxy_server, "general_settings", {"enable_drain_endpoint": True} - ) + monkeypatch.delenv("DRAIN_ENDPOINT_TOKEN", raising=False) + monkeypatch.setattr(proxy_server, "general_settings", {"enable_drain_endpoint": True}) @pytest.fixture @@ -68,44 +67,48 @@ def test_drain_disabled_ignores_token_header(client, monkeypatch): token side-channel would silently enable the endpoint.""" from litellm.proxy import proxy_server - monkeypatch.setattr( - proxy_server, "general_settings", {"drain_endpoint_token": "secret-123"} - ) + monkeypatch.setattr(proxy_server, "general_settings", {"drain_endpoint_token": "secret-123"}) resp = client.get("/health/drain", headers={"X-Drain-Token": "secret-123"}) assert resp.status_code == 404 assert GracefulShutdownManager.is_shutting_down() is False -def test_drain_when_enabled_without_token_sets_shutting_down_and_returns_drained( - client, enable_drain -): - resp = client.get("/health/drain") - assert resp.status_code == 200 - body = resp.json() - assert body["status"] == "drained" - assert body["drained_requests"] == 0 - assert GracefulShutdownManager.is_shutting_down() is True - - -def test_drain_with_token_configured_rejects_missing_header( - client, enable_drain_with_token -): +def test_drain_when_enabled_without_token_returns_401_with_no_side_effect(client, enable_drain): + """Fails closed: an enabled drain endpoint with no configured token must + reject every call instead of accepting them (issue #35527), otherwise any + pod-reachable caller can trigger a process-wide shutdown.""" resp = client.get("/health/drain") assert resp.status_code == 401 assert GracefulShutdownManager.is_shutting_down() is False -def test_drain_with_token_configured_rejects_wrong_header( - client, enable_drain_with_token -): +def test_drain_when_enabled_with_empty_string_token_returns_401(client, monkeypatch): + from litellm.proxy import proxy_server + + monkeypatch.delenv("DRAIN_ENDPOINT_TOKEN", raising=False) + monkeypatch.setattr( + proxy_server, + "general_settings", + {"enable_drain_endpoint": True, "drain_endpoint_token": ""}, + ) + resp = client.get("/health/drain") + assert resp.status_code == 401 + assert GracefulShutdownManager.is_shutting_down() is False + + +def test_drain_with_token_configured_rejects_missing_header(client, enable_drain_with_token): + resp = client.get("/health/drain") + assert resp.status_code == 401 + assert GracefulShutdownManager.is_shutting_down() is False + + +def test_drain_with_token_configured_rejects_wrong_header(client, enable_drain_with_token): resp = client.get("/health/drain", headers={"X-Drain-Token": "wrong-value"}) assert resp.status_code == 401 assert GracefulShutdownManager.is_shutting_down() is False -def test_drain_with_token_configured_accepts_correct_header( - client, enable_drain_with_token -): +def test_drain_with_token_configured_accepts_correct_header(client, enable_drain_with_token): resp = client.get("/health/drain", headers={"X-Drain-Token": "secret-123"}) assert resp.status_code == 200 assert resp.json()["status"] == "drained" diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index 491c4fb6a44..d195cc06afa 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -6741,11 +6741,12 @@ export interface paths { * * Because the kubelet calls preStop hooks without proxy credentials, the * endpoint does not require ``user_api_key_auth``. To prevent any - * pod-reachable caller from triggering shutdown, set + * pod-reachable caller from triggering shutdown, a token is required: set * ``general_settings.drain_endpoint_token`` (or the ``DRAIN_ENDPOINT_TOKEN`` * env var) and supply the same value on the ``X-Drain-Token`` header from * the preStop hook. Calls without the header (or with a wrong value) get a - * 401 and have no side effect. + * 401 and have no side effect. The endpoint fails closed: when enabled with + * no token configured, every call is rejected with 401. * * When enabled, it marks the worker as shutting down (so /health/readiness * and /health/liveliness immediately start returning 503, removing the pod