Repository navigation
security: timing-safe HMAC and CORS wildcard fix - #617
wahh3b-lgtm wants to merge 3 commits into
Conversation
- Use hmac.compare_digest with bitwise OR for constant-time JWT signature verification - Reject wildcard CORS origins when allow_credentials=True - Fix Python 2 exception syntax (except TypeError, ValueError -> except (TypeError, ValueError))
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe pull request changes legacy subscription-token signature validation to use constant-time comparisons. It also disables CORS credentials when the allowed origins include ChangesLegacy token validation
CORS credential handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change hardens legacy token signature comparison and stops sending CORS credentials for wildcard origins. No actionable merge-blocking risk is evident. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes both main changes: timing-safe HMAC validation and the CORS wildcard credential fix. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks each token’s mark,
And guards the origins after dark.
Two signatures meet a careful test,
While wildcard stars receive no crest.
The burrow’s headers settle right,
Then carrots crunch beneath moonlight.
Comment @coderabbitai help to get the list of available commands.
dev restructured app/utils/jwt.py (new HMAC token format, Python 3.14 except syntax), so take dev's version here; the constant-time compare is re-applied to the legacy branch in the next commit.
…e legacy signatures in constant time ALLOWED_ORIGINS defaults to "*". With allow_credentials on, Starlette then echoes every cross-origin request's Origin back together with Access-Control-Allow-Credentials, so any site could make credentialed requests and read the responses. Credentials are now only allowed when the origins are listed explicitly. The panel authenticates with bearer tokens, so the dashboard is unaffected; there is no startup warning, since the wildcard is the default. The legacy subscription token branch still matched its signature with `in`. It now uses hmac.compare_digest on bytes for both encodings (str input must be ASCII, and the signature comes from the URL).
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Changes
app/utils/jwt.py): Replacedinmembership check withhmac.compare_digestusing bitwise OR to ensure both comparisons always execute, preventing timing side-channelsapp/middlewares/__init__.py): Whenallow_credentials=True, any wildcard origin (*) in the allowed origins list now disables credentials with a warningapp/utils/jwt.py): Fixedexcept TypeError, ValueError:toexcept (TypeError, ValueError):What was removed (per project owner feedback)
get_secret_key: The JWT secret key is permanent and doesn't need a TTLinvalidate_secret_key_cache: Unused function removedFiles Changed
app/utils/jwt.pyapp/middlewares/__init__.pyVerification
Summary by CodeRabbit