Skip to content

fix(proxy): wire general_settings SSRF allowlist to litellm globals - #32243

Merged
Sameerlite merged 4 commits into
litellm_internal_stagingfrom
litellm_fix-general-settings-url-validation
Jul 7, 2026
Merged

fix(proxy): wire general_settings SSRF allowlist to litellm globals#32243
Sameerlite merged 4 commits into
litellm_internal_stagingfrom
litellm_fix-general-settings-url-validation

Conversation

@Sameerlite

@Sameerlite Sameerlite commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Before
image

After
image


Note

Medium Risk
Touches SSRF enforcement configuration and runtime globals; incorrect wiring could leave stale allowlists or unexpected validation behavior, though the change is intended to make documented admin settings effective.

Overview
Fixes #26599 by making proxy general_settings actually drive the SSRF globals that validate_url() / safe_get already read (litellm.user_url_validation, user_url_allowed_hosts, provider_url_destination_allowed_hosts).

Adds _apply_ssrf_general_settings (with string/"false" handling for user_url_validation and explicit updates when keys are present, including null to clear allowlists) and calls it on initial config load, merged general_settings updates, and update_config_general_settings so YAML and dashboard changes take effect without restart. Documents the three fields on ConfigGeneralSettings and extends the dashboard OpenAPI schema; regression tests cover load and live config updates.

Reviewed by Cursor Bugbot for commit 92722db. Bugbot is set up for automated code reviews on this repo. Configure here.

general_settings.user_url_allowed_hosts was documented in SSRF errors but
never applied at startup, so internal MCP/OpenAPI URLs stayed blocked.

Co-authored-by: Cursor <cursoragent@cursor.com>
@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a long-standing misconfiguration gap (issue #26599) where user_url_validation, user_url_allowed_hosts, and provider_url_destination_allowed_hosts could be declared under general_settings but were never wired into the litellm.* globals that the SSRF guard actually reads.

  • Adds _apply_ssrf_general_settings() and calls it in all three code paths that process general_settings: YAML startup (load_config), periodic DB overlay merge (_update_general_settings), and dashboard live update (update_config_general_settings).
  • Documents the three fields on ConfigGeneralSettings (Pydantic) and the OpenAPI TypeScript schema so they are recognized as valid keys at both parse time and in the UI.
  • Includes regression tests covering YAML load, null/string coercion, and live dashboard updates; all tests use mocks with no real network calls.

Confidence Score: 5/5

The change is safe to merge — it makes previously-inert admin config keys actually take effect without altering any existing behavior for deployments that never set those keys.

The wiring is straightforward: one helper function called in three well-defined config paths, each covered by a new mock test. The null/string-coercion edge cases are explicitly tested and behave sensibly. No existing tests are weakened, and no critical path code is touched.

No files require special attention.

Important Files Changed

Filename Overview
litellm/proxy/proxy_server.py Adds _normalize_user_url_validation and _apply_ssrf_general_settings helpers, then calls _apply_ssrf_general_settings in three places: initial config load, live DB overlay merge, and dashboard update_config_general_settings. Logic is correct; null handling asymmetry between allowlists (clears) and validation flag (no-op) is intentional and tested.
litellm/proxy/_types.py Adds three new Optional fields (user_url_validation, user_url_allowed_hosts, provider_url_destination_allowed_hosts) to ConfigGeneralSettings with accurate docstrings so the YAML keys are recognized and validated by Pydantic.
tests/test_litellm/proxy/proxy_server/test_proxy_config.py Adds a regression test confirming that SSRF fields in a YAML general_settings block correctly reach litellm.* globals after load_config, with proper teardown to restore originals.
tests/test_litellm/proxy/test_proxy_server.py Adds two new tests: one covering null/string-"false" edge cases in load_config, another verifying that live update_config_general_settings calls propagate all three SSRF globals, including clearing allowlists with None. Tests use only mocks — no real network calls.
ui/litellm-dashboard/src/lib/http/schema.d.ts Adds the three new SSRF fields to the ConfigGeneralSettings OpenAPI schema component so the dashboard TypeScript client reflects the updated API shape.

Reviews (3): Last reviewed commit: "fix: clear ssrf allowlists from null set..." | Re-trigger Greptile

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.85714% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
litellm/proxy/proxy_server.py 64.00% 9 Missing ⚠️

📢 Thoughts on this report? Let us know!

Use list[str] in ConfigGeneralSettings and run gen:api so schema.d.ts
matches the new SSRF general_settings fields.

Co-authored-by: Cursor <cursoragent@cursor.com>
@mateo-berri

Copy link
Copy Markdown
Contributor

@greptileai

@mateo-berri

Copy link
Copy Markdown
Contributor

@coderabbitai review

@mateo-berri

Copy link
Copy Markdown
Contributor

bugbot run

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mateo-berri

Copy link
Copy Markdown
Contributor

(note: @Sameerlite I'm just testing out coderabbit and comparing it with bugbot. Don't treat any of the issues it comes up with as blocking for now)

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

An error occurred during the review process. Please try again later.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch litellm_fix-general-settings-url-validation

Comment @coderabbitai help to get the list of available commands.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Null validation disables SSRF guard
    • Null user_url_validation values are ignored and string booleans are normalized before updating the SSRF validation global.
  • ✅ Fixed: Dashboard updates skip litellm globals
    • Dashboard and DB general-settings updates now immediately apply the SSRF allowlist and validation globals used at request time.

You can send follow-ups to the cloud agent here.

Comment thread litellm/proxy/proxy_server.py Outdated
Comment thread litellm/proxy/_types.py
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ Sameerlite
❌ cursoragent
You have signed the CLA already but the status is still pending? Let us recheck it.

@Sameerlite

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Null allowlists skip global reset
    • Updated SSRF settings application to assign allowlist globals whenever their config keys are present, including explicit null values, with regression coverage for load and update paths.

You can send follow-ups to the cloud agent here.

Comment thread litellm/proxy/proxy_server.py Outdated
@Sameerlite

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 92722db. Configure here.

@Sameerlite

Copy link
Copy Markdown
Contributor Author

@greptileai

@Sameerlite
Sameerlite requested a review from mateo-berri July 7, 2026 08:08

@mateo-berri mateo-berri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; thanks!

@Sameerlite
Sameerlite merged commit 42f5b0b into litellm_internal_staging Jul 7, 2026
126 checks passed
@Sameerlite
Sameerlite deleted the litellm_fix-general-settings-url-validation branch July 7, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: user_url_validation and user_url_allowed_hosts is not taking effect after https://github.com/BerriAI/litellm/pull/25906

4 participants