diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/client/connector_client.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/client/connector_client.py index 777c685ab..28562e7ef 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/client/connector_client.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/client/connector_client.py @@ -4,6 +4,7 @@ """Connector Client for Microsoft Agents.""" import logging +import re from typing import Any, Optional from aiohttp import ClientSession from io import BytesIO @@ -11,12 +12,15 @@ 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( diff --git a/tests/hosting_core/connector/test_connector_client.py b/tests/hosting_core/connector/test_connector_client.py index 59ae34cb3..2a8db01d3 100644 --- a/tests/hosting_core/connector/test_connector_client.py +++ b/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()