fix: rate-limit TURN credential minting to prevent quota abuse#5227
fix: rate-limit TURN credential minting to prevent quota abuse#5227RealDiligent wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces an in-memory rate limiter and origin allowlist validation for the TURN credential endpoint in the lobby worker to prevent abuse. A high-severity issue was identified regarding unbounded memory growth in the rate limiter's in-memory Map, which could lead to memory leaks and potential out-of-memory crashes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
580d33e to
7786586
Compare
|
Addressed review feedback in latest commits (rebased onto main): Superagent (P2): clientIp() now uses only CF-Connecting-IP for rate-limit keys — removed the spoofable X-Forwarded-For fallback. Gemini (memory growth): urn-rate-limit.ts prunes expired buckets on every check and caps the map at 10k entries (with eviction of oldest resetAt). CI re-running. |
7786586 to
cd43e34
Compare
Rate-limit /turn-credentials per client IP, reject disallowed browser Origins, and tighten ALLOWED_ORIGINS to official phase.rs hosts plus local dev. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Quick status update:
Ready for maintainer review/merge. |
| # Max TURN credential mints per client IP per hour (abuse guard). | ||
| TURN_RATE_LIMIT_PER_HOUR = "30" | ||
| # Origin allowlist for the /turn-credentials fetch. Missing Origin is still | ||
| # allowed (Tauri/desktop). Browsers with a present but disallowed Origin get 403. | ||
| ALLOWED_ORIGINS = "https://phase-rs.dev,https://preview.phase-rs.dev,http://localhost:5173,http://127.0.0.1:5173" |
There was a problem hiding this comment.
My main question here is that if a missing origin is still allowed, how big of a net benefit is this PR?
Summary
Root cause:
GET /turn-credentialson the lobby Worker mints 24-hour Cloudflare Realtime TURN credentials with no authentication, no per-client rate limit, andALLOWED_ORIGINS = *. Any caller could burn relay quota; symmetric-NAT peers then fall back to STUN-only and cannot connect.Fix:
TURN_RATE_LIMIT_PER_HOUR)Originheader is present but not on the allowlist (missingOriginstill allowed for Tauri/desktop)ALLOWED_ORIGINStophase-rs.dev,preview.phase-rs.dev, and local Vite dev hostsImpact: Reduces unauthenticated quota-burn attacks on production TURN relay capacity. Legitimate clients remain unaffected; over-limit callers receive HTTP 429 with
Retry-After.Risk / tradeoffs
Originheader.Test plan
lobby-workerunit tests for rate limiter (turn-rate-limit.test.mjs)