mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-21 00:21:49 +00:00
fix(proxy): refuse config-owned keys on POST /config/update
POST /config/update stored keys the config file owns and answered 200 while the file silently kept winning. Run the config-owned check for general_settings, litellm_settings, and router_settings before the first database read, the way /config/field/update already does, so a refused request stores nothing. The config reload re-loaded the merged settings as yaml settings, which turned every saved router setting read-only after one tick. Read the saved router settings row instead so database-owned values stay writable. The integration harness seeds num_retries through /config/update instead of the config file, which is what the effective-settings and observed routing tests need to keep exercising a database-owned value.
This commit is contained in:
parent
d42f448e41
commit
7cfec730d5
5 changed files with 123 additions and 34 deletions
|
|
@ -125,6 +125,9 @@ start_proxy() {
|
|||
start_proxy 4000 proxy.log
|
||||
proxy_pid="$launched_pid"
|
||||
.venv/bin/python .circleci/scripts/wait_integration_services.py
|
||||
curl --noproxy '*' -sSf -X POST "$INTEGRATION_PROXY_URL/config/update" \
|
||||
-H "Authorization: Bearer $LITELLM_MASTER_KEY" -H 'Content-Type: application/json' \
|
||||
-d '{"router_settings": {"num_retries": 0}}' > "$results/seed-router-settings.json"
|
||||
if [ "$suite" = management ]; then
|
||||
export INTEGRATION_PEER_URL=http://127.0.0.1:4001
|
||||
start_proxy 4001 peer.log
|
||||
|
|
|
|||
|
|
@ -6869,9 +6869,7 @@ class ProxyConfig:
|
|||
self._add_callbacks_from_db_config(config_data)
|
||||
|
||||
# router settings
|
||||
await self._add_router_settings_from_db_config(
|
||||
config_data=config_data, llm_router=llm_router, prisma_client=prisma_client
|
||||
)
|
||||
await self._add_router_settings_from_db_config(llm_router=llm_router, prisma_client=prisma_client)
|
||||
|
||||
return still_desired_ids
|
||||
|
||||
|
|
@ -7079,13 +7077,11 @@ class ProxyConfig:
|
|||
|
||||
async def _add_router_settings_from_db_config(
|
||||
self,
|
||||
config_data: Mapping[str, object],
|
||||
llm_router: Router | None,
|
||||
prisma_client: PrismaClient | None,
|
||||
) -> None:
|
||||
if llm_router is None or prisma_client is None:
|
||||
return
|
||||
self.router_settings.load_yaml(_as_settings_mapping(config_data.get("router_settings")))
|
||||
db_router_settings: Final[_ConfigParamRow | None] = await _config_param_table(prisma_client).find_first(
|
||||
where={"param_name": "router_settings"}
|
||||
)
|
||||
|
|
@ -16944,6 +16940,46 @@ async def update_config(
|
|||
if prisma_client is None:
|
||||
raise Exception("No DB Connected")
|
||||
|
||||
requested_general_settings: Final[Mapping[str, JsonValue]] = (
|
||||
config_info.general_settings.model_dump(exclude_none=True, exclude_unset=True)
|
||||
if config_info.general_settings is not None
|
||||
else {}
|
||||
)
|
||||
raw_litellm_settings: Final[Mapping[str, JsonValue]] = _CONFIG_SECTION_VALUES.validate_python(
|
||||
config_info.litellm_settings if config_info.litellm_settings is not None else {}
|
||||
)
|
||||
incoming_success_callback: Final = raw_litellm_settings.get("success_callback")
|
||||
updated_litellm_settings: Final[Mapping[str, JsonValue]] = _CONFIG_SECTION_VALUES.validate_python(
|
||||
{
|
||||
**raw_litellm_settings,
|
||||
**(
|
||||
{"success_callback": normalize_callback_names(incoming_success_callback)}
|
||||
if isinstance(incoming_success_callback, list)
|
||||
else {}
|
||||
),
|
||||
}
|
||||
)
|
||||
typed_router_settings: Final[Mapping[str, JsonValue]] = (
|
||||
config_info.router_settings.model_dump(exclude_none=True) if config_info.router_settings is not None else {}
|
||||
)
|
||||
router_settings_updates: Final[Mapping[str, JsonValue]] = {
|
||||
**typed_router_settings,
|
||||
**(
|
||||
{
|
||||
key: value
|
||||
for key, value in raw_router_settings.items()
|
||||
if key not in typed_router_settings and value is not None
|
||||
}
|
||||
if isinstance(raw_router_settings, dict)
|
||||
else {}
|
||||
),
|
||||
}
|
||||
proxy_config.reject_config_owned_writes(
|
||||
section_name="general_settings", changed_keys=requested_general_settings
|
||||
)
|
||||
proxy_config.reject_config_owned_writes(section_name="litellm_settings", changed_keys=updated_litellm_settings)
|
||||
proxy_config.reject_config_owned_writes(section_name="router_settings", changed_keys=router_settings_updates)
|
||||
|
||||
async def _read_section(param_name: str) -> dict:
|
||||
row: Final[_ConfigParamRow | None] = await _config_param_table(prisma_client).find_first(
|
||||
where={"param_name": param_name}
|
||||
|
|
@ -17014,15 +17050,9 @@ async def update_config(
|
|||
if config_info.litellm_settings is not None:
|
||||
existing = await _read_section("litellm_settings")
|
||||
before_litellm_settings: Final = copy.deepcopy(existing)
|
||||
updated_litellm_settings: Final = dict(config_info.litellm_settings)
|
||||
|
||||
incoming_cb = updated_litellm_settings.get("success_callback")
|
||||
if isinstance(incoming_cb, list):
|
||||
updated_litellm_settings["success_callback"] = normalize_callback_names(incoming_cb)
|
||||
|
||||
merged: Final = {**existing, **updated_litellm_settings}
|
||||
|
||||
incoming_cb = updated_litellm_settings.get("success_callback")
|
||||
incoming_cb: Final = updated_litellm_settings.get("success_callback")
|
||||
existing_cb: Final = existing.get("success_callback")
|
||||
if isinstance(incoming_cb, list):
|
||||
if isinstance(existing_cb, list):
|
||||
|
|
@ -17045,15 +17075,6 @@ async def update_config(
|
|||
if isinstance(raw_router_settings, dict):
|
||||
existing = await _read_section("router_settings")
|
||||
before_router_settings: Final = copy.deepcopy(existing)
|
||||
typed_router_settings: Final = (
|
||||
config_info.router_settings.dict(exclude_none=True) if config_info.router_settings is not None else {}
|
||||
)
|
||||
raw_router_settings_without_none: Final = {
|
||||
key: value
|
||||
for key, value in raw_router_settings.items()
|
||||
if key not in typed_router_settings and value is not None
|
||||
}
|
||||
router_settings_updates: Final = {**typed_router_settings, **raw_router_settings_without_none}
|
||||
new_router_settings: Final = {**existing, **router_settings_updates}
|
||||
await _upsert_section("router_settings", new_router_settings)
|
||||
asyncio.create_task(
|
||||
|
|
|
|||
|
|
@ -13,5 +13,4 @@ litellm_settings:
|
|||
host: os.environ/REDIS_HOST
|
||||
port: os.environ/REDIS_PORT
|
||||
router_settings:
|
||||
num_retries: 0
|
||||
disable_cooldowns: true
|
||||
|
|
|
|||
|
|
@ -139,6 +139,43 @@ def test_config_update_persists_disable_cooldowns(client, auth_as, mock_prisma,
|
|||
assert persisted["disable_cooldowns"] is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("section", "store_attr", "yaml_values", "changed_values"),
|
||||
[
|
||||
("general_settings", "settings", {"alerting": ["slack"]}, {"alerting": ["email"]}),
|
||||
("litellm_settings", "litellm_settings", {"success_callback": ["langfuse"]}, {"success_callback": ["otel"]}),
|
||||
("router_settings", "router_settings", {"num_retries": 0}, {"num_retries": 2}),
|
||||
],
|
||||
)
|
||||
def test_config_update_rejects_config_owned_keys_and_accepts_the_same_value(
|
||||
client, auth_as, mock_prisma, monkeypatch, section, store_attr, yaml_values, changed_values
|
||||
):
|
||||
from litellm.proxy import proxy_server as ps
|
||||
from litellm.proxy._types import LitellmUserRoles
|
||||
|
||||
table = _install_litellm_config(mock_prisma)
|
||||
monkeypatch.setattr(ps, "prisma_client", mock_prisma)
|
||||
monkeypatch.setattr(ps.proxy_config, "add_deployment", AsyncMock())
|
||||
store = getattr(ps.proxy_config, store_attr)
|
||||
store.load_yaml(yaml_values)
|
||||
try:
|
||||
with auth_as(LitellmUserRoles.PROXY_ADMIN):
|
||||
rejected = client.post("/config/update", json={section: changed_values})
|
||||
rejected_message = rejected.json()["error"]["message"]
|
||||
table.upsert.assert_not_called()
|
||||
accepted = client.post("/config/update", json={section: yaml_values})
|
||||
finally:
|
||||
store.load_yaml({})
|
||||
|
||||
assert rejected.status_code == 400
|
||||
assert f"{section} key '{next(iter(yaml_values))}' is set in the config file and cannot be changed here" in (
|
||||
rejected_message
|
||||
)
|
||||
assert accepted.status_code == 200
|
||||
persisted = json.loads(table.upsert.call_args.kwargs["data"]["create"]["param_value"])
|
||||
assert persisted[next(iter(yaml_values))] == yaml_values[next(iter(yaml_values))]
|
||||
|
||||
|
||||
def test_config_update_rejects_assistants_config(client, auth_as, mock_prisma, monkeypatch):
|
||||
from litellm.proxy import proxy_server as ps
|
||||
from litellm.proxy._types import LitellmUserRoles
|
||||
|
|
|
|||
|
|
@ -4919,8 +4919,8 @@ async def test_add_router_settings_from_db_config_merge_logic():
|
|||
mock_prisma_client.db.litellm_config.find_first = AsyncMock(return_value=mock_db_config)
|
||||
|
||||
# Call the method under test
|
||||
proxy_config.router_settings.load_yaml(config_data["router_settings"])
|
||||
await proxy_config._add_router_settings_from_db_config(
|
||||
config_data=config_data,
|
||||
llm_router=mock_router,
|
||||
prisma_client=mock_prisma_client,
|
||||
)
|
||||
|
|
@ -4980,8 +4980,8 @@ async def test_add_router_settings_from_db_config_empty_db_lists_do_not_clobber_
|
|||
mock_prisma_client = MagicMock()
|
||||
mock_prisma_client.db.litellm_config.find_first = AsyncMock(return_value=mock_db_config)
|
||||
|
||||
proxy_config.router_settings.load_yaml(config_data["router_settings"])
|
||||
await proxy_config._add_router_settings_from_db_config(
|
||||
config_data=config_data,
|
||||
llm_router=mock_router,
|
||||
prisma_client=mock_prisma_client,
|
||||
)
|
||||
|
|
@ -5017,8 +5017,8 @@ async def test_add_router_settings_from_db_config_empty_db_list_still_clears_unc
|
|||
mock_prisma_client = MagicMock()
|
||||
mock_prisma_client.db.litellm_config.find_first = AsyncMock(return_value=mock_db_config)
|
||||
|
||||
proxy_config.router_settings.load_yaml(config_data["router_settings"])
|
||||
await proxy_config._add_router_settings_from_db_config(
|
||||
config_data=config_data,
|
||||
llm_router=mock_router,
|
||||
prisma_client=mock_prisma_client,
|
||||
)
|
||||
|
|
@ -5042,8 +5042,8 @@ async def test_add_router_settings_from_db_config_edge_cases():
|
|||
mock_router.update_settings = MagicMock()
|
||||
|
||||
# Test Case 1: No router provided
|
||||
proxy_config.router_settings.load_yaml({"test": "value"})
|
||||
await proxy_config._add_router_settings_from_db_config(
|
||||
config_data={"router_settings": {"test": "value"}},
|
||||
llm_router=None,
|
||||
prisma_client=MagicMock(),
|
||||
)
|
||||
|
|
@ -5051,8 +5051,8 @@ async def test_add_router_settings_from_db_config_edge_cases():
|
|||
mock_router.update_settings.assert_not_called()
|
||||
|
||||
# Test Case 2: No prisma client provided
|
||||
proxy_config.router_settings.load_yaml({"test": "value"})
|
||||
await proxy_config._add_router_settings_from_db_config(
|
||||
config_data={"router_settings": {"test": "value"}},
|
||||
llm_router=mock_router,
|
||||
prisma_client=None,
|
||||
)
|
||||
|
|
@ -5065,8 +5065,8 @@ async def test_add_router_settings_from_db_config_edge_cases():
|
|||
|
||||
config_data = {"router_settings": {"routing_strategy": "usage-based"}}
|
||||
|
||||
proxy_config.router_settings.load_yaml(config_data["router_settings"])
|
||||
await proxy_config._add_router_settings_from_db_config(
|
||||
config_data=config_data,
|
||||
llm_router=mock_router,
|
||||
prisma_client=mock_prisma_client,
|
||||
)
|
||||
|
|
@ -5080,8 +5080,8 @@ async def test_add_router_settings_from_db_config_edge_cases():
|
|||
mock_db_config.param_value = {"db_setting": "db_value"}
|
||||
mock_prisma_client.db.litellm_config.find_first = AsyncMock(return_value=mock_db_config)
|
||||
|
||||
proxy_config.router_settings.load_yaml({})
|
||||
await proxy_config._add_router_settings_from_db_config(
|
||||
config_data={}, # No router_settings in config
|
||||
llm_router=mock_router,
|
||||
prisma_client=mock_prisma_client,
|
||||
)
|
||||
|
|
@ -5093,9 +5093,8 @@ async def test_add_router_settings_from_db_config_edge_cases():
|
|||
# Test Case 5: Both config and DB router_settings are None/empty
|
||||
mock_prisma_client.db.litellm_config.find_first = AsyncMock(return_value=None)
|
||||
|
||||
await proxy_config._add_router_settings_from_db_config(
|
||||
config_data={}, llm_router=mock_router, prisma_client=mock_prisma_client
|
||||
)
|
||||
proxy_config.router_settings.load_yaml({})
|
||||
await proxy_config._add_router_settings_from_db_config(llm_router=mock_router, prisma_client=mock_prisma_client)
|
||||
|
||||
# Should not call update_settings when no settings exist
|
||||
mock_router.update_settings.assert_not_called()
|
||||
|
|
@ -5107,8 +5106,8 @@ async def test_add_router_settings_from_db_config_edge_cases():
|
|||
|
||||
config_data = {"router_settings": {"config_setting": "config_value"}}
|
||||
|
||||
proxy_config.router_settings.load_yaml(config_data["router_settings"])
|
||||
await proxy_config._add_router_settings_from_db_config(
|
||||
config_data=config_data,
|
||||
llm_router=mock_router,
|
||||
prisma_client=mock_prisma_client,
|
||||
)
|
||||
|
|
@ -5157,8 +5156,8 @@ async def test_add_router_settings_shallow_merge_behavior():
|
|||
mock_prisma_client = MagicMock()
|
||||
mock_prisma_client.db.litellm_config.find_first = AsyncMock(return_value=mock_db_config)
|
||||
|
||||
proxy_config.router_settings.load_yaml(config_data["router_settings"])
|
||||
await proxy_config._add_router_settings_from_db_config(
|
||||
config_data=config_data,
|
||||
llm_router=mock_router,
|
||||
prisma_client=mock_prisma_client,
|
||||
)
|
||||
|
|
@ -5180,6 +5179,36 @@ async def test_add_router_settings_shallow_merge_behavior():
|
|||
assert merged_settings["top_level"] == "config_top"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_router_settings_reload_keeps_db_values_writable(tmp_path, monkeypatch):
|
||||
from litellm.proxy.proxy_server import ProxyConfig
|
||||
|
||||
config_path: Final = tmp_path / "config.yaml"
|
||||
config_path.write_text(yaml.safe_dump({"model_list": [], "router_settings": {"disable_cooldowns": True}}))
|
||||
db_row: Final = types.SimpleNamespace(param_value={"num_retries": 0})
|
||||
|
||||
async def read_config_row(_prisma_client, param_name):
|
||||
return db_row if param_name == "router_settings" else None
|
||||
|
||||
mock_prisma_client: Final = MagicMock()
|
||||
mock_prisma_client.db.litellm_config.find_first = AsyncMock(return_value=db_row)
|
||||
mock_router: Final = MagicMock()
|
||||
monkeypatch.setattr(proxy_server_module, "get_config_param", read_config_row)
|
||||
monkeypatch.setattr(proxy_server_module, "prisma_client", mock_prisma_client)
|
||||
monkeypatch.setattr(proxy_server_module, "store_model_in_db", True)
|
||||
monkeypatch.setattr(proxy_server_module, "user_config_file_path", None)
|
||||
proxy_config: Final = ProxyConfig()
|
||||
|
||||
for _ in range(2):
|
||||
await proxy_config.get_config(config_file_path=str(config_path))
|
||||
await proxy_config._add_router_settings_from_db_config(llm_router=mock_router, prisma_client=mock_prisma_client)
|
||||
|
||||
assert mock_router.update_settings.call_args.kwargs == {"disable_cooldowns": True, "num_retries": 0}
|
||||
assert proxy_config.router_settings.source("num_retries") == "db"
|
||||
assert proxy_config.router_settings.rejected_writes({"num_retries": 3}) == ()
|
||||
assert proxy_config.router_settings.rejected_writes({"disable_cooldowns": False}) == ("disable_cooldowns",)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_model_info_v1_oci_secrets_not_leaked():
|
||||
"""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue