Stop reflecting invalid API keys - #578
Merged
Merged
Conversation
Check the complete formatted log text so exception messages and tracebacks are covered, and reuse the existing client and sample file for the invalid-key request.
CyMule
marked this pull request as ready for review
July 21, 2026 00:50
There was a problem hiding this comment.
No issues found across 4 files
Shadow auto-approve: would auto-approve. Security fix that stops reflecting invalid API keys in responses and logs, with corresponding test coverage. The change is bounded (single line of logic) and clearly beneficial, with no operational or public-contract tradeoffs.
Re-trigger cubic
paulkarayan
approved these changes
Jul 21, 2026
paulkarayan
left a comment
There was a problem hiding this comment.
lgtm and thank you for a good find.
| if api_key != api_key_env: | ||
| raise HTTPException( | ||
| detail=f"API key {api_key} is invalid", status_code=status.HTTP_401_UNAUTHORIZED | ||
| detail="API key is invalid", status_code=status.HTTP_401_UNAUTHORIZED |
There was a problem hiding this comment.
nit: you could just slice the first 4 chars if you wanted. but i dont think that's actually very beneficial.
we know pretty well what the error is from this already.
CyMule
enabled auto-merge (squash)
July 21, 2026 15:59
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
API key is invaliddetail for authentication failuresWhy
The 401 detail interpolated the submitted API key (
f"API key {api_key} is invalid"),and that exception is also logged by the request path — so the value landed in both the
HTTP response body and the aggregated logs. A rejected key is not necessarily a
non-secret: it's frequently a real credential that simply didn't match (mistyped, trailing
whitespace, wrong environment, or the wrong value pasted into the header). Treating the
header value as untrusted-but-sensitive and never reflecting or logging it avoids turning
an auth failure into a credential-disclosure event.
Impact
The status code and authentication behavior are unchanged. Clients now receive
API key is invalidinstead of a message echoing the supplied value, and thecredential no longer appears in logs.
Validation
pytest test_general/api/test_app.py::test_general_api_returns_401— 1 passed (asserts the generic detail and that the submitted key is absent from captured log output)