mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-06-13 14:23:58 +00:00
fix(msteams): trust service URLs before replies
This commit is contained in:
parent
9d3fe7c34b
commit
5734c17ee0
@ -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
|
||||
|
||||
@ -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()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user