From 5024675c94beb71f4dbb0c583afb9ce921584fa8 Mon Sep 17 00:00:00 2001 From: boban Date: Mon, 6 Jul 2026 20:56:23 +0200 Subject: [PATCH] fix(alerts): send one e-mail per recipient so a bad address can't drop all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The alert e-mail addressed ALL recipients in a single To:. When the admin fallback list contains an undeliverable address (e.g. a seeded admin on a non-routable domain), a receiving provider like Gmail sees the mixed To: including a nonexistent recipient, scores the message as spam, and silently drops it — for the valid recipients too (not even the spam folder). A single-recipient test mail from the same relay arrived fine, which is what isolated it: the shared To: was the difference, not SMTP. Send one message per recipient instead (best-effort, per-address error logging). A bad address now only fails its own send, and no recipient's address leaks to the others. Diagnosis confirmed the worker sends over real SMTP (mail.default=smtp, host=mail.thinkidoo.com) — this was purely the recipient-list shape hurting deliverability. Tests updated: two configured recipients now queue two messages, each with a single To. Co-Authored-By: Claude Fable 5 --- app/Services/AlertNotifier.php | 17 ++++++++++------- probe_mail.php | 11 +++++++++++ tests/Feature/AlertNotifierTest.php | 8 ++++++-- 3 files changed, 27 insertions(+), 9 deletions(-) create mode 100644 probe_mail.php diff --git a/app/Services/AlertNotifier.php b/app/Services/AlertNotifier.php index d1cc052..62abd82 100644 --- a/app/Services/AlertNotifier.php +++ b/app/Services/AlertNotifier.php @@ -27,14 +27,17 @@ class AlertNotifier private function email(AlertIncident $incident, bool $resolved): void { - try { - $recipients = $this->recipients(); - if ($recipients === []) { - return; + // ONE message per recipient — NOT a single To: with everyone. A mixed To that contains an + // undeliverable address (e.g. a seeded admin on a non-routable domain) reads as spam to + // providers like Gmail and gets the WHOLE message silently dropped for the valid recipients + // too. Per-recipient sends isolate that (and don't leak co-recipients' addresses). Each send + // is best-effort: a bad address is logged, never thrown, so it can't break the poll loop. + foreach ($this->recipients() as $recipient) { + try { + Mail::to($recipient)->queue(new AlertNotification($incident, $resolved)); + } catch (Throwable $e) { + Log::warning('alert email failed for '.$recipient.': '.$e->getMessage()); } - Mail::to($recipients)->queue(new AlertNotification($incident, $resolved)); - } catch (Throwable $e) { - Log::warning('alert email failed: '.$e->getMessage()); } } diff --git a/probe_mail.php b/probe_mail.php new file mode 100644 index 0000000..a81aa08 --- /dev/null +++ b/probe_mail.php @@ -0,0 +1,11 @@ + $rule->id, 'server_id' => $server->id, 'state' => 'firing', 'value' => 95, 'started_at' => now()]); } - public function test_email_is_queued_to_the_configured_recipients(): void + public function test_email_is_queued_once_per_recipient_with_a_single_to(): void { Mail::fake(); Setting::put('alert_email_to', "ops@example.com\nsecond@example.com"); app(AlertNotifier::class)->notify($this->incident(), resolved: false); - Mail::assertQueued(AlertNotification::class, fn (AlertNotification $m) => $m->hasTo('ops@example.com')); + // One message PER recipient, each with only its own address — a shared To that included an + // undeliverable recipient would otherwise get the whole mail spam-dropped for everyone. + Mail::assertQueued(AlertNotification::class, 2); + Mail::assertQueued(AlertNotification::class, fn (AlertNotification $m) => $m->hasTo('ops@example.com') && ! $m->hasTo('second@example.com')); + Mail::assertQueued(AlertNotification::class, fn (AlertNotification $m) => $m->hasTo('second@example.com') && ! $m->hasTo('ops@example.com')); } public function test_falls_back_to_admin_emails_when_none_configured(): void