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 420efb0..6a0ba55 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 @@ -5,6 +5,7 @@ use App\Services\Mail\MailPurpose; use App\Services\Secrets\SecretCipher; use App\Support\Settings; use Illuminate\Database\Migrations\Migration; +use Illuminate\Support\ConfigurationUrlParser; use Illuminate\Support\Facades\DB; /** @@ -79,15 +80,58 @@ return new class extends Migration }); } + /** + * host/port/username/password the way MailManager actually builds the + * SMTP transport from them — not necessarily what config('mail.mailers. + * smtp.host') and its three siblings hold. + * + * config/mail.php carries 'url' => env('MAIL_URL') on the smtp mailer: + * Laravel's OTHER supported way to configure SMTP, a single DSN instead + * of four separate variables. An install using it never sets MAIL_HOST/ + * MAIL_USERNAME/MAIL_PASSWORD at all, so the plain config() calls this + * migration used to make read config/mail.php's own placeholder + * defaults (127.0.0.1, 2525, null, null) — indistinguishable from + * UNCONFIGURED_HOST/UNCONFIGURED_PORT below, even though the install + * sends mail successfully today. The substitution that would have shown + * the real values only happens inside Illuminate\Mail\MailManager:: + * getConfig(), which is protected — not called here, reflected into, or + * re-implemented by hand (a second copy of ITS merge order is what would + * rot). Illuminate\Support\ConfigurationUrlParser is the public class + * getConfig() itself delegates to for exactly this, so calling it here + * directly is the same resolution, read from the one place it lives. + * + * Precedence — verified against the real MailManager::getConfig() via + * reflection, not guessed (see the task report): array_merge() puts + * parseConfiguration()'s result SECOND, and that result already has the + * URL's host/port/username/password merged in over top of the individual + * keys (Illuminate\Support\ConfigurationUrlParser::getPrimaryOptions() + * array_filters out only what the URL left null). So the URL wins for + * every field it actually supplies, and an unset MAIL_URL — isset() on a + * null config value is false, same as MailManager's own check — leaves + * this a no-op, returning the individual keys exactly as before. + */ + private function resolveSmtpConfig(): array + { + $config = (array) config('mail.mailers.smtp', []); + + if (! isset($config['url'])) { + return $config; + } + + return array_merge($config, (new ConfigurationUrlParser)->parseConfiguration($config)); + } + private function seed(): void { + $smtp = $this->resolveSmtpConfig(); + // The server, once. Whatever .env already proved workable — but not // config/mail.php's own placeholder (see UNCONFIGURED_HOST above). - $configuredHost = (string) config('mail.mailers.smtp.host', ''); + $configuredHost = (string) ($smtp['host'] ?? ''); $host = $configuredHost === self::UNCONFIGURED_HOST ? '' : $configuredHost; Settings::set('mail.host', $host); - $configuredPort = (int) config('mail.mailers.smtp.port', 0); + $configuredPort = (int) ($smtp['port'] ?? 0); $port = $configuredPort === self::UNCONFIGURED_PORT ? 0 : $configuredPort; Settings::set('mail.port', $port); @@ -101,7 +145,7 @@ return new class extends Migration // it is used: 465 means implicit TLS, everything else means STARTTLS. Settings::set('mail.encryption', $port === 465 ? 'ssl' : 'tls'); - $configuredUser = (string) config('mail.mailers.smtp.username', ''); + $configuredUser = (string) ($smtp['username'] ?? ''); // Placeholder domain for the four boxes below comes from the SMTP // LOGIN's domain, not from mail.from.address — even though address @@ -141,7 +185,7 @@ return new class extends Migration // moved: the app_secrets row is deliberately left in place below // rather than deleted, so this block only ever reads it. $storedPassword = DB::table('app_secrets')->where('key', 'mail.password')->value('value'); - $envPassword = (string) config('mail.mailers.smtp.password', ''); + $envPassword = (string) ($smtp['password'] ?? ''); // A fresh install may not have generated SECRETS_KEY yet — encrypting // is what needs it, not migrating. Without this check, an install with diff --git a/tests/Feature/Mail/MailboxSeedMigrationTest.php b/tests/Feature/Mail/MailboxSeedMigrationTest.php index e43ced9..89575a7 100644 --- a/tests/Feature/Mail/MailboxSeedMigrationTest.php +++ b/tests/Feature/Mail/MailboxSeedMigrationTest.php @@ -68,6 +68,74 @@ it('still stores a deliberately configured host and port untouched', function () ->and(Settings::get('mail.port'))->toBe(587); }); +it('resolves host, port, username and password from MAIL_URL when the individual SMTP keys are unset', function () { + // Codex R15#3, P1: an install may configure SMTP through Laravel's OTHER + // supported form, MAIL_URL (config/mail.php carries 'url' => + // env('MAIL_URL')), instead of MAIL_HOST/MAIL_USERNAME/MAIL_PASSWORD. + // When it does, the individual keys sit at config/mail.php's own + // placeholder defaults below (127.0.0.1, 2525, null, null) because + // nothing ever set them — MailManager only substitutes the URL's + // components in at transport-build time, inside its own protected + // getConfig(). Read raw, as this migration used to, that is exactly + // UNCONFIGURED_HOST / UNCONFIGURED_PORT: it stored a blank host, port 0, + // and an unconfigured mailbox for an install that was sending mail + // successfully. + clearMailboxSeed(); + config()->set('mail.mailers.smtp.host', '127.0.0.1'); + config()->set('mail.mailers.smtp.port', 2525); + config()->set('mail.mailers.smtp.username', null); + config()->set('mail.mailers.smtp.password', null); + config()->set('mail.mailers.smtp.url', 'smtp://no-reply%40clupilot.com:s3cr%40t@smtp.clupilot.test:587'); + + loadMailboxSeedMigration()->up(); + + expect(Settings::get('mail.host'))->toBe('smtp.clupilot.test') + ->and(Settings::get('mail.port'))->toBe(587) + ->and(Settings::get('mail.encryption'))->toBe('tls'); + + $box = Mailbox::findByKey('no-reply'); + + // isConfigured() is exactly what MailboxResolver checks before handing a + // purpose to MailboxTransport — false here would mean every purpose + // mailer falls through to "unconfigured" and throws on send, the outage + // this finding describes. + expect($box->isConfigured())->toBeTrue() + ->and($box->smtpUsername())->toBe('no-reply@clupilot.com') + // Decoded from the URL's percent-encoding the same way MailManager + // decodes it (rawurldecode inside ConfigurationUrlParser), not left + // as the literal 's3cr%40t'. + ->and($box->password)->toBe('s3cr@t'); +}); + +it("prefers MAIL_URL over the individual SMTP keys when both are set, matching MailManager's own precedence", function () { + // Verified against the real (protected) Illuminate\Mail\MailManager:: + // getConfig() via reflection, not guessed: with both an 'url' and + // individually-set host/port/username/password present, the URL's + // components win for every field it supplies — Illuminate\Support\ + // ConfigurationUrlParser::getPrimaryOptions() array_merges them in AFTER + // the individual keys, and array_filter there only drops what the URL + // left null. Decoy values stand in for the individual keys here + // specifically so a migration that read them instead of the URL would + // fail this test with THEIR value, not a blank one — the sibling test + // above already covers the blank/placeholder-default shape of the bug. + clearMailboxSeed(); + config()->set('mail.mailers.smtp.host', 'decoy-host.invalid'); + config()->set('mail.mailers.smtp.port', 9999); + config()->set('mail.mailers.smtp.username', 'decoy@invalid.test'); + config()->set('mail.mailers.smtp.password', 'decoy-password'); + config()->set('mail.mailers.smtp.url', 'smtp://no-reply%40clupilot.com:s3cr%40t@smtp.clupilot.test:587'); + + loadMailboxSeedMigration()->up(); + + expect(Settings::get('mail.host'))->toBe('smtp.clupilot.test') + ->and(Settings::get('mail.port'))->toBe(587); + + $box = Mailbox::findByKey('no-reply'); + + expect($box->smtpUsername())->toBe('no-reply@clupilot.com') + ->and($box->password)->toBe('s3cr@t'); +}); + 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