whatsapp: close two group-chat security gaps (sender identity + fromMe handling) - #15413
Closed
jscholz wants to merge 2 commits into
Closed
whatsapp: close two group-chat security gaps (sender identity + fromMe handling)#15413jscholz wants to merge 2 commits into
jscholz wants to merge 2 commits into
Conversation
Bridge extracts senderId (participant JID) + senderName (pushName) on
every inbound message and posts them in the HTTP event dict, but the
gateway was dropping both: `text = body` carried only the plain message
content. That meant all group members' messages arrived at the LLM
indistinguishable from the session owner's, so (a) the LLM couldn't
attribute a question to the actual asker, (b) memory-write tool calls
triggered by a guest were stored under the owner's profile, and (c)
the trust-tier policy in AGENTS.md was effectively unenforceable.
Prepend `[<pushName>] ` to body for `isGroup` events only, after
@bot-mention stripping + doc-content injection so the prefix stays at
the very start of the text the LLM reads. Fallback order:
pushName → short JID → "unknown". DMs untouched (no ambiguity).
Completes the design intent already documented in
gateway/session.py:245-256 ("individual sender names are prefixed on
each user message") — that comment promised behavior that was never
implemented in code.
The fromMe handler in self-chat mode unconditionally bailed on group
messages with `if (isGroup || chatId.includes('status')) continue`,
meaning the user could never @mention their own bot in a group they
were in. This is an over-aggressive echo-loop guard — the real
loop-prevention already exists ~60 lines below at the REPLY_PREFIX +
recentlySentIds check, which catches messages the bridge itself sent.
Split the condition: still skip status broadcasts, but let group
messages continue. For 1:1 fromMe messages, preserve the existing
self-chat requirement (so the bot doesn't try to answer on the
user's behalf in random conversations).
Companion to the sender-identity prefix patch in gateway/platforms/
whatsapp.py — without this fix, the user themselves cannot
participate in a group conversation with their bot, so the sender
prefixing alone is only half the fix needed to make group chat
safely usable.
Collaborator
|
Partially overlaps with #15326 — both fix the fromMe group-message drop in WhatsApp bridge. This PR additionally addresses the sender identity prefix gap (security: memory-poisoning via indistinguishable group members). |
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related fixes that together make WhatsApp group chat safe and usable from the session owner's own paired account:
gateway/platforms/whatsapp.py— prepend[<pushName>]to thebodyon group messages so the LLM can attribute each turn to its sender.scripts/whatsapp-bridge/bridge.js— stop droppingfromMe && isGroupmessages in self-chat mode so the owner can @mention the bot in their own groups.Either fix alone is insufficient: without #1, group members' writes look indistinguishable from the owner's (memory-poisoning + trust-tier bypass); without #2, the owner can't participate in the group conversation at all.
Why it matters
Reproduced on my instance after a friend managed to have the bot address him as a custom honorific and trigger a profile memory-write into my
USER.md, all from a shared group chat. Root cause was twofold:senderId(participant JID) +senderName(pushName) on every inbound message and posts both in the HTTP event dict — butWhatsAppAdapter._handle_eventdrops them, passing onlytext = bodytoMessageEvent. From the LLM's perspective every group member's message looks like the owner's.fromMehandler in self-chat mode bails unconditionally on group messages (if (isGroup || chatId.includes('status')) continue). This was almost certainly intended as an echo-loop guard, but the real loop-prevention already exists ~60 lines below at theREPLY_PREFIX+recentlySentIdscheck. The early bail just blocks the owner from ever using the bot in their own groups.gateway/session.py:245-256has a comment documenting the intent that sender names are prefixed on each user message — this PR completes that design.Patch 1 — sender identity prefix (
whatsapp.py)Inserted immediately before the existing
MessageEvent(text=body, ...)construction. All prior transforms (_clean_bot_mention_text, document-content injection, media placeholder) run against the rawbodyfirst; the prefix appears at the very start of the text the LLM reads. Fallback chain:pushName→ short JID (before@) →"unknown".Patch 2 — fromMe group handling (
bridge.js)The downstream loop guard (around line 275-280 in the file) is unchanged: any fromMe message that starts with
REPLY_PREFIXor whosemsg.key.idis inrecentlySentIdsis still dropped, which is what actually prevents the bridge from re-ingesting its own sent messages. Removing the earlyisGroupbail just lets the owner's @mentions through to that guard.Scope / non-goals
fromMe; DM receiver path untouched).body.startswith("/")and@botnamemention stripping run before the prefix is added.Verification
On a running deploy:
[<pushName>] <message>.fromMe && isGroupbail).Reproduced and verified locally on
v0.11.0before rebasing both commits onto currentmain.