mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(whatsapp): resolve phone↔LID aliases in adapter DM/group allowlist (#53588)
Some checks failed
CI / Detect affected areas (push) Waiting to run
CI / Python tests (push) Blocked by required conditions
CI / Python lints (push) Blocked by required conditions
CI / TypeScript (push) Blocked by required conditions
CI / Docs Site (push) Blocked by required conditions
CI / Deny unrelated histories (push) Blocked by required conditions
CI / Check contributors (push) Blocked by required conditions
CI / Check uv.lock (push) Blocked by required conditions
CI / Lint Docker scripts (push) Blocked by required conditions
CI / Build&Test Docker image (push) Blocked by required conditions
CI / Supply-chain scan (push) Blocked by required conditions
CI / OSV scan (push) Waiting to run
CI / All required checks pass (push) Blocked by required conditions
Deploy Site / deploy-vercel (push) Waiting to run
Deploy Site / deploy-docs (push) Waiting to run
Build Skills Index / build-index (push) Has been cancelled
Build Skills Index / trigger-deploy (push) Has been cancelled
Some checks failed
CI / Detect affected areas (push) Waiting to run
CI / Python tests (push) Blocked by required conditions
CI / Python lints (push) Blocked by required conditions
CI / TypeScript (push) Blocked by required conditions
CI / Docs Site (push) Blocked by required conditions
CI / Deny unrelated histories (push) Blocked by required conditions
CI / Check contributors (push) Blocked by required conditions
CI / Check uv.lock (push) Blocked by required conditions
CI / Lint Docker scripts (push) Blocked by required conditions
CI / Build&Test Docker image (push) Blocked by required conditions
CI / Supply-chain scan (push) Blocked by required conditions
CI / OSV scan (push) Waiting to run
CI / All required checks pass (push) Blocked by required conditions
Deploy Site / deploy-vercel (push) Waiting to run
Deploy Site / deploy-docs (push) Waiting to run
Build Skills Index / build-index (push) Has been cancelled
Build Skills Index / trigger-deploy (push) Has been cancelled
The adapter-level intake gate (_is_dm_allowed / _is_group_allowed, reached via _should_process_message) did a raw set-membership check against the configured allowlist. WhatsApp now delivers inbound DM senders in LID form (<id>@lid) while operators configure allowlists with phone numbers, so the check never matched and every DM from an allowed contact was silently dropped before the gateway authz layer ran. Route both gates through the existing gateway.whatsapp_identity. expand_whatsapp_aliases helper (already used by gateway authz and session keys), which walks the bridge's lid-mapping-*.json session files. Phone and LID forms now resolve to each other in both directions; exact JID matches, wildcard, disabled/open policies, and empty-allowlist fail-closed behavior are all preserved. Fixes #14486
This commit is contained in:
parent
38e7bd8a08
commit
caf4dcc7ad
2 changed files with 203 additions and 2 deletions
|
|
@ -147,12 +147,51 @@ class WhatsAppBehaviorMixin:
|
|||
return False
|
||||
|
||||
# ------------------------------------------------------------------ gating
|
||||
@staticmethod
|
||||
def _matches_whatsapp_allowlist(candidate: str, allow_from) -> bool:
|
||||
"""Match a WhatsApp identifier against an allowlist across phone/LID forms.
|
||||
|
||||
WhatsApp delivers inbound senders in LID form (``<id>@lid``) while
|
||||
operators usually configure allowlists with phone numbers, and vice
|
||||
versa. A raw set-membership check therefore never matches a known
|
||||
contact. Resolve both the candidate and each allowlist entry through
|
||||
the bridge's ``lid-mapping-*.json`` files (the shared
|
||||
``gateway.whatsapp_identity`` helper that the gateway authz and
|
||||
session-key paths already use) so either configured form resolves to
|
||||
the inbound form.
|
||||
"""
|
||||
if not allow_from:
|
||||
return False
|
||||
# Fast path: exact match against the raw configured value (e.g. a full
|
||||
# ``@g.us`` group JID or an entry that already matches verbatim).
|
||||
if candidate in allow_from:
|
||||
return True
|
||||
|
||||
from gateway.whatsapp_identity import (
|
||||
expand_whatsapp_aliases,
|
||||
normalize_whatsapp_identifier,
|
||||
)
|
||||
|
||||
candidate_aliases = expand_whatsapp_aliases(candidate)
|
||||
if not candidate_aliases:
|
||||
return False
|
||||
for entry in allow_from:
|
||||
if entry == "*":
|
||||
return True
|
||||
if normalize_whatsapp_identifier(entry) in candidate_aliases:
|
||||
return True
|
||||
# Entry may itself be an unmapped form; expand it too so a phone
|
||||
# allowlist entry resolves when the inbound sender arrived as a LID.
|
||||
if expand_whatsapp_aliases(entry) & candidate_aliases:
|
||||
return True
|
||||
return False
|
||||
|
||||
def _is_dm_allowed(self, sender_id: str) -> bool:
|
||||
"""Check whether a DM from the given sender should be processed."""
|
||||
if self._dm_policy == "disabled":
|
||||
return False
|
||||
if self._dm_policy == "allowlist":
|
||||
return sender_id in self._allow_from
|
||||
return self._matches_whatsapp_allowlist(sender_id, self._allow_from)
|
||||
# "open" — all DMs allowed
|
||||
return True
|
||||
|
||||
|
|
@ -161,7 +200,7 @@ class WhatsAppBehaviorMixin:
|
|||
if self._group_policy == "disabled":
|
||||
return False
|
||||
if self._group_policy == "allowlist":
|
||||
return chat_id in self._group_allow_from
|
||||
return self._matches_whatsapp_allowlist(chat_id, self._group_allow_from)
|
||||
# "open" — all groups allowed
|
||||
return True
|
||||
|
||||
|
|
|
|||
162
tests/gateway/test_whatsapp_allowlist_lid_resolution.py
Normal file
162
tests/gateway/test_whatsapp_allowlist_lid_resolution.py
Normal file
|
|
@ -0,0 +1,162 @@
|
|||
"""WhatsApp DM/group allowlist must resolve phone↔LID aliases at intake.
|
||||
|
||||
Regression for #14486: WhatsApp now delivers inbound DM senders in LID form
|
||||
(``<id>@lid``) while operators configure the allowlist with phone numbers.
|
||||
The adapter-level gate (``_is_dm_allowed`` / ``_is_group_allowed`` →
|
||||
``_should_process_message``) did a raw set-membership check with no LID
|
||||
resolution, so every DM from an allowed user was silently dropped before the
|
||||
gateway authz layer ever ran.
|
||||
|
||||
The fix routes the adapter gate through the shared
|
||||
``gateway.whatsapp_identity.expand_whatsapp_aliases`` helper, which reads the
|
||||
bridge's ``lid-mapping-*.json`` session files (the same source the gateway
|
||||
authz and session-key paths already use).
|
||||
"""
|
||||
|
||||
import json
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
from gateway.config import Platform, PlatformConfig
|
||||
from hermes_constants import get_hermes_home
|
||||
|
||||
|
||||
PHONE = "351912345678"
|
||||
LID = "77214955630717"
|
||||
|
||||
|
||||
def _make_adapter(dm_policy=None, allow_from=None, group_policy=None, group_allow_from=None):
|
||||
from plugins.platforms.whatsapp.adapter import WhatsAppAdapter
|
||||
|
||||
extra = {}
|
||||
if dm_policy is not None:
|
||||
extra["dm_policy"] = dm_policy
|
||||
if allow_from is not None:
|
||||
extra["allow_from"] = allow_from
|
||||
if group_policy is not None:
|
||||
extra["group_policy"] = group_policy
|
||||
if group_allow_from is not None:
|
||||
extra["group_allow_from"] = group_allow_from
|
||||
|
||||
adapter = object.__new__(WhatsAppAdapter)
|
||||
adapter.platform = Platform.WHATSAPP
|
||||
adapter.config = PlatformConfig(enabled=True, extra=extra)
|
||||
adapter._message_handler = AsyncMock()
|
||||
adapter._dm_policy = str(extra.get("dm_policy", "open")).strip().lower()
|
||||
adapter._allow_from = WhatsAppAdapter._coerce_allow_list(extra.get("allow_from"))
|
||||
adapter._group_policy = str(extra.get("group_policy", "open")).strip().lower()
|
||||
adapter._group_allow_from = WhatsAppAdapter._coerce_allow_list(
|
||||
extra.get("group_allow_from")
|
||||
)
|
||||
return adapter
|
||||
|
||||
|
||||
def _write_lid_mapping(phone=PHONE, lid=LID):
|
||||
"""Mirror what the JS bridge writes: phone→lid and lid→phone (reverse)."""
|
||||
session_dir = get_hermes_home() / "whatsapp" / "session"
|
||||
session_dir.mkdir(parents=True, exist_ok=True)
|
||||
(session_dir / f"lid-mapping-{phone}.json").write_text(json.dumps(lid), encoding="utf-8")
|
||||
(session_dir / f"lid-mapping-{lid}_reverse.json").write_text(
|
||||
json.dumps(phone), encoding="utf-8"
|
||||
)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------- DM gate
|
||||
|
||||
def test_dm_phone_allowlist_matches_lid_sender():
|
||||
"""allow_from has the phone number; inbound sender arrives as @lid (the bug)."""
|
||||
_write_lid_mapping()
|
||||
adapter = _make_adapter(dm_policy="allowlist", allow_from=[PHONE])
|
||||
|
||||
assert adapter._is_dm_allowed(f"{LID}@lid") is True
|
||||
|
||||
|
||||
def test_dm_phone_with_plus_allowlist_matches_lid_sender():
|
||||
"""A ``+``-prefixed phone allowlist entry still resolves to the LID sender."""
|
||||
_write_lid_mapping()
|
||||
adapter = _make_adapter(dm_policy="allowlist", allow_from=[f"+{PHONE}"])
|
||||
|
||||
assert adapter._is_dm_allowed(f"{LID}@lid") is True
|
||||
|
||||
|
||||
def test_dm_lid_allowlist_matches_phone_sender():
|
||||
"""Reverse direction: allow_from has the LID, sender arrives as phone JID."""
|
||||
_write_lid_mapping()
|
||||
adapter = _make_adapter(dm_policy="allowlist", allow_from=[LID])
|
||||
|
||||
assert adapter._is_dm_allowed(f"{PHONE}@s.whatsapp.net") is True
|
||||
|
||||
|
||||
def test_dm_exact_phone_jid_still_matches():
|
||||
"""allow_from with the bare phone matches a phone-JID sender without any mapping."""
|
||||
adapter = _make_adapter(dm_policy="allowlist", allow_from=[PHONE])
|
||||
|
||||
assert adapter._is_dm_allowed(f"{PHONE}@s.whatsapp.net") is True
|
||||
|
||||
|
||||
def test_dm_wildcard_allows_any_sender():
|
||||
adapter = _make_adapter(dm_policy="allowlist", allow_from=["*"])
|
||||
|
||||
assert adapter._is_dm_allowed(f"{LID}@lid") is True
|
||||
|
||||
|
||||
def test_dm_unlisted_lid_sender_blocked():
|
||||
_write_lid_mapping()
|
||||
adapter = _make_adapter(dm_policy="allowlist", allow_from=[PHONE])
|
||||
|
||||
assert adapter._is_dm_allowed("99999999999999@lid") is False
|
||||
|
||||
|
||||
def test_dm_empty_allowlist_blocks_everyone():
|
||||
adapter = _make_adapter(dm_policy="allowlist", allow_from=[])
|
||||
|
||||
assert adapter._is_dm_allowed(f"{LID}@lid") is False
|
||||
|
||||
|
||||
def test_dm_disabled_policy_blocks_even_allowlisted():
|
||||
_write_lid_mapping()
|
||||
adapter = _make_adapter(dm_policy="disabled", allow_from=[PHONE])
|
||||
|
||||
assert adapter._is_dm_allowed(f"{LID}@lid") is False
|
||||
|
||||
|
||||
def test_dm_open_policy_allows_anyone():
|
||||
adapter = _make_adapter(dm_policy="open")
|
||||
|
||||
assert adapter._is_dm_allowed("anyone@lid") is True
|
||||
|
||||
|
||||
# ------------------------------------------------------------------ group gate
|
||||
|
||||
def test_group_jid_exact_match_still_works():
|
||||
"""Group allowlists use full ``@g.us`` JIDs — exact match must pass through."""
|
||||
adapter = _make_adapter(
|
||||
group_policy="allowlist", group_allow_from=["120363001234567890@g.us"]
|
||||
)
|
||||
|
||||
assert adapter._is_group_allowed("120363001234567890@g.us") is True
|
||||
|
||||
|
||||
def test_group_unlisted_jid_blocked():
|
||||
adapter = _make_adapter(
|
||||
group_policy="allowlist", group_allow_from=["120363001234567890@g.us"]
|
||||
)
|
||||
|
||||
assert adapter._is_group_allowed("120363009999999999@g.us") is False
|
||||
|
||||
|
||||
# ------------------------------------------------------ end-to-end intake gate
|
||||
|
||||
def test_should_process_message_dm_phone_allowlist_lid_sender():
|
||||
"""Full intake path: a DM from a phone-allowlisted contact arriving as @lid."""
|
||||
_write_lid_mapping()
|
||||
adapter = _make_adapter(dm_policy="allowlist", allow_from=[PHONE])
|
||||
|
||||
data = {
|
||||
"isGroup": False,
|
||||
"body": "hello",
|
||||
"senderId": f"{LID}@lid",
|
||||
"from": f"{LID}@lid",
|
||||
"botIds": [],
|
||||
"mentionedIds": [],
|
||||
}
|
||||
assert adapter._should_process_message(data) is True
|
||||
Loading…
Add table
Add a link
Reference in a new issue