From 2c874a7f3cb25a384336cb4061cebdce51457866 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Tue, 17 Mar 2026 23:00:05 -0700 Subject: [PATCH] fix: prevent body 'key' field from overriding Authorization header in auth On /key/block, /key/unblock, and /key/update, the request body 'key' field could contaminate the api_key Security dependency, causing the auth layer to authenticate against the target key instead of the caller's bearer token. This returned 401 for a nonexistent body key even when the Authorization header contained a valid master key. Added a guard in user_api_key_auth that re-reads the Authorization header directly from the request, ensuring the header is always the authoritative source for authentication. Co-Authored-By: Claude Opus 4.6 (1M context) --- litellm/proxy/auth/user_api_key_auth.py | 11 +++ .../test_key_management_endpoints.py | 85 +++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/litellm/proxy/auth/user_api_key_auth.py b/litellm/proxy/auth/user_api_key_auth.py index 376048e7a13..724184282fc 100644 --- a/litellm/proxy/auth/user_api_key_auth.py +++ b/litellm/proxy/auth/user_api_key_auth.py @@ -1535,6 +1535,17 @@ async def user_api_key_auth( Parent function to authenticate user api key / jwt token. """ + # Guard: re-read the Authorization header directly from the request. + # On endpoints whose request body contains a "key" field (e.g. /key/block, + # /key/unblock, /key/update), the body value can contaminate the api_key + # Security dependency, causing the auth layer to authenticate against the + # target key instead of the caller's bearer token. + _raw_auth_header = request.headers.get( + SpecialHeaders.openai_authorization.value + ) + if _raw_auth_header is not None: + api_key = _raw_auth_header + request_data = await _read_request_body(request=request) request_data = populate_request_with_path_params( request_data=request_data, request=request diff --git a/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py index cfc16808afb..f1fb973a797 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py @@ -7185,3 +7185,88 @@ def test_update_key_request_has_organization_id(): # Also verify it defaults to None data_no_org = UpdateKeyRequest(key="sk-test-key") assert data_no_org.organization_id is None + + +@pytest.mark.asyncio +async def test_body_key_does_not_override_authorization_header(monkeypatch): + """ + Verify that the request body 'key' field (used by /key/block, /key/unblock, + /key/update) does not contaminate the api_key used for authentication. + + Regression test: previously, sending a request like + POST /key/block + Authorization: Bearer + Body: {"key": "sk-does-not-exist"} + would authenticate against "sk-does-not-exist" instead of the master key, + returning a 401 for a non-existent key even though the caller had a valid + master key in the Authorization header. + """ + from litellm.proxy.auth.user_api_key_auth import user_api_key_auth + + master_key = "sk-master-test-key-1234" + + # Mock proxy_server globals + monkeypatch.setattr("litellm.proxy.proxy_server.master_key", master_key) + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", MagicMock()) + monkeypatch.setattr( + "litellm.proxy.proxy_server.general_settings", {} + ) + monkeypatch.setattr("litellm.proxy.proxy_server.jwt_handler", MagicMock()) + monkeypatch.setattr( + "litellm.proxy.proxy_server.litellm_proxy_admin_name", + "default_user_id", + ) + monkeypatch.setattr("litellm.proxy.proxy_server.llm_model_list", None) + monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", None) + monkeypatch.setattr("litellm.proxy.proxy_server.user_custom_auth", None) + monkeypatch.setattr( + "litellm.proxy.proxy_server.open_telemetry_logger", None + ) + monkeypatch.setattr( + "litellm.proxy.proxy_server.model_max_budget_limiter", MagicMock() + ) + mock_user_api_key_cache = MagicMock() + mock_user_api_key_cache.async_get_cache = AsyncMock(return_value=None) + mock_user_api_key_cache.get_cache = MagicMock(return_value=None) + monkeypatch.setattr( + "litellm.proxy.proxy_server.user_api_key_cache", + mock_user_api_key_cache, + ) + monkeypatch.setattr( + "litellm.proxy.proxy_server.proxy_logging_obj", MagicMock() + ) + + # Build a mock request that simulates: + # POST /key/block + # Authorization: Bearer + # Body: {"key": "sk-does-not-exist"} + mock_request = MagicMock() + mock_request.headers = { + "Authorization": f"Bearer {master_key}", + "Content-Type": "application/json", + } + mock_request.url.path = "/key/block" + mock_request.method = "POST" + mock_request.query_params = {} + mock_request.scope = { + "type": "http", + "path": "/key/block", + # Simulate a pre-parsed body with a 'key' field that differs from the master key + "parsed_body": ( + ("key",), + {"key": "sk-does-not-exist"}, + ), + } + mock_request.state = MagicMock() + + # Call user_api_key_auth the way FastAPI does. If the body 'key' field + # contaminates the api_key parameter, pass it explicitly to prove the + # guard works. + result = await user_api_key_auth( + request=mock_request, + api_key="sk-does-not-exist", # Simulates the contaminated value + ) + + # Auth should succeed — the guard re-reads the Authorization header + assert result is not None + assert result.user_role == LitellmUserRoles.PROXY_ADMIN