Sanitize conversation ID for agentic requests on agents channel

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/client/connector_client.py

@@ -4,19 +4,23 @@
 """Connector Client for Microsoft Agents."""
 
 import logging
+import re
 from typing import Any, Optional
 from aiohttp import ClientSession
 from io import BytesIO
 
 from microsoft_agents.activity import (
     Activity,
     ChannelAccount,
+    Channels,
     ConversationParameters,
     ConversationResourceResponse,
     ResourceResponse,
+    RoleTypes,
     ConversationsResult,
     PagedMembersResult,
 )
+from microsoft_agents.activity.channel_id import ChannelId
 from microsoft_agents.hosting.core.connector import ConnectorClientBase
 from ..attachments_base import AttachmentsBase
 from ..conversations_base import ConversationsBase
@@ -138,8 +142,33 @@ def __init__(self, client: ClientSession, **kwargs):
         self.client = client
         self._max_conversation_id_length = kwargs.get("max_conversation_id_length", 150)
 
-    def _normalize_conversation_id(self, conversation_id: str) -> str:
-        return conversation_id[: self._max_conversation_id_length]
+    def _normalize_conversation_id(
+        self, conversation_id: str, activity: Optional[Activity] = None
+    ) -> str:
+        trimmed = conversation_id[: self._max_conversation_id_length]
+        if activity is not None and self._should_sanitize_conversation_id(activity):
+            return re.sub(r"[/\\#?]", "_", trimmed)
+        return trimmed
+
+    @staticmethod
+    def _should_sanitize_conversation_id(activity: Activity) -> bool:
+        channel_id = activity.channel_id
+        if not channel_id:
+            return False
+        base_channel = (
+            channel_id.channel
+            if isinstance(channel_id, ChannelId)
+            else channel_id.split(":", 1)[0]
+        )
+        if base_channel != Channels.agents:
+            return False
+        from_property = activity.from_property
+        if not from_property or not from_property.role:
+            return False
+        return from_property.role in (
+            RoleTypes.agentic_identity,
+            RoleTypes.agentic_user,
+        )
 
     async def get_conversations(
         self, continuation_token: Optional[str] = None
@@ -221,7 +250,7 @@ async def reply_to_activity(
 
         with spans.ConnectorReplyToActivity(conversation_id, activity_id) as span:
 
-            conversation_id = self._normalize_conversation_id(conversation_id)
+            conversation_id = self._normalize_conversation_id(conversation_id, body)
             url = f"v3/conversations/{conversation_id}/activities/{activity_id}"
 
             logger.info(
@@ -283,7 +312,7 @@ async def send_to_conversation(
 
         with spans.ConnectorSendToConversation(conversation_id, body.id) as span:
 
-            conversation_id = self._normalize_conversation_id(conversation_id)
+            conversation_id = self._normalize_conversation_id(conversation_id, body)
             url = f"v3/conversations/{conversation_id}/activities"
 
             logger.info(
@@ -330,7 +359,7 @@ async def update_activity(
 
         with spans.ConnectorUpdateActivity(conversation_id, activity_id) as span:
 
-            conversation_id = self._normalize_conversation_id(conversation_id)
+            conversation_id = self._normalize_conversation_id(conversation_id, body)
             url = f"v3/conversations/{conversation_id}/activities/{activity_id}"
 
             logger.info(

tests/hosting_core/connector/test_connector_client.py

@@ -9,7 +9,8 @@
 from aiohttp import web, ClientSession
 from aiohttp.test_utils import TestServer
 
-from microsoft_agents.activity import Activity, ResourceResponse
+from microsoft_agents.activity import Activity, Channels, ResourceResponse, RoleTypes
+from microsoft_agents.activity.channel_account import ChannelAccount
 from microsoft_agents.hosting.core.connector.client.connector_client import (
     ConversationsOperations,
 )
@@ -134,3 +135,243 @@ async def handler(request):
             assert result.id is None
         finally:
             await server.close()
+
+
+class TestNormalizeConversationId:
+    """Tests for ConversationsOperations._normalize_conversation_id and _should_sanitize_conversation_id."""
+
+    def _make_ops(self):
+        return ConversationsOperations(None)
+
+    # --- _should_sanitize_conversation_id ---
+
+    @pytest.mark.parametrize(
+        "role",
+        [RoleTypes.agentic_identity, RoleTypes.agentic_user],
+    )
+    def test_should_sanitize_when_agents_channel_and_agentic_role(self, role):
+        activity = Activity(
+            type="message",
+            channel_id=Channels.agents,
+            from_property=ChannelAccount(id="user1", role=role),
+        )
+        assert (
+            ConversationsOperations._should_sanitize_conversation_id(activity) is True
+        )
+
+    @pytest.mark.parametrize(
+        "role",
+        [RoleTypes.user, RoleTypes.agent, RoleTypes.skill],
+    )
+    def test_should_not_sanitize_when_non_agentic_role(self, role):
+        activity = Activity(
+            type="message",
+            channel_id=Channels.agents,
+            from_property=ChannelAccount(id="user1", role=role),
+        )
+        assert (
+            ConversationsOperations._should_sanitize_conversation_id(activity) is False
+        )
+
+    @pytest.mark.parametrize(
+        "channel",
+        [
+            Channels.ms_teams,
+            Channels.email,
+            Channels.direct_line,
+            Channels.webchat,
+            Channels.emulator,
+        ],
+    )
+    def test_should_not_sanitize_when_non_agents_channel(self, channel):
+        activity = Activity(
+            type="message",
+            channel_id=channel,
+            from_property=ChannelAccount(id="user1", role=RoleTypes.agentic_identity),
+        )
+        assert (
+            ConversationsOperations._should_sanitize_conversation_id(activity) is False
+        )
+
+    def test_should_not_sanitize_when_no_channel_id(self):
+        activity = Activity(
+            type="message",
+            from_property=ChannelAccount(id="user1", role=RoleTypes.agentic_identity),
+        )
+        assert (
+            ConversationsOperations._should_sanitize_conversation_id(activity) is False
+        )
+
+    def test_should_not_sanitize_when_no_from(self):
+        activity = Activity(type="message", channel_id=Channels.agents)
+        assert (
+            ConversationsOperations._should_sanitize_conversation_id(activity) is False
+        )
+
+    def test_should_not_sanitize_when_no_role(self):
+        activity = Activity(
+            type="message",
+            channel_id=Channels.agents,
+            from_property=ChannelAccount(id="user1"),
+        )
+        assert (
+            ConversationsOperations._should_sanitize_conversation_id(activity) is False
+        )
+
+    # --- _normalize_conversation_id ---
+
+    def test_normalize_truncates_to_max_length(self):
+        ops = self._make_ops()
+        long_id = "a" * 200
+        result = ops._normalize_conversation_id(long_id)
+        assert result == "a" * 150
+
+    def test_normalize_does_not_sanitize_without_activity(self):
+        ops = self._make_ops()
+        conv_id = "conv/with/slashes"
+        result = ops._normalize_conversation_id(conv_id)
+        assert result == conv_id
+
+    def test_normalize_sanitizes_slashes_for_agents_channel_with_agentic_role(self):
+        ops = self._make_ops()
+        conv_id = "conv/with/slashes"
+        activity = Activity(
+            type="message",
+            channel_id=Channels.agents,
+            from_property=ChannelAccount(id="user1", role=RoleTypes.agentic_identity),
+        )
+        result = ops._normalize_conversation_id(conv_id, activity)
+        assert result == "conv_with_slashes"
+
+    def test_normalize_sanitizes_all_path_chars_for_agents_channel(self):
+        """Test that /, \\, #, and ? are all replaced with _."""
+        ops = self._make_ops()
+        conv_id = "conv/with\\special#chars?here"
+        activity = Activity(
+            type="message",
+            channel_id=Channels.agents,
+            from_property=ChannelAccount(id="user1", role=RoleTypes.agentic_user),
+        )
+        result = ops._normalize_conversation_id(conv_id, activity)
+        assert result == "conv_with_special_chars_here"
+
+    def test_normalize_sanitizes_for_agents_subchannel(self):
+        """Test that agents:email sub-channel also triggers sanitization."""
+        ops = self._make_ops()
+        conv_id = "conv/with/slashes"
+        activity = Activity(
+            type="message",
+            channel_id="agents:email",
+            from_property=ChannelAccount(id="user1", role=RoleTypes.agentic_user),
+        )
+        result = ops._normalize_conversation_id(conv_id, activity)
+        assert result == "conv_with_slashes"
+
+    def test_normalize_does_not_sanitize_for_msteams_with_agentic_role(self):
+        """msteams channel should NOT sanitize the conversation ID."""
+        ops = self._make_ops()
+        conv_id = "conv/with/slashes"
+        activity = Activity(
+            type="message",
+            channel_id=Channels.ms_teams,
+            from_property=ChannelAccount(id="user1", role=RoleTypes.agentic_user),
+        )
+        result = ops._normalize_conversation_id(conv_id, activity)
+        assert result == conv_id
+
+    def test_normalize_truncates_before_sanitizing(self):
+        ops = ConversationsOperations(None, max_conversation_id_length=5)
+        conv_id = "ab/cd/ef"
+        activity = Activity(
+            type="message",
+            channel_id=Channels.agents,
+            from_property=ChannelAccount(id="user1", role=RoleTypes.agentic_identity),
+        )
+        # Truncated to 5 chars first: "ab/cd", then sanitized
+        result = ops._normalize_conversation_id(conv_id, activity)
+        assert result == "ab_cd"
+
+    def test_normalize_no_sanitize_for_non_agentic_role_with_agents_channel(self):
+        ops = self._make_ops()
+        conv_id = "conv/with/slashes"
+        activity = Activity(
+            type="message",
+            channel_id=Channels.agents,
+            from_property=ChannelAccount(id="user1", role=RoleTypes.user),
+        )
+        result = ops._normalize_conversation_id(conv_id, activity)
+        assert result == conv_id
+
+
+class TestSendToConversationUrlEncoding:
+    """Integration tests: sanitization of conversation_id in send_to_conversation."""
+
+    @pytest.mark.asyncio
+    async def test_send_to_conversation_sanitizes_conversation_id_for_agentic_agents_channel(
+        self,
+    ):
+        captured = {}
+
+        async def handler(request):
+            captured["raw_path"] = request.raw_path
+            return web.json_response({"id": "resp-1"})
+
+        routes = [web.post("/v3/conversations/{tail:.*}/activities", handler)]
+        app = web.Application()
+        app.router.add_routes(routes)
+
+        server = TestServer(app)
+        await server.start_server()
+        try:
+            async with ClientSession(base_url=server.make_url("/")) as session:
+                ops = ConversationsOperations(session)
+                activity = Activity(
+                    type="message",
+                    channel_id=Channels.agents,
+                    from_property=ChannelAccount(
+                        id="user1", role=RoleTypes.agentic_identity
+                    ),
+                )
+                await ops.send_to_conversation("conv/sub/id", activity)
+
+            assert "conv_sub_id" in captured["raw_path"]
+        finally:
+            await server.close()
+
+
+class TestReplyToActivityUrlEncoding:
+    """Integration tests: sanitization of conversation_id in reply_to_activity."""
+
+    @pytest.mark.asyncio
+    async def test_reply_to_activity_sanitizes_conversation_id_for_agentic_agents_channel(
+        self,
+    ):
+        captured = {}
+
+        async def handler(request):
+            captured["raw_path"] = request.raw_path
+            return web.json_response({"id": "resp-1"})
+
+        routes = [
+            web.post("/v3/conversations/{tail:.*}/activities/{activity_id}", handler)
+        ]
+        app = web.Application()
+        app.router.add_routes(routes)
+
+        server = TestServer(app)
+        await server.start_server()
+        try:
+            async with ClientSession(base_url=server.make_url("/")) as session:
+                ops = ConversationsOperations(session)
+                activity = Activity(
+                    type="message",
+                    channel_id=Channels.agents,
+                    from_property=ChannelAccount(
+                        id="user1", role=RoleTypes.agentic_user
+                    ),
+                )
+                await ops.reply_to_activity("conv/sub/id", "act-1", activity)
+
+            assert "conv_sub_id" in captured["raw_path"]
+        finally:
+            await server.close()