markdown: Fix #-mention of private channel's topic made by system bots.

Previously when system bots used to `#-mention` a private
channel's topics in cases like moving messages of a private
channel, then the #-mentions were not rendered by the
markdown. This was because the system bots did not have
authorization to mention these channels.

This is fixed by passing down an `acting_user` parameter
in code paths involving sending these move message
notifications so that permission of acting_user to mention
the topic is verified for rendering the markdown, rather
than that of the system bot.

zerver/actions/message_edit.py

@@ -226,6 +226,7 @@ def maybe_send_resolve_topic_notifications(
             ),
             message_type=Message.MessageType.RESOLVE_TOPIC_NOTIFICATION,
             limit_unread_user_ids=affected_participant_ids,
+            acting_user=user_profile,
         )
 
     return resolved_topic_message_id, False
@@ -293,6 +294,7 @@ def send_message_moved_breadcrumbs(
                     user=user_mention,
                     changed_messages_count=changed_messages_count,
                 ),
+                acting_user=user_profile,
             )
 
     if old_thread_notification_string is not None:
@@ -307,6 +309,7 @@ def send_message_moved_breadcrumbs(
                     new_location=new_topic_link,
                     changed_messages_count=changed_messages_count,
                 ),
+                acting_user=user_profile,
             )
 
 

zerver/actions/message_send.py

@@ -156,6 +156,7 @@ def render_incoming_message(
     mention_data: MentionData | None = None,
     url_embed_data: dict[str, UrlEmbedData | None] | None = None,
     email_gateway: bool = False,
+    acting_user: UserProfile | None = None,
 ) -> MessageRenderingResult:
     realm_alert_words_automaton = get_alert_word_automaton(realm)
     try:
@@ -167,6 +168,7 @@ def render_incoming_message(
             mention_data=mention_data,
             url_embed_data=url_embed_data,
             email_gateway=email_gateway,
+            acting_user=acting_user,
         )
     except MarkdownRenderingError:
         raise JsonableError(_("Unable to render message"))
@@ -576,6 +578,7 @@ def build_message_send_dict(
     limit_unread_user_ids: set[int] | None = None,
     disable_external_notifications: bool = False,
     recipients_for_user_creation_events: dict[UserProfile, set[int]] | None = None,
+    acting_user: UserProfile | None = None,
 ) -> SendMessageRequest:
     """Returns a dictionary that can be passed into do_send_messages.  In
     production, this is always called by check_message, but some
@@ -620,6 +623,7 @@ def build_message_send_dict(
         realm,
         mention_data=mention_data,
         email_gateway=email_gateway,
+        acting_user=acting_user,
     )
     message.rendered_content = rendering_result.rendered_content
     message.rendered_content_version = markdown_version
@@ -1707,6 +1711,7 @@ def check_message(
     limit_unread_user_ids: set[int] | None = None,
     disable_external_notifications: bool = False,
     archived_channel_notice: bool = False,
+    acting_user: UserProfile | None = None,
 ) -> SendMessageRequest:
     """See
     https://zulip.readthedocs.io/en/latest/subsystems/sending-messages.html
