KamoCRM

Close three false-refusal paths the review found in RecipientDomainValidator

FixEmailService
Shipped
September 29, 2026 at 2:05 AM UTC
Author
Kamo
Commit
38cd3d5

Independent review of D-luna-1 (email-fix-review-1.md) found two BLOCKERs and a MAJOR, each a real false refusal of legitimate mail, demonstrated from code already in this repo: - Case (BLOCKER): AliasService.createAlias, SharedMailboxService.create and **************** all persist an address exactly as an admin typed it — a real row can read "Sales@kamocrm.com". Every lookup is now case-insensitive at the database (LOWER(col) = LOWER(?)), scoped by org_id, which still reaches this organization's own rows through each table's (org_id HASH, col) unique index rather than a cross-organization scan — it just cannot use the index's sorted range component to seek the exact value within them. A functional index on LOWER(col) per table (or normalizing case at write time in those three services) would restore a plain point lookup; neither is done here without sign-off, since both are bigger than this task. Implemented via EntityManager native queries in RecipientDomainValidator rather than new kamo-shared-library repository methods. - Plus-tags (BLOCKER): sage+urgent@kamocrm.com now folds to sage@kamocrm.com (EmailAddress.baseLocalPart(), the same fold the deliverability/suppression layer already applies) before the mailbox check — never for the alias check, since an alias is its own distinct, deliberately-created address. - Multiple root domains (MAJOR): an alias-domain address (kamouniverse.com) is now retried against every domain in **************** not only the UI default (allowedDomain) — a mailbox on a second owned root no longer reads as unknown just because it isn't the org's default domain. - Unindexed member lookup (MAJOR): the MemberRepository fallback is removed. **************** has no supporting index, so it ran a scan of every member row in the organization for every genuinely bad guess — exactly this feature's own reason to exist. It was also unnecessary: this class only ever runs for an organization on KamoMail, where a real, deliverable address always has a **************** row (that IS what provisions it); a member whose .email happens to match but has none of those rows has no mailbox on the shared server either, so a send to them fails at RCPT TO regardless of what this check said. See the class javadoc for the full argument. - Truncation (NIT): **************** now caps its prose at 5 named addresses ("and N more"), so DownstreamErrors' 300-char cut can no longer slice a many-bad-recipient send's message off mid-address before MailPack's directory-lookup hint is appended. rejectedRecipients() is never truncated. The MINOR (existence oracle) finding is accepted as reasoning, not a code change: the pre-flight check answers "does x@ownDomain exist" faster than the pre-existing SendFailedException->422 RECIPIENTS_REJECTED path already could (a real RCPT TO), for the same same-org, seat-gated caller; it opens no new privilege boundary. Tests (red confirmed, then green): RecipientDomainValidatorTest rewritten against the new EntityManager-based design (mirrors **************** RETURNS_SELF Query mock) with new cases for a stored-mixed-case address, a +tag against a real mailbox, a +tag that still doesn't resolve, and an alias-domain address whose real mailbox lives on a non-default owned root domain (with and without a +tag). SendFailureResponseTest gained cases for the capped prose vs. the untruncated structured list. heavy mvn test **************** 77 run, 0 failures.

All changes

Like what you see shipping?

All of it arrives in your workspace on its own. Start on the free plan and read this page again in a month.

Start Free ForeverView Pricing