Resolve MAIL_URL before seeding, so a takeover doesn't blank out a working relay

The seed migration read config('mail.mailers.smtp.host') and its three
siblings directly. An install configuring SMTP through MAIL_URL (the
other form config/mail.php supports) never sets those individual keys —
MailManager only substitutes the URL's components in at transport-build
time, inside its own protected getConfig(). Read raw, that left the
migration seeding config/mail.php's own placeholder defaults, which the
UNCONFIGURED_HOST/UNCONFIGURED_PORT guards then read as genuinely unset:
blank host, port 0, an unconfigured mailbox, for an install that was
sending mail successfully.

resolveSmtpConfig() calls the same Illuminate\Support\ConfigurationUrlParser
MailManager::getConfig() itself delegates to, so this is the same
resolution rather than a second copy of its merge order. Verified against
the real (protected) getConfig() via reflection: the URL wins for
host/port/username/password wherever it actually supplies one, and an
unset MAIL_URL leaves this a no-op.
feat/mailboxes
nexxo 2026-07-28 05:08:58 +02:00
parent e8a3b8d5be
commit f0c3bd9c2e
2 changed files with 116 additions and 4 deletions

View File

@ -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

View File

@ -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