Skip to content

fix: rate-limit TURN credential minting to prevent quota abuse#5227

Open
RealDiligent wants to merge 3 commits into
phase-rs:mainfrom
RealDiligent:fix/critical-issue-turn-credentials-rate-limit
Open

fix: rate-limit TURN credential minting to prevent quota abuse#5227
RealDiligent wants to merge 3 commits into
phase-rs:mainfrom
RealDiligent:fix/critical-issue-turn-credentials-rate-limit

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Summary

Root cause: GET /turn-credentials on the lobby Worker mints 24-hour Cloudflare Realtime TURN credentials with no authentication, no per-client rate limit, and ALLOWED_ORIGINS = *. Any caller could burn relay quota; symmetric-NAT peers then fall back to STUN-only and cannot connect.

Fix:

  • Per-IP sliding-window rate limit (default 30 mints/hour, configurable via TURN_RATE_LIMIT_PER_HOUR)
  • Reject browser requests whose Origin header is present but not on the allowlist (missing Origin still allowed for Tauri/desktop)
  • Tighten ALLOWED_ORIGINS to phase-rs.dev, preview.phase-rs.dev, and local Vite dev hosts

Impact: 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

  • Rate limiting is per-isolate (best-effort within a Worker instance); determined attackers can still spread load, but casual scraping/burning is blocked.
  • Browsers from non-listed origins are rejected when they send an Origin header.

Test plan

  • lobby-worker unit tests for rate limiter (turn-rate-limit.test.mjs)
  • CI lobby-worker + frontend checks

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread lobby-worker/src/turn-rate-limit.ts
@matthewevans matthewevans added the defer-fe Frontend/client/UI PR deferred to Matt's direct review label Jul 7, 2026
@RealDiligent RealDiligent force-pushed the fix/critical-issue-turn-credentials-rate-limit branch from 580d33e to 7786586 Compare July 7, 2026 13:32
Comment thread lobby-worker/src/turn.ts Outdated
@superagent-security superagent-security Bot added the pr:flagged Superagent: PR flagged for security review label Jul 7, 2026
@RealDiligent

Copy link
Copy Markdown
Contributor Author

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.

@RealDiligent RealDiligent force-pushed the fix/critical-issue-turn-credentials-rate-limit branch from 7786586 to cd43e34 Compare July 7, 2026 19:50
@superagent-security superagent-security Bot removed the pr:flagged Superagent: PR flagged for security review label Jul 7, 2026
RealDiligent and others added 3 commits July 8, 2026 03:54
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>
@RealDiligent

Copy link
Copy Markdown
Contributor Author

Quick status update:

  • Addressed Gemini HIGH finding by adding bucket pruning + hard cap in urn-rate-limit.ts.
  • Addressed Superagent P2 by removing X-Forwarded-For fallback; rate limiting now keys only on CF-Connecting-IP.
  • CI is fully green on current head commit.

Ready for maintainer review/merge.

Comment on lines +72 to +76
# 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"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main question here is that if a missing origin is still allowed, how big of a net benefit is this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

defer-fe Frontend/client/UI PR deferred to Matt's direct review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants