Skip to content

Stop reflecting invalid API keys - #578

Merged
CyMule merged 3 commits into
mainfrom
fix/redact-invalid-api-key
Jul 21, 2026
Merged

Stop reflecting invalid API keys#578
CyMule merged 3 commits into
mainfrom
fix/redact-invalid-api-key

Conversation

@CyMule

@CyMule CyMule commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • return a generic API key is invalid detail for authentication failures
  • keep the submitted credential out of both the HTTP response body and the error logs
  • add regression coverage asserting the generic response and that the key is absent from captured logs

Why

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 invalid instead of a message echoing the supplied value, and the
credential 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)

CyMule added 2 commits July 19, 2026 21:18
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
CyMule marked this pull request as ready for review July 21, 2026 00:50

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 paulkarayan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
CyMule enabled auto-merge (squash) July 21, 2026 15:59
@CyMule
CyMule merged commit d3038f4 into main Jul 21, 2026
11 checks passed
@CyMule
CyMule deleted the fix/redact-invalid-api-key branch July 21, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants