From 5734c17ee0c0bbf14f629b06eb11beada5c74f3b Mon Sep 17 00:00:00 2001 From: hinotoi-agent Date: Fri, 29 May 2026 09:24:32 +0800 Subject: [PATCH] fix(msteams): trust service URLs before replies --- nanobot/channels/msteams.py | 46 +++++++++++++++++++++++++++++++++ tests/test_msteams.py | 51 +++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/nanobot/channels/msteams.py b/nanobot/channels/msteams.py index 3487c276f..b184b9b07 100644 --- a/nanobot/channels/msteams.py +++ b/nanobot/channels/msteams.py @@ -53,6 +53,10 @@ if MSTEAMS_AVAILABLE: MSTEAMS_REF_TTL_DAYS = 30 MSTEAMS_WEBCHAT_HOST = "webchat.botframework.com" +MSTEAMS_DEFAULT_TRUSTED_SERVICE_URL_HOSTS = [ + "smba.trafficmanager.net", + "*.botframework.com", +] MSTEAMS_REF_META_FILENAME = "msteams_conversations_meta.json" MSTEAMS_REF_LOCK_FILENAME = "msteams_conversations.lock" MSTEAMS_REF_TOUCH_INTERVAL_S = 300 @@ -76,6 +80,9 @@ class MSTeamsConfig(Base): prune_web_chat_refs: bool = True prune_non_personal_refs: bool = True ref_touch_interval_s: int = Field(default=MSTEAMS_REF_TOUCH_INTERVAL_S, ge=0) + trusted_service_url_hosts: list[str] = Field( + default_factory=lambda: MSTEAMS_DEFAULT_TRUSTED_SERVICE_URL_HOSTS.copy() + ) @dataclass @@ -242,6 +249,11 @@ class MSTeamsChannel(BaseChannel): if not ref: raise RuntimeError(f"MSTeams conversation ref not found for chat_id={msg.chat_id}") + if not self._is_trusted_service_url(ref.service_url): + raise RuntimeError( + f"MSTeams conversation ref has untrusted service_url for chat_id={msg.chat_id}" + ) + token = await self._get_access_token() base_url = f"{ref.service_url.rstrip('/')}/v3/conversations/{ref.conversation_id}/activities" use_thread_reply = self.config.reply_in_thread and bool(ref.activity_id) @@ -284,6 +296,13 @@ class MSTeamsChannel(BaseChannel): if not sender_id or not conversation_id or not service_url: return + if not self._is_trusted_service_url(service_url): + self.logger.warning( + "Ignoring MSTeams activity with untrusted serviceUrl host: {}", + service_url, + ) + return + if recipient.get("id") and from_user.get("id") == recipient.get("id"): return @@ -626,6 +645,29 @@ class MSTeamsChannel(BaseChannel): return host == MSTEAMS_WEBCHAT_HOST or host.endswith(f".{MSTEAMS_WEBCHAT_HOST}") return MSTEAMS_WEBCHAT_HOST in normalized.lower() + def _is_trusted_service_url(self, service_url: str) -> bool: + """Return True for HTTPS Bot Framework service URLs trusted for bearer replies.""" + parsed = urlparse(service_url.strip()) + if parsed.scheme.lower() != "https": + return False + + host = (parsed.hostname or "").strip().lower().rstrip(".") + if not host: + return False + + for pattern in self.config.trusted_service_url_hosts: + trusted_host = str(pattern or "").strip().lower().rstrip(".") + if not trusted_host: + continue + if trusted_host.startswith("*."): + suffix = trusted_host[1:] + if host.endswith(suffix) and host != suffix.lstrip("."): + return True + continue + if host == trusted_host: + return True + return False + def _prune_conversation_refs(self, *, now: float | None = None) -> bool: """Remove stale and unsupported conversation refs from memory.""" if not self._conversation_refs: @@ -637,6 +679,10 @@ class MSTeamsChannel(BaseChannel): keys_to_drop: list[str] = [] for key, ref in self._conversation_refs.items(): + if not self._is_trusted_service_url(ref.service_url): + keys_to_drop.append(key) + continue + if self.config.prune_web_chat_refs and self._is_webchat_service_url(ref.service_url): keys_to_drop.append(key) continue diff --git a/tests/test_msteams.py b/tests/test_msteams.py index 39202ba02..4eb9f2146 100644 --- a/tests/test_msteams.py +++ b/tests/test_msteams.py @@ -434,6 +434,38 @@ async def test_handle_activity_denied_sender_does_not_store_ref(make_channel, tm assert not (tmp_path / "state" / "msteams_conversations.json").exists() +@pytest.mark.asyncio +async def test_handle_activity_rejects_untrusted_service_url(make_channel, tmp_path): + ch = make_channel(validateInboundAuth=False, allowFrom=["*"]) + + activity = { + "type": "message", + "id": "activity-poison", + "text": "Hello from forged Teams activity", + "serviceUrl": "https://attacker.example/collect", + "channelId": "msteams", + "conversation": { + "id": "conv-poison", + "conversationType": "personal", + }, + "from": { + "id": "29:attacker-user-id", + "aadObjectId": "attacker-user-id", + "name": "Attacker", + }, + "recipient": { + "id": "28:bot-id", + "name": "nanobot", + }, + } + + await ch._handle_activity(activity) + + assert ch.bus.inbound == [] + assert ch._conversation_refs == {} + assert not (tmp_path / "state" / "msteams_conversations.json").exists() + + @pytest.mark.asyncio async def test_handle_activity_mention_only_uses_default_response(make_channel): ch = make_channel() @@ -739,6 +771,25 @@ async def test_send_raises_when_conversation_ref_missing(make_channel): await ch.send(OutboundMessage(channel="msteams", chat_id="missing", content="Reply text")) +@pytest.mark.asyncio +async def test_send_rejects_untrusted_service_url_before_bearer_post(make_channel): + ch = make_channel() + fake_http = FakeHttpClient() + ch._http = fake_http + ch._token = "tok" + ch._token_expires_at = 9999999999 + ch._conversation_refs["conv-poison"] = ConversationRef( + service_url="https://attacker.example/collect", + conversation_id="conv-poison", + activity_id="activity-poison", + ) + + with pytest.raises(RuntimeError, match="untrusted service_url"): + await ch.send(OutboundMessage(channel="msteams", chat_id="conv-poison", content="Reply text")) + + assert fake_http.calls == [] + + @pytest.mark.asyncio async def test_send_raises_delivery_failures_for_retry(make_channel): ch = make_channel()