Skip to content

fix(security): isolate rejected OTP attempts - #7008

Merged
TheodoreSpeaks merged 3 commits into
stagingfrom
fix/otp-resource-admission
Aug 23, 2026
Merged

fix(security): isolate rejected OTP attempts#7008
TheodoreSpeaks merged 3 commits into
stagingfrom
fix/otp-resource-admission

Conversation

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator

Summary

  • isolate rejected public chat and file-share OTP attempts from legitimate send budgets
  • preserve resource-scoped throttling when the client IP cannot be resolved
  • add regression coverage for both public OTP surfaces

Type of Change

  • Bug fix

Testing

  • Targeted Vitest suites: 33 tests passed
  • bun run lint
  • block registry validation
  • bun run check:audits (33 audits passed)

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

vercel Bot commented Aug 23, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 23, 2026 7:15am

Request Review

@cursor

cursor Bot commented Aug 23, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Changes public OTP request flows: allow-list signaling, rate-limit responses, and when send budgets are consumed. Enumeration protection is improved, but resource/email limits no longer surface 429 to clients.

Overview
Public chat and file-share OTP POST handlers now always return a generic “Verification code sent” after IP checks, instead of 403/429/500 for unauthorized emails, resource/email throttles, or mail failures.

Unauthorized emails skip deliverOtp entirely so they no longer consume resource or per-email send buckets (including when no client IP is present). IP rate limits still return 429. Actual generation, remaining throttles, and email send run after the response via afterResponse.

Reviewed by Cursor Bugbot for commit abd36ff. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR prevents rejected public OTP attempts from consuming legitimate send budgets and removes allow-list response differences by moving authorized OTP delivery behind the response boundary.

  • Returns the same generic accepted response for rejected, throttled, failed-delivery, and successful OTP requests.
  • Preserves uniform pre-response IP throttling while applying resource and recipient limits only to authorized delivery attempts.
  • Adds deterministic regression coverage for public chat and file-share OTP routes.
  • Introduces a shared wrapper around Next.js post-response work.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/app/api/chat/[identifier]/otp/route.ts Moves authorized chat OTP throttling, persistence, and delivery after the generic response while preserving uniform IP throttling.
apps/sim/app/api/files/public/[token]/otp/route.ts Applies the same post-response OTP delivery model to public file shares without exposing allow-list or send-limit outcomes.
apps/sim/lib/core/utils/after-response.ts Adds a small shared wrapper around Next.js post-response task registration.
apps/sim/app/api/chat/[identifier]/otp/route.test.ts Adds regression coverage for generic responses, rejected requests, deferred execution, and retained throttling.
apps/sim/app/api/files/public/[token]/otp/route.test.ts Covers rejected addresses, absent client IPs, delivery failures, deferred work, and resource and recipient limits.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Request[Public OTP request] --> IP[Apply uniform IP throttle]
    IP --> Resolve[Resolve chat or file-share resource]
    Resolve --> Allow{Email allowed?}
    Allow --> Response[Return generic accepted response]
    Response --> Deferred[Run post-response callback]
    Allow -->|No| Skip[Skip send-budget and delivery work]
    Allow -->|Yes| Deferred
    Deferred --> Resource[Apply resource limit]
    Resource --> Recipient[Apply recipient limit]
    Recipient --> Store[Generate and store OTP]
    Store --> Send[Send verification email]
Loading

Reviews (3): Last reviewed commit: "fix(security): defer OTP delivery work" | Re-trigger Greptile

Comment thread apps/sim/app/api/chat/[identifier]/otp/route.ts Outdated
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/api/chat/[identifier]/otp/route.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 ed1b07d. Configure here.

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 abd36ff. Configure here.

@TheodoreSpeaks
TheodoreSpeaks merged commit d831c09 into staging Aug 23, 2026
30 checks passed
@TheodoreSpeaks
TheodoreSpeaks deleted the fix/otp-resource-admission branch August 23, 2026 07:23
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