fix(ui): surface env-var-sourced theme and logging-callback settings - #34156
Conversation
|
|
Greptile SummaryThis PR makes the Admin UI reflect theme and callback settings supplied through process environment variables. The main changes are:
Confidence Score: 5/5This looks safe to merge.
|
| Filename | Overview |
|---|---|
| litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py | Adds theme environment fallback and filters env-derived values before returning them publicly. |
| litellm/proxy/common_utils/callback_utils.py | Falls back to process environment values when callback variables are absent from stored configuration. |
| tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py | Tests theme fallback, stored-value precedence, missing values, and local-path filtering. |
| tests/test_litellm/proxy/test_proxy_server.py | Tests callback environment fallback and secret redaction for non-admin users. |
| tests/test_litellm/proxy/common_utils/test_callback_utils.py | Tests callback fallback, stored-value precedence, and absent variables. |
Reviews (3): Last reviewed commit: "fix(ui): surface env-var-sourced theme a..." | Re-trigger Greptile
Greptile SummaryThis PR makes UI configuration readers reflect settings supplied through process environment variables. The main changes are:
Confidence Score: 4/5The public theme-settings response needs to filter local filesystem paths before merging.
litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py
|
| Filename | Overview |
|---|---|
| litellm/proxy/common_utils/callback_utils.py | Adds process-environment fallback for callback variables while preserving stored-value precedence. |
| litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py | Adds theme environment fallback but can expose local branding paths through a public endpoint. |
| tests/test_litellm/proxy/common_utils/test_callback_utils.py | Tests callback environment fallback, stored precedence, and missing values. |
| tests/test_litellm/proxy/test_proxy_server.py | Tests callback endpoint fallback and secret redaction for non-admin users. |
| tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py | Tests theme fallback and precedence but does not cover local-path environment values. |
Reviews (2): Last reviewed commit: "fix(ui): surface env-var-sourced theme a..." | Re-trigger Greptile
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The UI theme and logging-callback read endpoints reported only stored config while the features resolve their values from the process environment, so a gateway configured purely through env vars showed blank settings pages even though branding rendered and callbacks fired. /get/ui_theme_settings read only litellm_settings.ui_theme_config; logo_url and favicon_url now fall back to UI_LOGO_PATH and LITELLM_FAVICON_URL when the stored config leaves them blank. process_callback (the logging-callbacks block of /get/config/callbacks) reported every callback env var as unset unless it lived in the config environment_variables overlay; it now falls back to os.getenv, matching the slack block. Secret values stay redacted for non-admins via the existing callback role gate. Stored values keep winning over the environment, so the UI-driven flow is unchanged. Resolves LIT-4667
08fc9c2 to
3b7de84
Compare
|
Addressed the Veria finding (Low: local branding path disclosure). @greptileai please review the current head 3b7de84 |
1aba849
into
litellm_internal_staging
Relevant issues
Linear ticket
Resolves LIT-4667
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Follow-up to #33576, which fixed the same env-blindness for SSO and SMTP and left the remaining read endpoints as separate tickets. These two endpoints are pure config readers with no LLM call, so the proof is a before/after curl of the effective config on one proxy configured the way an IaC shop configures one: UI theme and logging-callback settings supplied only as process env vars, nothing ever written through the UI.
Before, at
257ada88ccBoth features are live from the environment, yet the two endpoints the Admin UI reads report everything as unset
logo_urlbeing null renders the theme settings page as an empty-state placeholder, and every Langfuse field reading null makes a working callback look unconfigured, which is what the customer's leadership sees when they evaluate the gateway through the UI.After, at
08fc9c2122Same commands, same environment
The callback response above is for a full admin (master key). Secret callback values stay redacted for anyone below full admin exactly as before, since the fix only changes where the value is resolved from, not the role gate that masks it.
test_get_config_callback_env_secrets_redacted_for_non_adminpins that: an env-onlyLANGFUSE_SECRET_KEYcomes back asREDACTEDfor an internal user while the non-secretLANGFUSE_HOSTstill resolves. Only full admins can save config, and they see the plaintext, so there is no mask for a resubmit to persist over the real value.Admin UI
The same env-only gateway in the dashboard. UI Theme reflects the logo and favicon supplied via
UI_LOGO_PATH/LITELLM_FAVICON_URL, and Logging & Alerts lists the env-configured Langfuse callbackType
🐛 Bug Fix
Changes
The UI theme and logging-callback read endpoints reported only stored config while the features themselves resolve their values from the process environment, so a gateway configured purely through env vars showed blank settings pages even though branding rendered and callbacks fired.
/get/ui_theme_settingsread onlylitellm_settings.ui_theme_config. The update path writeslogo_url/favicon_urlthere and toUI_LOGO_PATH/LITELLM_FAVICON_URL, so a deployment that sets only those env vars has no stored row and read blank.logo_urlandfavicon_urlnow fall back to those env vars when the stored config leaves them blank, with the stored value still winning so the UI-driven flow is unchangedprocess_callback, which builds the logging-callbacks block of/get/config/callbacks, read each callback env var from the configenvironment_variablesoverlay and reportedNonewhen absent, never falling back toos.getenv. The slack block in the same handler already fell back to the environment; the callback block did not. It now goes through the same fallback, so a callback configured via env vars reports its live valuesSecret callback values continue to be redacted for non-admins by the existing callback role gate, and full admins continue to see them in plaintext as they did for stored config, so no new class of value is exposed to any role
QA runbook
Final Attestation