Skip to content

refactor(proxy): make the config file win over the database - #41779

Merged
yuneng-berri merged 10 commits into
mainfrom
litellm_settings_store_precedence
Sep 18, 2026
Merged

yuneng-berri merged 10 commits into
mainfrom
litellm_settings_store_precedence

Conversation

@yuneng-berri

@yuneng-berri yuneng-berri commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Precedence varied per key: some DB-wins, some config-wins, some merged
  • No way to tell which value is live without knowing the key
  • A dashboard write to a config-declared key was stored, then ignored
  • /config/field/info and /config/list disagreed inside one process

How it solves it:

  • One rule: if the config file declares a key, the file owns it
  • A key the file leaves out still comes from the database
  • Writes to a config-owned key now 400 instead of being swallowed
  • Both read endpoints resolve through the same store and report source

User Flow

Before: an admin edits a setting on the Admin UI settings page, the save succeeds, and nothing changes

  1. Their config file has general_settings.max_parallel_requests: 111
  2. They open http://localhost:4000/ui/?page=settings and change Max Parallel Requests to 999, then Save
  3. The save returns 200 and the field shows 999
  4. They send a burst of requests and the proxy still throttles at 111
  5. They re-check: GET /config/field/info?field_name=max_parallel_requests says 999, GET /config/list?config_type=general_settings says 111, on the same running proxy
  6. Nothing tells them the config file is the reason, so the only way out is to guess and remove the key from the file

After: the same save is refused up front and says exactly what to do

  1. Their config file has general_settings.max_parallel_requests: 111
  2. They open http://localhost:4000/ui/?page=settings, and Max Parallel Requests is shown as read-only with the value 111
  3. If they call the API directly, POST /config/field/update with max_parallel_requests: 999 comes back 400: "general_settings key 'max_parallel_requests' is set in the config file and cannot be changed here", naming the file to edit
  4. GET /config/field/info and GET /config/list both report 111 with "source": "config" and "editable": false
  5. A setting the file leaves out, e.g. max_request_size_mb, still saves from the UI, comes back 200, and reads back with "source": "db" and "editable": true

Relevant issues

Affected release

Linear ticket

Resolves LIT-7803

Pre-Submission checklist

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

Both runs use the same isolated Postgres and Redis and the same config file:

model_list:
  - model_name: qa-gpt
    litellm_params:
      model: openai/gpt-4o-mini
      api_key: os.environ/OPENAI_API_KEY

general_settings:
  master_key: sk-1234
  store_model_in_db: true
  max_parallel_requests: 111
  maximum_spend_logs_retention_period: 30d

litellm_settings:
  drop_params: true

The LiteLLM_Config table is emptied before each run, so both start from the same state. Case 1 is a real call to OpenAI, billed.

Before (8fc9c46)

#### case 1: a chat completion still goes through
$ curl -s -X POST http://localhost:4000/v1/chat/completions -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" -d '{"model":"qa-gpt","messages":[{"role":"user","content":"Reply with the single word: alive"}]}' | jq "{model, content: .choices[0].message.content, usage}"
{
  "model": "qa-gpt",
  "content": "Alive",
  "usage": {
    "prompt_tokens": 14,
    "completion_tokens": 1,
    "total_tokens": 15
  }
}

#### case 2: editing a setting the config file declares (max_parallel_requests: 111)
$ curl -s -w "
HTTP %{http_code}
" -X POST http://localhost:4000/config/field/update -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" -d '{"field_name":"max_parallel_requests","field_value":999,"config_type":"general_settings"}'
{"param_name":"general_settings","param_value":{"max_parallel_requests":999},"last_run_at":null,"reload_revision":0}
HTTP 200

#### case 3: reading back which value is live
$ curl -s "http://localhost:4000/config/field/info?field_name=max_parallel_requests" -H "Authorization: Bearer sk-1234"
{"field_name":"max_parallel_requests","field_value":999}
$ curl -s "http://localhost:4000/config/list?config_type=general_settings" -H "Authorization: Bearer sk-1234" | jq -c ".[] | select(.field_name==\"max_parallel_requests\")"
{"field_name": "max_parallel_requests", "field_type": "Integer", "field_description": "maximum parallel requests for each api key", "field_value": 111, "stored_in_db": true, "field_default_value": null, "premium_field": false, "nested_fields": null, "field_options": null, "field_tab": null}

