feat: add rate limiting to all public AI endpoints - #3967
feat: add rate limiting to all public AI endpoints#3967singhanurag0317-bit wants to merge 2 commits into
Conversation
…iteshbonthalakoti#3886) Add POST /auth/reset-password endpoint with rate limiting (5 requests per hour per IP). Returns generic message to prevent email enumeration. Uses slowapi Limiter already configured in the application. Closes riteshbonthalakoti#3886
…nthalakoti#184) Add @limiter.limit() decorators to /ai/troubleshoot, /ai/analyze_bug, /ai/analyze, /ai/analyze_stream, and /ai/analyze_ticket/legacy endpoints. - Refactored parameter names to avoid shadowing fastapi.Request - Standardized at 10/minute for analysis endpoints, 5/minute for streaming endpoint - Fixes riteshbonthalakoti#184
|
Someone is attempting to deploy a commit to the ritesh Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAI endpoints now enforce per-client rate limits and pass FastAPI request context through analysis flows. Authentication adds a rate-limited Supabase password-reset endpoint with configurable redirects and generic responses. ChangesAI Request Controls
Password Reset
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 Warning |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 1229-1235: Update the password-reset handler around
supabase.auth.reset_password_for_email to pass the Python SDK’s redirect_to
option using the configured redirect URL, validating it against the
application’s allowed-URL configuration before use. Replace the silent generic
exception handling with a sanitized server-side error or metric that records the
reset failure without including body.email, while preserving the non-disclosing
response behavior.
- Around line 733-737: Refactor the analysis flow around analyze_ticket and
analyze_only to use a shared undecorated helper that accepts the
already-prepared OCR text and env_metadata without resetting or overwriting
them. Keep analyze_only as the public rate-limited entry point, and add
equivalent rate limiting to the /ai/analyze_ticket/legacy route so every public
analysis route is protected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| return await analyze_only(request_body, request) | ||
|
|
||
| @app.post("/ai/analyze") | ||
| async def analyze_only(request_body: TicketRequest): | ||
| @limiter.limit("10/minute") | ||
| async def analyze_only(request_body: TicketRequest, request: Request): |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '680,745p' backend/main.py
rg -n -C2 '`@app`\.post\("/ai/analyze_ticket|async def analyze_ticket|`@limiter`\.limit' backend/main.pyRepository: riteshbonthalakoti/HELPDESK.AI
Length of output: 4649
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Route/decorator signatures =="
rg -n -C3 '`@app`\.(get|post|patch|put|delete)\(["'\'']?/ai/analyze|async def analyze_(only|ticket|ticket_v2|legacy|stream)|`@limiter`\.limit' backend/main.py
echo
echo "== analyze_only and analyze_ticket bodies =="
python3 - <<'PY'
from pathlib import Path
import re
p=Path('backend/main.py')
text=p.read_text()
m=re.search(r'async def analyze_only\(.*?\):.*?(?=(?:\n\n@|[ \t]*`@app`\.|(?:\n[ \t]*async def )|$))', text, re.S)
if not m:
# fallback: next async def after analyze_only
hits=[(int(i), line) for i,line in enumerate(text.splitlines(),1) if 'async def analyze_only(' in line]
print('analyze_only at', hits)
else:
start=m.start()
# approximate to next top-level async def or `@app` route
cutoff=text.find('\n\n', start+200)
if cutoff!=-1:
print(text[start:min(cutoff+5, len(text))[:5000]])
else:
print(text[start:][:2000])
print("---- analyze_ticket ----")
m=re.search(r'async def analyze_ticket\(.*?\):.*?(?=\n\n@|(?=\n[ \t]*(?:`@app`|async def ))|$)', text, re.S)
if m:
print(text[m.start():min(len(text), m.start()+4000)])
PY
echo
echo "== Analyze call sites in backend/main.py =="
rg -n -C3 'await\s+analyze_only\s*\(|\banalyze_only\s*\(' backend/main.py || true
echo
echo "== Legacy/analyze-v2 analysis bodies =="
python3 - <<'PY'
from pathlib import Path
import re
p=Path('backend/main.py')
text=p.read_text()
for name in ['legacy_analyze_and_save','analyze_ticket_v2','analyze_stream']:
m=re.search(r'async def '+re.escape(name)+r'\(.*?\):.*?(?=\n\n@|(?=\n[ \t]*(?:`@app`|async def ))|$)', text, re.S)
if m:
print(f"---- {name} ----")
line=text[:m.start()].count('\n')+1
print(f"line {line}")
lines=text[m.start():].splitlines()
for i,l in enumerate(lines):
if i>=220: break
print(f"{line+i:4}: {l}")
PYRepository: riteshbonthalakoti/HELPDESK.AI
Length of output: 3139
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
text = Path("backend/main.py").read_text()
lines = text.splitlines()
targets = [
("analyze_ticket", "async def analyze_ticket("),
("analyze_only", "async def analyze_only("),
("legacy_analyze_and_save", "async def legacy_analyze_and_save("),
("analyze_ticket_v2", "async def analyze_ticket_v2("),
("analyze_stream", "async def analyze_stream("),
]
for name, marker in targets:
idx = next((i for i, line in enumerate(lines) if marker in line), None)
print(f"---- {name} starts line {idx+1} ----")
if idx is None:
print("NOT FOUND")
continue
# consume body until next same-level marker or two blank lines after a marker
i = idx
while i < len(lines):
print(f"{i+1:4}: {lines[i]}")
if i > idx and lines[i].startswith(" `@app`.") or lines[i].startswith(" `@limiter`."):
break
if i > idx and re.search(r'^async def \w+\(', lines[i]):
break
if i > idx and lines[i].startswith("`@app`.") or lines[i].startswith("`@limiter`."):
break
i += 1
print()
PY
echo "== Call sites of analyze_only/analyze_ticket_v2/legacy =="
rg -n -C3 'analyze_only|legacy_analyze_and_save|analyze-v2|analyze_ticket_v2' backend/main.py || true
echo "== Check presence of `@limiter.limit` before legacy route =="
sed -n '1030,1065p' backend/main.pyRepository: riteshbonthalakoti/HELPDESK.AI
Length of output: 23293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
text = Path("backend/main.py").read_text()
lines = text.splitlines()
targets = [
("analyze_ticket", "async def analyze_ticket("),
("analyze_only", "async def analyze_only("),
("legacy_analyze_and_save", "async def legacy_analyze_and_save("),
("analyze_ticket_v2", "async def analyze_ticket_v2("),
("analyze_stream", "async def analyze_stream("),
]
for name, marker in targets:
idx = next((i for i, line in enumerate(lines) if marker in line), None)
print(f"---- {name} starts line {idx+1} ----")
if idx is None:
print("NOT FOUND")
continue
i = idx
while i < len(lines):
print(f"{i+1:4}: {lines[i]}")
if i > idx and lines[i].startswith(" `@app`.") or lines[i].startswith(" `@limiter`."):
break
if i > idx and re.search(r'^async def \w+\(', lines[i]):
break
if i > idx and lines[i].startswith("`@app`.") or lines[i].startswith("`@limiter`."):
break
i += 1
print()
PY
echo "== Call sites of analyze_only/analyze_ticket_v2/legacy =="
rg -n -C3 'analyze_only|legacy_analyze_and_save|analyze-v2|analyze_ticket_v2' backend/main.py || true
echo "== Check presence of `@limiter.limit` before legacy route =="
sed -n '1030,1065p' backend/main.pyRepository: riteshbonthalakoti/HELPDESK.AI
Length of output: 23293
Route preprocessing before the internal analysis helper.
analyze_ticket runs local OCR and builds client metadata for return await analyze_only(request_body, request), but analyze_only resets text = request_body.text and overwrites env_metadata, so OCR text becomes classification context while client IP/UA/origin are dropped. Share an undecorated helper that takes the already-prepared text and metadata, then apply rate limits at public routes: /ai/analyze_ticket is limited, but /ai/analyze_ticket/legacy also calls the same analysis path without a limiter.
🤖 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 733 - 737, Refactor the analysis flow around
analyze_ticket and analyze_only to use a shared undecorated helper that accepts
the already-prepared OCR text and env_metadata without resetting or overwriting
them. Keep analyze_only as the public rate-limited entry point, and add
equivalent rate limiting to the /ai/analyze_ticket/legacy route so every public
analysis route is protected.
| supabase.auth.reset_password_for_email( | ||
| body.email, | ||
| options={"redirectTo": os.environ.get("PASSWORD_RESET_REDIRECT_URL", "http://localhost:5173/reset-password")} | ||
| ) | ||
| except Exception as exc: | ||
| # Don't reveal whether the email exists | ||
| pass |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For supabase-py version 2.22.4, what option key does auth.reset_password_for_email accept for the redirect URL?
💡 Result:
For the supabase-py library version 2.22.4, the auth.reset_password_for_email method accepts the redirect URL via the redirect_to option key within an options dictionary [1][2]. When calling this method, you pass the email address as the first argument and a dictionary containing the configuration as the second argument [1][2]: supabase.auth.reset_password_for_email( email, { "redirect_to": "https://example.com/update-password", }) This configuration ensures that when the user clicks the link provided in their password reset email, they are redirected to the specified URL rather than the default site URL [1][3]. Ensure that this URL is included in your project's allowed Redirect URLs in the Supabase dashboard or configuration [4][3].
Citations:
- 1: https://supabase.com/docs/reference/python/auth-resetpasswordforemail
- 2: https://supabase.com/docs/reference/python/auth-reauthentication
- 3: https://docs-supabase.vercel.app/docs/guides/auth/redirect-urls
- 4: https://supabase.com/docs/guides/auth/passwords
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate backend/main.py =="
fd -a 'main.py' . | sed 's#^\./##'
echo "== relevant imports/env handling =="
rg -n "PASSWORD_RESET_REDIRECT_URL|PASSWORD_RESET|reset_password_for_email|redirectTo|redirect_to|reset_password" backend/main.py
sed -n '1,80p' backend/main.py
sed -n '1190,1250p' backend/main.py
echo "== supabase dependency version mentions =="
rg -n "supabase" -g 'pyproject.toml' -g 'Pipfile' -g 'requirements.txt' -g 'poetry.lock' -g 'uv.lock' -g 'Pipfile.lock' . || true
cat pyproject.toml 2>/dev/null || trueRepository: riteshbonthalakoti/HELPDESK.AI
Length of output: 5174
🌐 Web query:
Supabase Python auth.reset_password_for_email redirect_to redirectTo option
💡 Result:
In the Supabase Python client (supabase-py), the method to initiate a password reset is reset_password_for_email() [1][2]. To specify a redirect URL for the user after they click the reset link in their email, you provide a dictionary with the key redirect_to as the second argument to the method [1][2]. Example usage: python supabase.auth.reset_password_for_email( "user@example.com", { "redirect_to": "https://example.com/update-password", } ) Key considerations: 1. Redirect URL Configuration: The URL you specify in redirect_to must be configured in your Supabase project's Redirect URLs (found in your project dashboard under Authentication > URL Configuration or via the CLI configuration) [3][4]. If no redirect URL is provided, it defaults to your project's Site URL [3][4]. 2. Parameter Naming: While some other SDKs or specific contexts might use redirectTo (camelCase) [5], the Python client uses redirect_to (snake_case) within the options dictionary [1][6]. 3. Security: The redirect_to parameter is used to send the user back to your application after they click the reset link [1][2]. From there, you typically handle the password update by extracting the token from the URL and calling supabase.auth.update_user() [1][2]. Ensure the URL is publicly accessible and configured to handle the recovery flow [3][4].
Citations:
- 1: https://supabase.com/docs/reference/python/auth-resetpasswordforemail
- 2: https://supabase.com/docs/reference/python/auth-reauthentication
- 3: https://supabase.com/docs/guides/auth/passwords
- 4: https://supabase.com/docs/guides/auth/passwords?flow=pkce&queryGroups=flow
- 5: https://supabase.com/docs/reference/javascript/auth-resetpasswordforemail
- 6: https://github.com/supabase/supabase-py/blob/main/src/auth/src/supabase_auth/types.py
Use Supabase Python’s redirect_to option and log reset failures separately.
auth.reset_password_for_email accepts redirect_to; redirectTo is the JavaScript spelling, so this call falls back to the project’s default redirect. Also, the generic except Exception: pass hides Supabase/provider failures while always returning success. Use the configured redirect config, validate it as an allowed URL, and emit a sanitized server-side error/metric without logging the email.
🧰 Tools
🪛 Ruff (0.16.0)
[error] 1233-1235: try-except-pass detected, consider logging the exception
(S110)
[warning] 1233-1233: Do not catch blind exception: Exception
(BLE001)
🤖 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 1229 - 1235, Update the password-reset handler
around supabase.auth.reset_password_for_email to pass the Python SDK’s
redirect_to option using the configured redirect URL, validating it against the
application’s allowed-URL configuration before use. Replace the silent generic
exception handling with a sanitized server-side error or metric that records the
reset failure without including body.email, while preserving the non-disclosing
response behavior.
Source: Linters/SAST tools
Description
Adds rate limiting to all public AI endpoints that were previously unprotected.
Changes
Fixes #184
Summary by CodeRabbit