Skip to content

fix(rate-limit): close rate-limit bypass and tighten public route limits#4591

Merged
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/deepsec-fixes
May 14, 2026
Merged

fix(rate-limit): close rate-limit bypass and tighten public route limits#4591
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/deepsec-fixes

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Add enforceUserRateLimit / enforceIpRateLimit helpers and apply per-user 60/min limits to 8 A2A tool routes (previously unlimited)
  • Add per-IP limits to socket-token, telemetry, unsubscribe, and unauthenticated sso/providers
  • Close X-Forwarded-For: unknown bypass on chat otp/sso (spoofed traffic now shares one bucket)
  • Wire v1/copilot/chat into the standard v1 auth+rate-limit middleware — note: 401 response shape changes from {success:false, error} to {error} to match other v1 routes
  • Bound copilot timeout to 1s–1h
  • Dedup template view counter per viewer/10min to prevent inflation

Type of Change

  • Bug fix

Testing

Tested manually. Typecheck clean. 123/123 unit tests pass. Validated rate-limit numbers against 7 days of prod CloudWatch traffic — observed peak is 5–15× under each limit.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 14, 2026 2:03am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 14, 2026

PR Summary

Medium Risk
Touches multiple public/auth-adjacent API routes by introducing new rate-limit enforcement and changing some error/401 response behavior, which could impact clients if limits or response shapes are unexpected. Logic is straightforward but broad in surface area (auth, telemetry, unsubscribe, A2A tools, v1 copilot).

Overview
Adds centralized route rate limiting via new route-helpers (enforceUserRateLimit, enforceIpRateLimit, enforceUserOrIpRateLimit) with standard 429 responses and headers, and re-exports them from lib/core/rate-limiter with new unit tests.

Applies/strengthens throttling on multiple endpoints: per-IP limits for auth/socket-token, telemetry, users/me/settings/unsubscribe, and unauthenticated auth/sso/providers; per-user (fallback per-IP) limits for several A2A tool routes; and closes the unknown-IP bypass on chat otp/sso by always rate-limiting even when the client IP resolves to unknown.

Hardens a few abuse vectors: deduplicates public template view count increments per viewer per 10 minutes, routes v1/copilot/chat through the standard v1 auth+rate-limit middleware (changing the 401 error shape to match middleware responses), and constrains copilot timeout to 1s–1h in the request contract.

Reviewed by Cursor Bugbot for commit 2f3657c. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 14, 2026

Greptile Summary

This PR closes a long-standing X-Forwarded-For: unknown bypass on the chat OTP/SSO routes, adds per-user (60/min) rate limiting to 8 previously unlimited A2A tool routes, per-IP caps to socket-token, telemetry, unsubscribe, and unauthenticated sso/providers, wires v1/copilot/chat into the standard v1 auth+rate-limit middleware, and deduplicates template view counts per viewer per 10 minutes.

  • New route-helpers.ts introduces three composable helpers (enforceUserRateLimit, enforceIpRateLimit, enforceUserOrIpRateLimit) that wrap RateLimiter.checkRateLimitDirect, fail open on storage errors, and return a consistent { error, retryAfter } 429 response matching the v1 middleware shape.
  • A2A routes all apply enforceUserOrIpRateLimit after the existing checkSessionOrInternalAuth guard, so the userId is always present and the per-user bucket is always used.
  • v1/copilot/chat 401 response shape changes from { success: false, error } to { error } to match other v1 endpoints; the timeout field is now bounded to 1 s – 1 h.

Confidence Score: 5/5

Safe to merge; all new rate-limit paths fail open on Redis errors and are covered by tests.

The changes are additive and well-isolated: new helpers wrap an already-tested token-bucket primitive, A2A routes apply them after the existing auth guard so the IP fallback is unreachable in practice, and the copilot route migration aligns it with the rest of the v1 surface. The unknown-IP bypass fix is simple and its new behaviour is directly tested. No new auth bypass, data-loss, or crash paths were introduced.

apps/sim/app/api/auth/sso/providers/route.ts — the enforceIpRateLimit call sits after getSession() inside the try block, meaning a getSession() exception silently skips IP rate limiting for unauthenticated callers (flagged in a previous review comment).

Important Files Changed

Filename Overview
apps/sim/lib/core/rate-limiter/route-helpers.ts New file introducing three helper functions (enforceUserRateLimit, enforceIpRateLimit, enforceUserOrIpRateLimit) that centralise per-user and per-IP token-bucket enforcement; fails open on storage errors via checkRateLimitDirect's internal catch.
apps/sim/lib/core/rate-limiter/route-helpers.test.ts New test file with good coverage: per-user keying, per-IP keying, unknown-IP shared bucket, 429 response shape, fail-open on storage error, and user/IP dispatch in enforceUserOrIpRateLimit.
apps/sim/app/api/v1/copilot/chat/route.ts Switches from standalone authenticateV1Request (no rate limiting) to the standard authenticateRequest middleware, adding subscription-aware rate limiting; reconstructs the local auth object from the middleware result to preserve downstream usage.
apps/sim/app/api/templates/[id]/route.ts Adds per-viewer/per-template dedup via a 1-token bucket (1 per 10 min) using session userId or IP; authenticated and anonymous paths both handled cleanly.
apps/sim/app/api/auth/sso/providers/route.ts Adds IP rate limiting for unauthenticated callers, but the enforceIpRateLimit call sits inside the try block after getSession(), so a thrown exception bypasses the rate-limit check (flagged in previous review comment).
apps/sim/app/api/chat/[identifier]/otp/route.ts Removes the ip !== 'unknown' guard so spoofed unknown IPs now share one bucket per identifier rather than bypassing IP-based rate limiting entirely.
apps/sim/app/api/tools/a2a/send-message/route.ts Adds enforceUserOrIpRateLimit after the auth guard; userId is always defined at that point so the user bucket is always used.

Reviews (2): Last reviewed commit: "fix(rate-limit): address PR review — dro..." | Re-trigger Greptile

Comment thread apps/sim/lib/core/rate-limiter/route-helpers.ts
Comment thread apps/sim/app/api/tools/a2a/cancel-task/route.ts Outdated
…, fall back to per-IP when JWT auth lacks userId
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2f3657c. Configure here.

@waleedlatif1 waleedlatif1 merged commit 568a552 into staging May 14, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/deepsec-fixes branch May 14, 2026 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant