mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
test: deflake fuzzy picker, breached-password HIBP, and MCP stdio timeout tests (rolling deflake 2026-09-22) (#42125)
* test(autoroute): wait for a valid fuzzy selection index and cancel the prompt on driver failure Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(autoroute): read the fuzzy selection through the public InquirerPy property Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(proxy): inject the HIBP client into change_password so the breached-password test never touches the network Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(mcp): only use the 200ms read timeout in the silent mode of the transport completion test Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(proxy): record HIBP requests so the ordering test asserts no lookup happened Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * ci(codeql): filter the weak-sensitive-data-hashing false positive on the HIBP k-anonymity lookup Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
19556952d9
commit
cddc53464e
6 changed files with 92 additions and 36 deletions
8
.github/workflows/codeql.yml
vendored
8
.github/workflows/codeql.yml
vendored
|
|
@ -67,12 +67,18 @@ jobs:
|
|||
# further up the stack are modified. The suppression is scoped to this one
|
||||
# file/rule pair via SARIF post-filtering so every other callsite of
|
||||
# py/weak-sensitive-data-hashing in the repository continues to be analyzed.
|
||||
- name: Filter SARIF (OCI sha256)
|
||||
# The same query fires on the HIBP k-anonymity lookup in
|
||||
# litellm/proxy/auth/password_policy.py, where the password's SHA-1 is only
|
||||
# a lookup key into the haveibeenpwned range API (the protocol mandates
|
||||
# SHA-1) and the digest itself never leaves the proxy beyond its first 5
|
||||
# characters.
|
||||
- name: Filter SARIF (OCI sha256, HIBP sha1)
|
||||
if: matrix.language == 'python'
|
||||
uses: advanced-security/filter-sarif@2da736ff05ef065cb2894ac6892e47b5eac2c3c0 # v1.1
|
||||
with:
|
||||
patterns: |
|
||||
-litellm/llms/oci/common_utils.py:py/weak-sensitive-data-hashing
|
||||
-litellm/proxy/auth/password_policy.py:py/weak-sensitive-data-hashing
|
||||
input: sarif-results/python.sarif
|
||||
output: sarif-results/python.sarif
|
||||
|
||||
|
|
|
|||
|
|
@ -107,7 +107,7 @@ def validate_password_policy(password: str, general_settings: Mapping[str, objec
|
|||
)
|
||||
|
||||
|
||||
def _hibp_client() -> AsyncHTTPHandler:
|
||||
def get_hibp_client() -> AsyncHTTPHandler:
|
||||
return get_async_httpx_client(
|
||||
llm_provider=httpxSpecialProvider.PasswordBreachCheck,
|
||||
params={"timeout": HIBP_TIMEOUT_SECONDS}, # mutable-ok: callee takes a bare dict (PEP 589)
|
||||
|
|
@ -155,7 +155,7 @@ async def is_password_breached(
|
|||
corpus, or HIBP is unreachable (fail open)."""
|
||||
if not is_breach_check_enabled(general_settings):
|
||||
return False
|
||||
return await _is_password_breached(password, client if client is not None else _hibp_client())
|
||||
return await _is_password_breached(password, client if client is not None else get_hibp_client())
|
||||
|
||||
|
||||
def breached_password_error() -> ProxyException:
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ from fastapi import APIRouter, Depends, HTTPException
|
|||
from pydantic import TypeAdapter
|
||||
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
from litellm.llms.custom_httpx.http_handler import AsyncHTTPHandler
|
||||
from litellm.proxy._types import (
|
||||
UI_TEAM_ID,
|
||||
ChangePasswordRequest,
|
||||
|
|
@ -24,7 +25,11 @@ from litellm.proxy._types import (
|
|||
UserAPIKeyAuth,
|
||||
)
|
||||
from litellm.proxy.auth.login_utils import PASSWORD_SESSION_METADATA
|
||||
from litellm.proxy.auth.password_policy import validate_password_not_breached, validate_password_policy
|
||||
from litellm.proxy.auth.password_policy import (
|
||||
get_hibp_client,
|
||||
validate_password_not_breached,
|
||||
validate_password_policy,
|
||||
)
|
||||
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
|
||||
from litellm.proxy.management_endpoints.session_endpoints import revoke_ui_session_keys
|
||||
from litellm.proxy.management_helpers.audit_logs import create_object_audit_log
|
||||
|
|
@ -71,6 +76,7 @@ def _user_table(
|
|||
async def change_password(
|
||||
data: ChangePasswordRequest,
|
||||
user_api_key_dict: Annotated[UserAPIKeyAuth, Depends(user_api_key_auth)],
|
||||
hibp_client: Annotated[AsyncHTTPHandler, Depends(get_hibp_client)],
|
||||
) -> ChangePasswordResponse:
|
||||
"""
|
||||
Change the calling user's own password.
|
||||
|
|
@ -133,7 +139,7 @@ async def change_password(
|
|||
)
|
||||
|
||||
validate_password_policy(data.new_password, general_settings)
|
||||
await validate_password_not_breached(data.new_password, general_settings)
|
||||
await validate_password_not_breached(data.new_password, general_settings, hibp_client)
|
||||
|
||||
password_update: Final[prisma_types.LiteLLM_UserTableUpdateInput] = {
|
||||
"password": hash_password(data.new_password),
|
||||
|
|
|
|||
|
|
@ -1890,8 +1890,9 @@ async def test_transport_completion_and_normal_messages(transport: MCPTransport,
|
|||
from litellm.proxy._experimental.mcp_server.rest_endpoints import _connection_error_message
|
||||
|
||||
logging_callback: Final = AsyncMock()
|
||||
read_timeout: Final = 0.2 if mode == "silent" else 30
|
||||
client: Final = MCPClient(
|
||||
server_url="https://example.com/sse", transport_type=transport, timeout=0.2, logging_callback=logging_callback
|
||||
server_url="https://example.com/sse", transport_type=transport, timeout=read_timeout, logging_callback=logging_callback
|
||||
)
|
||||
|
||||
async def operation(session: ClientSession) -> CallToolResult:
|
||||
|
|
@ -1909,7 +1910,7 @@ async def test_transport_completion_and_normal_messages(transport: MCPTransport,
|
|||
with pytest.raises(MCPError) as caught:
|
||||
await asyncio.wait_for(pending, timeout=3)
|
||||
if mode == "closed":
|
||||
assert "connection was closed" in _connection_error_message(caught.value, client.server_url, 0.2)
|
||||
assert "connection was closed" in _connection_error_message(caught.value, client.server_url, read_timeout)
|
||||
else:
|
||||
assert isinstance(as_mcp_read_timeout(caught.value), TimeoutError)
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
import asyncio
|
||||
import contextlib
|
||||
import contextvars
|
||||
from typing import Any, Dict, List, Optional, Tuple
|
||||
from unittest.mock import patch
|
||||
|
||||
|
|
@ -288,9 +290,12 @@ def _highlighted_choice(session: AppSession) -> Optional[str]:
|
|||
if session.app is None:
|
||||
return None
|
||||
controls = [c for c in session.app.layout.find_all_controls() if isinstance(c, InquirerPyFuzzyControl)]
|
||||
if not controls or controls[0].choice_count == 0:
|
||||
if not controls:
|
||||
return None
|
||||
try:
|
||||
return controls[0].selection["name"]
|
||||
except IndexError:
|
||||
return None
|
||||
return controls[0].selection["name"]
|
||||
|
||||
|
||||
async def _wait_until_highlighted(session: AppSession, name: str) -> None:
|
||||
|
|
@ -309,21 +314,35 @@ def _drive_fuzzy_pick(
|
|||
) -> List[str]:
|
||||
"""Drives the real InquirerPy fuzzy prompt through prompt_toolkit's own test input/output,
|
||||
exercising the actual widget (filtering, tab-to-toggle, enter-to-confirm) rather than mocking
|
||||
it away. asyncio.to_thread propagates the create_app_session context into the worker thread
|
||||
running _fuzzy_pick's synchronous .execute() call. Each key event names the choice the widget
|
||||
must highlight before the next key is sent (None sends the next key immediately)."""
|
||||
it away. The worker thread running _fuzzy_pick's synchronous .execute() call inherits the
|
||||
create_app_session context. Each key event names the choice the widget must highlight before
|
||||
the next key is sent (None sends the next key immediately). The widget swaps its filtered list
|
||||
before it clamps the highlight index on the next redraw, so the poller only reads a name once
|
||||
the index is in range. If driving the widget fails, ctrl-c ends the prompt so the worker thread
|
||||
exits and the failure surfaces instead of hanging the event loop shutdown."""
|
||||
|
||||
async def _run() -> List[str]:
|
||||
with create_pipe_input() as pipe_input:
|
||||
with create_app_session(input=pipe_input, output=DummyOutput()) as session:
|
||||
task = asyncio.ensure_future(
|
||||
asyncio.to_thread(wizard_module._fuzzy_pick, models, prompt_label, multiselect)
|
||||
prompt = asyncio.get_running_loop().run_in_executor(
|
||||
None,
|
||||
contextvars.copy_context().run,
|
||||
wizard_module._fuzzy_pick,
|
||||
models,
|
||||
prompt_label,
|
||||
multiselect,
|
||||
)
|
||||
for text, highlighted in key_events:
|
||||
pipe_input.send_text(text)
|
||||
if highlighted is not None:
|
||||
await _wait_until_highlighted(session, highlighted)
|
||||
return await task
|
||||
try:
|
||||
for text, highlighted in key_events:
|
||||
pipe_input.send_text(text)
|
||||
if highlighted is not None:
|
||||
await _wait_until_highlighted(session, highlighted)
|
||||
except BaseException:
|
||||
pipe_input.send_text("\x03")
|
||||
with contextlib.suppress(BaseException):
|
||||
await prompt
|
||||
raise
|
||||
return await prompt
|
||||
|
||||
return asyncio.run(_run())
|
||||
|
||||
|
|
|
|||
|
|
@ -1,17 +1,19 @@
|
|||
"""
|
||||
Tests for POST /user/password/change (litellm/proxy/management_endpoints/password_endpoints.py).
|
||||
|
||||
HIBP traffic is intercepted with respx; no test here touches the network.
|
||||
HIBP is served by an AsyncHTTPHandler wrapping an httpx.MockTransport that is
|
||||
injected straight into change_password; no test here touches the network.
|
||||
"""
|
||||
|
||||
import hashlib
|
||||
from typing import Final
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
import respx
|
||||
from fastapi import HTTPException
|
||||
|
||||
from litellm.llms.custom_httpx.http_handler import AsyncHTTPHandler
|
||||
from litellm.proxy._types import UI_TEAM_ID, LitellmTableNames, ProxyErrorTypes, ProxyException, UserAPIKeyAuth
|
||||
from litellm.proxy.auth.login_utils import PASSWORD_SESSION_METADATA
|
||||
from litellm.proxy.management_endpoints.password_endpoints import change_password
|
||||
|
|
@ -49,15 +51,29 @@ def _virtual_key_caller() -> UserAPIKeyAuth:
|
|||
return UserAPIKeyAuth(user_id="user-123", team_id="team-abc", metadata=dict(PASSWORD_SESSION_METADATA))
|
||||
|
||||
|
||||
def _hibp_url_for(password: str) -> str:
|
||||
sha1 = hashlib.sha1(password.encode(), usedforsecurity=False).hexdigest().upper()
|
||||
return f"https://api.pwnedpasswords.com/range/{sha1[:5]}"
|
||||
|
||||
|
||||
def _hibp_suffix_for(password: str) -> str:
|
||||
return hashlib.sha1(password.encode(), usedforsecurity=False).hexdigest().upper()[5:]
|
||||
|
||||
|
||||
def _hibp_client_returning(body: str) -> AsyncHTTPHandler:
|
||||
return AsyncHTTPHandler(transport=httpx.MockTransport(lambda request: httpx.Response(200, text=body)))
|
||||
|
||||
|
||||
def _hibp_client_never_called() -> AsyncHTTPHandler:
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
raise AssertionError(f"unexpected HIBP call to {request.url}")
|
||||
|
||||
return AsyncHTTPHandler(transport=httpx.MockTransport(handler))
|
||||
|
||||
|
||||
def _hibp_client_recording(calls: list[httpx.Request]) -> AsyncHTTPHandler:
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
calls.append(request)
|
||||
return httpx.Response(200, text="")
|
||||
|
||||
return AsyncHTTPHandler(transport=httpx.MockTransport(handler))
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_change_password_success_writes_new_scrypt_hash():
|
||||
from litellm.proxy._types import ChangePasswordRequest
|
||||
|
|
@ -75,6 +91,7 @@ async def test_change_password_success_writes_new_scrypt_hash():
|
|||
response = await change_password(
|
||||
data=ChangePasswordRequest(current_password=CURRENT_PASSWORD, new_password=NEW_PASSWORD),
|
||||
user_api_key_dict=_caller(),
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
assert response.user_id == "user-123"
|
||||
|
|
@ -107,6 +124,7 @@ async def test_change_password_rejects_wrong_current_password():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password="not-the-password", new_password=NEW_PASSWORD),
|
||||
user_api_key_dict=_caller(),
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 400
|
||||
|
|
@ -132,6 +150,7 @@ async def test_change_password_rejects_unchanged_password():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password=CURRENT_PASSWORD, new_password=CURRENT_PASSWORD),
|
||||
user_api_key_dict=_caller(),
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 400
|
||||
|
|
@ -167,6 +186,7 @@ async def test_change_password_rejects_non_password_login_session(caller: UserAP
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password=CURRENT_PASSWORD, new_password=NEW_PASSWORD),
|
||||
user_api_key_dict=caller,
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 403
|
||||
|
|
@ -193,6 +213,7 @@ async def test_change_password_rejects_session_without_user():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password=CURRENT_PASSWORD, new_password=NEW_PASSWORD),
|
||||
user_api_key_dict=_caller(user_id=None),
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 400
|
||||
|
|
@ -219,6 +240,7 @@ async def test_change_password_rejects_account_without_password():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password=CURRENT_PASSWORD, new_password=NEW_PASSWORD),
|
||||
user_api_key_dict=_caller(),
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 400
|
||||
|
|
@ -244,6 +266,7 @@ async def test_change_password_enforces_min_length():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password=CURRENT_PASSWORD, new_password="Short1!"),
|
||||
user_api_key_dict=_caller(),
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
assert exc_info.value.code == "400"
|
||||
|
|
@ -254,15 +277,11 @@ async def test_change_password_enforces_min_length():
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@respx.mock
|
||||
async def test_change_password_rejects_breached_password():
|
||||
"""With the default policy, the new password is screened against HIBP."""
|
||||
from litellm.proxy._types import ChangePasswordRequest
|
||||
|
||||
breached_password = "Password123!"
|
||||
respx.get(_hibp_url_for(breached_password)).mock(
|
||||
return_value=httpx.Response(200, text=f"{_hibp_suffix_for(breached_password)}:1")
|
||||
)
|
||||
prisma = _make_prisma(_make_user_row(hash_password(CURRENT_PASSWORD)))
|
||||
|
||||
with (
|
||||
|
|
@ -277,6 +296,7 @@ async def test_change_password_rejects_breached_password():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password=CURRENT_PASSWORD, new_password=breached_password),
|
||||
user_api_key_dict=_caller(),
|
||||
hibp_client=_hibp_client_returning(f"{_hibp_suffix_for(breached_password)}:1"),
|
||||
)
|
||||
|
||||
assert exc_info.value.code == "400"
|
||||
|
|
@ -287,16 +307,14 @@ async def test_change_password_rejects_breached_password():
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@respx.mock
|
||||
async def test_change_password_verifies_current_password_before_hibp_lookup():
|
||||
"""A caller who fails current-password verification must not trigger any
|
||||
HIBP traffic. The HIBP check fails open on errors, so an unmocked lookup
|
||||
could not prove ordering; instead the route is registered and asserted
|
||||
uncalled."""
|
||||
HIBP traffic: the injected client records each request it serves and the
|
||||
test asserts none were made."""
|
||||
from litellm.proxy._types import ChangePasswordRequest
|
||||
|
||||
hibp_route = respx.get(_hibp_url_for(NEW_PASSWORD)).mock(return_value=httpx.Response(200, text=""))
|
||||
prisma = _make_prisma(_make_user_row(hash_password(CURRENT_PASSWORD)))
|
||||
hibp_calls: Final[list[httpx.Request]] = []
|
||||
|
||||
with (
|
||||
patch( # test-quality-ok: change_password reads proxy_server module globals; no injection seam
|
||||
|
|
@ -310,11 +328,12 @@ async def test_change_password_verifies_current_password_before_hibp_lookup():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password="not-the-password", new_password=NEW_PASSWORD),
|
||||
user_api_key_dict=_caller(),
|
||||
hibp_client=_hibp_client_recording(hibp_calls),
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 400
|
||||
assert "Current password is incorrect" in exc_info.value.detail["error"]
|
||||
assert not hibp_route.called
|
||||
assert hibp_calls == []
|
||||
prisma.db.litellm_usertable.update.assert_not_called()
|
||||
|
||||
|
||||
|
|
@ -341,6 +360,7 @@ async def test_change_password_success_emits_redacted_audit_log():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password=CURRENT_PASSWORD, new_password=NEW_PASSWORD),
|
||||
user_api_key_dict=_caller(),
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
audit_mock.assert_awaited_once()
|
||||
|
|
@ -375,6 +395,7 @@ async def test_change_password_failure_emits_no_audit_log():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password="not-the-password", new_password=NEW_PASSWORD),
|
||||
user_api_key_dict=_caller(),
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
audit_mock.assert_not_awaited()
|
||||
|
|
@ -411,6 +432,7 @@ async def test_change_password_revokes_other_sessions_keeping_callers():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password=CURRENT_PASSWORD, new_password=NEW_PASSWORD),
|
||||
user_api_key_dict=caller,
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
revoke_mock.assert_awaited_once()
|
||||
|
|
@ -442,6 +464,7 @@ async def test_change_password_failure_revokes_no_sessions():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password="not-the-password", new_password=NEW_PASSWORD),
|
||||
user_api_key_dict=_caller(),
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
revoke_mock.assert_not_awaited()
|
||||
|
|
@ -463,6 +486,7 @@ async def test_change_password_requires_db():
|
|||
await change_password(
|
||||
data=ChangePasswordRequest(current_password=CURRENT_PASSWORD, new_password=NEW_PASSWORD),
|
||||
user_api_key_dict=_caller(),
|
||||
hibp_client=_hibp_client_never_called(),
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 500
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue