Skip to content

fix(proxy): make /config/update env-var encryption idempotent - #28022

Merged
yuneng-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_/hardcore-albattani-e592b4
May 15, 2026
Merged

fix(proxy): make /config/update env-var encryption idempotent#28022
yuneng-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_/hardcore-albattani-e592b4

Conversation

@yuneng-berri

@yuneng-berri yuneng-berri commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • POST /config/update re-encrypted environment_variables on every save with no decrypt-first guard. When the Admin UI read config back via /get/config/callbacks (which returns the stored, still-encrypted value) and re-POSTed it on the next save, the handler stacked a second encryption layer. The doubly-encrypted value decrypted to ciphertext at load time, was pushed into os.environ as the API key, and silently broke the callback.
  • The sibling path save_config was already idempotent (decrypt-then-encrypt); update_config had dropped that guard — the env-var write logic was duplicated and the two copies diverged.
  • This extracts a single idempotent chokepoint, ProxyConfig._encrypt_env_variables_for_db, and routes both update_config and save_config through it. It decrypts via the pure _decrypt_db_variables (no os.environ mutation on the write path) then encrypts exactly once. update_config merges only the sent keys, so untouched env vars keep their stored ciphertext byte-for-byte.

Test plan

  • New unit test test_encrypt_env_variables_for_db_is_idempotent — plaintext → ciphertext, then ciphertext fed back in ×3, asserts the stored value always decrypts to the original plaintext in exactly one layer, and that os.environ is not mutated on the write path.
  • pytest tests/test_litellm/proxy/test_proxy_server.py -k "encrypt_env_variables_for_db or update_config or env_var or environment" — 14 passed.
  • pytest tests/test_litellm/proxy/management_endpoints/test_config_override_endpoints.py test_delete_callbacks_endpoint.py — 6 passed.
  • End-to-end against a running proxy: re-submitting a stored ciphertext through POST /config/update now decrypts back to plaintext (failed before the change, passes after); first-write, plaintext re-save, and cross-section update adjacency all pass before and after.

Note

Medium Risk
Touches environment-variable encryption on config write paths; a mistake could corrupt stored secrets or break integrations relying on decrypted values.

Overview
Fixes a double-encryption bug when updating environment_variables via POST /config/update by routing writes through a new idempotent helper, ProxyConfig._encrypt_env_variables_for_db, which decrypts-then-encrypts once and avoids mutating os.environ on the write path.

Updates save_config to use the same helper and adjusts /config/update merging so only provided keys are re-written while untouched env vars remain byte-identical in the DB. Adds unit and endpoint-level regression tests to ensure repeated ciphertext round-trips never stack encryption layers.

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

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.
@greptile-apps

greptile-apps Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes double-encryption of environment_variables in POST /config/update by introducing a single idempotent write helper, ProxyConfig._encrypt_env_variables_for_db, that always decrypts first (via _decrypt_db_variables with return_original_value=True) before encrypting, so re-submitting stored ciphertext never stacks a second layer.

  • Both save_config and update_config now share the same chokepoint, closing the divergence that caused the bug; update_config still only rewrites sent keys so untouched ciphertext is preserved byte-for-byte.
  • A focused regression test covers first-write, ciphertext round-trip ×3, and confirms the write path does not mutate os.environ.

Confidence Score: 4/5

Safe to merge; the fix is narrowly scoped to the env-var encryption write path and does not touch auth, routing, or any other critical proxy logic.

The decrypt-then-encrypt pattern is correct: _decrypt_db_variables uses return_original_value=True, so plaintext values that fail decryption fall back to the original string and are then encrypted once, while existing ciphertext is properly unwrapped before being re-encrypted. The only subtle invariant is that idempotency relies on return_original_value=True being hardcoded inside _decrypt_db_variables — a future change to that default would silently break the guarantee, but that is a future maintenance concern rather than a current defect.

No files require special attention; both changed files are self-contained and the new test covers the key edge cases.

Important Files Changed

Filename Overview
litellm/proxy/proxy_server.py Introduces _encrypt_env_variables_for_db (decrypt-then-encrypt chokepoint) and routes both save_config and update_config through it, eliminating the double-encryption stacking bug; logic is sound and backward-compatible.
tests/test_litellm/proxy/test_proxy_server.py Adds test_encrypt_env_variables_for_db_is_idempotent — a pure mock test that verifies plaintext→ciphertext, ciphertext re-feed ×3, and no os.environ mutation; no real network calls, matches existing test-file conventions.

Reviews (1): Last reviewed commit: "fix(proxy): make /config/update env-var ..." | Re-trigger Greptile

@codecov

codecov Bot commented May 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

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

📢 Thoughts on this report? Let us know!

…encryption

Adds test_update_config_env_var_round_trip_not_double_encrypted, which
drives the real /config/update handler: first write plaintext, then
re-POST the stored ciphertext (the Admin UI round-trip) and assert the
value is not stacked with a second encryption layer and untouched keys
stay byte-identical. Verified to fail against the pre-fix handler and
pass after. Also tightens the unit test to exactly three ciphertext
re-feeds.
@yuneng-berri
yuneng-berri merged commit 361a84c into litellm_internal_staging May 15, 2026
116 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_/hardcore-albattani-e592b4 branch May 15, 2026 23:15
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
…tani-e592b4

fix(proxy): make /config/update env-var encryption idempotent
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.

2 participants