fix(security): remove unauthenticated open email relay (SES) - #2862
Merged
Conversation
POST /api/emails/ took emailFrom, emailTo, subject, htmlBody and textBody straight from the request body -- with no auth, no allowlist on the SES Source, and no rate limiting -- and passed emailFrom through as the SES Source. This turned the app's verified SES identity into an open relay: anyone could send arbitrary phishing/spam email, to any recipient, "From" any address, at the project's expense and reputation (security audit finding #3, Critical). The generic relay has no legitimate caller (client, embed, or server) -- the only references were in the thunder-tests manual collection. The public contact form (POST /api/emails/contact) already covers the real use case safely: a fixed template with a server-controlled Source (EMAIL_USER) and server-side staff recipient. Rather than bolt auth onto a capability nothing uses, remove it entirely: - Drop the POST / route. - Remove the send controller handler. - Remove ses-service.send (the arbitrary-Source sender). - Remove the now-orphaned Email type. /contact and all templated system emails (registration, password reset) are unchanged. tsc and lint pass clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
VirginiaWu11
approved these changes
Aug 22, 2026
VirginiaWu11
left a comment
Contributor
There was a problem hiding this comment.
Clean fix — reviewed the full diff plus the surrounding call graph, no issues found.
Verified:
- No remaining callers of the removed
send(email-controller.ts),emailService.send(ses-service.ts), or theEmailtype anywhere inserver/,client/, orembed/— grepped for all three.account-service.tsonly importssendRegistrationConfirmation/sendResetPasswordConfirmationfromses-service.ts, neversend. SendEmailCommandOutputstays correctly imported inemail-controller.tssincesendContactFormstill uses it in its type signature — nothing orphaned.- Every remaining SES sender (
sendContactEmail, registration/reset-password confirmations, etc.) sourcesSourcefrom the server-controlledEMAIL_USERenv var, not from request input — confirms the removedsendreally was the only arbitrary-Sourceprimitive. POST /api/emails/contactis untouched and still mounted correctly under/api/emailsinroutes/index.ts.tsc --noEmitpasses clean on the resulting tree.
Removing the relay outright (rather than bolting on auth) is the right call here since nothing legitimate called it. Approving.
🤖 Review generated with Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes security audit finding #3 (Critical) —
POST /api/emails/was an open email relay. It reademailFrom,emailTo,subject,htmlBody, andtextBodystraight from the request body with no auth, noSourceallowlist, and no rate limiting, then passedemailFromthrough as the SESSource. Anyone could send arbitrary phishing/spam email, to any recipient, "From" any address, on the project's verified SES identity — risking AWS billing abuse and domain deliverability/reputation being blacklisted.Why remove rather than lock down
The generic relay has no legitimate caller — not the client, the embed, or the server. The only references were in the
thunder-tests/manual API collection (a dev tool). The public contact form (POST /api/emails/contact) already covers the real use case safely: a fixed template with a server-controlledSource(EMAIL_USER) and a server-side staff recipient. Bolting auth onto a capability nothing uses would just retain a dangerous arbitrary-send primitive, so the relay is removed entirely.Changes
POST /route (email-router.ts).sendcontroller handler (email-controller.ts).ses-service.send— the arbitrary-Sourcesender.Emailtype (email-type.ts).Unaffected
POST /api/emails/contactand all templated system emails (registration confirmation, password reset) are unchanged — they already use a server-controlledSource.Verification
tsc --noEmitandnpm run lintboth pass clean. No email unit tests exist to update.🤖 This PR was written by Claude on behalf of @hanapotski.
🤖 Generated with Claude Code