#### case 4: editing a setting the config file leaves out (max_request_size_mb)
$ curl -s -w "
HTTP %{http_code}
" -X POST http://localhost:4000/config/field/update -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" -d '{"field_name":"max_request_size_mb","field_value":42,"config_type":"general_settings"}'
{"param_name":"general_settings","param_value":{"max_request_size_mb":42,"max_parallel_requests":999},"last_run_at":null,"reload_revision":0}
HTTP 200
$ curl -s "http://localhost:4000/config/field/info?field_name=max_request_size_mb" -H "Authorization: Bearer sk-1234"
{"field_name":"max_request_size_mb","field_value":42}

After (0d9c515)

#### case 1: a chat completion still goes through
$ curl -s -X POST http://localhost:4000/v1/chat/completions -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" -d '{"model":"qa-gpt","messages":[{"role":"user","content":"Reply with the single word: alive"}]}' | jq "{model, content: .choices[0].message.content, usage}"
{
  "model": "qa-gpt",
  "content": "alive",
  "usage": {
    "prompt_tokens": 14,
    "completion_tokens": 1,
    "total_tokens": 15
  }
}

#### case 2: editing a setting the config file declares (max_parallel_requests: 111)
$ curl -s -w "
HTTP %{http_code}
" -X POST http://localhost:4000/config/field/update -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" -d '{"field_name":"max_parallel_requests","field_value":999,"config_type":"general_settings"}'
{"detail":{"error":"general_settings key 'max_parallel_requests' is set in the config file and cannot be changed here","keys":["max_parallel_requests"],"section":"general_settings","resolution":"edit /private/tmp/precedence-qa/config.yaml to change it, or remove it from the file to let the database own it"}}
HTTP 400

#### case 3: reading back which value is live
$ curl -s "http://localhost:4000/config/field/info?field_name=max_parallel_requests" -H "Authorization: Bearer sk-1234"
{"field_name":"max_parallel_requests","field_value":111,"source":"config","editable":false}
$ curl -s "http://localhost:4000/config/list?config_type=general_settings" -H "Authorization: Bearer sk-1234" | jq -c ".[] | select(.field_name==\"max_parallel_requests\")"
{"field_name": "max_parallel_requests", "field_type": "Integer", "field_description": "maximum parallel requests for each api key", "field_value": 111, "stored_in_db": false, "field_default_value": null, "premium_field": false, "nested_fields": null, "field_options": null, "field_tab": null, "source": "config", "editable": false}

#### case 4: editing a setting the config file leaves out (max_request_size_mb)
$ curl -s -w "
HTTP %{http_code}
" -X POST http://localhost:4000/config/field/update -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" -d '{"field_name":"max_request_size_mb","field_value":42,"config_type":"general_settings"}'
{"param_name":"general_settings","param_value":{"max_request_size_mb":42},"last_run_at":null,"reload_revision":0}
HTTP 200
$ curl -s "http://localhost:4000/config/field/info?field_name=max_request_size_mb" -H "Authorization: Bearer sk-1234"
{"field_name":"max_request_size_mb","field_value":42,"source":"db","editable":true}

Type

🧹 Refactoring

Caveats (if any)

Severe

  • Breaking: a write to a config-declared setting now 400s
  • Operators who edit those settings from the UI must remove them from the file
  • A pass-through entry in the DB can no longer flip auth on a config-declared path

High

  • Deleting a stored pass-through row used to leave the deleted route serving until restart; fixed here, but it is a live bug on main

Medium

  • Deleting the last allowed IP locks everyone out, on main too: /delete/allowed_ip leaves allowed_ips: [], which denies every address, and the cleanup call is already locked out. Reproduced on the merge base, so not from this PR, but it makes the rest of test_config_misc_endpoints_e2e.py fail once that test runs
  • The Admin UI still renders config-owned fields as editable until the dashboard PR lands
  • Until then the user sees the 400 on save instead of a disabled input
  • /config/field/info answers from the resolved settings now, not the stored row

Low

  • source is exposed on both read endpoints so the dashboard can grey out fields
  • The 786-case JSON fixture is gone; cases are generated from the rule table now

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

@yuneng-berri
yuneng-berri requested a review from a team September 18, 2026 07:05
@codspeed

codspeed Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_settings_store_precedence (0d9c515) with main (8fc9c46)

Open in CodSpeed

@codecov

codecov Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
litellm/proxy/proxy_server.py 96.44% 6 Missing ⚠️
..._experimental/mcp_server/discoverable_endpoints.py 0.00% 1 Missing ⚠️
litellm/proxy/config_resolvers/settings_rules.py 97.77% 1 Missing ⚠️
litellm/proxy/config_resolvers/settings_store.py 98.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment thread litellm/proxy/config_resolvers/settings_rules.py Fixed
@greptile-apps

