From 0ff468ced88d26b83eb5c497c9accc1e48d4691d Mon Sep 17 00:00:00 2001 From: sanyamk23 Date: Wed, 26 Aug 2026 01:32:50 +0530 Subject: [PATCH] fix(security): reject /health/drain when enabled with no token configured An enabled drain endpoint with no drain_endpoint_token or DRAIN_ENDPOINT_TOKEN configured accepted every caller, letting anyone with network access to the health port trigger a process-wide shutdown. _authorize_drain_request now fails closed: when no token is configured the endpoint returns 401 naming both configuration options. A configured token continues to be enforced via X-Drain-Token using secrets.compare_digest. The previously-green path (enabled + no token + no header) is covered by a regression test asserting 401 with no shutting-down side effect, plus an empty-string-token edge case. Helm chart comments and route docstrings updated to state the token requirement. --- helm/litellm-helm/values.yaml | 8 +-- .../health_endpoints/_health_endpoints.py | 20 +++++-- .../test_graceful_shutdown_endpoints.py | 55 ++++++++++--------- ui/litellm-dashboard/src/lib/http/schema.d.ts | 5 +- 4 files changed, 50 insertions(+), 38 deletions(-) diff --git a/helm/litellm-helm/values.yaml b/helm/litellm-helm/values.yaml index f8df98de102..c0325da5547 100644 --- a/helm/litellm-helm/values.yaml +++ b/helm/litellm-helm/values.yaml @@ -322,11 +322,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 cc49ae574cc..43cdf02babc 100644 --- a/litellm/proxy/health_endpoints/_health_endpoints.py +++ b/litellm/proxy/health_endpoints/_health_endpoints.py @@ -1593,13 +1593,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( @@ -1692,11 +1699,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 c77b6ad2e46..81d2f5577bc 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -6160,11 +6160,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