From 3ff2c3ff74acd5f1b7a4ddec8a612a247e3c2974 Mon Sep 17 00:00:00 2001 From: IanM Date: Thu, 18 Jun 2026 22:06:44 +0100 Subject: [PATCH] fix(core): set an explicit title on notification emails so it can't leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The notification email template renders `{{ $title ?? trans('...default_title') }}`. The view factory is a singleton, so a `title` shared by an earlier informational email (password reset, "send test email", account activation) lingered on it, and NotificationMailer rendered without its own title — so the notification body heading showed the previous email's title (e.g. "Reset Your Password") instead of "Notification". NotificationMailer now passes an explicit default title in its render data, overriding any stale shared value. Fixes #4767 --- .../src/Notification/NotificationMailer.php | 6 +- .../integration/mail/EmailTitleLeakTest.php | 105 ++++++++++++++++++ .../Notification/NotificationMailerTest.php | 27 +++++ 3 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 framework/core/tests/integration/mail/EmailTitleLeakTest.php diff --git a/framework/core/src/Notification/NotificationMailer.php b/framework/core/src/Notification/NotificationMailer.php index 9d7034d8a5..822bb3b96f 100644 --- a/framework/core/src/Notification/NotificationMailer.php +++ b/framework/core/src/Notification/NotificationMailer.php @@ -45,8 +45,12 @@ public function send(MailableInterface&BlueprintInterface $blueprint, User $user $forumTitle = $this->settings->get('forum_title'); $username = $user->display_name; $userEmail = $user->email; + // Pass an explicit title so the body heading can't inherit a stale `title` + // left on the shared (singleton) view factory by an earlier email — e.g. a + // password reset's "Reset Your Password". See flarum/framework#4767. + $title = $this->translator->trans('core.email.notification.default_title'); - $data = compact('blueprint', 'user', 'unsubscribeLink', 'settingsLink', 'type', 'forumTitle', 'username', 'userEmail'); + $data = compact('blueprint', 'user', 'unsubscribeLink', 'settingsLink', 'type', 'forumTitle', 'username', 'userEmail', 'title'); $this->view->share($data); diff --git a/framework/core/tests/integration/mail/EmailTitleLeakTest.php b/framework/core/tests/integration/mail/EmailTitleLeakTest.php new file mode 100644 index 0000000000..708a057fc3 --- /dev/null +++ b/framework/core/tests/integration/mail/EmailTitleLeakTest.php @@ -0,0 +1,105 @@ +share(['title' => 'Reset Your Password', ...])`, which persists on the + * shared factory. NotificationMailer renders the notification WITHOUT a `title` + * key, and the blade falls back `{{ $title ?? trans('...default_title') }}`. With + * a stale shared `title` present, the fallback never fires, so the notification + * shows the previous email's title. + * + * This test exercises the real singleton view factory and the real notification + * blade — the actual buggy components — rather than mocks. + */ +class EmailTitleLeakTest extends TestCase +{ + private function viewFactory(): Factory + { + return $this->app()->getContainer()->make(Factory::class); + } + + /** + * The data NotificationMailer hands to the notification view. Since the fix for + * #4767 it includes an explicit `title` (the default notification title), so the + * heading can't inherit a stale `title` left on the shared view factory. + */ + private function notificationData(): array + { + return [ + 'user' => (object) ['email' => 'recipient@example.com'], + 'unsubscribeLink' => 'https://example.com/unsubscribe', + 'settingsLink' => 'https://example.com/settings', + 'type' => 'testNotification', + 'forumTitle' => 'Test Forum', + 'username' => 'Recipient', + 'userEmail' => 'recipient@example.com', + 'body' => 'The notification body.', + 'title' => 'Notification', + ]; + } + + /** + * Extract the body heading (`

`) the notification template renders, so we + * assert on the title specifically — the word "Notification" also appears in + * the footer, which would make a whole-document match unreliable. + */ + private function renderedHeading(string $html): string + { + $this->assertMatchesRegularExpression('#

(.*?)

#s', $html); + preg_match('#

(.*?)

#s', $html, $m); + + return trim($m[1]); + } + + #[Test] + public function notification_uses_its_default_title_when_rendered_in_isolation(): void + { + $view = $this->viewFactory(); + + // Render the way NotificationMailer does: share the data, then make. + $data = $this->notificationData(); + $view->share($data); + $html = $view->make('mail::html.notification', $data)->render(); + + // Baseline: with nothing leaked, the heading is the default notification title. + $this->assertSame('Notification', $this->renderedHeading($html)); + } + + #[Test] + public function notification_title_does_not_leak_from_a_previously_sent_informational_email(): void + { + $view = $this->viewFactory(); + + // Simulate SendInformationalEmailJob (e.g. password reset) having run first + // in this process: it shares a `title` onto the singleton view factory. + $view->share(['title' => 'Reset Your Password']); + + // Now render a notification the way the fixed NotificationMailer does: + // the data carries its own explicit `title`, so the stale shared one is + // overridden rather than inherited. + $data = $this->notificationData(); + $view->share($data); + $html = $view->make('mail::html.notification', $data)->render(); + + // Regression guard for #4767: the heading is the notification's own title, + // NOT the leaked password-reset title. + $this->assertSame('Notification', $this->renderedHeading($html)); + } +} diff --git a/framework/core/tests/unit/Notification/NotificationMailerTest.php b/framework/core/tests/unit/Notification/NotificationMailerTest.php index 06b4662463..9a5ad015bc 100644 --- a/framework/core/tests/unit/Notification/NotificationMailerTest.php +++ b/framework/core/tests/unit/Notification/NotificationMailerTest.php @@ -46,6 +46,7 @@ protected function setUp(): void // Common stub setup $this->translator->shouldReceive('setLocale')->once(); + $this->translator->shouldReceive('trans')->with('core.email.notification.default_title')->andReturn('Notification')->byDefault(); $this->settings->shouldReceive('get')->with('default_locale')->andReturn('en'); $this->settings->shouldReceive('get')->with('forum_title')->andReturn('Test Forum'); @@ -76,6 +77,32 @@ public function successful_send_delegates_to_mailer(): void $this->notificationMailer->send($this->makeBlueprint(), $this->makeUser()); } + #[Test] + public function send_provides_a_default_title_so_the_body_heading_cannot_leak_from_a_previous_email(): void + { + // The notification template renders `{{ $title ?? trans('...default_title') }}`. + // Because the view factory is a singleton, a `title` shared by an earlier + // informational email (password reset etc.) lingers and is picked up here + // unless NotificationMailer supplies its own. So NotificationMailer must pass + // an explicit `title` in the data it renders with. (See #4767.) + $this->translator->shouldReceive('trans') + ->with('core.email.notification.default_title') + ->andReturn('Notification'); + + $captured = null; + $this->mailer->shouldReceive('send')->once() + ->with(m::any(), m::on(function ($data) use (&$captured) { + $captured = $data; + + return true; + }), m::any()); + + $this->notificationMailer->send($this->makeBlueprint(), $this->makeUser()); + + $this->assertArrayHasKey('title', $captured, 'NotificationMailer must pass an explicit title.'); + $this->assertSame('Notification', $captured['title']); + } + #[Test] public function mailer_exception_propagates_to_caller(): void {