greptile-apps Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with both previous findings resolved and no new actionable issue established

Summary

This PR establishes config-file ownership for explicitly declared settings, falls back to database values for omitted keys, rejects conflicting control-plane writes, and reports configuration provenance through the read APIs. The latest changes also ensure deleted pass-through routes are unregistered and move prompt-injection heuristics off the event loop

  • Introduces centralized settings precedence and provenance handling
  • Rejects writes that conflict with config-owned values
  • Aligns /config/field/info and /config/list
  • Refreshes runtime side effects and route deletion behavior
  • Adds focused unit and end-to-end coverage

Reviews (3) · Last reviewed commit: "test(e2e): assert config ownership inste..."

Comment thread litellm/proxy/proxy_server.py Outdated
Comment thread litellm/proxy/proxy_server.py Outdated
@yuneng-berri

Copy link
Copy Markdown
Contributor Author

@greptileai review

The precedence used to vary per key: some keys let a stored row win, some
let the file win, some merged the two. That meant an operator could not
answer "which value is live?" without knowing the key.

Now file presence decides ownership. A key the config file declares is
config-owned, whatever the database holds, and a key the file omits falls
back to the stored row. KeyRule no longer carries a RuleKind, only which
row the stored value lives in.

Writes to a config-owned key are refused at the two surfaces that reach
the database instead of being stored and silently ignored: save_config
and /config/field/update both 400 naming the key and the config file path.

Both read endpoints now report source and editable off the same
SettingsStore, so /config/field/info and /config/list can no longer
disagree inside one process.

Replaces the 786-case checked-in JSON fixture with cases generated from
the rule table, so the matrix tests no longer assert that resolve() agrees
with a snapshot of resolve().

BREAKING CHANGE: a dashboard or /config/field/update write to a setting
the config file declares now returns 400 instead of being stored. Remove
the key from the config file to let the database own it.
… the store

Both write paths now go through the same refusal, so /config/field/update and
/config/update answer identically instead of each phrasing its own rule.

A successful write now applies to the SettingsStore, so the next read sees it.
Without this, /config/field/info reported a key the dashboard had just stored
as "not set" until the process reloaded from the database.

resolve() no longer takes a KeyRule it never reads; the store picks the row.
The matrix tests resolve through SettingsStore instead of calling resolve
directly, so the section and key in each case actually route a lookup.

ConfigFieldInfo and ConfigList type `source` as the FieldSource literal, and
the dashboard API types are regenerated for the two new fields.
The two dashboard toggles under litellm_settings wrote through save_config,
so the refusal applied, but they mutated the litellm module global first: a
refused write still took effect in the running process until the next reload.
Both now check before they mutate.

/config/field/delete drops the stored key without touching the store, so a
deleted key kept reading back from the process. It now refreshes the store
like the other write paths.

/config/list reported source and editable for the general_settings rows but
not for the litellm_settings ones, so the dashboard would have shown a
config-declared toggle as editable.
@yuneng-berri yuneng-berri changed the title refactor(proxy): resolve config and DB settings precedence in one SettingsStore refactor(proxy): make the config file win over the database Sep 18, 2026
…anges

The reload only re-registered pass-through endpoints when the stored row
still carried the key, so deleting the row left the deleted routes serving
traffic until the process restarted.

It now compares the resolved list before and after the row is applied and
rebuilds on any difference, including a deletion that resolves back to the
config file's list or to nothing.

This matches what _apply_retention_settings already does with the retention
values, so the two reload effects no longer disagree about what counts as a
change.

The tests assert the proxy's registry of live pass-through routes, which is
what decides whether a request is routed upstream or falls through to the
auth error, rather than that the registration helper was called.
…spatcher

_apply_general_settings_side_effects grew a fourth argument when the reload
started comparing the resolved pass-through list, and this dispatch test calls
it positionally, so it failed with a TypeError.
… row

/config/field/info used to answer from the LiteLLM_Config row, so "the field
400s" proved the row did not carry it. It now answers from the resolved
settings, and the CI stack config declares general_settings.max_parallel_requests,
so the endpoint returns that value and the old assertion could never hold.

The check that /add/allowed_ip writes only what the caller changed moves to
/config/list, which still reports stored_in_db off the row, and the field/info
call now asserts the ownership the endpoint reports: the config file owns the
key, so it reads back as source=config and editable=false.

