diff --git a/app/Services/Mail/MailboxResolver.php b/app/Services/Mail/MailboxResolver.php index 6fc8e3c..86ec076 100644 --- a/app/Services/Mail/MailboxResolver.php +++ b/app/Services/Mail/MailboxResolver.php @@ -21,7 +21,7 @@ final class MailboxResolver throw new InvalidArgumentException("Unknown mail purpose [{$purpose}]."); } - $box = $this->named(Settings::get(MailPurpose::settingKey($purpose))); + $box = $this->active($this->named(Settings::get(MailPurpose::settingKey($purpose)))); if ($box !== null) { return $box; @@ -31,11 +31,27 @@ final class MailboxResolver // address is recoverable, one that is never sent may not be noticed. return $purpose === MailPurpose::SYSTEM ? null - : $this->named(Settings::get(MailPurpose::settingKey(MailPurpose::SYSTEM))); + : $this->active($this->named(Settings::get(MailPurpose::settingKey(MailPurpose::SYSTEM)))); } private function named(mixed $key): ?Mailbox { return is_string($key) && $key !== '' ? Mailbox::findByKey($key) : null; } + + /** + * Deactivating a mailbox is an explicit operator action meaning "do not + * use this" — it must fall back exactly like a mapping that points + * nowhere or a row that is gone, so it is filtered here, at BOTH lookup + * sites, rather than left for a caller to notice. + * + * A mailbox with no password is deliberately NOT filtered here: silently + * sending support mail from billing@ because support@ has no password + * yet is worse than a loud failure naming the mailbox that needs one — + * that check stays a MailboxTransport concern, not this one. + */ + private function active(?Mailbox $box): ?Mailbox + { + return $box?->active === true ? $box : null; + } } diff --git a/tests/Feature/Mail/MailboxResolverTest.php b/tests/Feature/Mail/MailboxResolverTest.php index bff7a43..151c8b2 100644 --- a/tests/Feature/Mail/MailboxResolverTest.php +++ b/tests/Feature/Mail/MailboxResolverTest.php @@ -35,6 +35,31 @@ it('falls back to system when the named mailbox no longer exists', function () { ->toBe('no-reply@clupilot.com'); }); +it('falls back to system when the mailbox is deactivated, not deleted', function () { + // Deactivating is an explicit operator action. Reaching this mailbox by + // its row surviving would silently send from an address the operator + // just told the console to stop using. + Mailbox::factory()->create(['key' => 'support', 'address' => 'support@clupilot.com', 'active' => false]); + Mailbox::factory()->create(['key' => 'no-reply', 'address' => 'no-reply@clupilot.com']); + Settings::set('mail.purpose.support', 'support'); + Settings::set('mail.purpose.system', 'no-reply'); + + expect(app(MailboxResolver::class)->for(MailPurpose::SUPPORT)?->address) + ->toBe('no-reply@clupilot.com'); +}); + +it('returns null rather than an inactive mailbox when the fallback is deactivated too', function () { + // The fallback lookup is filtered the SAME way as the direct one — an + // inactive system mailbox must not be handed out just because it is the + // last candidate. + Mailbox::factory()->create(['key' => 'support', 'address' => 'support@clupilot.com', 'active' => false]); + Mailbox::factory()->create(['key' => 'no-reply', 'address' => 'no-reply@clupilot.com', 'active' => false]); + Settings::set('mail.purpose.support', 'support'); + Settings::set('mail.purpose.system', 'no-reply'); + + expect(app(MailboxResolver::class)->for(MailPurpose::SUPPORT))->toBeNull(); +}); + it('returns null rather than guessing when even system is unset', function () { expect(app(MailboxResolver::class)->for(MailPurpose::SYSTEM))->toBeNull(); });