From 0d8c9137fb118f873dfd5d73baf761bd85fcce55 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 15 May 2026 15:14:18 -0700 Subject: [PATCH] fix(proxy): make /config/update env-var encryption idempotent A single decrypt-then-encrypt chokepoint (_encrypt_env_variables_for_db) now backs both update_config and save_config. Re-submitting a value the Admin UI read back from /get/config/callbacks as ciphertext no longer stacks a second encryption layer, which previously decrypted to garbage and silently broke the callback. The chokepoint decrypts with the pure _decrypt_db_variables (no os.environ mutation on the write path) and encrypts exactly once; update_config merges only the sent keys so untouched env vars keep their stored ciphertext byte-for-byte. --- litellm/proxy/proxy_server.py | 54 +++++++++++++----- tests/test_litellm/proxy/test_proxy_server.py | 56 +++++++++++++++++++ 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 46ebff515b..268b244c86 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -3415,20 +3415,18 @@ class ProxyConfig: # Make a copy to avoid mutating the original config config_to_save = new_config.copy() - # SECURITY: Always encrypt environment_variables before DB write + # SECURITY: Always encrypt environment_variables before DB write. + # _encrypt_env_variables_for_db is idempotent — a caller that + # already encrypted the values (or re-submitted ciphertext read + # back from the DB) will not get a stacked second layer. if ( "environment_variables" in config_to_save and config_to_save["environment_variables"] ): - # decrypt the environment_variables - in case a caller function has already encrypted the environment_variables - decrypted_env_vars = self._decrypt_and_set_db_env_variables( - environment_variables=config_to_save["environment_variables"], - return_original_value=True, - ) - - # encrypt the environment_variables, - config_to_save["environment_variables"] = self._encrypt_env_variables( - environment_variables=decrypted_env_vars + config_to_save["environment_variables"] = ( + self._encrypt_env_variables_for_db( + environment_variables=config_to_save["environment_variables"] + ) ) config_to_save.pop("model_list", None) @@ -5076,6 +5074,29 @@ class ProxyConfig: decrypted_variables[k] = decrypted_value return decrypted_variables + def _encrypt_env_variables_for_db( + self, environment_variables: dict, new_encryption_key: Optional[str] = None + ) -> dict: + """ + Idempotently encrypt environment variables for a DB write. + + Config writers may pass either plaintext (first write) or values that + are already ciphertext — e.g. the Admin UI reads config back via + /get/config/callbacks (which returns the stored, still-encrypted + value) and re-POSTs it on the next save. Decrypt first so an + already-encrypted value is not stacked with a second encryption + layer, then encrypt exactly once. + + Decryption here deliberately uses _decrypt_db_variables (not + _decrypt_and_set_db_env_variables): this is a write path, and + loading values into os.environ is the read path's responsibility. + """ + decrypted_env_vars = self._decrypt_db_variables(environment_variables) + return self._encrypt_env_variables( + environment_variables=decrypted_env_vars, + new_encryption_key=new_encryption_key, + ) + @staticmethod def _parse_router_settings_value(value: Any) -> Optional[dict]: """ @@ -13788,11 +13809,18 @@ async def update_config( # noqa: PLR0915 existing[k] = v await _upsert_section("general_settings", existing) - # environment_variables: encrypt request values, then merge into existing. + # environment_variables: idempotently encrypt the request values + # (plaintext on first write, OR ciphertext the UI read back via + # /get/config/callbacks and re-submitted on save), then merge into + # existing. Only the sent keys are re-written; untouched keys keep + # their stored ciphertext byte-for-byte. if config_info.environment_variables is not None: existing = await _read_section("environment_variables") - for k, v in config_info.environment_variables.items(): - existing[k] = encrypt_value_helper(value=v) + existing.update( + proxy_config._encrypt_env_variables_for_db( + environment_variables=config_info.environment_variables + ) + ) await _upsert_section("environment_variables", existing) # litellm_settings: merge existing + request, request wins (matching diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index 6eade95701..be019b2a08 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -3850,6 +3850,62 @@ def test_update_config_fields_uppercases_env_vars(monkeypatch): assert os.environ.get("DD_SITE") == "us5.datadoghq.com" +def test_encrypt_env_variables_for_db_is_idempotent(monkeypatch): + """ + Regression: /config/update and save_config must not stack a second + encryption layer when a caller re-submits a value that is already + ciphertext (the Admin UI reads config back from /get/config/callbacks — + which returns the stored, still-encrypted value — and re-POSTs it on the + next save). _encrypt_env_variables_for_db must yield a value that decrypts + to the original plaintext in exactly ONE layer, no matter how many times + its own output is fed back in. It must also not mutate os.environ (write + path — loading into the process env is the read path's job). + """ + from litellm.proxy.common_utils.encrypt_decrypt_utils import ( + decrypt_value_helper, + ) + from litellm.proxy.proxy_server import ProxyConfig + + monkeypatch.setenv("LITELLM_SALT_KEY", "sk-test-salt-key") + monkeypatch.delenv("LANGFUSE_PUBLIC_KEY", raising=False) + + proxy_config = ProxyConfig() + plaintext = "pk-langfuse-secret-value" + + # First write: plaintext in -> single-encrypted out. + enc1 = proxy_config._encrypt_env_variables_for_db( + {"LANGFUSE_PUBLIC_KEY": plaintext} + ) + assert enc1["LANGFUSE_PUBLIC_KEY"] != plaintext + assert ( + decrypt_value_helper( + value=enc1["LANGFUSE_PUBLIC_KEY"], key="LANGFUSE_PUBLIC_KEY" + ) + == plaintext + ) + + # UI round-trip: feed the ciphertext back in. Must NOT double-encrypt. + enc2 = proxy_config._encrypt_env_variables_for_db(enc1) + assert ( + decrypt_value_helper( + value=enc2["LANGFUSE_PUBLIC_KEY"], key="LANGFUSE_PUBLIC_KEY" + ) + == plaintext + ) + + # And again — still exactly one layer, never stacked. + enc3 = proxy_config._encrypt_env_variables_for_db(enc2) + assert ( + decrypt_value_helper( + value=enc3["LANGFUSE_PUBLIC_KEY"], key="LANGFUSE_PUBLIC_KEY" + ) + == plaintext + ) + + # Write path must not leak the value into the process environment. + assert os.environ.get("LANGFUSE_PUBLIC_KEY") is None + + def test_get_prompt_spec_for_db_prompt_with_versions(): """ Test that _get_prompt_spec_for_db_prompt correctly converts database prompts