fix(proxy): close the config-ownership gaps QA found in the settings store

- apply_db_row only clears runtime values for keys the row actually changed, so an env-resolved DB-owned setting survives a reload
- DELETE /config/field/delete refuses a key the config file owns instead of silently rewriting the row
- GET /config/field/info reports the declared value of a config-owned key, not the env-resolved secret
- SettingsStore gains a short-circuiting __bool__ so truthiness checks stop at the first key
- _initialize_jwt_auth resolves os.environ refs into a local mapping instead of mutating the shared general_settings dict
- rejected_writes compares against the resolved value, matching what __setitem__ accepts
- a stored value identical to the config template is no longer reported as shadowed
- the enterprise email-settings and coordination-redis writers go through reject_config_owned_writes
This commit is contained in:
Yuneng Jiang 2026-09-19 12:17:20 -07:00
parent eda1faba14
commit 3dff41f369
No known key found for this signature in database
8 changed files with 373 additions and 11 deletions

View file

@ -60,6 +60,11 @@ async def _get_email_settings(prisma_client) -> Dict[str, bool]:
async def _save_email_settings(prisma_client, settings: Dict[str, bool]):
"""Helper function to save email settings to general_settings in db"""
from litellm.proxy.proxy_server import proxy_config
proxy_config.reject_config_owned_writes(
section_name="general_settings", changed_keys={"email_settings": settings}
)
try:
verbose_proxy_logger.debug(
f"Saving email settings to general_settings: {settings}"
@ -168,6 +173,8 @@ async def update_event_settings(
await _save_email_settings(prisma_client, settings_dict)
return {"message": "Email event settings updated successfully"}
except HTTPException:
raise
except Exception as e:
verbose_proxy_logger.exception(f"Error updating email settings: {str(e)}")
raise HTTPException(status_code=500, detail=str(e))
@ -197,6 +204,8 @@ async def reset_event_settings(
await _save_email_settings(prisma_client, default_settings)
return {"message": "Email event settings reset to defaults"}
except HTTPException:
raise
except Exception as e:
verbose_proxy_logger.exception(f"Error resetting email settings: {str(e)}")
raise HTTPException(status_code=500, detail=str(e))

View file

@ -60,9 +60,7 @@ class SettingsStore(MutableMapping[str, JsonValue]):
def rejected_writes(self, incoming: Mapping[str, JsonValue]) -> tuple[str, ...]:
return tuple(
sorted(
key for key, value in incoming.items() if self.owned_by_config(key) and value != self._yaml_values[key]
)
sorted(key for key, value in incoming.items() if self.owned_by_config(key) and value != self.get(key))
)
def shadowed_db_keys(self) -> tuple[str, ...]:
@ -74,8 +72,13 @@ class SettingsStore(MutableMapping[str, JsonValue]):
def apply_db_row(self, row: DbRow, db_row: Mapping[str, JsonValue]) -> None:
previous_row: Final = self._database_rows.get(row, _EMPTY_VALUES)
changed: Final = frozenset(
key
for key in (*previous_row, *db_row)
if previous_row.get(key, ABSENT) != db_row.get(key, ABSENT) # pyright: ignore[reportUnknownArgumentType] # JsonValue vs Absent compare
)
self._database_rows = MappingProxyType({**self._database_rows, row: MappingProxyType(dict(db_row))})
self._clear_runtime_keys(frozenset((*previous_row, *db_row)))
self._clear_runtime_keys(changed)
def resolved(self) -> Mapping[str, JsonValue]:
return MappingProxyType(dict(self))
@ -130,6 +133,9 @@ class SettingsStore(MutableMapping[str, JsonValue]):
def __len__(self) -> int:
return sum(1 for _ in self)
def __bool__(self) -> bool:
return any(True for _ in self)
def _clear_runtime(self) -> None:
self._runtime_values = _EMPTY_VALUES
self._deleted_runtime_keys = frozenset()
@ -160,7 +166,12 @@ class SettingsStore(MutableMapping[str, JsonValue]):
def _db_value_is_shadowed(self, key: str) -> bool:
db_value: Final = self._db_value(key)
return not isinstance(db_value, Absent) and db_value is not None and db_value != self.get(key)
return (
not isinstance(db_value, Absent)
and db_value is not None
and db_value != self.get(key)
and db_value != self.config_value(key)
)
def _resolution_for(self, key: str) -> Resolved:
yaml_value: Final[SettingValue] = self._yaml_values.get(key, ABSENT)

View file

@ -364,6 +364,11 @@ async def update_coordination_redis_settings(
settings: Final = _merge_over_saved(request.settings, saved_settings or {})
_validated_params(settings)
from litellm.proxy.proxy_server import proxy_config
proxy_config.reject_config_owned_writes(
section_name=_GENERAL_SETTINGS_PARAM_NAME, changed_keys={_COORDINATION_REDIS_KEY: settings}
)
general_settings: Final = await _read_general_settings()
before_settings: Final = general_settings.get(_COORDINATION_REDIS_KEY)
action: Final[AUDIT_ACTIONS] = "updated" if isinstance(before_settings, dict) else "created"

View file

@ -5117,6 +5117,15 @@ class ProxyConfig:
store.apply_db_row(cast(DbRow, section_name), wrote_section)
await invalidate_config_param(section_name)
def reject_config_owned_deletes(self, *, section_name: str, keys: tuple[str, ...]) -> None:
"""Refuse a delete of a setting the config file owns; unlike a write, the value never makes it allowed."""
store: Final = self._settings_stores.get(cast(Section, section_name))
if store is None:
return
owned: Final = tuple(sorted(key for key in keys if store.owned_by_config(key)))
if owned:
self._raise_config_owned(section_name=section_name, rejected=owned, store=store)
def reject_config_owned_writes(self, *, section_name: str, changed_keys: Mapping[str, JsonValue]) -> None:
"""Refuse a write to a setting the config file owns, rather than storing a value that never applies."""
store: Final = self._settings_stores.get(cast(Section, section_name))
@ -5125,6 +5134,9 @@ class ProxyConfig:
rejected: Final = store.rejected_writes(changed_keys)
if not rejected:
return
self._raise_config_owned(section_name=section_name, rejected=rejected, store=store)
def _raise_config_owned(self, *, section_name: str, rejected: tuple[str, ...], store: SettingsStore) -> None:
subject: Final = (
f"key '{rejected[0]}' is" if len(rejected) == 1 else f"keys {', '.join(repr(key) for key in rejected)} are"
)
@ -9684,10 +9696,12 @@ class ProxyStartupEvent:
user_api_key_cache: UserApiKeyCache,
):
"""Initialize JWT auth on startup"""
if general_settings.get("litellm_jwtauth", None) is not None:
for k, v in general_settings["litellm_jwtauth"].items():
if isinstance(v, str) and v.startswith("os.environ/"):
general_settings["litellm_jwtauth"][k] = get_secret(v)
declared_jwtauth: Final = general_settings.get("litellm_jwtauth", None)
if declared_jwtauth is not None:
resolved_jwtauth: Final = {
key: (get_secret(value) if isinstance(value, str) and value.startswith("os.environ/") else value)
for key, value in declared_jwtauth.items()
}
# ``user_config_file_path`` is set by ``ProxyConfig._get_config_from_file``
# during startup. Threading it through lets an operator-
# configured ``custom_validate: s3://...`` resolve through
@ -9695,7 +9709,7 @@ class ProxyStartupEvent:
# file context) hit the gate and refuse remote loads.
litellm_jwtauth = LiteLLM_JWTAuth(
config_file_path=user_config_file_path,
**general_settings["litellm_jwtauth"],
**resolved_jwtauth,
)
else:
litellm_jwtauth = LiteLLM_JWTAuth()
@ -17665,9 +17679,12 @@ async def get_config_general_settings(
detail={"error": f"Field name={field_name} is not set"},
)
declared: Final = (
settings.config_value(field_name) if settings.owned_by_config(field_name) else settings[field_name]
)
field_value = _redact_general_setting_value(
field_name,
settings[field_name],
declared,
user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN,
)
if field_name == "plugins" and isinstance(field_value, list):
@ -18041,6 +18058,8 @@ async def delete_config_general_settings(
detail={"error": f"Invalid field={data.field_name} passed in."},
)
proxy_config.reject_config_owned_deletes(section_name="general_settings", keys=(data.field_name,))
## get general settings from db
db_general_settings: Final[_ConfigParamRow | None] = await _config_param_table(prisma_client).find_first(
where={"param_name": "general_settings"}

View file

@ -260,3 +260,74 @@ async def test_endpoint_with_no_prisma_client(mock_user_api_key_auth):
with pytest.raises(HTTPException) as exc_info:
await reset_event_settings(user_api_key_dict=mock_user_api_key_auth)
assert exc_info.value.status_code == 500
def _prisma_recording_upserts(upserts):
client = mock.MagicMock()
async def find_unique(*args, **kwargs):
return None
async def upsert(*args, **kwargs):
upserts.append(kwargs)
return None
client.db.litellm_config.find_unique = find_unique
client.db.litellm_config.upsert = upsert
return client
def _proxy_config_owning(general_settings):
from litellm.proxy.proxy_server import ProxyConfig
proxy_config = ProxyConfig()
proxy_config._load_yaml_settings_stores({"general_settings": general_settings})
return proxy_config
@pytest.mark.asyncio
async def test_save_email_settings_refuses_a_config_owned_email_settings():
upserts = []
client = _prisma_recording_upserts(upserts)
proxy_config = _proxy_config_owning({"email_settings": {EmailEvent.new_user_invitation.value: True}})
with mock.patch("litellm.proxy.proxy_server.proxy_config", proxy_config):
with pytest.raises(HTTPException) as refused:
await _save_email_settings(client, {EmailEvent.new_user_invitation.value: False})
assert refused.value.status_code == 400
assert refused.value.detail["keys"] == ["email_settings"]
assert upserts == []
@pytest.mark.asyncio
async def test_update_event_settings_surfaces_the_config_owned_refusal(mock_user_api_key_auth):
upserts = []
client = _prisma_recording_upserts(upserts)
proxy_config = _proxy_config_owning({"email_settings": {EmailEvent.virtual_key_created.value: False}})
request = EmailEventSettingsUpdateRequest(
settings=[EmailEventSettings(event=EmailEvent.virtual_key_created, enabled=True)]
)
with mock.patch("litellm.proxy.proxy_server.prisma_client", client):
with mock.patch("litellm.proxy.proxy_server.proxy_config", proxy_config):
with pytest.raises(HTTPException) as refused:
await update_event_settings(request=request, user_api_key_dict=mock_user_api_key_auth)
assert refused.value.status_code == 400
assert refused.value.detail["keys"] == ["email_settings"]
assert upserts == []
@pytest.mark.asyncio
async def test_save_email_settings_still_writes_when_the_config_file_is_silent():
upserts = []
client = _prisma_recording_upserts(upserts)
proxy_config = _proxy_config_owning({})
with mock.patch("litellm.proxy.proxy_server.proxy_config", proxy_config):
await _save_email_settings(client, {EmailEvent.new_user_invitation.value: False})
assert len(upserts) == 1
written = json.loads(upserts[0]["data"]["create"]["param_value"])
assert written["email_settings"] == {EmailEvent.new_user_invitation.value: False}

View file

@ -359,3 +359,93 @@ def test_settings_store_refusal_stays_quiet_about_the_database_when_nothing_is_s
assert refused.value.shadows_db_value is False
assert "stored in the database" not in str(refused.value)
assert "config file" in str(refused.value)
def test_settings_store_keeps_a_resolved_runtime_value_when_a_db_row_repeats_it() -> None:
store: Final = SettingsStore("general_settings")
store.apply_db_row("general_settings", {"litellm_key_header_name": "os.environ/HDR"})
store.apply_runtime_values({"litellm_key_header_name": "X-Resolved-Header"})
store.apply_db_row("general_settings", {"litellm_key_header_name": "os.environ/HDR"})
assert store["litellm_key_header_name"] == "X-Resolved-Header"
def test_settings_store_drops_a_resolved_runtime_value_when_a_db_row_changes_it() -> None:
store: Final = SettingsStore("general_settings")
store.apply_db_row("general_settings", {"litellm_key_header_name": "os.environ/HDR"})
store.apply_runtime_values({"litellm_key_header_name": "X-Resolved-Header"})
store.apply_db_row("general_settings", {"litellm_key_header_name": "os.environ/OTHER"})
assert store["litellm_key_header_name"] == "os.environ/OTHER"
def test_settings_store_accepts_the_writes_it_does_not_report_as_rejected() -> None:
store: Final = SettingsStore("general_settings")
store.load_yaml({"litellm_key_header_name": "os.environ/HDR"})
store.apply_runtime_values({"litellm_key_header_name": "X-Resolved-Header"})
incoming: Final[dict[str, JsonValue]] = {"litellm_key_header_name": "X-Resolved-Header"}
assert store.rejected_writes(incoming) == ()
store["litellm_key_header_name"] = "X-Resolved-Header"
assert store["litellm_key_header_name"] == "X-Resolved-Header"
def test_settings_store_reports_a_rejected_write_the_store_itself_refuses() -> None:
store: Final = SettingsStore("general_settings")
store.load_yaml({"litellm_key_header_name": "os.environ/HDR"})
store.apply_runtime_values({"litellm_key_header_name": "X-Resolved-Header"})
assert store.rejected_writes({"litellm_key_header_name": "X-Other-Header"}) == ("litellm_key_header_name",)
with pytest.raises(ConfigOwnedKeyError):
store["litellm_key_header_name"] = "X-Other-Header"
def test_settings_store_reports_no_shadowing_when_the_database_repeats_the_config_template() -> None:
store: Final = SettingsStore("general_settings")
store.load_yaml({"litellm_key_header_name": "os.environ/HDR"})
store.apply_db_row("general_settings", {"litellm_key_header_name": "os.environ/HDR"})
store.apply_runtime_values({"litellm_key_header_name": "X-Resolved-Header"})
assert store.shadowed_db_keys() == ()
assert store.shadows_db_value("litellm_key_header_name") is False
def test_settings_store_still_reports_shadowing_when_the_database_holds_another_template() -> None:
store: Final = SettingsStore("general_settings")
store.load_yaml({"litellm_key_header_name": "os.environ/HDR"})
store.apply_db_row("general_settings", {"litellm_key_header_name": "os.environ/OTHER"})
store.apply_runtime_values({"litellm_key_header_name": "X-Resolved-Header"})
assert store.shadowed_db_keys() == ("litellm_key_header_name",)
def test_settings_store_truthiness_stops_at_the_first_key() -> None:
store: Final = SettingsStore("general_settings")
store.load_yaml({f"key_{index}": index for index in range(25)})
resolutions: Final[list[str]] = []
original: Final = SettingsStore._resolution_for
def counted(self: SettingsStore, key: str): # type: ignore[no-untyped-def]
resolutions.append(key)
return original(self, key)
with patch.object(SettingsStore, "_resolution_for", counted):
assert bool(store) is True
truthiness_resolutions: Final = len(resolutions)
resolutions.clear()
assert len(store) == 25
assert len(resolutions) == 25
assert truthiness_resolutions <= 1
def test_settings_store_truthiness_matches_emptiness() -> None:
store: Final = SettingsStore("general_settings")
assert bool(store) is False
store["max_parallel_requests"] = 3
assert bool(store) is True
del store["max_parallel_requests"]
assert bool(store) is False

View file

@ -616,3 +616,64 @@ async def test_connection_test_rejects_proxy_admin_viewer():
user_api_key_dict=UserAPIKeyAuth(api_key="hashed", user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY),
)
assert exc_info.value.status_code == 403
def _real_proxy_config(file_general_settings: dict) -> "object":
from litellm.proxy.proxy_server import ProxyConfig
proxy_config = ProxyConfig()
proxy_config._load_yaml_settings_stores({"general_settings": file_general_settings})
proxy_config.get_config_state = MagicMock( # type: ignore[method-assign]
return_value={"general_settings": file_general_settings}
)
return proxy_config
@pytest.mark.asyncio
async def test_update_refuses_a_config_owned_coordination_redis_block(monkeypatch):
monkeypatch.setattr(litellm, "store_audit_logs", False)
mock_prisma = _prisma_with_general_settings({"master_key": "sk-1234"})
from_file = {"coordination_redis": {"host": "yaml-redis.example.com", "port": 6379}}
with (
patch("litellm.proxy.proxy_server.prisma_client", mock_prisma),
patch("litellm.proxy.proxy_server.proxy_config", _real_proxy_config(from_file)),
patch("litellm.proxy.proxy_server.store_model_in_db", True),
):
with pytest.raises(HTTPException) as refused:
await update_coordination_redis_settings(
request=CoordinationRedisSettingsRequest(settings={"host": "db-redis.example.com", "port": 6380}),
user_api_key_dict=_admin_auth(),
litellm_changed_by=None,
)
assert refused.value.status_code == 400
assert refused.value.detail["keys"] == ["coordination_redis"]
mock_prisma.db.litellm_config.upsert.assert_not_called()
@pytest.mark.asyncio
async def test_update_still_persists_when_the_config_file_declares_no_block(monkeypatch):
monkeypatch.setattr(litellm, "store_audit_logs", False)
mock_prisma = _prisma_with_general_settings({"master_key": "sk-1234"})
async def _capture_invalidate(param_name: str) -> None:
return None
with (
patch("litellm.proxy.proxy_server.prisma_client", mock_prisma),
patch("litellm.proxy.proxy_server.proxy_config", _real_proxy_config({"master_key": "sk-1234"})),
patch("litellm.proxy.proxy_server.store_model_in_db", True),
patch(
"litellm.proxy.management_endpoints.coordination_redis_endpoints.invalidate_config_param",
new=_capture_invalidate,
),
):
await update_coordination_redis_settings(
request=CoordinationRedisSettingsRequest(settings={"host": "db-redis.example.com", "port": 6380}),
user_api_key_dict=_admin_auth(),
litellm_changed_by=None,
)
persisted = json.loads(mock_prisma.db.litellm_config.upsert.call_args.kwargs["data"]["update"]["param_value"])
assert persisted["coordination_redis"] == {"host": "db-redis.example.com", "port": 6380}

View file

@ -14737,3 +14737,99 @@ async def test_auth_cache_invalidation_subscriber_evicts_byok_credentials_cached
byok_credential_cache.flush_cache()
assert evicted, "the subscriber does not evict the BYOK credential cache on a peer worker's broadcast"
@pytest.mark.asyncio
async def test_delete_config_general_settings_refuses_a_key_the_config_file_owns(monkeypatch):
from litellm.proxy._types import ConfigFieldDelete
from litellm.proxy.proxy_server import ProxyConfig, delete_config_general_settings
pc = ProxyConfig()
pc._load_yaml_settings_stores({"general_settings": {"max_request_size_mb": 42}})
monkeypatch.setattr(proxy_server_module, "proxy_config", pc)
monkeypatch.setattr(proxy_server_module, "prisma_client", _fake_prisma_with_config({"max_request_size_mb": 99}))
admin = UserAPIKeyAuth(api_key="hashed-admin", user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN)
with pytest.raises(HTTPException) as refused:
await delete_config_general_settings(
data=ConfigFieldDelete(field_name="max_request_size_mb", config_type="general_settings"),
user_api_key_dict=admin,
)
assert refused.value.status_code == 400
assert refused.value.detail["keys"] == ["max_request_size_mb"]
assert "config file" in refused.value.detail["error"]
assert pc.settings["max_request_size_mb"] == 42
@pytest.mark.asyncio
async def test_delete_config_general_settings_still_removes_a_key_the_database_owns(monkeypatch):
from litellm.proxy._types import ConfigFieldDelete
from litellm.proxy.proxy_server import ProxyConfig, delete_config_general_settings
pc = ProxyConfig()
pc._load_yaml_settings_stores({"general_settings": {}})
pc.settings.apply_db_row("general_settings", {"max_request_size_mb": 42})
monkeypatch.setattr(proxy_server_module, "proxy_config", pc)
monkeypatch.setattr(proxy_server_module, "prisma_client", _fake_prisma_with_config({"max_request_size_mb": 42}))
admin = UserAPIKeyAuth(api_key="hashed-admin", user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN)
await delete_config_general_settings(
data=ConfigFieldDelete(field_name="max_request_size_mb", config_type="general_settings"),
user_api_key_dict=admin,
)
assert "max_request_size_mb" not in pc.settings
@pytest.mark.asyncio
async def test_config_field_info_reports_the_declared_value_of_a_config_owned_secret(monkeypatch):
from litellm.proxy.proxy_server import ProxyConfig, get_config_general_settings
pc = ProxyConfig()
pc._load_yaml_settings_stores({"general_settings": {"master_key": "os.environ/PROXY_MASTER_KEY"}})
pc.settings.apply_runtime_values({"master_key": "sk-resolved-secret"})
monkeypatch.setattr(proxy_server_module, "proxy_config", pc)
monkeypatch.setattr(proxy_server_module, "prisma_client", _fake_prisma_with_config({}))
admin = UserAPIKeyAuth(api_key="hashed-admin", user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN)
info = await get_config_general_settings(field_name="master_key", user_api_key_dict=admin)
assert info.field_value == "os.environ/PROXY_MASTER_KEY"
assert info.source == "config"
assert info.editable is False
@pytest.mark.asyncio
async def test_config_field_info_still_reports_a_database_owned_value(monkeypatch):
from litellm.proxy.proxy_server import ProxyConfig, get_config_general_settings
pc = ProxyConfig()
pc._load_yaml_settings_stores({"general_settings": {}})
pc.settings.apply_db_row("general_settings", {"max_request_size_mb": 42})
monkeypatch.setattr(proxy_server_module, "proxy_config", pc)
monkeypatch.setattr(proxy_server_module, "prisma_client", _fake_prisma_with_config({"max_request_size_mb": 42}))
admin = UserAPIKeyAuth(api_key="hashed-admin", user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN)
info = await get_config_general_settings(field_name="max_request_size_mb", user_api_key_dict=admin)
assert info.field_value == 42
assert info.source == "db"
@pytest.mark.asyncio
async def test_initialize_jwt_auth_leaves_the_declared_jwtauth_mapping_unresolved(monkeypatch):
from litellm.proxy.proxy_server import ProxyStartupEvent
declared = {"public_key_ttl": "600", "team_id_jwt_field": "os.environ/JWT_TEAM_FIELD"}
general_settings = {"litellm_jwtauth": declared}
monkeypatch.setattr(proxy_server_module, "get_secret", lambda value: "resolved-team-field")
ProxyStartupEvent._initialize_jwt_auth(
general_settings=general_settings,
prisma_client=None,
user_api_key_cache=DualCache(),
)
assert declared["team_id_jwt_field"] == "os.environ/JWT_TEAM_FIELD"
assert proxy_server_module.jwt_handler.litellm_jwtauth.team_id_jwt_field == "resolved-team-field"