Fix cross-bot infinite reply loop: honor the existing other_bots config

muc_message() only ever ignored the bot's own nick. With multiple sibling
bot instances (urlbug, pibug, urlbrot) in the same room, each correctly
ignores itself but happily reacts to the others' replies - discovered
live when a smoke-test message containing a CVE id caused
SecurityTracker's URL reply to be picked up by the other bot's
URLResolver, whose title-scrape reply contained the same CVE id, causing
SecurityTracker to fire again, forever, across both bots, hammering
security-tracker.debian.org indefinitely until manually restarted.

persistent_config.ini.spec already had an `other_bots` key seemingly
designed for exactly this, but nothing in the codebase ever read it. Wired
it into muc_message()'s existing self-nick check. Populating the actual
other_bots list per host is a separate, per-deployment config change (not
committed here, since persistent_config.ini is gitignored per-host state).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

diff --git a/src/distbot/bot/bot.py b/src/distbot/bot/bot.py
index e29f1ab869d15c2590538091aab06cb4fc59784d..34bbda063b5c21d914d25b899825d6bf86c99146 100644 (file)
--- a/src/distbot/bot/bot.py
+++ b/src/distbot/bot/bot.py
@@ -118,7 +118,15 @@ class Bot(slixmpp.ClientXMPP):
             logger.debug("%s: self-ping timed out; assuming still joined", room)
 
     def muc_message(self, msg: Message):
-        if msg['mucnick'] == self.nick or 'groupchat' != msg['type']:
+        if 'groupchat' != msg['type']:
+            return False
+        if msg['mucnick'] == self.nick or msg['mucnick'] in (conf_get('other_bots') or []):
+            # Ignoring only our own nick isn't enough: two sibling bot
+            # instances in the same room each correctly ignore themselves
+            # but will happily react to each other's replies, which can
+            # produce an infinite cross-bot reply loop (e.g. one bot's
+            # catch-all posts a URL, the other's URL-resolver replies with
+            # a title that re-triggers the first bot's catch-all, forever).
             return False
         return self.message(msg)
 

diff --git a/tests/test_unit/test_muc_message.py b/tests/test_unit/test_muc_message.py
new file mode 100644 (file)
index 0000000..d26db8d
--- /dev/null
+++ b/tests/test_unit/test_muc_message.py
@@ -0,0 +1,50 @@
+# -*- coding: utf-8 -*-
+from unittest.mock import Mock
+
+import distbot.bot.bot as bot_module
+from distbot.bot.bot import Bot
+
+
+def _fake_bot(nick="urlbug"):
+    fake = Mock()
+    fake.nick = nick
+    return fake
+
+
+def test_ignores_own_messages():
+    fake = _fake_bot(nick="urlbug")
+    msg = {"mucnick": "urlbug", "type": "groupchat"}
+    assert Bot.muc_message(fake, msg) is False
+    fake.message.assert_not_called()
+
+
+def test_ignores_other_configured_bots(monkeypatch):
+    monkeypatch.setattr(bot_module, "conf_get", lambda key: ["pibug", "urlbrot"] if key == "other_bots" else None)
+    fake = _fake_bot(nick="urlbug")
+    msg = {"mucnick": "pibug", "type": "groupchat"}
+    assert Bot.muc_message(fake, msg) is False
+    fake.message.assert_not_called()
+
+
+def test_reacts_to_real_users(monkeypatch):
+    monkeypatch.setattr(bot_module, "conf_get", lambda key: ["pibug", "urlbrot"] if key == "other_bots" else None)
+    fake = _fake_bot(nick="urlbug")
+    fake.message.return_value = "handled"
+    msg = {"mucnick": "someRealUser", "type": "groupchat"}
+    assert Bot.muc_message(fake, msg) == "handled"
+    fake.message.assert_called_once_with(msg)
+
+
+def test_other_bots_unset_defaults_gracefully(monkeypatch):
+    monkeypatch.setattr(bot_module, "conf_get", lambda key: None)
+    fake = _fake_bot(nick="urlbug")
+    fake.message.return_value = "handled"
+    msg = {"mucnick": "someRealUser", "type": "groupchat"}
+    assert Bot.muc_message(fake, msg) == "handled"
+
+
+def test_ignores_non_groupchat():
+    fake = _fake_bot(nick="urlbug")
+    msg = {"mucnick": "someRealUser", "type": "chat"}
+    assert Bot.muc_message(fake, msg) is False
+    fake.message.assert_not_called()