diff --git a/database/migrations/2026_07_28_100000_seed_mailboxes_from_environment.php b/database/migrations/2026_07_28_100000_seed_mailboxes_from_environment.php index efb1d13..420efb0 100644 --- a/database/migrations/2026_07_28_100000_seed_mailboxes_from_environment.php +++ b/database/migrations/2026_07_28_100000_seed_mailboxes_from_environment.php @@ -58,6 +58,20 @@ return new class extends Migration private const UNCONFIGURED_PORT = 2525; + /** + * config/mail.php's own fallback when MAIL_FROM_ADDRESS is unset. + * + * Same masking problem as UNCONFIGURED_HOST above, and the same fix: + * env('MAIL_FROM_ADDRESS', 'hello@example.com') has already substituted + * by the time this migration reads config('mail.from.address'), and + * 'hello@example.com' is not '', so without this check it would be + * seeded as a real From address instead of recognised as "nothing + * configured, fall back to the SMTP login". Same trade-off as the host + * placeholder too: an install that deliberately wants to send from the + * literal address hello@example.com reads as unset — nobody does that. + */ + private const UNCONFIGURED_FROM_ADDRESS = 'hello@example.com'; + public function up(): void { DB::transaction(function () { @@ -88,10 +102,41 @@ return new class extends Migration Settings::set('mail.encryption', $port === 465 ? 'ssl' : 'tls'); $configuredUser = (string) config('mail.mailers.smtp.username', ''); + + // Placeholder domain for the four boxes below comes from the SMTP + // LOGIN's domain, not from mail.from.address — even though address + // itself (below) now prefers mail.from.address for the no-reply box. + // The login and the From address can point at different domains (an + // API-key login on a shared relay next to a From address on the + // install's own domain, or the reverse), and only the login's domain + // is proven to accept mail: it is the one .env's MAIL_HOST/ + // MAIL_USERNAME pair has actually been sending through. Building + // support@/billing@/etc. from mail.from.address's domain instead + // could seed a placeholder on a domain nobody could ever turn into a + // working mailbox. $domain = str_contains($configuredUser, '@') ? substr($configuredUser, strpos($configuredUser, '@') + 1) : 'clupilot.com'; + // The From address actually in force today may not be the SMTP + // login — a relay's own account name (an API-key username, a shared + // login) is routinely not the address recipients see and can reply + // to. A takeover migration's job is to reproduce what the system + // already sends, not to improve on it, so address and username are + // read and assigned separately rather than collapsing the login into + // both. + $configuredFromAddress = (string) config('mail.from.address', ''); + $fromAddress = $configuredFromAddress === self::UNCONFIGURED_FROM_ADDRESS ? '' : $configuredFromAddress; + + // An install with no MAIL_FROM_ADDRESS has nothing better than the + // SMTP login to send From — which is what Laravel's own mailer was + // already falling back to before this migration, so this reproduces + // that install's mail rather than changing it. + $resolvedAddress = $fromAddress !== '' ? $fromAddress : $configuredUser; + + $configuredFromName = (string) config('mail.from.name', ''); + $displayName = $configuredFromName !== '' ? $configuredFromName : 'CluPilot'; + // A password may already be stored in the vault. It is CARRIED, not // moved: the app_secrets row is deliberately left in place below // rather than deleted, so this block only ever reads it. @@ -111,8 +156,14 @@ return new class extends Migration $box = Mailbox::firstOrNew(['key' => $key]); $box->fill([ - 'address' => $isConfigured ? $configuredUser : $key.'@'.$domain, - 'display_name' => 'CluPilot', + 'address' => $isConfigured ? $resolvedAddress : $key.'@'.$domain, + 'display_name' => $displayName, + // Null when the login and address match: Mailbox:: + // smtpUsername() already falls back to address in that case, + // so writing the identical string into both columns would + // just be a redundant copy — one more thing to keep in sync + // as either changes later through the console. + 'username' => $isConfigured && $configuredUser !== $resolvedAddress ? $configuredUser : null, 'no_reply' => $meta['no_reply'], 'active' => true, ]); diff --git a/tests/Feature/Mail/MailboxSeedMigrationTest.php b/tests/Feature/Mail/MailboxSeedMigrationTest.php index 7fd6b83..e43ced9 100644 --- a/tests/Feature/Mail/MailboxSeedMigrationTest.php +++ b/tests/Feature/Mail/MailboxSeedMigrationTest.php @@ -68,6 +68,84 @@ it('still stores a deliberately configured host and port untouched', function () ->and(Settings::get('mail.port'))->toBe(587); }); +it('seeds address from mail.from.address and keeps the SMTP login as username when they differ', function () { + // The installation this reproduces: MAIL_USERNAME is a relay login, + // MAIL_FROM_ADDRESS is the address actually in force today. Before this + // fix the login was seeded as the From address and username was left + // null — invalid mail, or mail sent from the relay login instead of the + // sender the system was actually configured to use. + clearMailboxSeed(); + config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com'); + config()->set('mail.from.address', 'hello@clupilot.local'); + + loadMailboxSeedMigration()->up(); + + $box = Mailbox::findByKey('no-reply'); + + expect($box->address)->toBe('hello@clupilot.local') + ->and($box->username)->toBe('no-reply@clupilot.com'); +}); + +it('leaves username null when the SMTP login and the From address are the same', function () { + // Mailbox::smtpUsername() already falls back to address when username is + // null — writing the identical string into both columns is a redundant + // copy that only has to drift once (an operator changes one in the + // console but not the other) to become a real, harder-to-spot bug. + clearMailboxSeed(); + config()->set('mail.mailers.smtp.username', 'ops@clupilot.com'); + config()->set('mail.from.address', 'ops@clupilot.com'); + + loadMailboxSeedMigration()->up(); + + $box = Mailbox::findByKey('no-reply'); + + expect($box->address)->toBe('ops@clupilot.com') + ->and($box->username)->toBeNull(); +}); + +it('falls back to the SMTP login for address when MAIL_FROM_ADDRESS was never set', function () { + // config/mail.php's own env('MAIL_FROM_ADDRESS', 'hello@example.com') has + // already substituted by the time this migration reads config() — an + // install that never set MAIL_FROM_ADDRESS reads back that placeholder, + // not null, so it has to be recognised the same way UNCONFIGURED_HOST is. + clearMailboxSeed(); + config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com'); + config()->set('mail.from.address', 'hello@example.com'); + + loadMailboxSeedMigration()->up(); + + $box = Mailbox::findByKey('no-reply'); + + expect($box->address)->toBe('no-reply@clupilot.com') + ->and($box->username)->toBeNull(); +}); + +it('seeds display_name from mail.from.name instead of a hardcoded CluPilot', function () { + clearMailboxSeed(); + config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com'); + config()->set('mail.from.name', 'Example Org Support'); + + loadMailboxSeedMigration()->up(); + + expect(Mailbox::findByKey('no-reply')->display_name)->toBe('Example Org Support') + ->and(Mailbox::findByKey('support')->display_name)->toBe('Example Org Support'); +}); + +it('still derives the placeholder domain from the SMTP login, not from mail.from.address', function () { + // The two can point at different domains. Only the login's domain is + // proven to accept mail for this install — switching the placeholder + // domain to mail.from.address's domain could seed support@/billing@/etc. + // on a domain nobody could ever turn into a working mailbox. + clearMailboxSeed(); + config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com'); + config()->set('mail.from.address', 'hello@clupilot.local'); + + loadMailboxSeedMigration()->up(); + + expect(Mailbox::findByKey('support')->address)->toBe('support@clupilot.com') + ->and(Mailbox::findByKey('billing')->address)->toBe('billing@clupilot.com'); +}); + it('carries the vault password across as ciphertext, never decrypting and re-encrypting it', function () { clearMailboxSeed(); config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com');