fix(secret_managers): check vault rotation error return and mask raw API keys in logs

This commit is contained in:
michelligabriele 2026-03-20 21:20:00 +01:00
parent 50f88c8642
commit 476e11e207
4 changed files with 111 additions and 6 deletions

View file

@ -322,7 +322,7 @@ class KeyManagementEventHooks:
optional_params = await KeyManagementEventHooks._get_secret_manager_optional_params(
team_id
)
await litellm.secret_manager_client.async_rotate_secret(
result = await litellm.secret_manager_client.async_rotate_secret(
current_secret_name=KeyManagementEventHooks._get_secret_name(
current_secret_name
),
@ -332,6 +332,13 @@ class KeyManagementEventHooks:
new_secret_value=new_secret_value,
optional_params=optional_params,
)
if (
isinstance(result, dict)
and result.get("status") == "error"
):
raise ValueError(
f"Secret manager rotation failed: {result.get('message', 'unknown error')}"
)
@staticmethod
def _get_secret_name(secret_name: str) -> str:

View file

@ -537,12 +537,17 @@ class HashicorpSecretManager(BaseSecretManager):
json_resp.get("data", {}).get("data", {}).get(data_key, None)
)
if new_secret_value_from_vault != new_secret_value:
verbose_logger.exception(
f"New secret value mismatch. Expected: {new_secret_value}, Got: {new_secret_value_from_vault}"
# Mask key values to avoid logging raw API keys
masked_expected = f"...{new_secret_value[-4:]}" if new_secret_value else "<empty>"
masked_got = f"...{new_secret_value_from_vault[-4:]}" if new_secret_value_from_vault else "<empty>"
verbose_logger.error(
"New secret value mismatch. Expected: %s, Got: %s",
masked_expected,
masked_got,
)
return {
"status": "error",
"message": f"New secret value mismatch. Expected: {new_secret_value}, Got: {new_secret_value_from_vault}",
"message": f"New secret value mismatch. Expected: {masked_expected}, Got: {masked_got}",
}
except httpx.HTTPStatusError as e:
if e.response.status_code == 404:

View file

@ -759,4 +759,7 @@ async def test_hashicorp_secret_manager_rotate_secret_value_mismatch(hashicorp_s
# Verify error response
assert response["status"] == "error"
assert "mismatch" in response["message"].lower()
assert "expected-value" in response["message"]
# Raw key values should be masked — only last 4 chars shown
assert "expected-value" not in response["message"]
assert "...alue" in response["message"] # last 4 of "expected-value"
assert "...alue" in response["message"] # last 4 of "different-value"

View file

@ -385,6 +385,96 @@ class TestRotateVirtualKeyInSecretManager:
new_secret_value="sk-new-value",
team_id="team-123",
)
# Verify async_rotate_secret was NOT called
mock_secret_manager.async_rotate_secret.assert_not_called()
class TestRotateVirtualKeyErrorHandling:
"""Tests that _rotate_virtual_key_in_secret_manager propagates error dicts as exceptions."""
@pytest.mark.asyncio
async def test_rotate_raises_on_error_dict(self):
"""Test that error dict from async_rotate_secret is raised as ValueError."""
from litellm.types.secret_managers.main import KeyManagementSystem, KeyManagementSettings
from litellm.secret_managers.base_secret_manager import BaseSecretManager
import litellm
mock_secret_manager = MagicMock(spec=BaseSecretManager)
mock_secret_manager.async_rotate_secret = AsyncMock(
return_value={"status": "error", "message": "New secret value mismatch. Expected: ...3lcA, Got: ...zt4w"}
)
litellm.secret_manager_client = mock_secret_manager
litellm._key_management_system = KeyManagementSystem.HASHICORP_VAULT
litellm._key_management_settings = KeyManagementSettings(
store_virtual_keys=True,
prefix_for_stored_virtual_keys="litellm/",
)
import builtins
original_isinstance = builtins.isinstance
def mock_isinstance(obj, cls):
if cls == BaseSecretManager and obj == mock_secret_manager:
return True
return original_isinstance(obj, cls)
with patch.object(
KeyManagementEventHooks,
"_get_secret_manager_optional_params",
return_value=None,
), patch(
"litellm.proxy.hooks.key_management_event_hooks.isinstance",
side_effect=mock_isinstance,
):
with pytest.raises(ValueError, match="Secret manager rotation failed"):
await KeyManagementEventHooks._rotate_virtual_key_in_secret_manager(
current_secret_name="old-key",
new_secret_name="new-key",
new_secret_value="sk-new-value",
team_id=None,
)
@pytest.mark.asyncio
async def test_rotate_success_dict_does_not_raise(self):
"""Test that a success response from async_rotate_secret does not raise."""
from litellm.types.secret_managers.main import KeyManagementSystem, KeyManagementSettings
from litellm.secret_managers.base_secret_manager import BaseSecretManager
import litellm
mock_secret_manager = MagicMock(spec=BaseSecretManager)
mock_secret_manager.async_rotate_secret = AsyncMock(
return_value={"request_id": "abc", "data": {}}
)
litellm.secret_manager_client = mock_secret_manager
litellm._key_management_system = KeyManagementSystem.HASHICORP_VAULT
litellm._key_management_settings = KeyManagementSettings(
store_virtual_keys=True,
prefix_for_stored_virtual_keys="litellm/",
)
import builtins
original_isinstance = builtins.isinstance
def mock_isinstance(obj, cls):
if cls == BaseSecretManager and obj == mock_secret_manager:
return True
return original_isinstance(obj, cls)
with patch.object(
KeyManagementEventHooks,
"_get_secret_manager_optional_params",
return_value=None,
), patch(
"litellm.proxy.hooks.key_management_event_hooks.isinstance",
side_effect=mock_isinstance,
):
# Should not raise
await KeyManagementEventHooks._rotate_virtual_key_in_secret_manager(
current_secret_name="old-key",
new_secret_name="new-key",
new_secret_value="sk-new-value",
team_id=None,
)