Add abuse protection to /api/query (closes #168) - #376
Open
SuhrudhC wants to merge 4 commits into
Open
Conversation
Add a reproduction script and solution plan for the unprotected /api/query endpoint. The script bursts both endpoints against the real app with the LLM and email sender mocked: /api/query returns 200 for all 20 requests (no 429), while /api/feedback is throttled to 3/minute, confirming the gap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
/api/query fans out to Vertex AI RAG plus a Gemini completion on every call. Without a rate limit, scripted traffic is a direct cost and availability risk. flask_limiter was already wired up but only applied to /api/feedback. Wrap the ChatView class-based view with limiter.limit() before registering the route, defaulting to 10 per minute (tunable via QUERY_RATE_LIMIT env var without a redeploy). Add two tests mirroring the existing feedback rate-limit test: one confirming requests within the budget return 200, one confirming the 11th request returns 429. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Covers six angles to confirm the fix in the previous commit holds:
Boundary — exactly 10 requests succeed, the 11th returns 429.
Per-IP — two source IPs each get their own independent quota.
Window reset — limiter.reset() refills the window; traffic resumes.
Env-var — QUERY_RATE_LIMIT is read from os.environ at startup.
Regression — /api/feedback limit unchanged; the two endpoints have
independent counters in both directions.
Response — 429 reply has a non-empty HTML error body.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
yangm2
requested changes
Jul 1, 2026
Contributor
There was a problem hiding this comment.
did you mean to check this in?
Author
There was a problem hiding this comment.
That was a personal planning scratch file that got accidentally committed. I just deleted that.
Contributor
There was a problem hiding this comment.
seems like this should go in the backend/scripts dir
Author
There was a problem hiding this comment.
Moved to backend/scripts/ alongside the other utility scripts. Let me know if there's anything else to fix before the pr can get merged.
Contributor
There was a problem hiding this comment.
Please make sure you pass the code-quality checks. I enabled the github action checks, but you can run these checks locally as well (see the README.md).
- Delete SOLUTION_PLAN.md (internal scratch file, not intended for the repo) - Move backend/repro_rate_limit.py → backend/scripts/ alongside other utility scripts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
@claude code-review |
This comment was marked as resolved.
This comment was marked as resolved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this? (check all applicable)
Description
/api/queryis the most expensive thing this app does — every request fans out to Vertex AI RAG plus a Gemini completion. Right now it has no rate limit at all, even thoughflask_limiteris already set up and used on/api/feedback. Anyone (or any script) can hit it as fast as they want, which is a real cost and availability risk.This PR wires the existing limiter up to
/api/querythe same way it's already used on/api/feedback. It defaults to 10 requests per minute per IP, which should be plenty for a normal back-and-forth conversation but stops a script from hammering the endpoint. The limit is also configurable through aQUERY_RATE_LIMITenv var, so it can be tuned in production without a code change if it turns out to be too tight or too loose.I also wrote a small script (
backend/repro_rate_limit.py) that reproduces the bug by bursting both endpoints and comparing results — before the fix/api/queryreturned 200 for all 20 requests with zero rejections, and after the fix it correctly starts returning 429s past the 10th request.Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Easiest way to check this locally: send 11+ POST requests to
/api/queryfrom the same machine within a minute and confirm the 11th comes back with a 429. You can also just runuv run python backend/repro_rate_limit.pyfrom thebackend/directory — it mocks out the LLM call so it doesn't need real GCP credentials, and prints out a before/after-style comparison of/api/queryvs/api/feedback.Added/updated tests?
test_app.py(within-limit returns 200, over-limit returns 429), plus a new filetest_query_rate_limit.pywith 13 tests covering edge cases: the exact boundary (10th request succeeds, 11th doesn't), that two different IPs get separate quotas, that the window resets properly, that the env var is actually being read, and that/api/queryand/api/feedbackdon't share counters with each other.Documentation
[optional] Are there any post deployment tasks we need to perform?
Worth double-checking in production that the rate limiter is actually seeing real client IPs and not the IP of whatever proxy sits in front of the app (Digital Ocean). If it's only seeing the proxy's IP, every user would share one bucket. I didn't have access to verify the production networking setup, so flagging this for whoever reviews/deploys.