mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
fix(auth): compare a short secret whole when scanning for a reflected credential
The reflection scan only ever compared eight-character windows of the response against the secret, so a secret with fewer credential characters than that could never match once an endpoint echoed it percent-encoded or split up, and the verbatim check needs the raw form. Cap the run at the secret's length so a short client secret is compared whole in every wire shape.
This commit is contained in:
parent
48a59a1668
commit
9f2b59cc96
2 changed files with 22 additions and 3 deletions
|
|
@ -169,17 +169,23 @@ def _drop_reflected_assertion(rendered: str, assertion: SecretStr | None) -> str
|
|||
def _shares_a_credential_run(rendered: str, compacted_secret: str) -> bool:
|
||||
"""``unquote`` covers a credential sent form-encoded, without every caller enumerating that
|
||||
shape for itself: percent-escaping is reversible and applies to any field, query string
|
||||
included."""
|
||||
included.
|
||||
|
||||
A secret shorter than the probe run is compared whole: a window longer than the secret can
|
||||
never be found inside it, which would leave a short client secret unprotected in every shape
|
||||
but the verbatim one.
|
||||
"""
|
||||
# unquote covers %XX; unquote_plus additionally covers the "+" a form-encoded body uses for a
|
||||
# space. Both are kept rather than only the wider one, because "+" is a base64 character and
|
||||
# decoding it away would lose a run that the undecoded candidate still matches on.
|
||||
run: Final = min(_REFLECTION_MIN_RUN, len(compacted_secret))
|
||||
compacted_candidates: Final = tuple(
|
||||
_CREDENTIAL_CHARS.sub("", candidate) for candidate in (rendered, unquote(rendered), unquote_plus(rendered))
|
||||
)
|
||||
return any(
|
||||
compacted[start : start + _REFLECTION_MIN_RUN] in compacted_secret
|
||||
compacted[start : start + run] in compacted_secret
|
||||
for compacted in compacted_candidates
|
||||
for start in range(len(compacted) - _REFLECTION_MIN_RUN + 1)
|
||||
for start in range(len(compacted) - run + 1)
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -703,6 +703,19 @@ class TestRedactionAndCaps:
|
|||
|
||||
assert "short-secret" not in result.redacted_body
|
||||
|
||||
def test_a_short_secret_echoed_in_its_wire_shape_is_dropped(self):
|
||||
"""Regression: the run scan only ever compared eight-character windows, so a secret
|
||||
with fewer credential characters than that could never match once it came back
|
||||
percent-encoded rather than verbatim, and the whole-value check needs the raw form."""
|
||||
secret = SecretStr("p@ss w0rd!")
|
||||
echoed = quote(secret.get_secret_value(), safe="")
|
||||
body = {"error": "invalid_client", "error_description": f"rejected {echoed}"}
|
||||
|
||||
assert secret.get_secret_value() not in echoed
|
||||
result = redact_oauth_error_body(400, json.dumps(body), secret)
|
||||
|
||||
assert echoed not in result.redacted_body
|
||||
|
||||
def test_an_unrelated_body_is_not_falsely_redacted(self):
|
||||
"""The scan must not fire on a body that merely shares short runs with the assertion."""
|
||||
assertion = SecretStr("eyJhbGciOiJSUzI1NiJ9." + "Z" * 60 + ".signature")
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue