When fixing #32 (delivery engine writing back to sent_emails), the bounce reason and SMTP response values come from remote mail servers. A few things to lock down:
1. Unbounded error strings stored in the database
Remote SMTP servers can return arbitrarily long error responses. These get stored verbatim into bounce_reason, smtp_response (on sent_emails), and error_message (on delivery_attempts). A malicious server could send multi-megabyte responses to bloat the database.
Fix: Truncate all externally-sourced strings before storage (1024 bytes is plenty for any real SMTP error).
2. Empty Message-ID bypass in deferred path
recordDeliveryAttempt is called directly (not through updateSentEmailStatus) in the temporary failure path. If extractMessageID fails to parse the email file, the call would execute a pointless query with message_id = '<>'.
Fix: Guard the call site with an empty check, and add a defense-in-depth check inside recordDeliveryAttempt itself.
3. Unknown status vs CHECK constraint
The delivery_attempts.status column has a CHECK constraint limiting values to ('pending', 'sent', 'deferred', 'failed', 'bounced'). If an unexpected status string somehow reaches recordDeliveryAttempt, the INSERT would fail silently.
Fix: Explicit default case in the status mapping switch that logs a warning and returns early instead of letting the database reject it.
When fixing #32 (delivery engine writing back to
sent_emails), the bounce reason and SMTP response values come from remote mail servers. A few things to lock down:1. Unbounded error strings stored in the database
Remote SMTP servers can return arbitrarily long error responses. These get stored verbatim into
bounce_reason,smtp_response(onsent_emails), anderror_message(ondelivery_attempts). A malicious server could send multi-megabyte responses to bloat the database.Fix: Truncate all externally-sourced strings before storage (1024 bytes is plenty for any real SMTP error).
2. Empty Message-ID bypass in deferred path
recordDeliveryAttemptis called directly (not throughupdateSentEmailStatus) in the temporary failure path. IfextractMessageIDfails to parse the email file, the call would execute a pointless query withmessage_id = '<>'.Fix: Guard the call site with an empty check, and add a defense-in-depth check inside
recordDeliveryAttemptitself.3. Unknown status vs CHECK constraint
The
delivery_attempts.statuscolumn has a CHECK constraint limiting values to('pending', 'sent', 'deferred', 'failed', 'bounced'). If an unexpected status string somehow reachesrecordDeliveryAttempt, the INSERT would fail silently.Fix: Explicit
defaultcase in the status mapping switch that logs a warning and returns early instead of letting the database reject it.