regression: add rate limiters to the thread REST endpoints
Proposed changes (including videos or screenshots)
Two of the thread pane's data sources moved off DDP during the 8.8.0 cycle and landed on REST endpoints that never declared rateLimiterOptions, so they inherited the default REST rule — 10 calls per 60 seconds per IP, a ceiling designed for machine-to-machine traffic. No call site changed, only the budget.
Neither DDP method had a dedicated rule, so both ran on the generic per-method limits in apps/meteor/server/startup/rateLimiter.js: Connection_By_Method (10 / 10s per connection) and User_By_Method (20 / 10s per user), both applied at once. This PR restores the per-user ceiling on all three thread routes.
| Endpoint | Moved to REST in | Before | After the migration | This PR |
|---|---|---|---|---|
POST /v1/chat.readThread | #41593 (was readThreads DDP) | 20 / 10s per user | 10 / 60s per IP | 20 / 10s per IP |
GET /v1/chat.getThreadMessages | #40998 (was getThreadMessages DDP) | 20 / 10s per user | 10 / 60s per IP | 20 / 10s per IP |
GET /v1/chat.getThreadsList | always REST | 10 / 60s per IP | 10 / 60s per IP | 20 / 10s per IP |
Why the per-user rule and not the per-connection one. The REST limiter keys on IP, a coarser bucket than a DDP connection — one tab was one connection, and a user with two tabs had two 10-call buckets under a shared 20-call ceiling. Copying the per-connection number onto an IP key would be stricter than DDP ever was, and would fail this ticket's own reproduction: 11 replies arriving as a burst is 11 calls in one window, which 10/10s rejects exactly like 10/60s does.
chat.getThreadsList is not part of the regression — it has been REST since 2022 and has no earlier value to restore. It is included because it is the third leg of the same pane with the same paginated shape, so leaving it on 10/60s would just move the failure one panel over. It can be dropped without affecting the reported bug.
Issue(s)
Steps to test or reproduce
chat.readThread (the reported case):
- Enable
API_Enable_Rate_Limiter; leave the defaults at 10 calls / 60000 ms. - As user A, open a thread and keep it open.
- As user B, send 11 replies to that thread within 60 seconds.
- Watch A's network tab: before this change, the 11th
POST /api/v1/chat.readThreadreturns 429; after it, all succeed. A'stunreadshould not retain the thread'stmid.
chat.getThreadMessages — open 11 different threads within 60 seconds (each open costs one page request). Before this change the 11th thread fails to load its replies; after it, all open normally. Same result by dragging the scrollbar of a single thread past ~550 replies.
Further comments
1. Per-user keying (follow-up: CORE-2637). This restores the DDP budget, but keyed on IP, not userId — all the REST limiter can do today. A single user is now at or above what DDP gave them; users sharing a NAT still share one bucket. These three routes should switch over once per-user keying lands.
2. The migration guide needs updating in the meantime. docs/api-endpoint-migration.md never mentions carrying the method's rate limit over, which is what produced both this regression and CORE-2629. It needs a step: copy the method's dedicated RateLimiter.limitMethod rule, or if it had none, declare rateLimiterOptions explicitly — the REST default is a machine-to-machine ceiling, not a UI one.
Summary by CodeRabbit
- Bug Fixes
- Added request rate limits to thread-reading and thread-listing endpoints, allowing up to 20 requests every 10 seconds.