Verified against a live proxy on an isolated Postgres rather than in CI, where
this check has never run: it waits on protected-environment approval.
@yuneng-berri

Copy link
Copy Markdown
Contributor Author

@greptile

@yuneng-berri
yuneng-berri merged commit c4ab1d9 into main Sep 18, 2026
87 of 88 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_settings_store_precedence branch September 18, 2026 16:52
yuneng-berri added a commit that referenced this pull request Oct 1, 2026
Config-wins (#41779) made general_settings.pass_through_endpoints a config-owned key. The DB reader then got the config list back as if it were DB rows, re-registered each entry without forward_headers on every DB sync, and the stripped copy won the route lookup, so a config pass-through with forward_headers: true stopped forwarding Authorization. UI create, update and delete of pass-throughs were also rejected while the config declared any.

This puts pass-throughs back on their pre-#41779 path: the settings store no longer lets the config own the key, the config list is captured env-resolved at load_config, each DB sync merges DB entries with config entries on paths the DB does not declare, and /config/field/info reads the stored rows only. A UI pass-through write re-applies that merge immediately so the config entries stay served until the next sync.
yuneng-berri added a commit that referenced this pull request Oct 1, 2026
#43962)

* fix(proxy): restore pre-config-wins handling of pass-through endpoints

Config-wins (#41779) made general_settings.pass_through_endpoints a config-owned key. The DB reader then got the config list back as if it were DB rows, re-registered each entry without forward_headers on every DB sync, and the stripped copy won the route lookup, so a config pass-through with forward_headers: true stopped forwarding Authorization. UI create, update and delete of pass-throughs were also rejected while the config declared any.

This puts pass-throughs back on their pre-#41779 path: the settings store no longer lets the config own the key, the config list is captured env-resolved at load_config, each DB sync merges DB entries with config entries on paths the DB does not declare, and /config/field/info reads the stored rows only. A UI pass-through write re-applies that merge immediately so the config entries stay served until the next sync.

* fix(proxy): keep config pass-throughs in every reload of the merged list

get_config now returns DB pass-throughs plus config ones on other paths,
each DB sync republishes that merged list, and /config/field/info reads
pass_through_endpoints from the DB row so a UI write never drops stored
entries when models are not stored in the DB

* fix(proxy): keep serving pass-throughs while the config file reloads

load_yaml cleared the runtime pass-through list, so auth: false routes
answered 401 while get_config awaited the database

* fix(proxy): read stored pass-throughs from the writer before a UI write

A lagging read replica could return an older list, and the UI create and
edit flows write the whole field back

* fix(proxy): apply config file pass-through auth changes on reload

The kept runtime list was merged as if it were DB entries, so an edited
config entry on the same path was dropped. Merge the stored DB row with
the fresh config instead, and give the field-info test mock a writer

* fix(proxy): keep pass-throughs served while a DB sync reads the database

get_config resets the stored DB rows before reading them again, which
cleared the served pass-through list and made auth: false routes answer
401 for the length of the read

* refactor(proxy): move the settings store reload out of the loop

basedpyright rejects a Final variable assigned inside a loop
stvnksslr pushed a commit to stvnksslr/litellm that referenced this pull request Oct 1, 2026
BerriAI#43962)

* fix(proxy): restore pre-config-wins handling of pass-through endpoints

Config-wins (BerriAI#41779) made general_settings.pass_through_endpoints a config-owned key. The DB reader then got the config list back as if it were DB rows, re-registered each entry without forward_headers on every DB sync, and the stripped copy won the route lookup, so a config pass-through with forward_headers: true stopped forwarding Authorization. UI create, update and delete of pass-throughs were also rejected while the config declared any.

This puts pass-throughs back on their pre-BerriAI#41779 path: the settings store no longer lets the config own the key, the config list is captured env-resolved at load_config, each DB sync merges DB entries with config entries on paths the DB does not declare, and /config/field/info reads the stored rows only. A UI pass-through write re-applies that merge immediately so the config entries stay served until the next sync.

* fix(proxy): keep config pass-throughs in every reload of the merged list

get_config now returns DB pass-throughs plus config ones on other paths,
each DB sync republishes that merged list, and /config/field/info reads
pass_through_endpoints from the DB row so a UI write never drops stored
entries when models are not stored in the DB

* fix(proxy): keep serving pass-throughs while the config file reloads

load_yaml cleared the runtime pass-through list, so auth: false routes
answered 401 while get_config awaited the database

* fix(proxy): read stored pass-throughs from the writer before a UI write

A lagging read replica could return an older list, and the UI create and
edit flows write the whole field back

* fix(proxy): apply config file pass-through auth changes on reload

The kept runtime list was merged as if it were DB entries, so an edited
config entry on the same path was dropped. Merge the stored DB row with
the fresh config instead, and give the field-info test mock a writer

* fix(proxy): keep pass-throughs served while a DB sync reads the database

get_config resets the stored DB rows before reading them again, which
cleared the served pass-through list and made auth: false routes answer
401 for the length of the read

* refactor(proxy): move the settings store reload out of the loop

basedpyright rejects a Final variable assigned inside a loop

(cherry picked from commit 2eb2bf1)
yuneng-berri added a commit that referenced this pull request Oct 1, 2026
#43962) (#44054)

* fix(proxy): restore pre-config-wins handling of pass-through endpoints

Config-wins (#41779) made general_settings.pass_through_endpoints a config-owned key. The DB reader then got the config list back as if it were DB rows, re-registered each entry without forward_headers on every DB sync, and the stripped copy won the route lookup, so a config pass-through with forward_headers: true stopped forwarding Authorization. UI create, update and delete of pass-throughs were also rejected while the config declared any.

This puts pass-throughs back on their pre-#41779 path: the settings store no longer lets the config own the key, the config list is captured env-resolved at load_config, each DB sync merges DB entries with config entries on paths the DB does not declare, and /config/field/info reads the stored rows only. A UI pass-through write re-applies that merge immediately so the config entries stay served until the next sync.

* fix(proxy): keep config pass-throughs in every reload of the merged list

get_config now returns DB pass-throughs plus config ones on other paths,
each DB sync republishes that merged list, and /config/field/info reads
pass_through_endpoints from the DB row so a UI write never drops stored
entries when models are not stored in the DB

* fix(proxy): keep serving pass-throughs while the config file reloads

load_yaml cleared the runtime pass-through list, so auth: false routes
answered 401 while get_config awaited the database

* fix(proxy): read stored pass-throughs from the writer before a UI write

A lagging read replica could return an older list, and the UI create and
edit flows write the whole field back

* fix(proxy): apply config file pass-through auth changes on reload

The kept runtime list was merged as if it were DB entries, so an edited
config entry on the same path was dropped. Merge the stored DB row with
the fresh config instead, and give the field-info test mock a writer

* fix(proxy): keep pass-throughs served while a DB sync reads the database

get_config resets the stored DB rows before reading them again, which
cleared the served pass-through list and made auth: false routes answer
401 for the length of the read

* refactor(proxy): move the settings store reload out of the loop

basedpyright rejects a Final variable assigned inside a loop

(cherry picked from commit 2eb2bf1)
yuneng-berri added a commit that referenced this pull request Oct 2, 2026
…th (#44267)

The failure spend-log row from #42695 is written for pass-through routes that
run as LLM API routes, which a config route only does with auth: true. The
test omitted auth and passed only while config wins (#41779) registered config
entries through the typed model, where auth defaults to true. #43962 restored
the pre-config-wins registration, so the route lost that status and the row
was never written. Set auth: true on the route so the test covers the logging
it was written for without depending on that side effect

(cherry picked from commit 1d9cd9b)
yuneng-berri added a commit that referenced this pull request Oct 2, 2026
…th (#44269)

The failure spend-log row from #42695 is written for pass-through routes that
run as LLM API routes, which a config route only does with auth: true. The
test omitted auth and passed only while config wins (#41779) registered config
entries through the typed model, where auth defaults to true. #43962 restored
the pre-config-wins registration, so the route lost that status and the row
was never written. Set auth: true on the route so the test covers the logging
it was written for without depending on that side effect

(cherry picked from commit 1d9cd9b)
yuneng-berri added a commit that referenced this pull request Oct 2, 2026
…th (#44265)

The failure spend-log row from #42695 is written for pass-through routes that
run as LLM API routes, which a config route only does with auth: true. The
test omitted auth and passed only while config wins (#41779) registered config
entries through the typed model, where auth defaults to true. #43962 restored
the pre-config-wins registration, so the route lost that status and the row
was never written. Set auth: true on the route so the test covers the logging
it was written for without depending on that side effect

This branch is waiting to be deployed

1 waiting deployment
e2e-changed — 0d9c5159 Waiting Sep 18, 2026 by yuneng-berri via Run changed e2e tests against the stage-mirror stack #6839
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.

3 participants