From bc80a712b74ac5672350c92bab482f55e7f40658 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sun, 11 Oct 2026 05:38:47 +0200 Subject: [PATCH] fix(proxy): fail closed, zero-trust proxies, in-stack guard ordering Review-response pass on the guard middleware: - Enabled-but-missing fastapi-guard now rejects with 503 (OpenAI-error envelope, logged once) instead of forwarding unscreened. - Trusted proxies default to none: X-Forwarded-For is honored only from explicitly listed ingress addresses, so the header cannot spoof allowlists, limits, or bans. - Attach GuardMiddleware inside CORS/SecurityHeaders so guard rejections keep their headers. - Retype the optional-dep seam (Callable[..., object]/ASGIApp) and drop the type comment; config built as one literal so conditional inserts cannot re-trip the argument-type ceiling. - Tests: fail-closed 503 + log-once, CORS/security-header pin on rejections, untrusted XFF ignored. --- litellm/proxy/middleware/guard_middleware.py | 132 +++++++++++------- litellm/proxy/proxy_server.py | 22 +-- .../proxy/middleware/test_guard_middleware.py | 91 ++++++++++-- 3 files changed, 177 insertions(+), 68 deletions(-) diff --git a/litellm/proxy/middleware/guard_middleware.py b/litellm/proxy/middleware/guard_middleware.py index 46ba276ea71..3012cdf9168 100644 --- a/litellm/proxy/middleware/guard_middleware.py +++ b/litellm/proxy/middleware/guard_middleware.py @@ -17,30 +17,41 @@ Design constraints honored here: matching the middleware house style and its streaming-preservation test. - Opt-in via ``LITELLM_GUARD_ENABLED``; when unset, the wrapper delegates straight through with no guard import and no config work. -- fastapi-guard is NOT added to pyproject dependencies by this module: the - import is lazy and raises an actionable error when the flag is enabled - without the package installed. (Fork PRs may not modify dependency files - per the guard-fork-dependencies CI policy, so the dependency addition is +- Fail-closed when enabled: the proxy does not depend on fastapi-guard (it + is deliberately absent from pyproject), so the import is lazy. If the + flag is set and the package is missing, every screened request is + rejected with a 503 in litellm's OpenAI-error envelope and an error is + logged once: an operator who explicitly enabled the guard must not get + unscreened passthrough. (Fork PRs may not modify dependency files per + the guard-fork-dependencies CI policy, so the dependency addition is left to the maintainers.) -- Rejections are built in litellm's OpenAI-error JSON shape (the - ``AdmissionControlMiddleware`` model), so clients see the same error - envelope for 403/429 as for other proxy rejections. +- Zero-trust client identity by default: ``X-Forwarded-For`` is honored + only from proxies explicitly listed in ``LITELLM_GUARD_TRUSTED_PROXIES`` + (guard ignores forwarded headers from unlisted peers), so a caller + cannot spoof its way past allowlists, limits, or bans by setting the + header. Deployments behind a load balancer must list their ingress + addresses there. +- Rejections guard itself builds use guard's response shapes; the one + rejection this wrapper builds (guard unavailable) uses litellm's + OpenAI-error JSON shape (the ``AdmissionControlMiddleware`` model), so + clients see a familiar envelope. """ from __future__ import annotations import importlib import os +from collections.abc import Callable from typing import Final +from starlette.responses import JSONResponse from starlette.types import ASGIApp, Receive, Scope, Send from litellm._logging import verbose_proxy_logger GUARD_ENABLED_ENV: Final = "LITELLM_GUARD_ENABLED" GUARD_EXCLUDED_PATHS_ENV: Final = "LITELLM_GUARD_EXCLUDED_PATHS" -DEFAULT_EXCLUDED_PATHS: Final = "/health,/health/liveliness,/health/readiness,/metrics" -DEFAULT_TRUSTED_PROXIES: Final = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16" +DEFAULT_EXCLUDED_PATHS: Final = ("/health", "/health/liveliness", "/health/readiness", "/metrics") def _env_str(name: str, default: str = "") -> str: @@ -70,12 +81,29 @@ def guard_enabled() -> bool: return _env_bool(GUARD_ENABLED_ENV) -def _build_guard_config(): # noqa: ANN202 # return type is from the lazily imported, untyped optional dependency - # type: () -> object - """Build the guard config from the LITELLM_GUARD_* environment variables.""" - guard_module: Final = importlib.import_module("guard") - security_config_cls: Final = guard_module.SecurityConfig +def _build_guard_config() -> object: + """Build the guard config from the LITELLM_GUARD_* environment variables. + Returns guard's ``SecurityConfig``. The ``object`` return and the + dynamic import are the seam: fastapi-guard is an optional dependency + that is deliberately absent from the environments litellm type-checks, + so the class can only be reached through a runtime lookup. + """ + guard_module: Final = importlib.import_module("guard") + # fastapi-guard is an optional dependency that is deliberately absent + # from the environments litellm type-checks, so attributes read off the + # dynamically imported module are Any; the Callable annotation re-types + # the seam so no Any escapes this line. + security_config_cls: Final[Callable[..., object]] = guard_module.SecurityConfig # pyright: ignore[reportAny] # optional dep absent from the typecheck venv; seam re-typed below + + redis_url_env: Final = _env_str("LITELLM_GUARD_REDIS_URL") + ipinfo_token_env: Final = _env_str("LITELLM_GUARD_IPINFO_TOKEN") + # One literal, no conditional inserts: optional list-valued features are + # passed as their empty/None form, which SecurityConfig treats the same + # as unset. In-memory state unless a Redis URL is configured: never + # implicitly depend on a Redis server being reachable. The proxy + # defaults to a single uvicorn worker; multi-worker deployments should + # set LITELLM_GUARD_REDIS_URL so counters and bans are shared. kwargs: Final = { "enable_rate_limiting": True, "rate_limit": _env_int("LITELLM_GUARD_RATE_LIMIT", 100), @@ -84,39 +112,47 @@ def _build_guard_config(): # noqa: ANN202 # return type is from the lazily imp "auto_ban_threshold": _env_int("LITELLM_GUARD_AUTO_BAN_THRESHOLD", 10), "auto_ban_duration": _env_int("LITELLM_GUARD_AUTO_BAN_DURATION", 300), "enable_penetration_detection": True, - # In-memory state unless a Redis URL is configured: never implicitly - # depend on a Redis server being reachable. The proxy defaults to a - # single uvicorn worker; multi-worker deployments should set - # LITELLM_GUARD_REDIS_URL so counters and bans are shared. - "enable_redis": False, + "enable_redis": bool(redis_url_env), + "redis_url": redis_url_env or None, + "redis_prefix": "litellm_guard:", "passive_mode": _env_bool("LITELLM_GUARD_PASSIVE_MODE"), "blacklist": _csv(_env_str("LITELLM_GUARD_BLOCKED_IPS")), + "whitelist": _csv(_env_str("LITELLM_GUARD_ALLOWED_IPS")) or None, "blocked_user_agents": list(_csv(_env_str("LITELLM_GUARD_BLOCKED_USER_AGENTS"))), - "trusted_proxies": _csv(_env_str("LITELLM_GUARD_TRUSTED_PROXIES")) or _csv(DEFAULT_TRUSTED_PROXIES), + "blocked_countries": frozenset(_csv(_env_str("LITELLM_GUARD_BLOCKED_COUNTRIES"))), + "whitelist_countries": frozenset(_csv(_env_str("LITELLM_GUARD_ALLOWED_COUNTRIES"))), + "block_cloud_providers": frozenset(_csv(_env_str("LITELLM_GUARD_BLOCK_CLOUD_PROVIDERS"))) or None, + # Zero-trust default: no proxies are trusted, so forwarded headers + # are ignored and the connecting peer is the client. List real + # ingress addresses to enable X-Forwarded-For resolution. + "trusted_proxies": _csv(_env_str("LITELLM_GUARD_TRUSTED_PROXIES")), "trusted_proxy_depth": _env_int("LITELLM_GUARD_TRUSTED_PROXY_DEPTH", 1), - "exclude_paths": list(_csv(_env_str(GUARD_EXCLUDED_PATHS_ENV)) or DEFAULT_EXCLUDED_PATHS.split(",")), + "exclude_paths": list(_csv(_env_str(GUARD_EXCLUDED_PATHS_ENV)) or DEFAULT_EXCLUDED_PATHS), + "ipinfo_token": ipinfo_token_env or None, } - - if allowed_ips := _csv(_env_str("LITELLM_GUARD_ALLOWED_IPS")): - kwargs["whitelist"] = allowed_ips - if blocked_countries := _csv(_env_str("LITELLM_GUARD_BLOCKED_COUNTRIES")): - kwargs["blocked_countries"] = frozenset(blocked_countries) - if allowed_countries := _csv(_env_str("LITELLM_GUARD_ALLOWED_COUNTRIES")): - kwargs["whitelist_countries"] = frozenset(allowed_countries) - if cloud_providers := _csv(_env_str("LITELLM_GUARD_BLOCK_CLOUD_PROVIDERS")): - kwargs["block_cloud_providers"] = frozenset(cloud_providers) - - if redis_url := _env_str("LITELLM_GUARD_REDIS_URL"): - kwargs["enable_redis"] = True - kwargs["redis_url"] = redis_url - kwargs["redis_prefix"] = "litellm_guard:" - - if ipinfo_token := _env_str("LITELLM_GUARD_IPINFO_TOKEN"): - kwargs["ipinfo_token"] = ipinfo_token - return security_config_cls(**kwargs) +def _guard_unavailable_response() -> JSONResponse: + """Fail-closed rejection for when the guard is enabled but cannot be + initialized (litellm's OpenAI-error envelope, the + ``AdmissionControlMiddleware`` model).""" + return JSONResponse( + status_code=503, + content={ + "error": { + "message": ( + "LITELLM_GUARD_ENABLED is set but the fastapi-guard package " + "is not installed. Install it with: pip install fastapi-guard. " + "Requests are rejected until then." + ), + "type": "guard_unavailable_error", + "code": "503", + } + }, + ) + + class GuardMiddleware: """Pure-ASGI fastapi-guard wrapper. @@ -133,22 +169,23 @@ class GuardMiddleware: def _ensure_guard(self) -> ASGIApp | None: """Build and return the guard ASGI app, or None when the package is - missing (logged loudly; the flag-on misconfiguration must stay - visible).""" + missing (logged loudly once; the caller then fails closed).""" if self._guard_asgi is not None: return self._guard_asgi if self._guard_failed: return None try: guard_module: Final = importlib.import_module("guard") - middleware_cls: Final = guard_module.SecurityMiddleware + # Same optional-dep seam as _build_guard_config: the attribute + # is Any; the Callable annotation keeps it from escaping. + middleware_cls: Final[Callable[..., ASGIApp]] = guard_module.SecurityMiddleware # pyright: ignore[reportAny] # optional dep absent from the typecheck venv; seam re-typed below except ImportError as exc: self._guard_failed = True # Log loudly once; security misconfiguration must be visible. verbose_proxy_logger.error( "LITELLM_GUARD_ENABLED is set but fastapi-guard is not " "installed: %s. Install it with: pip install fastapi-guard. " - "Guard is DISABLED until then.", + "Requests are REJECTED until then.", exc, ) return None @@ -163,10 +200,11 @@ class GuardMiddleware: guard_asgi: Final = self._ensure_guard() if guard_asgi is None: - # Flag on, package missing: fail open with a loud log rather - # than taking the proxy down (matches the disabled path; the - # misconfiguration is logged at startup of the first request). - await self.app(scope, receive, send) + # Flag on, package missing: fail closed with 503 rather than + # forward unscreened. The operator explicitly enabled the + # guard; silently honoring the opposite of that intent would + # make the flag a no-op. The misconfiguration is logged once. + await _guard_unavailable_response()(scope, receive, send) return await guard_asgi(scope, receive, send) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 61e07f74d87..775591e6c71 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -2563,6 +2563,18 @@ current_dir = os.path.dirname(os.path.abspath(__file__)) # app.mount("/ui", StaticFiles(directory=ui_path, html=True), name="ui") +# Optional fastapi-guard security middleware (off unless LITELLM_GUARD_ENABLED +# is set). Added before CORSMiddleware and SecurityHeadersMiddleware so it +# nests *inside* both (Starlette makes the last-added middleware outermost) +# and guard's own 403/429 rejections still leave with CORS and security +# headers. It stays outside the auth dependencies, so screened-and-blocked +# requests never reach key validation. See middleware/guard_middleware.py. +from litellm.proxy.middleware.guard_middleware import ( # noqa: E402 # mid-file attach: the middleware stack freezes once the lifespan runs + GuardMiddleware, +) + +app.add_middleware(GuardMiddleware) + app.add_middleware( CORSMiddleware, allow_origins=origins, @@ -2610,16 +2622,6 @@ app.add_middleware(InFlightRequestsMiddleware) app.add_middleware(SecurityHeadersMiddleware) app.add_middleware(GZipBufferedResponseMiddleware) -# Optional fastapi-guard security middleware (off unless LITELLM_GUARD_ENABLED -# is set). Added inside SecurityHeaders so screened responses are still -# headered, and outside the auth dependencies so screened-and-blocked -# requests never reach key validation. See middleware/guard_middleware.py. -from litellm.proxy.middleware.guard_middleware import ( # noqa: E402 # mid-file attach: the middleware stack freezes once the lifespan runs - GuardMiddleware, -) - -app.add_middleware(GuardMiddleware) - def mount_swagger_ui(): swagger_directory: Final = os.path.join(current_dir, "swagger") diff --git a/tests/unit/proxy/middleware/test_guard_middleware.py b/tests/unit/proxy/middleware/test_guard_middleware.py index aa7d47040f3..95c1e3cb084 100644 --- a/tests/unit/proxy/middleware/test_guard_middleware.py +++ b/tests/unit/proxy/middleware/test_guard_middleware.py @@ -4,17 +4,19 @@ Unit tests for the optional fastapi-guard middleware wiring. Mirrors test_security_headers_middleware.py: minimal app, TestClient, monkeypatched env, default-off assertion, and the pure-ASGI house-style assertion. fastapi-guard is an optional dependency (not added to -pyproject by this change), so behavior tests skip when it is absent. +pyproject by this change), so behavior tests skip when it is absent; the +fail-closed path is exercised unconditionally via a poisoned sys.modules +entry. """ -import asyncio import itertools -import os +import sys from typing import Final import pytest from starlette.testclient import TestClient +from litellm._logging import verbose_proxy_logger from litellm.proxy.middleware.guard_middleware import ( GuardMiddleware, guard_enabled, @@ -63,6 +65,34 @@ def _build_app(): return app +def _build_stacked_app(): + """The proxy's nesting: guard innermost, then CORS, then security + headers outermost (Starlette makes the last-added middleware + outermost), so guard rejections leave with their headers.""" + from starlette.applications import Starlette + from starlette.middleware.cors import CORSMiddleware + from starlette.responses import JSONResponse + from starlette.routing import Route + + from litellm.proxy.middleware.security_headers_middleware import ( + SecurityHeadersMiddleware, + ) + + async def ping(request): + return JSONResponse({"ok": True}) + + app: Final = Starlette(routes=[Route("/ping", ping, methods=["GET"])]) + app.add_middleware(GuardMiddleware) + app.add_middleware( + CORSMiddleware, + allow_origins=["https://ok.example"], + allow_methods=["GET"], + allow_headers=["*"], + ) + app.add_middleware(SecurityHeadersMiddleware) + return app + + class _ClientIPInjector: """TestClient hardcodes scope["client"]; the middleware stack must see the test IP, so mutate the scope before it reaches the app.""" @@ -78,8 +108,8 @@ class _ClientIPInjector: await self._asgi_app(screened, receive, send) -def _client(client_ip): - return TestClient(_ClientIPInjector(_build_app(), client_ip)) +def _client(client_ip, asgi_app=None): + return TestClient(_ClientIPInjector(asgi_app or _build_app(), client_ip)) def test_disabled_by_default(monkeypatch): @@ -88,14 +118,21 @@ def test_disabled_by_default(monkeypatch): assert client.get("/ping").status_code == 200 -def test_fail_loud_without_package(monkeypatch): - if _GUARD_AVAILABLE: - pytest.skip("fastapi-guard is installed") +def test_enabled_without_package_fails_closed(monkeypatch): + """Flag on + package missing: every request is rejected with 503 and + the misconfiguration is logged exactly once. An operator who enabled + the guard must not get unscreened passthrough.""" + monkeypatch.setitem(sys.modules, "guard", None) monkeypatch.setenv("LITELLM_GUARD_ENABLED", "true") + error_messages: Final = [] + monkeypatch.setattr(verbose_proxy_logger, "error", lambda msg, *args: error_messages.append(msg)) client: Final = _client(_unique_ip()) - # Flag on, package missing: request still passes (logged loudly), but - # the guard attribute proves the wiring attempted the import. - assert client.get("/ping").status_code == 200 + first: Final = client.get("/ping") + second: Final = client.get("/ping") + assert first.status_code == 503 + assert second.status_code == 503 + assert first.json()["error"]["type"] == "guard_unavailable_error" + assert len(error_messages) == 1 @pytest.mark.skipif(not _GUARD_AVAILABLE, reason="fastapi-guard not installed") @@ -107,6 +144,37 @@ def test_blocked_ip_is_rejected(monkeypatch): assert client.get("/ping").status_code == 403 +@pytest.mark.skipif(not _GUARD_AVAILABLE, reason="fastapi-guard not installed") +def test_untrusted_forwarded_header_is_ignored(monkeypatch): + """No trusted proxies configured: X-Forwarded-For must not override the + connecting address, or a private-network caller could spoof its way + past allowlists, limits, and bans.""" + blocked: Final = "10.4.5.6" + monkeypatch.setenv("LITELLM_GUARD_ENABLED", "true") + monkeypatch.setenv("LITELLM_GUARD_BLOCKED_IPS", blocked) + with _client(blocked) as client: + rejected: Final = client.get("/ping", headers={"x-forwarded-for": "198.51.100.9"}) + assert rejected.status_code == 403 + + +@pytest.mark.skipif(not _GUARD_AVAILABLE, reason="fastapi-guard not installed") +def test_rejected_response_keeps_cors_and_security_headers(monkeypatch): + """Guard is nested inside the CORS and security-headers wrappers, so a + guard rejection carries their headers (readable cross-origin errors).""" + blocked: Final = _unique_ip() + monkeypatch.setenv("LITELLM_GUARD_ENABLED", "true") + monkeypatch.setenv("LITELLM_GUARD_BLOCKED_IPS", blocked) + with _client(blocked, _build_stacked_app()) as client: + rejected: Final = client.get("/ping", headers={"Origin": "https://ok.example"}) + assert rejected.status_code == 403 + # Both wrappers ran on the guard rejection. guard sets its own + # anti-framing header value and litellm's setdefault defers to it, so + # presence (not a specific value) is the property pinned here. + assert rejected.headers["x-frame-options"] in ("DENY", "SAMEORIGIN") + assert rejected.headers["content-security-policy"] == "frame-ancestors 'none'" + assert rejected.headers["access-control-allow-origin"] == "https://ok.example" + + @pytest.mark.skipif(not _GUARD_AVAILABLE, reason="fastapi-guard not installed") def test_rate_limit_returns_429(monkeypatch): client_ip: Final = _unique_ip() @@ -135,3 +203,4 @@ def test_redis_stays_off_without_config(monkeypatch): config: Final = _build_guard_config() assert config.enable_redis is False + assert config.redis_url is None