Repository navigation
fix(api): strip Bearer scheme so keyed requests reach per-key rate-limit bucket (#2428) - #2490
Conversation
🤖 Open Code ReviewTarget: PR #2490 🔍 OpenCodeReview found 2 issue(s) in this PR. 1.
|
✅ Automated Test Results: PASSEDAll tests passed (7/7 executed). memos_python_core/changed-repo-python: 7/7. Duration: 9s Branch: |
…mit bucket (MemTensor#2428) _get_client_key in src/memos/api/middleware/rate_limit.py tested whether request.headers['Authorization'] started with 'krlk_', but real clients send 'Authorization: Bearer krlk_<hex>' per RFC 6750. The prefix check never matched and every request — including ones carrying a valid API key — fell through to IP-based limiting. Behind a shared egress IP all keyed clients shared one bucket, so the per-key rate-limit was dead. The fix strips the 'Bearer ' scheme (case-insensitive, per RFC 7235) before the prefix check, so 'Bearer krlk_<hex>' correctly maps to 'ratelimit:key:<first 20 of stripped key>'. Backwards compatibility is preserved: a bare 'krlk_<hex>' header still matches. - New tests/api/test_rate_limit_bearer.py with 7 cases covering Bearer prefix, lowercase bearer, non-key Bearer tokens, missing header, X-Forwarded-For preference, and the bare-krlk_ backwards-compat path. - No new imports, no new dependencies, no public API change. Fixes MemTensor#2428.
67f7bc5 to
af0108d
Compare
|
Thanks @endxxxx for the review. This PR strips the |
✅ Automated Test Results: PASSEDAll tests passed (7/7 executed). memos_python_core/changed-repo-python: 7/7. Duration: 11s Branch: |
Summary
_get_client_keyinsrc/memos/api/middleware/rate_limit.pytested whetherrequest.headers["Authorization"]started withkrlk_, but real clients sendAuthorization: Bearer krlk_<hex>per RFC 6750. The prefix check never matched and every request — including ones carrying a valid API key — fell through to IP-based limiting.Root cause
Real
Authorizationvalues areBearer <token>; theBearerprefix is what made this branch unreachable. Behind a shared egress IP all keyed clients shared one bucket and the per-key rate-limit was dead code.Fix
Strip the
Bearerscheme (case-insensitive, per RFC 7235) before the prefix check, soBearer krlk_<hex>correctly maps toratelimit:key:<first 20 of stripped key>. Backwards compatibility is preserved: a barekrlk_<hex>header still matches.Tests
New
tests/api/test_rate_limit_bearer.py, 7 cases:test_bearer_krlk_key_uses_per_key_bucket— red against unpatchedmain, green with the fixtest_bearer_krlk_key_first_twenty_chars_of_actual_key— confirms the key slicetest_lowercase_bearer_scheme— RFC 7235 case-insensitivitytest_bearer_with_non_key_token_falls_back_to_ip— non-API-key Bearer tokens still go to IPtest_missing_authorization_header_falls_back_to_iptest_x_forwarded_for_header_preferred_over_iptest_krlk_prefix_without_bearer_scheme_still_matches— backwards compatVerification
Ruff:
ruff format --checkandruff checkboth clean on the touched test file; the source file has the same 2 pre-existing BLE001 findings as onmain(this PR introduces 0 new lint issues).Out of scope
Closes #2428