fix(web): rate-limit expensive endpoints and stop analytics tenant spoofing

[richiemcilroy]

Rate-limits the unauthenticated Loom download, messenger, analytics tracking, guest-checkout and desktop-log endpoints, and derives the analytics tenant from the video record instead of trusting client-supplied ids.

Greptile Summary

This PR hardens five unauthenticated or lightly-authenticated endpoints by adding Vercel Firewall-backed rate limiting through a new shared isRateLimited helper, and fixes a tenant-spoofing path in the analytics tracking route by deriving tenantId from the DB-fetched video record instead of trusting client-supplied body.orgId/body.ownerId.

• apps/web/lib/rate-limit.ts: New helper that wraps @vercel/firewall's checkRateLimit, fails open on any error, is a no-op outside production, and accepts an optional per-user key or explicit headers (needed for Hono handlers where next/headers is unavailable).
• apps/web/app/api/analytics/track/route.ts: Tenant derivation now reads orgId and ownerId exclusively from the looked-up videos row, eliminating the spoofing vector; rate limit guard added before the Tinybird ingest path.
• apps/web/actions/messenger.ts: Rate limit guard correctly placed before the messengerMessages insert and conversation timestamp update, addressing the DB-spam concern raised in the previous review.

Confidence Score: 5/5

Safe to merge — all five endpoints are guarded correctly, the tenant-spoofing fix is sound, and the previously raised messenger DB-write ordering issue is resolved.

The rate-limit helper fails open so it cannot break self-hosted or development deployments. The analytics tenant fix removes client trust entirely for existing videos. The messenger guard is now placed before any database writes. No new defects were introduced in the changed paths.

No files require special attention beyond the pre-existing isRateLimited name shadowing inside importFromLoom in apps/web/actions/loom.ts, which was flagged in a previous review pass.

Important Files Changed

Filename	Overview
apps/web/lib/rate-limit.ts	New shared rate-limit helper backed by Vercel Firewall; fails open on error, skips in non-production, and accepts an optional per-user key and explicit headers for non-App-Router callers. Well-documented with clear rule-ID registry.
apps/web/app/api/analytics/track/route.ts	Adds rate limiting before expensive work and fixes tenant spoofing by deriving tenantId exclusively from the DB-fetched video record (orgId → ownerId) instead of trusting client-supplied body fields.
apps/web/actions/messenger.ts	Rate limit guard moved before the DB insert and conversation timestamp update, fixing the previously flagged issue where DB spam was possible even on rate-limited requests.
apps/web/actions/loom.ts	Adds IP-based rate limiting to the unauthenticated downloadLoomVideo action; note that the imported isRateLimited is shadowed by a local const of the same name inside importFromLoom (previously flagged).
apps/web/app/api/tools/loom-download/route.ts	Rate limit guard added as the first check in the GET handler, before any URL parsing or video conversion work.
apps/web/app/api/settings/billing/guest-checkout/route.ts	Rate limit correctly placed before request.json() parsing, preventing Stripe object creation on abusive requests.
apps/web/app/api/desktop/[...route]/root.ts	Rate limit guard added to the desktop log endpoint; correctly passes c.req.raw.headers explicitly since nextHeaders() is unavailable in Hono handlers.

Comments Outside Diff (2)

1. apps/web/actions/messenger.ts, line 180-226 (link)

   Rate limit check placed after DB writes

   The user message is inserted into messengerMessages (line 180) and messengerConversations is updated (line 192) before isRateLimited is evaluated at line 221. When a request is rejected by the rate limiter, the message already lives in the database without a corresponding AI reply. On the user's next (successful) retry the orphaned message is still in the history, so the AI receives the same user turn twice with no assistant turn between them, corrupting the conversation state.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/web/actions/messenger.ts
   Line: 180-226
   
   Comment:
   **Rate limit check placed after DB writes**
   
   The user message is inserted into `messengerMessages` (line 180) and `messengerConversations` is updated (line 192) before `isRateLimited` is evaluated at line 221. When a request is rejected by the rate limiter, the message already lives in the database without a corresponding AI reply. On the user's next (successful) retry the orphaned message is still in the history, so the AI receives the same user turn twice with no assistant turn between them, corrupting the conversation state.
   
   How can I resolve this? If you propose a fix, please make it concise.

2. apps/web/actions/loom.ts, line 451-457 (link)

   Imported isRateLimited shadowed by local variable

   The new top-level import import { isRateLimited, RATE_LIMIT_IDS } from "@/lib/rate-limit" is shadowed inside importFromLoom by const isRateLimited = await createLoomImportRateLimitCheck(user.id). The local binding wins within that function scope, so there is no runtime breakage today, but the identical names make the code misleading — a future reader may confuse the two, or a linter may flag it as a shadowing error. Consider renaming the local variable (e.g. loomImportRateLimitCheck) to avoid the collision.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/web/actions/loom.ts
   Line: 451-457
   
   Comment:
   **Imported `isRateLimited` shadowed by local variable**
   
   The new top-level import `import { isRateLimited, RATE_LIMIT_IDS } from "@/lib/rate-limit"` is shadowed inside `importFromLoom` by `const isRateLimited = await createLoomImportRateLimitCheck(user.id)`. The local binding wins within that function scope, so there is no runtime breakage today, but the identical names make the code misleading — a future reader may confuse the two, or a linter may flag it as a shadowing error. Consider renaming the local variable (e.g. `loomImportRateLimitCheck`) to avoid the collision.
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

_{Reviews (3): Last reviewed commit: "fix(web): rate-limit messenger before pe..." | Re-trigger Greptile}

[polarityinc]

Paragon Review Skipped

Hi @richiemcilroy! Your Polarity credit balance is insufficient to complete this review.

Please visit https://app.paragon.run to finish your review.

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[richiemcilroy]

@greptileai please review the PR

[richiemcilroy]

@greptileai please review the PR