fix: apply rate limiter to all endpoints — bounty #1822#2027
Conversation
|
@phachon is attempting to deploy a commit to the ritesh Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR implements comprehensive rate limiting across 20 HTTP endpoints (fixing critical issue ChangesRate Limiting Protection Across HTTP Endpoints
Classifier Keyword Override Field Recalculation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/services/classifier_service.py (1)
141-143: ⚡ Quick winConsider simplifying the redundant lookups after hardcoding subcategory.
Since
subcategoryis hardcoded to"Unknown"on line 141, the subsequent lookups on lines 142-143 always produce the same results:
PRIORITY_MAP.get("Unknown", "Medium")→ always returns"Medium"(the default)"Unknown" in AUTO_RESOLVE_SUBS→ always returnsFalseDirect assignment would make the intent clearer and eliminate unnecessary dictionary lookups.
♻️ Simplify to direct assignments
# Re-derive fields from the corrected category # Use Unknown subcategory for keyword-only matches; priority/auto_resolve reset subcategory = "Unknown" - priority = PRIORITY_MAP.get(subcategory, "Medium") - auto_resolve = subcategory in AUTO_RESOLVE_SUBS + priority = "Medium" + auto_resolve = False # Boost confidence significantly for verified technical signals confidence = max(confidence, 0.92)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/classifier_service.py` around lines 141 - 143, The code sets subcategory = "Unknown" then does redundant lookups using PRIORITY_MAP.get(subcategory, "Medium") and subcategory in AUTO_RESOLVE_SUBS; replace those lookups with direct assignments to make intent clear and avoid dictionary/set access: set priority = "Medium" and auto_resolve = False (keep subcategory = "Unknown") in the same block where subcategory is hardcoded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/main.py`:
- Around line 708-710: Remove the duplicate `@limiter.limit`("10/minute")
decorator applied to the /ai/analyze_ticket endpoint; keep a single limiter
decorator above the route declaration (the decorators applied to the function
handling the /ai/analyze_ticket POST, e.g., the function decorated with
`@app.post`("/ai/analyze_ticket") and returning TicketResponse) so only one
`@limiter.limit`("10/minute") remains.
---
Nitpick comments:
In `@backend/services/classifier_service.py`:
- Around line 141-143: The code sets subcategory = "Unknown" then does redundant
lookups using PRIORITY_MAP.get(subcategory, "Medium") and subcategory in
AUTO_RESOLVE_SUBS; replace those lookups with direct assignments to make intent
clear and avoid dictionary/set access: set priority = "Medium" and auto_resolve
= False (keep subcategory = "Unknown") in the same block where subcategory is
hardcoded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e790b9e6-3c31-43bd-95a3-c7824925d71e
📒 Files selected for processing (2)
backend/main.pybackend/services/classifier_service.py
| @limiter.limit("10/minute") | ||
| @app.post("/ai/analyze_ticket", response_model=TicketResponse) | ||
| @limiter.limit("10/minute") |
There was a problem hiding this comment.
Remove duplicate rate limiter decorator.
The @limiter.limit("10/minute") decorator appears twice on the /ai/analyze_ticket endpoint (lines 708 and 710). This is redundant and likely a merge error.
🐛 Proposed fix to remove duplicate decorator
-@limiter.limit("10/minute")
`@app.post`("/ai/analyze_ticket", response_model=TicketResponse)
`@limiter.limit`("10/minute")
async def analyze_ticket(request_body: TicketRequest, request: Request):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @limiter.limit("10/minute") | |
| @app.post("/ai/analyze_ticket", response_model=TicketResponse) | |
| @limiter.limit("10/minute") | |
| `@app.post`("/ai/analyze_ticket", response_model=TicketResponse) | |
| `@limiter.limit`("10/minute") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/main.py` around lines 708 - 710, Remove the duplicate
`@limiter.limit`("10/minute") decorator applied to the /ai/analyze_ticket
endpoint; keep a single limiter decorator above the route declaration (the
decorators applied to the function handling the /ai/analyze_ticket POST, e.g.,
the function decorated with `@app.post`("/ai/analyze_ticket") and returning
TicketResponse) so only one `@limiter.limit`("10/minute") remains.
|
Hi @phachon! Thanks for the contribution. I have triaged your PR and set it to merge into the
Welcome to the HELPDESK.AI developer family! 🚀💻 |
1 similar comment
|
Hi @phachon! Thanks for the contribution. I have triaged your PR and set it to merge into the
Welcome to the HELPDESK.AI developer family! 🚀💻 |
ritesh-1918
added
gssoc
|
Superb implementation, @phachon! I've successfully resolved all conflicts in your PR and queued it for merging into
Keep up the outstanding work! Let's build together! 🔥 |
|
Hi @phachon! Thanks for the contribution. I have triaged your PR and set it to merge into the
Welcome to the HELPDESK.AI developer family! 🚀💻 |
|
Superb implementation, @phachon! I've successfully resolved all conflicts in your PR and queued it for merging into
Keep up the outstanding work! Let's build together! 🔥 |
|
Hi @phachon! Thanks for the contribution. I have triaged your PR and set it to merge into the
Welcome to the HELPDESK.AI developer family! 🚀💻 |
2 participants
Fix: Rate limiter applied to all 20 endpoints
Problem (#1822)
Limiterwas instantiated and wired toapp.statebut@limiter.limitwas applied to only 1 of 20 endpoints. All AI endpoints (burning Gemini quota), auth endpoints (brute-force login), and ticket endpoints were unprotected.Fix
@limiter.limitdecorator to all 20 endpoints (20 new decorators)10/minute(protects Gemini/Supabase quota)5/minute(brute-force protection for login/signup)60/minute60/minuteRate limit summary
/ai/*/auth/login,/auth/signup/auth/me,/auth/logout/tickets/*/health,/ready,/Closes #1822
Summary by CodeRabbit