From 003febe399a94c99c2da6d6184cb7aa2d6f6f938 Mon Sep 17 00:00:00 2001 From: Noah Markert Date: Thu, 4 Jun 2026 00:36:58 +0200 Subject: [PATCH] Add tests to ensure that email notification links are sanitized (#19741) Fix #2860 Also cleans up comments around plans to define `ALLOWED_SCHEMES` as we can rely on Bleach's `ALLOWED_PROTOCOLS` defaults (`http`, `https` and `mailto`). --- changelog.d/19741.misc | 1 + synapse/push/mailer.py | 4 --- tests/push/test_email.py | 67 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 changelog.d/19741.misc diff --git a/changelog.d/19741.misc b/changelog.d/19741.misc new file mode 100644 index 0000000000..55347570d8 --- /dev/null +++ b/changelog.d/19741.misc @@ -0,0 +1 @@ +Added tests to ensure that email notification links are sanitized. Contributed by Noah Markert. diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index 1ebbc6d4f3..c1cd070d8e 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -111,8 +111,6 @@ ALLOWED_ATTRS = { # would make sense if we did "img": ["src"], } -# When bleach release a version with this option, we can specify schemes -# ALLOWED_SCHEMES = ["http", "https", "ftp", "mailto"] class Mailer: @@ -971,8 +969,6 @@ def safe_markup(raw_html: str) -> Markup: raw_html, tags=ALLOWED_TAGS, attributes=ALLOWED_ATTRS, - # bleach master has this, but it isn't released yet - # protocols=ALLOWED_SCHEMES, strip=True, ) ) diff --git a/tests/push/test_email.py b/tests/push/test_email.py index d3822b8643..b1dcd38704 100644 --- a/tests/push/test_email.py +++ b/tests/push/test_email.py @@ -379,6 +379,73 @@ class EmailPusherTests(HomeserverTestCase): self.assertIn("_matrix/media/v1/thumbnail/DUMMY_MEDIA_ID", html) + @parameterized.expand( + [ + ( + "https allowed", + 'test click me', + 'test click me', + ), + ( + "http allowed", + 'test click me', + 'test click me', + ), + ( + "mailto allowed", + 'test click me', + 'test click me', + ), + ( + "javascript disallowed", + 'test click me', + "test click me", + ), + ( + "data disallowed", + 'test click me', + "test click me", + ), + ], + ) + def test_link_schemes_sanitized( + self, + _test_label: str, + input: str, + expected: str, + ) -> None: + # Create a room + room = self.helper.create_room_as(self.user_id, tok=self.access_token) + + self.helper.invite( + room=room, src=self.user_id, tok=self.access_token, targ=self.others[0].id + ) + self.helper.join(room=room, user=self.others[0].id, tok=self.others[0].token) + + self.helper.send_event( + room, + type="m.room.message", + content={ + "msgtype": "m.text", + "format": "org.matrix.custom.html", + "formatted_body": input, + "body": "test click me", # Plain-text Fallback + }, + tok=self.others[0].token, + ) + + args, kwargs = self._check_for_mail() + msg: bytes = args[5] + html_message = email.message_from_bytes(msg).get_payload(i=1) + assert isinstance(html_message, email.message.Message) + + # Extract the `bytes` from the html Message object, and decode to a `str`. + html = html_message.get_payload(decode=True) + assert isinstance(html, bytes) + html = html.decode() + + self.assertIn(expected, html) + def test_empty_room(self) -> None: """All users leaving a room shouldn't cause the pusher to break.""" # Create a simple room with two users