fix(team_callback_endpoints): omit credential callback_vars instead of masking them

GET /team/{team_id}/callback masked credential-bearing callback_vars as the
literal ***REDACTED***. No write path recognises that marker, so a caller that
cloned the response into POST /team/{team_id}/callback or POST /team/update
stored the marker itself as the credential. It is encrypted at rest like any
other secret, so the row looks legitimate while the team's logging silently
stops working.

Drop the credential keys from the response instead, so no marker exists for a
caller to echo back. The response is a partial config rather than a safe round
trip: the metadata write paths replace logging wholesale, so echoing it back
still drops the stored credential, it just cannot substitute a fabricated one.

Resolves LIT-5110
This commit is contained in:
Yucheng Zhu 2026-08-18 11:11:53 -07:00
parent 00e1f25e9b
commit 433aef58e5
3 changed files with 104 additions and 16 deletions

View file

@ -76,21 +76,28 @@ def _redact_callback_secrets(metadata: Any) -> Any:
return redacted
def _mask_sensitive_callback_vars(callbacks: TeamCallbackMetadata) -> None:
"""Mask credential-bearing callback vars in place, keeping the rest readable.
def _drop_sensitive_callback_vars(callbacks: TeamCallbackMetadata) -> None:
"""Remove credential-bearing callback vars in place, keeping the rest readable.
``callback_vars`` mixes credentials (``langsmith_api_key``,
``langfuse_secret_key``, ``gcs_path_service_account``) with plain
configuration (project names, bucket names, hosts). The configuration is
what makes a read of this endpoint useful, so only the sensitive keys are
replaced, using the same marker as the audit-log redaction above.
dropped.
A value that still carries the encrypted prefix here failed to decrypt, so
it is masked too. Handing back a ciphertext blob under a key that is not
it is dropped too. Handing back a ciphertext blob under a key that is not
classified as sensitive would give the caller something it cannot use and
cannot tell apart from a real value.
Masking in place rather than rebuilding the mapping keeps this under the
Keys are omitted rather than replaced with the ``***REDACTED***`` marker
used for audit logs, because a marker survives a read-modify-write and is
then stored as the credential itself. Omitting does not make the response a
safe round trip: the metadata write paths replace ``logging`` wholesale, so
echoing this payload back still drops the stored credential. It only stops
a fabricated one being stored in its place.
Deleting in place rather than rebuilding the mapping keeps this under the
LIT002 mutable-collection-construction budget. It is safe because the only
caller passes an object it just built from a decrypted deep copy of the
row, so nothing here is reachable from the team's stored metadata.
@ -100,7 +107,7 @@ def _mask_sensitive_callback_vars(callbacks: TeamCallbackMetadata) -> None:
for key in tuple(callbacks.callback_vars):
value = callbacks.callback_vars[key]
if is_sensitive_callback_key(key) or str(value).startswith(_CALLBACK_VAR_ENCRYPTED_PREFIX):
callbacks.callback_vars[key] = _CALLBACK_VARS_REDACTED
del callbacks.callback_vars[key]
def _resolve_team_callbacks(team_metadata: object) -> TeamCallbackMetadata:
@ -142,7 +149,7 @@ def _resolve_team_callbacks(team_metadata: object) -> TeamCallbackMetadata:
TeamCallbackMetadata(**callback_settings) if isinstance(callback_settings, dict) else TeamCallbackMetadata()
)
_mask_sensitive_callback_vars(resolved)
_drop_sensitive_callback_vars(resolved)
return resolved
@ -530,8 +537,10 @@ async def get_team_callbacks(
Covers callbacks registered through POST /team/{team_id}/callback and the Admin UI as well as
teams still on the deprecated callback_settings shape, resolved from the team's stored metadata
with the same precedence used at request time. A key-level logging config overrides the team's
at request time and is not reflected here. Credential-bearing callback_vars are returned masked
as `***REDACTED***`
at request time and is not reflected here. Credential-bearing callback_vars are omitted from the
response rather than masked, so a marker can never be posted back and stored as a credential. The
response is therefore a partial config; credentials must be supplied again before it is written
anywhere, and a team left without one falls back to the proxy-level credential for that callback
Returns {
"status": "success",

View file

@ -19,6 +19,10 @@ from litellm.proxy._types import (
LitellmUserRoles,
UserAPIKeyAuth,
)
from litellm.proxy.common_utils.callback_utils import (
decrypt_callback_vars,
encrypt_callback_vars,
)
from litellm.proxy.management_endpoints.team_callback_endpoints import (
add_team_callbacks,
disable_team_logging,
@ -526,14 +530,87 @@ async def test_get_team_callbacks_returns_callbacks_registered_via_post(monkeypa
assert response["data"]["success_callbacks"] == ["langsmith"]
assert response["data"]["failure_callbacks"] == []
# Non-secret vars come back usable, the credential is masked, and the
# Non-secret vars come back usable, the credential key is absent, and the
# ciphertext that is stored on the row never reaches the response.
assert response["data"]["callback_vars"]["langsmith_project"] == "tenant-project"
assert response["data"]["callback_vars"]["langsmith_api_key"] == "***REDACTED***"
assert "langsmith_api_key" not in response["data"]["callback_vars"]
assert "lsv2-real-secret" not in json.dumps(response)
assert "litellm_enc::" not in json.dumps(response)
@pytest.mark.asyncio
async def test_get_team_callbacks_response_does_not_write_a_marker_as_a_credential_when_cloned(monkeypatch):
"""The read response must be safe to post back into a write path.
Cloning one team's logging config onto another is the obvious use of this
endpoint. When the response carried a ``***REDACTED***`` marker in place of
the credential, that replay stored the marker itself as the credential:
it is encrypted at rest like any real secret, so the row looks legitimate
while the target team's logging silently does nothing. Omitting the key
instead means the replay writes no credential at all, which is a visibly
incomplete config rather than a plausible broken one.
"""
monkeypatch.setenv("LITELLM_SALT_KEY", "test-salt-32-bytes-aaaaaaaaaaaaaa")
source_metadata = {
"logging": [
{
"callback_name": "langsmith",
"callback_type": "success",
"callback_vars": {
"langsmith_api_key": "lsv2-source-secret",
"langsmith_project": "source-project",
},
}
]
}
source_row = _team_row(team_id="team-source", metadata=encrypt_callback_vars(source_metadata))
source_prisma = _patch_prisma(source_row)
with (
patch("litellm.proxy.proxy_server.prisma_client", source_prisma),
patch("litellm.proxy.proxy_server.master_key", None),
):
read = await get_team_callbacks(
http_request=MagicMock(spec=Request),
team_id="team-source",
user_api_key_dict=_admin_auth(),
)
# Nothing a caller could echo into any write path is a marker, so this
# holds whichever of the metadata write routes the clone goes through.
assert "***REDACTED***" not in json.dumps(read)
target_row = _team_row(team_id="team-target", metadata={})
target_prisma = _patch_prisma(target_row)
with (
patch("litellm.proxy.proxy_server.prisma_client", target_prisma),
patch("litellm.proxy.proxy_server.litellm_proxy_admin_name", "admin"),
patch("litellm.proxy.proxy_server.master_key", None),
):
await add_team_callbacks(
data=AddTeamCallback(
callback_name="langsmith",
callback_type="success",
callback_vars=read["data"]["callback_vars"],
),
http_request=MagicMock(spec=Request),
team_id="team-target",
user_api_key_dict=_admin_auth(),
litellm_changed_by=None,
)
persisted = decrypt_callback_vars(
json.loads(target_prisma.db.litellm_teamtable.update.await_args.kwargs["data"]["metadata"])
)
cloned_vars = persisted["logging"][0]["callback_vars"]
# The non-secret config clones, the credential does not follow, and no
# marker is ever stored where a credential belongs.
assert cloned_vars["langsmith_project"] == "source-project"
assert "langsmith_api_key" not in cloned_vars
assert "***REDACTED***" not in json.dumps(persisted)
@pytest.mark.asyncio
async def test_get_team_callbacks_prefers_logging_over_deprecated_callback_settings():
"""A team carrying both shapes must report only the one that actually fires.
@ -646,7 +723,7 @@ async def test_get_team_callbacks_decrypts_vars_stored_under_non_sensitive_keys(
@pytest.mark.asyncio
async def test_get_team_callbacks_masks_values_that_fail_to_decrypt(monkeypatch):
async def test_get_team_callbacks_drops_values_that_fail_to_decrypt(monkeypatch):
"""A value that cannot be decrypted must never leave as ciphertext.
After a salt-key rotation an existing value no longer decrypts, and the
@ -683,7 +760,7 @@ async def test_get_team_callbacks_masks_values_that_fail_to_decrypt(monkeypatch)
)
assert response["data"]["success_callbacks"] == ["langsmith"]
assert response["data"]["callback_vars"]["langsmith_project"] == "***REDACTED***"
assert "langsmith_project" not in response["data"]["callback_vars"]
assert _CALLBACK_VAR_ENCRYPTED_PREFIX not in json.dumps(response)
@ -716,7 +793,7 @@ async def test_get_team_callbacks_falls_back_to_deprecated_callback_settings():
assert response["data"]["callback_vars"]["gcs_bucket_name"] == "legacy-bucket"
# Legacy rows predate encryption at rest, so this endpoint is where the
# plaintext secret would otherwise escape.
assert response["data"]["callback_vars"]["langfuse_secret_key"] == "***REDACTED***"
assert "langfuse_secret_key" not in response["data"]["callback_vars"]
assert "sk-lf-legacy-plaintext" not in json.dumps(response)

View file

@ -14750,8 +14750,10 @@ export interface paths {
* Covers callbacks registered through POST /team/{team_id}/callback and the Admin UI as well as
* teams still on the deprecated callback_settings shape, resolved from the team's stored metadata
* with the same precedence used at request time. A key-level logging config overrides the team's
* at request time and is not reflected here. Credential-bearing callback_vars are returned masked
* as `***REDACTED***`
* at request time and is not reflected here. Credential-bearing callback_vars are omitted from the
* response rather than masked, so a marker can never be posted back and stored as a credential. The
* response is therefore a partial config; credentials must be supplied again before it is written
* anywhere, and a team left without one falls back to the proxy-level credential for that callback
*
* Returns {
* "status": "success",