From 15aad0c17d57740e0f442128c8b0fd69592f4c97 Mon Sep 17 00:00:00 2001 From: jesus Date: Tue, 3 Nov 2026 22:54:57 +0000 Subject: [PATCH] fix(usage-export): drop callback and SCIM slots from both metadata objects at any depth and mask camelCase credential names --- .../sensitive_data_masker.py | 63 ++++++++++++------- .../repositories/daily_activity_repository.py | 14 +++-- litellm/repositories/daily_activity_sql.py | 5 +- tests/e2e/models.py | 1 + .../test_usage_export_metadata_e2e.py | 20 ++++-- .../spend/_daily_activity_fixtures.py | 4 +- .../spend/test_daily_activity_repository.py | 1 + .../test_sensitive_data_masker.py | 42 +++++++++++++ .../test_daily_activity_repository.py | 22 +++++-- .../repositories/test_daily_activity_sql.py | 12 ++-- 10 files changed, 139 insertions(+), 45 deletions(-) diff --git a/litellm/litellm_core_utils/sensitive_data_masker.py b/litellm/litellm_core_utils/sensitive_data_masker.py index 8c402fe9b44..2cf8899cf3a 100644 --- a/litellm/litellm_core_utils/sensitive_data_masker.py +++ b/litellm/litellm_core_utils/sensitive_data_masker.py @@ -1,3 +1,4 @@ +import re from collections.abc import ItemsView, Mapping, Sequence from collections.abc import Set as AbstractSet from dataclasses import dataclass, field @@ -30,6 +31,9 @@ _DEFAULT_SENSITIVE_PATTERNS: Final = frozenset( ) +_CAMEL_BOUNDARY: Final = re.compile(r"(?<=[a-z0-9])(?=[A-Z])") + + def _object_items(data: Mapping[str, object]) -> ItemsView[str, object]: return data.items() @@ -51,7 +55,9 @@ class SensitiveDataMasker: # If any key segment matches one of these, the key is not considered sensitive # even if it also matches a sensitive pattern. For example, "input_cost_per_token" # contains "token" but "cost" overrides that — it's a pricing field, not a secret. - self.non_sensitive_overrides = non_sensitive_overrides or frozenset(("cost",)) + self.non_sensitive_overrides = ( + non_sensitive_overrides if non_sensitive_overrides is not None else frozenset(("cost",)) + ) self.visible_prefix = visible_prefix self.visible_suffix = visible_suffix @@ -82,10 +88,10 @@ class SensitiveDataMasker: if excluded_keys and key in excluded_keys: return False - key_lower: Final = str(key).lower() - # Split on underscores/hyphens and check if any segment matches the pattern + # Split on underscores, hyphens and camelCase boundaries and check if any segment matches the pattern # This avoids false positives like "max_tokens" matching "token" - # but still catches "api_key", "access_token", etc. + # but still catches "api_key", "access_token", "apiKey", "clientSecret", etc. + key_lower: Final = _CAMEL_BOUNDARY.sub("_", str(key)).lower() key_segments: Final = key_lower.replace("-", "_").split("_") # If any segment matches a non-sensitive override, the key is not sensitive. @@ -255,10 +261,16 @@ def mask_sensitive_keys(data: Mapping[str, object], sensitive_fields: set[str]) return masked -def redact_credentials_in_payload(data: Mapping[str, object]) -> Mapping[str, object]: +def redact_credentials_in_payload( + data: Mapping[str, object], + masker: SensitiveDataMasker = _default_masker, + *, + dropped_keys: AbstractSet[str] = frozenset(), +) -> Mapping[str, object]: """Return a copy of ``data`` where every value under a credential-named key is - replaced by the shared ``REDACTED`` marker, nested mappings are recursed into, - and every other value is preserved by identity. + replaced by the shared ``REDACTED`` marker, every entry whose key is in + ``dropped_keys`` is removed outright at any depth, nested mappings are recursed + into, and every other value is preserved by identity. Sensitive-key detection is delegated to the shared :class:`SensitiveDataMasker`, so the credential names stay in one place. Unlike @@ -271,28 +283,31 @@ def redact_credentials_in_payload(data: Mapping[str, object]) -> Mapping[str, ob rather than passed through, so burying a credential deeper than the walk goes hides it instead of exposing it. """ - return _redact_mapping(data, 0) + return _PayloadRedactor(masker, frozenset(dropped_keys)).mapping(data, 0) -def _redact_mapping(data: Mapping[str, object], depth: int) -> Mapping[str, object]: - return {key: _redact_entry(key, value, depth) for key, value in data.items()} +@dataclass(frozen=True, slots=True) +class _PayloadRedactor: + masker: SensitiveDataMasker + dropped_keys: frozenset[str] + def mapping(self, data: Mapping[str, object], depth: int) -> Mapping[str, object]: + return {key: self.entry(key, value, depth) for key, value in data.items() if key not in self.dropped_keys} -def _redact_entry(key: str, value: object, depth: int) -> object: - if value is not None and _default_masker.is_sensitive_key(key): - return REDACTED - if not isinstance(value, (Mapping, list, tuple)): - return value - if depth >= DEFAULT_MAX_RECURSE_DEPTH: - return REDACTED - if isinstance(value, Mapping): - return _redact_mapping(value, depth + 1) - return _redact_sequence(value, depth + 1) + def entry(self, key: str, value: object, depth: int) -> object: + if value is not None and self.masker.is_sensitive_key(key): + return REDACTED + if not isinstance(value, (Mapping, list, tuple)): + return value + if depth >= DEFAULT_MAX_RECURSE_DEPTH: + return REDACTED + if isinstance(value, Mapping): + return self.mapping(value, depth + 1) + return self.sequence(value, depth + 1) - -def _redact_sequence(values: Sequence[object], depth: int) -> Sequence[object]: - redacted: Final = tuple(_redact_entry("", item, depth) for item in values) - return redacted if isinstance(values, tuple) else list(redacted) + def sequence(self, values: Sequence[object], depth: int) -> Sequence[object]: + redacted: Final = tuple(self.entry("", item, depth) for item in values) + return redacted if isinstance(values, tuple) else list(redacted) # Usage example: diff --git a/litellm/repositories/daily_activity_repository.py b/litellm/repositories/daily_activity_repository.py index 0527def01a5..ae4d9992036 100644 --- a/litellm/repositories/daily_activity_repository.py +++ b/litellm/repositories/daily_activity_repository.py @@ -12,9 +12,10 @@ from typing_extensions import assert_never from litellm import constants from litellm._logging import verbose_proxy_logger from litellm.constants import UNKNOWN_MODEL_SPEND_LOG_MODEL -from litellm.litellm_core_utils.sensitive_data_masker import redact_credentials_in_payload +from litellm.litellm_core_utils.sensitive_data_masker import SensitiveDataMasker, redact_credentials_in_payload from litellm.repositories.chunked_in import find_many_in from litellm.repositories.daily_activity_sql import ( + EXPORT_METADATA_DROPPED_KEYS, ExportCursor, SqlQuery, adjust_dates_for_timezone, @@ -142,13 +143,18 @@ def _daily_rows_table( raise AssertionError("unreachable") +_EXPORT_METADATA_MASKER: Final = SensitiveDataMasker(non_sensitive_overrides=frozenset()) + + +def _redacted_metadata(metadata: Mapping[str, object]) -> Mapping[str, object]: + return redact_credentials_in_payload(metadata, _EXPORT_METADATA_MASKER, dropped_keys=EXPORT_METADATA_DROPPED_KEYS) + + def _with_redacted_metadata(row: ExportRow) -> ExportRow: if not row.key_metadata and not row.user_metadata: return row return replace( - row, - key_metadata=redact_credentials_in_payload(row.key_metadata), - user_metadata=redact_credentials_in_payload(row.user_metadata), + row, key_metadata=_redacted_metadata(row.key_metadata), user_metadata=_redacted_metadata(row.user_metadata) ) diff --git a/litellm/repositories/daily_activity_sql.py b/litellm/repositories/daily_activity_sql.py index 756bba85278..055135b22c0 100644 --- a/litellm/repositories/daily_activity_sql.py +++ b/litellm/repositories/daily_activity_sql.py @@ -537,9 +537,10 @@ def _metadata_object_field(expression: str, redacted_keys: frozenset[str], alias _EMPTY_KEY_METADATA_FIELD: Final = "'{}'::jsonb AS key_metadata" _EMPTY_USER_METADATA_FIELD: Final = "'{}'::jsonb AS user_metadata" _EMPTY_METADATA_FIELDS: Final = f", {_EMPTY_KEY_METADATA_FIELD}, {_EMPTY_USER_METADATA_FIELD}" -_USER_METADATA_FIELD: Final = _metadata_object_field("u.metadata", SCIM_DIRECTORY_METADATA_KEYS, "user_metadata") +EXPORT_METADATA_DROPPED_KEYS: Final[frozenset[str]] = CALLBACK_CONFIG_METADATA_SLOTS | SCIM_DIRECTORY_METADATA_KEYS +_USER_METADATA_FIELD: Final = _metadata_object_field("u.metadata", EXPORT_METADATA_DROPPED_KEYS, "user_metadata") _KEY_METADATA_FIELD: Final = _metadata_object_field( - "COALESCE(vt.metadata, dvt.metadata)", CALLBACK_CONFIG_METADATA_SLOTS, "key_metadata" + "COALESCE(vt.metadata, dvt.metadata)", EXPORT_METADATA_DROPPED_KEYS, "key_metadata" ) diff --git a/tests/e2e/models.py b/tests/e2e/models.py index 7467d2fd085..776faf33966 100644 --- a/tests/e2e/models.py +++ b/tests/e2e/models.py @@ -60,6 +60,7 @@ class KeyMetadata(BaseModel): tag: str | None = None guardrails: list[str] | None = None webhook_token: str | None = None + apiKey: str | None = None class ObjectPermission(BaseModel): diff --git a/tests/e2e/quota_management/spend_tracking/test_usage_export_metadata_e2e.py b/tests/e2e/quota_management/spend_tracking/test_usage_export_metadata_e2e.py index 2e27dbc9a0d..6c908915034 100644 --- a/tests/e2e/quota_management/spend_tracking/test_usage_export_metadata_e2e.py +++ b/tests/e2e/quota_management/spend_tracking/test_usage_export_metadata_e2e.py @@ -47,11 +47,13 @@ class UsageExportParams(BaseModel, frozen=True): class UsageExportKeyMetadata(BaseModel, frozen=True): tag: str | None = None webhook_token: str | None = None + apiKey: str | None = None class UsageExportUserMetadata(BaseModel, frozen=True): department: str | None = None ldap_password: str | None = None + clientSecret: str | None = None class UsageExportRow(BaseModel, frozen=True): @@ -107,7 +109,11 @@ class TestUsageExportMetadata: user_email=f"e2e-usage-export-{marker}@example.com", user_role="internal_user", user_id=f"e2e-usage-export-{marker}", - metadata={"department": f"research-{marker}", "ldap_password": f"pw-{marker}"}, + metadata={ + "department": f"research-{marker}", + "ldap_password": f"pw-{marker}", + "clientSecret": f"cs-{marker}", + }, ), response_type=UserNewResponse, ) @@ -118,7 +124,9 @@ class TestUsageExportMetadata: user_id=user_id, models=[model], key_alias=f"e2e-usage-export-{marker}", - metadata=KeyMetadata(tag=f"cost-center-{marker}", webhook_token=f"whsec-{marker}"), + metadata=KeyMetadata( + tag=f"cost-center-{marker}", webhook_token=f"whsec-{marker}", apiKey=f"ak-{marker}" + ), ) ) resources.defer(lambda: client.proxy.delete_key(record.key)) @@ -160,9 +168,13 @@ class TestUsageExportMetadata: assert row.user_metadata.department == f"research-{marker}", ( f"user metadata not exported: {row.user_metadata}" ) - assert (row.key_metadata.webhook_token, row.user_metadata.ldap_password) == ("REDACTED", "REDACTED"), ( - f"credential-named metadata values reached the export unmasked: {row}" + masked = ( + row.key_metadata.webhook_token, + row.key_metadata.apiKey, + row.user_metadata.ldap_password, + row.user_metadata.clientSecret, ) + assert masked == ("REDACTED",) * 4, f"credential-named metadata values reached the export unmasked: {row}" own_view: Final = client.proxy.transport.probe( EXPORT_ROUTE, diff --git a/tests/integration/spend/_daily_activity_fixtures.py b/tests/integration/spend/_daily_activity_fixtures.py index 85c434c6877..515457126d5 100644 --- a/tests/integration/spend/_daily_activity_fixtures.py +++ b/tests/integration/spend/_daily_activity_fixtures.py @@ -177,6 +177,7 @@ def seed_daily_activity_fixture(connection: psycopg.Connection, *, schema: str, "team-1", "user-1", '{"tags": ["blue", "gold"], "cost_center": "cc-1", "webhook_token": "whsec_1", ' + '"scim_roles": ["r"], "langfuse": {"apiKey": "lf", "logging": [{"callback_name": "langfuse"}]}, ' '"logging": [{"callback_name": "langfuse", "callback_vars": {"langfuse_secret_key": "sk"}}], ' '"callback_settings": {"otel": {"headers": "x"}}, "secret_manager_settings": {"vault": "v"}}', [], @@ -214,7 +215,8 @@ def seed_daily_activity_fixture(connection: psycopg.Connection, *, schema: str, "user-1", "user@example.com", [], - '{"department": "research", "ldap_password": "pw", "scim_enterprise": {"employeeNumber": "E-1"}, ' + '{"department": "research", "ldap_password": "pw", "logging": [{"callback_name": "langfuse"}], ' + '"scim_enterprise": {"employeeNumber": "E-1"}, ' '"scim_entitlements": ["ent"], "scim_roles": ["role"]}', ), ) diff --git a/tests/integration/spend/test_daily_activity_repository.py b/tests/integration/spend/test_daily_activity_repository.py index 3df2b934112..d312262a638 100644 --- a/tests/integration/spend/test_daily_activity_repository.py +++ b/tests/integration/spend/test_daily_activity_repository.py @@ -248,6 +248,7 @@ async def test_repository_queries_and_exports_seeded_daily_activity(monkeypatch: "tags": ["blue", "gold"], "cost_center": "cc-1", "webhook_token": "REDACTED", + "langfuse": {"apiKey": "REDACTED"}, } assert active_key_export.user_metadata == {"department": "research", "ldap_password": "REDACTED"} unprivileged_exports: Final = tuple( diff --git a/tests/unit/litellm_core_utils/test_sensitive_data_masker.py b/tests/unit/litellm_core_utils/test_sensitive_data_masker.py index 2182620164c..9039ab99371 100644 --- a/tests/unit/litellm_core_utils/test_sensitive_data_masker.py +++ b/tests/unit/litellm_core_utils/test_sensitive_data_masker.py @@ -612,3 +612,45 @@ def test_redact_credentials_in_payload_leaves_a_realistic_tool_schema_intact(): assert "REDACTED" not in str(result["tools"]) assert result["tools"][0] == tool assert result["api_key"] == "REDACTED" + + +@pytest.mark.parametrize("key", ["apiKey", "clientSecret", "accessToken", "secretAccessKey", "x-apiKey"]) +def test_is_sensitive_key_splits_camel_case_names(key: str): + assert SensitiveDataMasker().is_sensitive_key(key) is True + + +def test_is_sensitive_key_camel_case_split_keeps_pricing_and_count_fields_clear(): + masker = SensitiveDataMasker() + + assert masker.is_sensitive_key("maxTokens") is False + assert masker.is_sensitive_key("inputCostPerToken") is False + + +def test_empty_non_sensitive_overrides_disables_the_cost_exemption(): + strict = SensitiveDataMasker(non_sensitive_overrides=frozenset()) + + assert SensitiveDataMasker().is_sensitive_key("cost_center_api_key") is False + assert strict.is_sensitive_key("cost_center_api_key") is True + assert strict.is_sensitive_key("cost_center") is False + + +def test_redact_credentials_in_payload_drops_listed_keys_at_any_depth_with_the_given_masker(): + from litellm.litellm_core_utils.sensitive_data_masker import redact_credentials_in_payload + + payload = { + "logging": [{"callback_vars": {"langfuse_secret_key": "s"}}], + "cost_center_api_key": "sk-cost", + "nested": {"scim_roles": ["r"], "keep": 1, "deeper": [{"logging": {}, "apiKey": "k"}]}, + } + + result = redact_credentials_in_payload( + payload, + SensitiveDataMasker(non_sensitive_overrides=frozenset()), + dropped_keys=frozenset({"logging", "scim_roles"}), + ) + + assert result == { + "cost_center_api_key": "REDACTED", + "nested": {"keep": 1, "deeper": [{"apiKey": "REDACTED"}]}, + } + assert redact_credentials_in_payload(payload)["cost_center_api_key"] == "sk-cost" diff --git a/tests/unit/repositories/test_daily_activity_repository.py b/tests/unit/repositories/test_daily_activity_repository.py index c4021725cd7..e14f49cfbe6 100644 --- a/tests/unit/repositories/test_daily_activity_repository.py +++ b/tests/unit/repositories/test_daily_activity_repository.py @@ -687,11 +687,19 @@ async def test_export_redacts_credential_named_values_inside_metadata_objects() **_export_row("key-1"), "key_metadata": { "cost_center": "cc-1", + "cost_center_api_key": "sk-cost-1234567890", "api_key": "sk-live-1234567890", - "webhook": {"url": "https://hooks.example", "auth_token": "t0k3n"}, + "webhook": {"url": "https://hooks.example", "auth_token": "t0k3n", "clientSecret": "cs"}, "owners": [{"name": "ops", "password": "pw"}], + "scim_roles": ["admin"], + "langfuse": {"apiKey": "lf", "logging": [{"callback_vars": {"langfuse_secret_key": "s"}}]}, + }, + "user_metadata": { + "department": "research", + "github_token": "ghp_secret", + "logging": [{"callback_name": "langfuse"}], + "profile": {"scim_enterprise": {"employeeNumber": "E-1"}, "team": "ops"}, }, - "user_metadata": {"department": "research", "github_token": "ghp_secret"}, }, {**_export_row("key-2"), "key_metadata": {}, "user_metadata": {}}, ), @@ -703,11 +711,17 @@ async def test_export_redacts_credential_named_values_inside_metadata_objects() assert rows[0].key_metadata == { "cost_center": "cc-1", + "cost_center_api_key": "REDACTED", "api_key": "REDACTED", - "webhook": {"url": "https://hooks.example", "auth_token": "REDACTED"}, + "webhook": {"url": "https://hooks.example", "auth_token": "REDACTED", "clientSecret": "REDACTED"}, "owners": [{"name": "ops", "password": "REDACTED"}], + "langfuse": {"apiKey": "REDACTED"}, + } + assert rows[0].user_metadata == { + "department": "research", + "github_token": "REDACTED", + "profile": {"team": "ops"}, } - assert rows[0].user_metadata == {"department": "research", "github_token": "REDACTED"} assert (rows[1].key_metadata, rows[1].user_metadata) == ({}, {}) diff --git a/tests/unit/repositories/test_daily_activity_sql.py b/tests/unit/repositories/test_daily_activity_sql.py index 7257fd207a3..5514129ea55 100644 --- a/tests/unit/repositories/test_daily_activity_sql.py +++ b/tests/unit/repositories/test_daily_activity_sql.py @@ -451,14 +451,14 @@ def test_export_uses_latest_deleted_key_metadata(export_type: ExportType) -> Non assert "COALESCE(vt.user_id, dvt.user_id)" in query.sql +_DROPPED_KEYS: Final = ( + "- 'callback_settings' - 'logging' - 'scim_enterprise' - 'scim_entitlements' - 'scim_roles' " + "- 'secret_manager_settings'" +) _REDACTED_KEY_METADATA: Final = ( - "THEN (COALESCE(vt.metadata, dvt.metadata) - 'callback_settings' - 'logging' - 'secret_manager_settings')" - "::text END), '{}')::jsonb AS key_metadata" -) -_REDACTED_USER_METADATA: Final = ( - "THEN (u.metadata - 'scim_enterprise' - 'scim_entitlements' - 'scim_roles')::text END), '{}')::jsonb " - "AS user_metadata" + f"THEN (COALESCE(vt.metadata, dvt.metadata) {_DROPPED_KEYS})::text END), '{{}}')::jsonb AS key_metadata" ) +_REDACTED_USER_METADATA: Final = f"THEN (u.metadata {_DROPPED_KEYS})::text END), '{{}}')::jsonb AS user_metadata" _EMPTY_KEY_METADATA: Final = "'{}'::jsonb AS key_metadata" _EMPTY_USER_METADATA: Final = "'{}'::jsonb AS user_metadata"