feat(proxy): configurable response headers and login-page hint#30792
Conversation
Adds a small ASGI middleware that sets standard response headers (X-Frame-Options, Content-Security-Policy frame-ancestors, X-Content-Type-Options) on proxy and UI responses. Strict-Transport-Security is optional and gated behind LITELLM_ENABLE_HSTS for HTTPS deployments. Values use setdefault so a route that sets its own header is preserved.
build_ui_login_form accepts a hide_default_credentials_hint parameter and google_login reads LITELLM_HIDE_DEFAULT_CREDENTIALS_HINT (or general_settings) so the legacy login page behaves consistently with the new UI. Also collapses a duplicated branch and removes an unused variable and module-level constant.
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR adds two operator-facing hardening features: a pure-ASGI
Confidence Score: 5/5Safe to merge — both features default to current behavior, the middleware is pure ASGI and tested, and the two issues raised in the previous review are fully addressed. Both changes are purely additive and opt-in. The duplicate branch collapse in google_login and fallback_login is a pure simplification with no behavioral difference. The test suite is comprehensive and properly isolated. No regressions are introduced. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/middleware/security_headers_middleware.py | New pure-ASGI middleware adding X-Frame-Options, CSP frame-ancestors, and X-Content-Type-Options to every HTTP response, with opt-in HSTS; uses setdefault to avoid overriding existing headers. |
| litellm/proxy/proxy_server.py | Adds SecurityHeadersMiddleware to the app and collapses the duplicate fallback_login branches while wiring up hide_default_credentials_hint; redirect_url is computed but unused (pre-existing dead code). |
| litellm/proxy/management_endpoints/ui_sso.py | Replaces the pre-built html_form constant with a call to build_ui_login_form, collapses the duplicate branch in google_login, and forwards hide_default_credentials_hint from env var and general_settings. |
| litellm/proxy/common_utils/html_forms/ui_login.py | Adds hide_default_credentials_hint parameter to build_ui_login_form, makes the info-box conditional, and removes the now-unnecessary module-level html_form constant. |
| tests/test_litellm/proxy/middleware/test_security_headers_middleware.py | New tests covering static headers, HSTS toggle, setdefault semantics, and that the middleware is pure ASGI (not BaseHTTPMiddleware). |
| tests/test_litellm/proxy/management_endpoints/test_ui_sso.py | Adds three async tests for the legacy login page via a helper that now correctly uses patch.dict to isolate env var mutations. |
| tests/test_litellm/proxy/proxy_server/test_routes_login_sso.py | Adds regression tests for /fallback/login showing and hiding the credentials hint; also includes minor formatting cleanups. |
| tests/test_litellm/proxy/common_utils/html_forms/test_ui_login.py | New unit tests verifying build_ui_login_form shows/hides the credentials hint independently of the deprecation banner flag. |
Reviews (2): Last reviewed commit: "fix(proxy/ui): apply credentials hint fl..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def _hsts_enabled() -> bool: | ||
| return os.getenv("LITELLM_ENABLE_HSTS", "false").strip().lower() == "true" |
There was a problem hiding this comment.
_hsts_enabled() calls os.getenv on every HTTP response. Since the env var is set once at process startup, this value is effectively constant at runtime. Caching it as a class attribute in __init__ (e.g. self._hsts_enabled = _hsts_enabled()) would avoid the repeated lookup and also make the intent clearer — HSTS is a deployment-time toggle, not something that changes mid-flight.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
The /fallback/login handler still rendered the default-credentials hint regardless of LITELLM_HIDE_DEFAULT_CREDENTIALS_HINT. Collapse its duplicate branch and forward the flag, matching google_login, so all login surfaces behave consistently. Adds regression tests for /fallback/login and makes the ui_sso test helper restore os.environ so env vars do not leak across tests.
|
@greptileai please re-review. Both items from the previous review are addressed in commit 551a0ad:
|
Adds the LITELLM_ENABLE_HSTS flag to the environment variables reference. This env var was introduced in BerriAI/litellm#30792; the litellm repo's documentation_test_env_keys check requires every env var to be listed here.
yucheng-berri
merged commit 5637b32
into
litellm_internal_staging
yucheng-berri
deleted the
litellm_ui_security_headers_and_credential_hint
branch
…AI#30792) * feat(proxy): add configurable response headers middleware Adds a small ASGI middleware that sets standard response headers (X-Frame-Options, Content-Security-Policy frame-ancestors, X-Content-Type-Options) on proxy and UI responses. Strict-Transport-Security is optional and gated behind LITELLM_ENABLE_HSTS for HTTPS deployments. Values use setdefault so a route that sets its own header is preserved. * feat(proxy/ui): make login page credentials hint configurable build_ui_login_form accepts a hide_default_credentials_hint parameter and google_login reads LITELLM_HIDE_DEFAULT_CREDENTIALS_HINT (or general_settings) so the legacy login page behaves consistently with the new UI. Also collapses a duplicated branch and removes an unused variable and module-level constant. * fix(proxy/ui): apply credentials hint flag on /fallback/login The /fallback/login handler still rendered the default-credentials hint regardless of LITELLM_HIDE_DEFAULT_CREDENTIALS_HINT. Collapse its duplicate branch and forward the flag, matching google_login, so all login surfaces behave consistently. Adds regression tests for /fallback/login and makes the ui_sso test helper restore os.environ so env vars do not leak across tests.
3 participants
Relevant issues
Linear ticket
Resolves LIT-3821
Changes
Two small admin UI and proxy improvements.
A new ASGI middleware sets a few standard response headers (X-Frame-Options, Content-Security-Policy frame-ancestors, X-Content-Type-Options) on proxy and UI responses. Strict-Transport-Security is optional via
LITELLM_ENABLE_HSTSfor deployments served over HTTPS. Existing behavior is preserved: header values use setdefault so a route that sets its own value is kept, and HSTS is off by default.The login page "default credentials" hint is now configurable. The legacy login page previously always showed it while the new UI could already hide it;
build_ui_login_formandgoogle_loginnow both honorLITELLM_HIDE_DEFAULT_CREDENTIALS_HINT(orgeneral_settings.hide_default_credentials_hint), so the two login surfaces behave consistently. This also includes a small cleanup of the login handler that collapses a duplicated branch and removes an unused variable and an unused module-level constant.Both settings default to existing behavior, so nothing changes unless an operator opts in.
Pre-Submission checklist
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewType
🆕 New Feature
Screenshots / Proof of Fix
Verified on a live proxy on localhost.
Response headers — iframe embedding behavior
The login page is loaded inside an iframe from a different page.
Before — the page renders when embedded:
After — the same embed now declines to render:
Login page credentials hint (
LITELLM_HIDE_DEFAULT_CREDENTIALS_HINT)Before — flag unset, the hint card is shown:
After — flag set, the hint card is hidden: