mirror of
https://github.com/MervinPraison/PraisonAI.git
synced 2026-11-07 12:47:55 -10:00
Merge pull request #6085 from MervinPraison/claude/issue-6084-20261027
fix: refuse agent sends to unattested explicit chat destinations
This commit is contained in:
commit
8bd67b83bc
4 files changed
+120
-4
No files matched your search
@@ -468,10 +468,15 @@ async def deliver_media_to_adapter(
|
||||
)
|
||||
return False
|
||||
|
||||
target_id = thread_id or channel_id
|
||||
channel = client.get_channel(int(target_id))
|
||||
if channel is None and thread_id is not None:
|
||||
channel = client.get_channel(int(channel_id))
|
||||
# Resolve the thread *under* the parent channel (as ``send_message``
|
||||
# does) rather than by global id, so a target's thread segment can
|
||||
# never redirect the upload to an unrelated channel (#6084).
|
||||
channel = client.get_channel(int(channel_id))
|
||||
if channel is not None and thread_id is not None:
|
||||
get_thread = getattr(channel, "get_thread", None)
|
||||
thread = get_thread(int(thread_id)) if callable(get_thread) else None
|
||||
if thread is not None:
|
||||
channel = thread
|
||||
if channel is not None:
|
||||
await channel.send(
|
||||
content=caption or None, file=discord.File(path)
|
||||
|
||||
@@ -102,6 +102,27 @@ class BotOutboundMessenger:
|
||||
|
||||
resolved = f"{platform}:{channel_id}"
|
||||
|
||||
# Destination authorization (#6084): the target is model-controlled, so
|
||||
# an explicit "<platform>:<chat_id>" must name a chat this gateway
|
||||
# already knows (the turn's origin, or a home/alias/observed channel)
|
||||
# before anything — text or media — is delivered. Symbolic tokens
|
||||
# ("origin", a platform home, an alias) resolve to known chats by
|
||||
# construction. Operators widen the set via the alias overlay.
|
||||
if not self._is_attested(platform, channel_id):
|
||||
logger.warning(
|
||||
"BotOutboundMessenger: refusing send to unattested destination %s",
|
||||
resolved,
|
||||
)
|
||||
return DeliveryResult(
|
||||
ok=False,
|
||||
target=resolved,
|
||||
summary=f"Refused: {resolved} is not a known destination for this gateway.",
|
||||
detail=(
|
||||
"destination not attested (not the origin chat, a home "
|
||||
"channel, an alias, or an observed channel)"
|
||||
),
|
||||
)
|
||||
|
||||
# Idempotency covers the *whole* text+media envelope (issue #2578). A
|
||||
# duplicate proactive send must skip both the text and any media upload,
|
||||
# otherwise a re-fired job with attachments would double-upload media
|
||||
@@ -183,6 +204,21 @@ class BotOutboundMessenger:
|
||||
detail=detail,
|
||||
)
|
||||
|
||||
def _is_attested(self, platform: str, channel_id: str) -> bool:
|
||||
"""Whether ``platform:channel_id`` is a chat this gateway knows."""
|
||||
origin = self._origin
|
||||
if (
|
||||
origin is not None
|
||||
and (origin.platform or "").lower() == platform.lower()
|
||||
and str(origin.channel_id) == str(channel_id)
|
||||
):
|
||||
return True
|
||||
try:
|
||||
return bool(self._router.directory.has_channel(platform, str(channel_id)))
|
||||
except Exception as e: # pragma: no cover — defensive, fail closed
|
||||
logger.warning("BotOutboundMessenger: attestation check failed: %s", e)
|
||||
return False
|
||||
|
||||
async def _deliver_media(
|
||||
self,
|
||||
target: str,
|
||||
|
||||
@@ -417,3 +417,50 @@ def test_discord_media_transient_error_propagates_for_retry(tmp_path):
|
||||
ok = asyncio.run(_run())
|
||||
assert ok is True
|
||||
assert channel.calls == 2
|
||||
|
||||
|
||||
def test_discord_media_thread_is_scoped_to_parent_channel(tmp_path):
|
||||
# #6084: the thread segment of "discord:A:B" must not redirect the upload
|
||||
# to an arbitrary channel B by global id — only a thread under the
|
||||
# (attested) parent A may receive it, otherwise the file lands in A.
|
||||
pytest.importorskip("discord")
|
||||
|
||||
f = tmp_path / "report.pdf"
|
||||
f.write_bytes(b"%PDF-1.4 data")
|
||||
|
||||
class _Channel:
|
||||
def __init__(self, threads=None):
|
||||
self.sent = 0
|
||||
self._threads = threads or {}
|
||||
|
||||
async def send(self, *args, **kwargs):
|
||||
self.sent += 1
|
||||
|
||||
def get_thread(self, tid):
|
||||
return self._threads.get(tid)
|
||||
|
||||
real_thread = _Channel()
|
||||
parent = _Channel(threads={555: real_thread})
|
||||
unrelated = _Channel()
|
||||
channels = {42: parent, 999: unrelated}
|
||||
|
||||
class Adapter:
|
||||
platform = "discord"
|
||||
|
||||
class _client:
|
||||
@staticmethod
|
||||
def get_channel(cid):
|
||||
return channels.get(cid)
|
||||
|
||||
def _run(thread_id):
|
||||
return asyncio.run(
|
||||
deliver_media_to_adapter(Adapter(), "42", str(f), thread_id=thread_id)
|
||||
)
|
||||
|
||||
assert _run("999") is True
|
||||
assert unrelated.sent == 0
|
||||
assert parent.sent == 1
|
||||
|
||||
assert _run("555") is True
|
||||
assert real_thread.sent == 1
|
||||
assert parent.sent == 1
|
||||
@@ -88,6 +88,34 @@ def test_send_to_platform_home_channel():
|
||||
assert sent == [("999", "Nightly summary")]
|
||||
|
||||
|
||||
def test_send_to_unattested_explicit_chat_is_refused():
|
||||
# #6084: a model-supplied "<platform>:<chat_id>" naming a chat the gateway
|
||||
# has no relationship with must fail closed instead of delivering.
|
||||
router, sent = _make_router(home="999")
|
||||
origin = SessionSource(platform="telegram", channel_id="123")
|
||||
messenger = BotOutboundMessenger(router, origin=origin)
|
||||
|
||||
result = asyncio.run(messenger.send("telegram:666", "leak"))
|
||||
|
||||
assert result.ok is False
|
||||
assert "Refused" in result.summary
|
||||
assert sent == []
|
||||
|
||||
|
||||
def test_send_to_known_explicit_chat_is_allowed():
|
||||
router, sent = _make_router(home="999", aliases={"ops": "555"})
|
||||
origin = SessionSource(platform="telegram", channel_id="123")
|
||||
messenger = BotOutboundMessenger(router, origin=origin)
|
||||
|
||||
for target, chat in (
|
||||
("telegram:123", "123"), # origin chat
|
||||
("telegram:999", "999"), # home channel
|
||||
("telegram:555:42", "555"), # aliased chat, thread under it
|
||||
):
|
||||
assert asyncio.run(messenger.send(target, "hi")).ok is True, target
|
||||
assert [c for c, _ in sent] == ["123", "999", "555"]
|
||||
|
||||
|
||||
def test_send_unresolvable_target_fails_cleanly():
|
||||
router, sent = _make_router()
|
||||
messenger = BotOutboundMessenger(router) # no origin
|
||||
|
||||
Reference in new issue
Block a user