@@ -1850,6 +1855,7 @@ def check_message(
         limit_unread_user_ids=limit_unread_user_ids,
         disable_external_notifications=disable_external_notifications,
         recipients_for_user_creation_events=recipients_for_user_creation_events,
+        acting_user=acting_user,
     )
 
     if (
@@ -1888,6 +1894,7 @@ def _internal_prep_message(
     forged: bool = False,
     forged_timestamp: float | None = None,
     archived_channel_notice: bool = False,
+    acting_user: UserProfile | None = None,
 ) -> SendMessageRequest | None:
     """
     Create a message object and checks it, but doesn't send it or save it to the database.
@@ -1920,6 +1927,7 @@ def _internal_prep_message(
             forged=forged,
             forged_timestamp=forged_timestamp,
             archived_channel_notice=archived_channel_notice,
+            acting_user=acting_user,
         )
     except JsonableError as e:
         logging.exception(
@@ -1944,6 +1952,7 @@ def internal_prep_stream_message(
     forged: bool = False,
     forged_timestamp: float | None = None,
     archived_channel_notice: bool = False,
+    acting_user: UserProfile | None = None,
 ) -> SendMessageRequest | None:
     """
     See _internal_prep_message for details of how this works.
@@ -1962,6 +1971,7 @@ def internal_prep_stream_message(
         forged=forged,
         forged_timestamp=forged_timestamp,
         archived_channel_notice=archived_channel_notice,
+        acting_user=acting_user,
     )
 
 
@@ -2041,6 +2051,7 @@ def internal_send_stream_message(
     message_type: int = Message.MessageType.NORMAL,
     limit_unread_user_ids: set[int] | None = None,
     archived_channel_notice: bool = False,
+    acting_user: UserProfile | None = None,
 ) -> int | None:
     message = internal_prep_stream_message(
         sender,
@@ -2051,6 +2062,7 @@ def internal_send_stream_message(
         message_type=message_type,
         limit_unread_user_ids=limit_unread_user_ids,
         archived_channel_notice=archived_channel_notice,
+        acting_user=acting_user,
     )
 
     if message is None:

zerver/lib/markdown/__init__.py

@@ -67,7 +67,7 @@ from zerver.lib.timezone import common_timezones
 from zerver.lib.types import LinkifierDict
 from zerver.lib.url_encoding import encode_stream, hash_util_encode
 from zerver.lib.url_preview.types import UrlEmbedData, UrlOEmbedData
-from zerver.models import Message, Realm
+from zerver.models import Message, Realm, UserProfile
 from zerver.models.linkifiers import linkifiers_for_realm
 from zerver.models.realm_emoji import EmojiInfo, get_name_keyed_dict_for_active_realm_emoji
 
@@ -2697,6 +2697,7 @@ def do_convert(
     mention_data: MentionData | None = None,
     email_gateway: bool = False,
     no_previews: bool = False,
+    acting_user: UserProfile | None = None,
 ) -> MessageRenderingResult:
     """Convert Markdown to HTML, with Zulip-specific settings and hacks."""
     # This logic is a bit convoluted, but the overall goal is to support a range of use cases:
@@ -2772,13 +2773,16 @@ def do_convert(
             mention_backend = MentionBackend(message_realm.id)
             mention_data = MentionData(mention_backend, content, message_sender)
 
+        if acting_user is None:
+            acting_user = message_sender
+
         stream_names = possible_linked_stream_names(content)
-        stream_name_info = mention_data.get_stream_name_map(
-            stream_names, acting_user=message_sender
-        )
+        stream_name_info = mention_data.get_stream_name_map(stream_names, acting_user=acting_user)
 
         linked_stream_topic_data = possible_linked_topics(content)
-        topic_info = mention_data.get_topic_info_map(linked_stream_topic_data)
+        topic_info = mention_data.get_topic_info_map(
+            linked_stream_topic_data, acting_user=acting_user
+        )
 
         if content_has_emoji_syntax(content):
             active_realm_emoji = get_name_keyed_dict_for_active_realm_emoji(message_realm.id)
@@ -2877,6 +2881,7 @@ def markdown_convert(
     mention_data: MentionData | None = None,
     email_gateway: bool = False,
     no_previews: bool = False,
+    acting_user: UserProfile | None = None,
 ) -> MessageRenderingResult:
     markdown_stats_start()
     ret = do_convert(
@@ -2890,6 +2895,7 @@ def markdown_convert(
         mention_data,
         email_gateway,
         no_previews=no_previews,
+        acting_user=acting_user,
     )
     markdown_stats_finish()
     return ret
@@ -2903,6 +2909,7 @@ def render_message_markdown(
     url_embed_data: dict[str, UrlEmbedData | None] | None = None,
     mention_data: MentionData | None = None,
     email_gateway: bool = False,
+    acting_user: UserProfile | None = None,
 ) -> MessageRenderingResult:
     """
     This is basically just a wrapper for do_render_markdown.
@@ -2925,6 +2932,7 @@ def render_message_markdown(
         url_embed_data=url_embed_data,
         mention_data=mention_data,
         email_gateway=email_gateway,
+        acting_user=acting_user,
     )
 
     return rendering_result

zerver/lib/mention.py

@@ -423,16 +423,12 @@ class MentionData:
     def get_stream_name_map(
         self, stream_names: set[str], acting_user: UserProfile | None
     ) -> dict[str, int]:
-        return self.mention_backend.get_stream_name_map(
-            stream_names, acting_user=self.message_sender
-        )
+        return self.mention_backend.get_stream_name_map(stream_names, acting_user=acting_user)
 
     def get_topic_info_map(
-        self, channel_topics: set[ChannelTopicInfo]
+        self, channel_topics: set[ChannelTopicInfo], acting_user: UserProfile | None
     ) -> dict[ChannelTopicInfo, int | None]:
-        return self.mention_backend.get_topic_info_map(
-            channel_topics, acting_user=self.message_sender
-        )
+        return self.mention_backend.get_topic_info_map(channel_topics, acting_user=acting_user)
 
 
 def silent_mention_syntax_for_user(user_profile: UserProfile) -> str:

zerver/tests/test_markdown.py

@@ -72,6 +72,7 @@ from zerver.models.groups import SystemGroups
 from zerver.models.linkifiers import linkifiers_for_realm
 from zerver.models.realms import get_realm
 from zerver.models.streams import get_stream
+from zerver.models.users import get_system_bot
 
 
 class SimulatedFencedBlockPreprocessor(FencedBlockPreprocessor):
@@ -3236,6 +3237,21 @@ class MarkdownStreamTopicMentionTests(ZulipTestCase):
             f'<p><a class="stream-topic" data-stream-id="{core_stream.id}" href="/#narrow/channel/{core_stream.id}-core/topic/testing/with/{msg_id}">#{core_stream.name} &gt; testing</a></p>',
         )
 
+        # Test permalinks generated when a user with no access to the channel
+        # sends a topic link on behalf of a user who has access to the channel.
+        notification_bot = get_system_bot(settings.NOTIFICATION_BOT, iago.realm_id)
+
+        msg = Message(
+            sender=notification_bot,
+            sending_client=get_client("test"),
+            realm=realm,
+        )
+        content = "#**core>testing**"
+        self.assertEqual(
+            render_message_markdown(msg, content, acting_user=iago).rendered_content,
+            f'<p><a class="stream-topic" data-stream-id="{core_stream.id}" href="/#narrow/channel/{core_stream.id}-core/topic/testing/with/{msg_id}">#{core_stream.name} &gt; testing</a></p>',
+        )
+
         # Test newly subscribed user to a channel with protected history
         # won't have accessed to this message, and hence, the topic
         # link would not be a permalink.
@@ -3252,6 +3268,22 @@ class MarkdownStreamTopicMentionTests(ZulipTestCase):
             f'<p><a class="stream-topic" data-stream-id="{core_stream.id}" href="/#narrow/channel/{core_stream.id}-core/topic/testing">#{core_stream.name} &gt; testing</a></p>',
         )
 
+        # Test permalinks would not be generated when a user with no access to the
+        # channel sends a topic link on behalf of another user who has no access to
+        # the channel.
+        cordelia = self.example_user("cordelia")
+
+        msg = Message(
+            sender=notification_bot,
+            sending_client=get_client("test"),
+            realm=realm,
+        )
+        content = "#**core>testing**"
+        self.assertEqual(
+            render_message_markdown(msg, content, acting_user=cordelia).rendered_content,
+            "<p>#<strong>core&gt;testing</strong></p>",
+        )
+
         # Test when trying to render a topic link of a channel with protected
         # history, if message_sender is None, topic link is not permalink.
         content = "#**core>testing**"