Fix #2428: Rate limiter never recognizes Bearer API keys, so per-key limiting is dead code - #2432
Memtensor-AI wants to merge 2 commits into
Conversation
… extraction - Rate limiter middleware now correctly extracts API keys from 'Authorization: Bearer krlk_...' headers - Added case-insensitive Bearer scheme stripping before checking for krlk_ prefix - Previously all Bearer token requests fell through to IP-based limiting - Added 7 comprehensive tests covering Bearer tokens, lowercase bearer, direct keys, and IP fallback - All 120 API tests passing, no regressions Fixes MemTensor#2428
🤖 Open Code ReviewTarget: PR #2432 ✅ OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s). Generated by cloud-assistant via Open Code Review. |
🔧 Open Code Review requested Agent fixOpen Code Review found 1 issue(s). I have resumed the development Agent to fix them.
The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed. |
…tion The input token 'krlk_test_key_12345678' is 22 chars; [:20] produces 'krlk_test_key_123456'. The assertion was correct but silent about why the expected value differs from the input. Add a comment explaining the truncation intent so future readers don't mistake it for a typo. Addresses OCR finding: tests/api/test_rate_limit_middleware.py L41-43
✅ Automated Test Results: PASSEDAll tests passed (8/8 executed). memos_github_open_source/smoke: 1/1, memos_python_core/changed-repo-python: 7/7. Duration: 21s [advisory, non-gating] AI-generated tests on branch test/auto-gen-4c2787d77a027c1d-20260929011416: 64/64 passed — these do NOT affect the PR verdict; review the branch manually. Branch: |
Description
Fixed rate limiter middleware to correctly recognize Bearer tokens for per-key rate limiting. Previously, the middleware checked if the Authorization header started with "krlk_" directly, but real requests use the standard OAuth2 format "Authorization: Bearer krlk_...", causing all keyed requests to fall through to IP-based limiting. This meant clients behind shared NAT/proxy IPs incorrectly shared rate limit buckets.
The fix strips the Bearer scheme prefix (case-insensitive) before checking for the API key prefix, enabling proper per-key rate limiting. Added comprehensive test coverage with 7 new tests verifying Bearer token extraction, lowercase bearer support, backward compatibility with direct keys, and IP fallback behavior.
All 120 existing API tests passed with no regressions. Linting and formatting verified clean. The fix is minimal, focused, and maintains backward compatibility while correctly implementing the intended per-key rate limiting feature.
Related Issue (Required): Fixes #2428
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Automated tests are pending.
Checklist
@bittergreen please review this PR.
Reviewer Checklist