Skip to content

fix: stale config state + admin role check + inline imports#2

Merged
bussyjd merged 1 commit intomainfrom
feat/config-only-model-management
Apr 9, 2026
Merged

fix: stale config state + admin role check + inline imports#2
bussyjd merged 1 commit intomainfrom
feat/config-only-model-management

Conversation

@bussyjd
Copy link
Copy Markdown
Collaborator

@bussyjd bussyjd commented Apr 9, 2026

Addresses review feedback from BerriAI#25413 (Greptile bot):

  • P1 (data loss): Call proxy_config.update_config_state() after save_config() in all three config-only helpers. Without this, sequential model operations silently drop earlier changes because get_config_state() returns stale startup config.
  • P2 (style): Move ModelInfo to module-level imports, remove two inline imports.
  • P3 (security): Add PROXY_ADMIN role check in all four config-only code paths. user_api_key_auth validates the key but not the role.

All 41 tests pass.

Address review feedback from BerriAI#25413:

- P1: Call proxy_config.update_config_state() after save_config() in all
  three config-only helpers. Without this, sequential model operations
  in the same session silently drop earlier changes because
  get_config_state() returns the stale startup config.

- P2: Move ModelInfo import to the module-level import block. Remove
  two inline `from litellm.types.router import ModelInfo` statements
  in patch_model and _update_team_model_in_db.

- P3: Add PROXY_ADMIN role check in all four config-only code paths
  (add_new_model, delete_model, update_model, patch_model). The
  user_api_key_auth dependency validates the key but not the role —
  a non-admin virtual key holder could manage models without this guard.
@bussyjd bussyjd merged commit 778111d into main Apr 9, 2026
31 of 36 checks passed
bussyjd added a commit to ObolNetwork/obol-stack that referenced this pull request Apr 9, 2026
Includes fixes from ObolNetwork/litellm#2:
- P1: stale in-memory config after save_config (sequential write data loss)
- P2: inline ModelInfo imports moved to module-level
- P3: PROXY_ADMIN role check in config-only code paths
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.

1 participant