fix(msteams): prune stale and unsupported conversation refs
This commit is contained in:
@@ -651,6 +651,7 @@ Create or reuse a Microsoft Teams / Azure bot app registration. Set the bot mess
|
|||||||
> - `replyInThread: true` replies to the triggering Teams activity when a stored `activity_id` is available.
|
> - `replyInThread: true` replies to the triggering Teams activity when a stored `activity_id` is available.
|
||||||
> - `mentionOnlyResponse` controls what Nanobot receives when a user sends only a bot mention (`<at>Nanobot</at>`). Set to `""` to ignore mention-only messages.
|
> - `mentionOnlyResponse` controls what Nanobot receives when a user sends only a bot mention (`<at>Nanobot</at>`). Set to `""` to ignore mention-only messages.
|
||||||
> - `validateInboundAuth: true` enables inbound Bot Framework bearer-token validation (signature, issuer, audience, lifetime, `serviceUrl`). This is the safe default for public deployments. Only set it to `false` for local development or tightly controlled testing.
|
> - `validateInboundAuth: true` enables inbound Bot Framework bearer-token validation (signature, issuer, audience, lifetime, `serviceUrl`). This is the safe default for public deployments. Only set it to `false` for local development or tightly controlled testing.
|
||||||
|
> - Conversation refs are auto-pruned to avoid bad outbound routing: Web Chat refs, non-`personal` refs, and refs older than 30 days are removed.
|
||||||
|
|
||||||
**4. Run**
|
**4. Run**
|
||||||
|
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ import time
|
|||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
|
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
|
||||||
from typing import TYPE_CHECKING, Any
|
from typing import TYPE_CHECKING, Any
|
||||||
|
from urllib.parse import urlparse
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
from loguru import logger
|
from loguru import logger
|
||||||
@@ -43,6 +44,10 @@ if TYPE_CHECKING:
|
|||||||
if MSTEAMS_AVAILABLE:
|
if MSTEAMS_AVAILABLE:
|
||||||
import jwt
|
import jwt
|
||||||
|
|
||||||
|
MSTEAMS_REF_TTL_DAYS = 30
|
||||||
|
MSTEAMS_REF_TTL_S = MSTEAMS_REF_TTL_DAYS * 24 * 60 * 60
|
||||||
|
MSTEAMS_WEBCHAT_HOST = "webchat.botframework.com"
|
||||||
|
|
||||||
|
|
||||||
class MSTeamsConfig(Base):
|
class MSTeamsConfig(Base):
|
||||||
"""Microsoft Teams channel configuration."""
|
"""Microsoft Teams channel configuration."""
|
||||||
@@ -70,6 +75,7 @@ class ConversationRef:
|
|||||||
activity_id: str | None = None
|
activity_id: str | None = None
|
||||||
conversation_type: str | None = None
|
conversation_type: str | None = None
|
||||||
tenant_id: str | None = None
|
tenant_id: str | None = None
|
||||||
|
updated_at: float | None = None
|
||||||
|
|
||||||
|
|
||||||
class MSTeamsChannel(BaseChannel):
|
class MSTeamsChannel(BaseChannel):
|
||||||
@@ -103,6 +109,8 @@ class MSTeamsChannel(BaseChannel):
|
|||||||
self._refs_path = get_workspace_path() / "state" / "msteams_conversations.json"
|
self._refs_path = get_workspace_path() / "state" / "msteams_conversations.json"
|
||||||
self._refs_path.parent.mkdir(parents=True, exist_ok=True)
|
self._refs_path.parent.mkdir(parents=True, exist_ok=True)
|
||||||
self._conversation_refs: dict[str, ConversationRef] = self._load_refs()
|
self._conversation_refs: dict[str, ConversationRef] = self._load_refs()
|
||||||
|
if self._prune_conversation_refs():
|
||||||
|
self._save_refs(prune=False)
|
||||||
|
|
||||||
async def start(self) -> None:
|
async def start(self) -> None:
|
||||||
"""Start the Teams webhook listener."""
|
"""Start the Teams webhook listener."""
|
||||||
@@ -289,6 +297,7 @@ class MSTeamsChannel(BaseChannel):
|
|||||||
activity_id=activity_id or None,
|
activity_id=activity_id or None,
|
||||||
conversation_type=conversation_type or None,
|
conversation_type=conversation_type or None,
|
||||||
tenant_id=str((channel_data.get("tenant") or {}).get("id") or "") or None,
|
tenant_id=str((channel_data.get("tenant") or {}).get("id") or "") or None,
|
||||||
|
updated_at=time.time(),
|
||||||
)
|
)
|
||||||
self._save_refs()
|
self._save_refs()
|
||||||
|
|
||||||
@@ -491,9 +500,59 @@ class MSTeamsChannel(BaseChannel):
|
|||||||
logger.warning("Failed to load MSTeams conversation refs: {}", e)
|
logger.warning("Failed to load MSTeams conversation refs: {}", e)
|
||||||
return {}
|
return {}
|
||||||
|
|
||||||
def _save_refs(self) -> None:
|
def _is_webchat_service_url(self, service_url: str) -> bool:
|
||||||
|
"""Return True when service URL points to unsupported Bot Framework Web Chat."""
|
||||||
|
normalized = service_url.strip()
|
||||||
|
if not normalized:
|
||||||
|
return False
|
||||||
|
host = (urlparse(normalized).hostname or "").strip().lower()
|
||||||
|
if host:
|
||||||
|
return host == MSTEAMS_WEBCHAT_HOST or host.endswith(f".{MSTEAMS_WEBCHAT_HOST}")
|
||||||
|
return MSTEAMS_WEBCHAT_HOST in normalized.lower()
|
||||||
|
|
||||||
|
def _prune_conversation_refs(self, *, now: float | None = None) -> bool:
|
||||||
|
"""Remove stale and unsupported conversation refs from memory."""
|
||||||
|
if not self._conversation_refs:
|
||||||
|
return False
|
||||||
|
|
||||||
|
now_ts = time.time() if now is None else now
|
||||||
|
stale_before = now_ts - MSTEAMS_REF_TTL_S
|
||||||
|
keys_to_drop: list[str] = []
|
||||||
|
|
||||||
|
for key, ref in self._conversation_refs.items():
|
||||||
|
if self._is_webchat_service_url(ref.service_url):
|
||||||
|
keys_to_drop.append(key)
|
||||||
|
continue
|
||||||
|
|
||||||
|
conv_type = str(ref.conversation_type or "").strip().lower()
|
||||||
|
if conv_type and conv_type != "personal":
|
||||||
|
keys_to_drop.append(key)
|
||||||
|
continue
|
||||||
|
|
||||||
|
try:
|
||||||
|
updated_at = float(ref.updated_at) if ref.updated_at is not None else 0.0
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
updated_at = 0.0
|
||||||
|
if updated_at <= 0 or updated_at < stale_before:
|
||||||
|
keys_to_drop.append(key)
|
||||||
|
|
||||||
|
if not keys_to_drop:
|
||||||
|
return False
|
||||||
|
|
||||||
|
for key in keys_to_drop:
|
||||||
|
self._conversation_refs.pop(key, None)
|
||||||
|
logger.info(
|
||||||
|
"MSTeams pruned {} stale/unsupported conversation refs (ttl={} days)",
|
||||||
|
len(keys_to_drop),
|
||||||
|
MSTEAMS_REF_TTL_DAYS,
|
||||||
|
)
|
||||||
|
return True
|
||||||
|
|
||||||
|
def _save_refs(self, *, prune: bool = True) -> None:
|
||||||
"""Persist conversation references."""
|
"""Persist conversation references."""
|
||||||
try:
|
try:
|
||||||
|
if prune:
|
||||||
|
self._prune_conversation_refs()
|
||||||
data = {
|
data = {
|
||||||
key: {
|
key: {
|
||||||
"service_url": ref.service_url,
|
"service_url": ref.service_url,
|
||||||
@@ -502,6 +561,7 @@ class MSTeamsChannel(BaseChannel):
|
|||||||
"activity_id": ref.activity_id,
|
"activity_id": ref.activity_id,
|
||||||
"conversation_type": ref.conversation_type,
|
"conversation_type": ref.conversation_type,
|
||||||
"tenant_id": ref.tenant_id,
|
"tenant_id": ref.tenant_id,
|
||||||
|
"updated_at": ref.updated_at,
|
||||||
}
|
}
|
||||||
for key, ref in self._conversation_refs.items()
|
for key, ref in self._conversation_refs.items()
|
||||||
}
|
}
|
||||||
|
|||||||
+90
-3
@@ -17,7 +17,7 @@ from cryptography.hazmat.primitives.asymmetric import rsa
|
|||||||
|
|
||||||
import nanobot.channels.msteams as msteams_module
|
import nanobot.channels.msteams as msteams_module
|
||||||
from nanobot.bus.events import OutboundMessage
|
from nanobot.bus.events import OutboundMessage
|
||||||
from nanobot.channels.msteams import ConversationRef, MSTeamsChannel, MSTeamsConfig
|
from nanobot.channels.msteams import ConversationRef, MSTeamsChannel
|
||||||
|
|
||||||
|
|
||||||
class DummyBus:
|
class DummyBus:
|
||||||
@@ -115,6 +115,95 @@ async def test_handle_activity_personal_message_publishes_and_stores_ref(make_ch
|
|||||||
saved = json.loads((tmp_path / "state" / "msteams_conversations.json").read_text(encoding="utf-8"))
|
saved = json.loads((tmp_path / "state" / "msteams_conversations.json").read_text(encoding="utf-8"))
|
||||||
assert saved["conv-123"]["conversation_id"] == "conv-123"
|
assert saved["conv-123"]["conversation_id"] == "conv-123"
|
||||||
assert saved["conv-123"]["tenant_id"] == "tenant-id"
|
assert saved["conv-123"]["tenant_id"] == "tenant-id"
|
||||||
|
assert float(saved["conv-123"]["updated_at"]) > 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_prunes_stale_and_unsupported_conversation_refs(make_channel, tmp_path, monkeypatch):
|
||||||
|
now = 1_800_000_000.0
|
||||||
|
monkeypatch.setattr(msteams_module.time, "time", lambda: now)
|
||||||
|
|
||||||
|
state_dir = tmp_path / "state"
|
||||||
|
state_dir.mkdir(parents=True, exist_ok=True)
|
||||||
|
refs_path = state_dir / "msteams_conversations.json"
|
||||||
|
refs_path.write_text(
|
||||||
|
json.dumps(
|
||||||
|
{
|
||||||
|
"conv-valid": {
|
||||||
|
"service_url": "https://smba.trafficmanager.net/amer/",
|
||||||
|
"conversation_id": "conv-valid",
|
||||||
|
"conversation_type": "personal",
|
||||||
|
"updated_at": now - 60,
|
||||||
|
},
|
||||||
|
"conv-webchat": {
|
||||||
|
"service_url": "https://webchat.botframework.com/",
|
||||||
|
"conversation_id": "conv-webchat",
|
||||||
|
"conversation_type": "personal",
|
||||||
|
"updated_at": now - 60,
|
||||||
|
},
|
||||||
|
"conv-group": {
|
||||||
|
"service_url": "https://smba.trafficmanager.net/amer/",
|
||||||
|
"conversation_id": "conv-group",
|
||||||
|
"conversation_type": "channel",
|
||||||
|
"updated_at": now - 60,
|
||||||
|
},
|
||||||
|
"conv-stale": {
|
||||||
|
"service_url": "https://smba.trafficmanager.net/amer/",
|
||||||
|
"conversation_id": "conv-stale",
|
||||||
|
"conversation_type": "personal",
|
||||||
|
"updated_at": now - msteams_module.MSTEAMS_REF_TTL_S - 1,
|
||||||
|
},
|
||||||
|
"conv-missing-ts": {
|
||||||
|
"service_url": "https://smba.trafficmanager.net/amer/",
|
||||||
|
"conversation_id": "conv-missing-ts",
|
||||||
|
"conversation_type": "personal",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
indent=2,
|
||||||
|
),
|
||||||
|
encoding="utf-8",
|
||||||
|
)
|
||||||
|
|
||||||
|
ch = make_channel()
|
||||||
|
|
||||||
|
assert set(ch._conversation_refs.keys()) == {"conv-valid"}
|
||||||
|
assert ch._conversation_refs["conv-valid"].conversation_id == "conv-valid"
|
||||||
|
|
||||||
|
persisted = json.loads(refs_path.read_text(encoding="utf-8"))
|
||||||
|
assert set(persisted.keys()) == {"conv-valid"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_save_prunes_unsupported_conversation_refs(make_channel, tmp_path, monkeypatch):
|
||||||
|
now = 1_800_000_000.0
|
||||||
|
monkeypatch.setattr(msteams_module.time, "time", lambda: now)
|
||||||
|
|
||||||
|
ch = make_channel()
|
||||||
|
ch._conversation_refs = {
|
||||||
|
"conv-valid": ConversationRef(
|
||||||
|
service_url="https://smba.trafficmanager.net/amer/",
|
||||||
|
conversation_id="conv-valid",
|
||||||
|
conversation_type="personal",
|
||||||
|
updated_at=now,
|
||||||
|
),
|
||||||
|
"conv-webchat": ConversationRef(
|
||||||
|
service_url="https://webchat.botframework.com/",
|
||||||
|
conversation_id="conv-webchat",
|
||||||
|
conversation_type="personal",
|
||||||
|
updated_at=now,
|
||||||
|
),
|
||||||
|
"conv-group": ConversationRef(
|
||||||
|
service_url="https://smba.trafficmanager.net/amer/",
|
||||||
|
conversation_id="conv-group",
|
||||||
|
conversation_type="groupChat",
|
||||||
|
updated_at=now,
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
ch._save_refs()
|
||||||
|
|
||||||
|
assert set(ch._conversation_refs.keys()) == {"conv-valid"}
|
||||||
|
|
||||||
|
saved = json.loads((tmp_path / "state" / "msteams_conversations.json").read_text(encoding="utf-8"))
|
||||||
|
assert set(saved.keys()) == {"conv-valid"}
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -558,5 +647,3 @@ def test_msteams_default_config_includes_restart_notify_fields():
|
|||||||
assert "restartNotifyEnabled" not in cfg
|
assert "restartNotifyEnabled" not in cfg
|
||||||
assert "restartNotifyPreMessage" not in cfg
|
assert "restartNotifyPreMessage" not in cfg
|
||||||
assert "restartNotifyPostMessage" not in cfg
|
assert "restartNotifyPostMessage" not in cfg
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user