From ed01f7316b298df04a77db59efb24b8796ed9e09 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Thu, 13 Aug 2026 10:19:37 -0700 Subject: [PATCH] test(e2e): assert the model allow-list permits, not only denies Every case in TestAccessControl asserted that something was refused. A gateway that denied the allow-listed model too would have passed all of them, so the suite could not tell "denied correctly" from "broken outright". Adds the positive half: a key allow-listed for gemini-2.5-flash can call it and gets back a real completion rather than a 200-wrapped error. Also tightens the unknown-model case. It accepted any valid JSON, so a bare "{}" or even "null" satisfied it. It now requires the OpenAI-shaped error envelope with a message a client can actually surface, parsed through a typed model instead of json.loads. --- .../access_control/access_control_client.py | 20 +++++++++ .../access_control/test_access_control_e2e.py | 43 +++++++++++++------ 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/tests/e2e/access_control/access_control_client.py b/tests/e2e/access_control/access_control_client.py index e95ad1f57ce..7ace036f433 100644 --- a/tests/e2e/access_control/access_control_client.py +++ b/tests/e2e/access_control/access_control_client.py @@ -4,6 +4,8 @@ from __future__ import annotations from dataclasses import dataclass +from pydantic import BaseModel, ValidationError + from proxy_client import ProxyClient from e2e_http import StreamingResponse from models import ( @@ -19,6 +21,24 @@ MODEL_ACCESS_DENIED_MARKER = "key_model_access_denied" ROUTE_NOT_ALLOWED_MARKER = "not allowed to call this route" +class ApiErrorDetail(BaseModel): + message: str | None = None + type: str | None = None + code: str | int | None = None + + +class ApiErrorEnvelope(BaseModel): + error: ApiErrorDetail + + +def error_envelope(body: str) -> ApiErrorEnvelope | None: + """The OpenAI-shaped `{"error": {...}}` a client parses, or None if absent.""" + try: + return ApiErrorEnvelope.model_validate_json(body) + except ValidationError: + return None + + @dataclass(frozen=True, slots=True) class AccessControlClient: proxy: ProxyClient diff --git a/tests/e2e/access_control/test_access_control_e2e.py b/tests/e2e/access_control/test_access_control_e2e.py index e24b721d831..af7e9a099fd 100644 --- a/tests/e2e/access_control/test_access_control_e2e.py +++ b/tests/e2e/access_control/test_access_control_e2e.py @@ -13,19 +13,18 @@ management route). from __future__ import annotations -import json - import pytest from access_control_client import ( AccessControlClient, MODEL_ACCESS_DENIED_MARKER, ROUTE_NOT_ALLOWED_MARKER, + error_envelope, ) from e2e_config import unique_marker from e2e_http import Success, UnauthorizedError, UnknownApiError, unwrap from lifecycle import ResourceManager -from models import ChatBody, ChatMessage, LiteLLMParamsBody +from models import ChatBody, ChatMessage, ChatResponse, LiteLLMParamsBody from proxy_client import ProxyClient pytestmark = pytest.mark.e2e @@ -35,16 +34,28 @@ DISALLOWED_MODEL = "gpt-5.5" VIRTUAL_KEY_BACKEND = "anthropic/claude-haiku-4-5-20251001" -def _is_json(body: str) -> bool: - try: - json.loads(body) - return True - except ValueError: - return False - - - class TestAccessControl: + def test_allowed_model_is_permitted( + self, client: AccessControlClient, resources: ResourceManager + ) -> None: + """The allow-list's positive half. + + Without this, every other case in this class passes just as happily + against a gateway that denies the allowed model too, because they only + ever assert that something was refused. + """ + key = resources.key(models=[ALLOWED_MODEL]) + result = client.chat_status( + key, ALLOWED_MODEL, f"capital of France? {unique_marker()}" + ) + assert result.status_code == 200, ( + f"key allow-listed for {ALLOWED_MODEL!r} must be able to call it, got " + f"{result.status_code}: {result.body[:300]}" + ) + assert ChatResponse.model_validate_json(result.body).choices, ( + f"200 must carry a real completion, not an error envelope: {result.body[:300]}" + ) + def test_disallowed_model_is_denied_403( self, client: AccessControlClient, resources: ResourceManager ) -> None: @@ -85,7 +96,13 @@ class TestAccessControl: f"unknown model must be rejected 400 before forwarding, got " f"{result.status_code}: {result.body[:300]}" ) - assert _is_json(result.body), f"400 body must be valid JSON: {result.body[:300]}" + envelope = error_envelope(result.body) + assert envelope is not None, ( + f"400 body must be an OpenAI-shaped error envelope, got: {result.body[:300]}" + ) + assert envelope.error.message, ( + f"400 error must carry a message a client can surface: {result.body[:300]}" + ) class TestVirtualKeyAuth: