fix(whatsapp): bridge gets the adapter's resolved group policy and group allowlist; one allowlist reader

The bridge now gates groups by WHATSAPP_GROUP_POLICY / WHATSAPP_GROUP_ALLOWED_USERS
(#73465), but only the DM policy and DM allowlist were exported from the adapter's
resolved config — a YAML group_allow_from never reached the bridge and, under a
multiplexed gateway, the launch process's WHATSAPP_GROUP_* values did. _bridge_env
exports both like it already did for the DM pair, so unlisted-group traffic is cut
before media download instead of after the POST to Python.

whatsapp_common._select_allowlist generalises the DM precedence reader (config key
presence wins, an explicit empty list stays authoritative, then the first truthy env
carrier); the adapter's group list and the Cloud sibling's group list use it instead
of hand-rolled `or` chains, so `group_allow_from: []` means the same everywhere.

Tests: bridge env carries group policy + group allowlist from the secondary profile's
YAML (scoped precedence file); env fallback test trimmed to the two invariants (red on
the merge base); bare test adapters seed _group_policy/_group_allow_from, which
_bridge_env now reads.
This commit is contained in:
kshitijk4poor
2026-09-19 02:56:28 +05:30
committed by kshitij
parent 477afd2420
commit 35b38f1f4e
7 changed files with 73 additions and 124 deletions

View File

@@ -197,9 +197,9 @@ class WhatsAppCloudAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
extra.get("group_policy") or _get_wsecret("WHATSAPP_CLOUD_GROUP_POLICY")
or _get_wsecret("WHATSAPP_GROUP_POLICY", default="open") or "open"
).strip().lower()
self._group_allow_from: set[str] = self._normalize_allow_ids(self._coerce_allow_list(
extra.get("group_allow_from") or extra.get("groupAllowFrom") or _get_wsecret("WHATSAPP_CLOUD_GROUP_ALLOW_FROM")
))
_, raw_groups = self._select_allowlist(
extra, ("group_allow_from", "groupAllowFrom"), ("WHATSAPP_CLOUD_GROUP_ALLOW_FROM",), _get_wsecret)
self._group_allow_from: set[str] = self._normalize_allow_ids(self._coerce_allow_list(raw_groups))
self._mention_patterns = self._compile_mention_patterns()
# Webhook dedup state (in-memory, FIFO-evicted) and counters.
self._seen_wamids: "OrderedDict[str, bool]" = OrderedDict()

View File

@@ -5,7 +5,7 @@ to-bot detection, broadcast filtering, WhatsApp markdown conversion, chunk budge
Mixin contract — the host adapter sets these on ``self`` before calling any mixin
method: ``config`` (PlatformConfig), ``name``, ``_dm_policy`` / ``_group_policy``
("open" | "allowlist" | "disabled"), ``_allow_from`` / ``_group_allow_from`` (set[str]),
("open" | "allowlist" | "disabled" | "pairing"), ``_allow_from`` / ``_group_allow_from`` (set[str]),
``_mention_patterns`` (list[re.Pattern]), ``_reply_prefix`` (Optional[str]).
"""
@@ -105,20 +105,24 @@ class WhatsAppBehaviorMixin(OwnAccessPolicyMixin):
parts = raw if isinstance(raw, list) else str(raw).split(",")
return {str(part).strip() for part in parts if str(part).strip()}
def _select_dm_allowlist(self, extra: Dict[str, Any], env_keys, read_env) -> Any:
"""Pick the raw DM allowlist by key *presence*: ``allow_from``/``allowFrom`` in config (an
explicit empty list stays authoritative), then the first truthy env carrier. Records the
winning source in ``_dm_allowlist_source`` so live DM checks keep the same precedence."""
for key in ("allow_from", "allowFrom"):
@staticmethod
def _select_allowlist(extra: Dict[str, Any], config_keys, env_keys, read_env) -> tuple[Optional[str], Any]:
"""``(source, raw)`` by key *presence*: a config key wins (an explicit empty list stays authoritative),
then the first truthy env carrier; ``(None, None)`` when neither is set."""
for key in config_keys:
if key in extra:
self._dm_allowlist_source = "config"
return extra.get(key)
return "config", extra.get(key)
for env in env_keys:
if read_env(env):
self._dm_allowlist_source = env
return read_env(env)
self._dm_allowlist_source = None
return None
raw = read_env(env)
if raw:
return env, raw
return None, None
def _select_dm_allowlist(self, extra: Dict[str, Any], env_keys, read_env) -> Any:
"""Raw DM allowlist; records the winning source in ``_dm_allowlist_source`` so live DM checks keep
the same precedence."""
self._dm_allowlist_source, raw = self._select_allowlist(extra, ("allow_from", "allowFrom"), env_keys, read_env)
return raw
def _live_dm_allow_from(self) -> set[str]:
"""Allowlist currently enforced for DM intake / strict DM auth. Env-seeded adapters re-read

View File

@@ -275,15 +275,11 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
self._dm_policy = str(_extra_or_secret(extra, "dm_policy", "WHATSAPP_DM_POLICY", "pairing")).strip().lower()
self._allow_from = self._coerce_allow_list(self._select_dm_allowlist(extra, ("WHATSAPP_ALLOWED_USERS",), _wenv))
self._group_policy = str(_extra_or_secret(extra, "group_policy", "WHATSAPP_GROUP_POLICY", "pairing")).strip().lower()
# Group allowlist: config keys win, then the profile-scoped env CSV — mirroring the DM
# path (_select_dm_allowlist) so a group_allow_from set only via WHATSAPP_GROUP_ALLOWED_USERS
# reaches the adapter (#72529: the env carrier was bridged to the Node bridge but never read here,
# so env-only installs silently ran group gating with an empty allowlist).
self._group_allow_from = self._coerce_allow_list(
extra.get("group_allow_from") if "group_allow_from" in extra
else (extra.get("groupAllowFrom") if "groupAllowFrom" in extra
else (_wenv("WHATSAPP_GROUP_ALLOW_FROM") or _wenv("WHATSAPP_GROUP_ALLOWED_USERS") or None))
)
# Same precedence as the DM list. Until #72529 the env carrier only reached the Node bridge, so
# env-only installs gated groups on an empty allowlist.
_, raw_groups = self._select_allowlist(
extra, ("group_allow_from", "groupAllowFrom"), ("WHATSAPP_GROUP_ALLOW_FROM", "WHATSAPP_GROUP_ALLOWED_USERS"), _wenv)
self._group_allow_from = self._coerce_allow_list(raw_groups)
rr = extra.get("send_read_receipts", False)
self._send_read_receipts = rr if isinstance(rr, bool) else str(rr or "").strip().lower() in {"1", "true", "yes", "on"}
self._mention_patterns = self._compile_mention_patterns()
@@ -400,11 +396,12 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
# adapter resolved (scoped env → this profile's YAML → default), or a secondary's YAML
# ``dm_policy: pairing`` runs under the default profile's allowlist and drops valid pairing DMs.
bridge_env["WHATSAPP_DM_POLICY"] = self._dm_policy
allowed = ",".join(sorted(self._allow_from))
if allowed:
bridge_env["WHATSAPP_ALLOWED_USERS"] = allowed
else:
bridge_env.pop("WHATSAPP_ALLOWED_USERS", None)
bridge_env["WHATSAPP_GROUP_POLICY"] = self._group_policy
for env_key, ids in (("WHATSAPP_ALLOWED_USERS", self._allow_from), ("WHATSAPP_GROUP_ALLOWED_USERS", self._group_allow_from)):
if ids:
bridge_env[env_key] = ",".join(sorted(ids))
else:
bridge_env.pop(env_key, None)
# Without these the bridge hardcodes ~/.hermes/{image,audio,document}_cache (wrong under HERMES_HOME/profiles/cache layout).
img_dir, audio_dir, _video_dir, doc_dir = _cache_dirs()
bridge_env.update(HERMES_IMAGE_CACHE_DIR=str(img_dir), HERMES_AUDIO_CACHE_DIR=str(audio_dir), HERMES_DOCUMENT_CACHE_DIR=str(doc_dir))

View File

@@ -69,13 +69,13 @@ def test_secondary_reads_own_yaml_and_never_the_launch_env(homes, monkeypatch):
"matrix:\n process_notices: true\n session_scope: room\n"
"discord:\n reactions: false\n allow_mentions:\n everyone: true\n"
"slack:\n reactions: false\n ignored_channels: [C_LAUNCH]\n"
"telegram:\n reactions: true\n")
"telegram:\n reactions: true\n", encoding="utf-8")
load_gateway_config() # launch profile bridges its YAML into os.environ (single-profile contract)
(secondary / "config.yaml").write_text(
"matrix:\n enabled: true\n user_id: '@bot:example.org'\n"
" allowed_users: ['@owner:example.org']\n ignore_user_patterns: ['^@ignored:']\n"
"discord:\n enabled: true\nslack:\n enabled: true\n"
"telegram:\n enabled: true\n proxy_url: http://127.0.0.1:18080\n")
"telegram:\n enabled: true\n proxy_url: http://127.0.0.1:18080\n", encoding="utf-8")
from plugins.platforms.discord.adapter import DiscordAdapter
from plugins.platforms.matrix.adapter import MatrixAdapter
from plugins.platforms.slack.adapter import SlackAdapter
@@ -107,7 +107,7 @@ def test_explicit_env_beats_yaml_for_the_owning_profile(homes, monkeypatch):
``everyone: true`` and TELEGRAM_REACTIONS=true beats the stock ``reactions: false`` (#109032)."""
launch, _ = homes
(launch / "config.yaml").write_text(
"discord:\n allow_mentions:\n everyone: true\ntelegram:\n reactions: false\n")
"discord:\n allow_mentions:\n everyone: true\ntelegram:\n reactions: false\n", encoding="utf-8")
monkeypatch.setenv("DISCORD_ALLOW_MENTION_EVERYONE", "false")
monkeypatch.setenv("TELEGRAM_REACTIONS", "true")
from plugins.platforms.telegram.adapter import TelegramAdapter
@@ -123,7 +123,7 @@ def test_central_allow_bots_gate_honours_a_secondary_yaml_policy(homes):
from plugins.platforms.slack.adapter import SlackAdapter
_, secondary = homes
(secondary / "config.yaml").write_text(
"discord:\n enabled: true\n allow_bots: all\nslack:\n enabled: true\n allow_bots: all\n")
"discord:\n enabled: true\n allow_bots: all\nslack:\n enabled: true\n allow_bots: all\n", encoding="utf-8")
with _secondary_scope(secondary):
cfg = load_gateway_config()
d, s = DiscordAdapter(cfg.platforms[Platform.DISCORD]), SlackAdapter(cfg.platforms[Platform.SLACK])
@@ -141,7 +141,7 @@ def test_matrix_yaml_lists_gate_intake_and_approval(homes):
_, secondary = homes
(secondary / "config.yaml").write_text(
"matrix:\n enabled: true\n user_id: '@bot:example.org'\n"
" allowed_users: ['@owner:example.org']\n ignore_user_patterns: ['^@ignored:']\n")
" allowed_users: ['@owner:example.org']\n ignore_user_patterns: ['^@ignored:']\n", encoding="utf-8")
with _secondary_scope(secondary):
a = MatrixAdapter(load_gateway_config().platforms[Platform.MATRIX])
a._user_id = "@bot:example.org"
@@ -164,7 +164,7 @@ def test_yuanbao_secondary_home_channel_is_live_and_reloadable(homes):
from gateway.platforms.yuanbao import AutoSetHomeMiddleware
_, secondary = homes
(secondary / "config.yaml").write_text(
"platforms:\n yuanbao:\n enabled: true\n extra:\n app_id: a\n app_secret: b\n")
"platforms:\n yuanbao:\n enabled: true\n extra:\n app_id: a\n app_secret: b\n", encoding="utf-8")
adapter = types.SimpleNamespace(name="yuanbao-b2")
ctx = types.SimpleNamespace(chat_id="dm:tenant-b2", chat_name="b2")
with _secondary_scope(secondary):
@@ -177,15 +177,21 @@ def test_yuanbao_secondary_home_channel_is_live_and_reloadable(homes):
def test_whatsapp_bridge_env_carries_the_secondary_effective_policy(homes, monkeypatch):
"""bridge.js gates DMs before Python: it must receive the adapter's resolved dm_policy/allow_from, not the
launch process's WHATSAPP_* values."""
"""bridge.js gates DMs and group intake before Python: it must receive the adapter's resolved
dm_policy/allow_from/group_policy, not the launch process's WHATSAPP_* values."""
from plugins.platforms.whatsapp.adapter import WhatsAppAdapter
_, secondary = homes
monkeypatch.setenv("WHATSAPP_DM_POLICY", "allowlist")
monkeypatch.setenv("WHATSAPP_GROUP_POLICY", "disabled")
monkeypatch.setenv("WHATSAPP_ALLOWED_USERS", "15550001111")
(secondary / "config.yaml").write_text("whatsapp:\n enabled: true\n dm_policy: pairing\n")
monkeypatch.setenv("WHATSAPP_GROUP_ALLOWED_USERS", "120363000000000000@g.us")
(secondary / "config.yaml").write_text(
"whatsapp:\n enabled: true\n dm_policy: pairing\n group_policy: allowlist\n"
" group_allow_from: [120363001234567890@g.us]\n", encoding="utf-8")
with _secondary_scope(secondary):
a = WhatsAppAdapter(load_gateway_config().platforms[Platform.WHATSAPP])
env = a._bridge_env()
assert a._dm_policy == "pairing" == env["WHATSAPP_DM_POLICY"]
assert a._group_policy == "allowlist" == env["WHATSAPP_GROUP_POLICY"]
assert env["WHATSAPP_GROUP_ALLOWED_USERS"] == "120363001234567890@g.us"
assert "WHATSAPP_ALLOWED_USERS" not in env

View File

@@ -54,8 +54,8 @@ def _make_adapter():
adapter._bridge_process = None
adapter._reply_prefix = None
adapter._send_read_receipts = False
adapter._dm_policy = "pairing"
adapter._allow_from = set()
adapter._dm_policy = adapter._group_policy = "pairing"
adapter._allow_from = adapter._group_allow_from = set()
adapter._running = False
adapter._message_handler = None
adapter._fatal_error_code = None
@@ -514,11 +514,11 @@ class TestNoCredsPreflight:
adapter.config = MagicMock()
adapter._bridge_port = 19877
bridge = tmp_path / "bridge.js"
bridge.write_text("// stub")
bridge.write_text("// stub", encoding="utf-8")
adapter._bridge_script = str(bridge)
session_dir = tmp_path / "session"
session_dir.mkdir()
(session_dir / "creds.json").write_text("{}")
(session_dir / "creds.json").write_text("{}", encoding="utf-8")
adapter._session_path = session_dir
adapter._bridge_log_fh = None
adapter._fatal_error_code = None

View File

@@ -1,27 +1,12 @@
"""Regression test for #72529 — the WhatsApp adapter never read WHATSAPP_GROUP_ALLOWED_USERS.
"""Regression for #72529 — the WhatsApp adapter never read WHATSAPP_GROUP_ALLOWED_USERS.
The Node bridge receives WHATSAPP_GROUP_ALLOWED_USERS via _BRIDGE_PASSTHROUGH_ENV, and the
YAML bridge maps config.yaml ``whatsapp.group_allow_from`` onto the same env carrier — but
``WhatsAppAdapter.__init__`` seeded ``_group_allow_from`` from ``extra`` alone. An install
that configured the group allowlist only through the documented env var (or relied on the
YAML bridge to populate it) silently ran group gating with an empty allowlist: with
``group_policy: allowlist`` every group message was rejected, and with ``group_policy: open``
nothing changed — the triage on #72529 flagged exactly this half as "no PR yet".
The DM path already had the right shape (``_select_dm_allowlist`` reads config by key
*presence*, then the scoped env). This test pins the group path to the same precedence:
1. ``extra["group_allow_from"]`` (explicit list or CSV) wins, even when empty-ish forms differ;
2. the legacy ``groupAllowFrom`` key follows;
3. with neither config key present, WHATSAPP_GROUP_ALLOW_FROM / WHATSAPP_GROUP_ALLOWED_USERS
(profile-scoped, multiplex-safe) seed the allowlist;
4. config wins over env — an env value must never broaden a config-seeded list.
The Node bridge received the env carrier via _BRIDGE_PASSTHROUGH_ENV, but ``WhatsAppAdapter.__init__``
seeded ``_group_allow_from`` from ``extra`` alone, so an env-only install ran ``group_policy: allowlist``
against an empty allowlist and rejected every group. Precedence mirrors the DM path: config key presence
wins, then the profile-scoped env.
"""
from unittest.mock import patch
import pytest
from gateway.config import Platform, PlatformConfig
from gateway.config import PlatformConfig
def _adapter_with_extra(extra):
@@ -29,71 +14,28 @@ def _adapter_with_extra(extra):
return WhatsAppAdapter(PlatformConfig(enabled=True, extra=extra))
class TestGroupAllowlistEnvFallback:
"""Env-only group allowlists must reach ``_group_allow_from``."""
def test_env_only_group_allowlist_is_read(self, monkeypatch):
monkeypatch.setenv("WHATSAPP_GROUP_ALLOWED_USERS", "120363001234567890@g.us, 86316009876@s.whatsapp.net")
monkeypatch.delenv("WHATSAPP_GROUP_ALLOW_FROM", raising=False)
adapter = _adapter_with_extra({})
assert adapter._group_allow_from == {"120363001234567890@g.us", "86316009876@s.whatsapp.net"}
def test_group_allow_from_env_alias(self, monkeypatch):
"""WHATSAPP_GROUP_ALLOW_FROM (the allow_from-style spelling) is honoured too."""
monkeypatch.setenv("WHATSAPP_GROUP_ALLOW_FROM", "86316009876@s.whatsapp.net")
monkeypatch.delenv("WHATSAPP_GROUP_ALLOWED_USERS", raising=False)
adapter = _adapter_with_extra({})
assert adapter._group_allow_from == {"86316009876@s.whatsapp.net"}
def test_config_group_allow_from_still_wins(self, monkeypatch):
"""An explicit config list stays authoritative — the env value must not broaden it."""
monkeypatch.setenv("WHATSAPP_GROUP_ALLOWED_USERS", "1111111111@g.us")
adapter = _adapter_with_extra({"group_allow_from": ["120363001234567890@g.us"]})
assert adapter._group_allow_from == {"120363001234567890@g.us"}
def test_legacy_group_allow_from_key_wins_over_env(self, monkeypatch):
monkeypatch.setenv("WHATSAPP_GROUP_ALLOWED_USERS", "1111111111@g.us")
adapter = _adapter_with_extra({"groupAllowFrom": ["86316009876@s.whatsapp.net"]})
assert adapter._group_allow_from == {"86316009876@s.whatsapp.net"}
def test_empty_env_and_no_config_means_empty_allowlist(self, monkeypatch):
monkeypatch.delenv("WHATSAPP_GROUP_ALLOWED_USERS", raising=False)
monkeypatch.delenv("WHATSAPP_GROUP_ALLOW_FROM", raising=False)
adapter = _adapter_with_extra({})
assert adapter._group_allow_from == set()
def test_scoped_env_read_through_secret_scope(self, tmp_path, monkeypatch):
"""The env read is profile-scoped (multiplex-safe) like every other WHATSAPP_* read."""
class TestGroupAllowlistEnv:
def test_env_only_group_allowlist_gates_intake_through_the_secret_scope(self, tmp_path, monkeypatch):
"""No config key: the profile-scoped WHATSAPP_GROUP_ALLOWED_USERS seeds the allowlist ``_is_group_allowed``
reads (multiplex-safe like every other WHATSAPP_* read)."""
from agent import secret_scope as ss
(tmp_path / ".env").write_text("WHATSAPP_GROUP_ALLOWED_USERS=86316009876@s.whatsapp.net\n")
(tmp_path / ".env").write_text("WHATSAPP_GROUP_ALLOWED_USERS=120363001234567890@g.us\nWHATSAPP_GROUP_POLICY=allowlist\n", encoding="utf-8")
monkeypatch.delenv("WHATSAPP_GROUP_ALLOWED_USERS", raising=False)
monkeypatch.delenv("WHATSAPP_GROUP_ALLOW_FROM", raising=False)
ss.set_multiplex_active(True)
tok = ss.set_secret_scope(ss.build_profile_secret_scope(tmp_path))
try:
adapter = _adapter_with_extra({})
assert adapter._group_allow_from == {"86316009876@s.whatsapp.net"}
finally:
ss.reset_secret_scope(tok)
ss.set_multiplex_active(False)
class TestGroupGatingUsesEnvSeededAllowlist:
"""#72529's symptom: authorized group senders rejected because the allowlist was empty."""
def test_allowlisted_group_chat_passes_intake(self, monkeypatch):
"""group_policy=allowlist + env-only allowlist → the group chat is admitted."""
monkeypatch.setenv("WHATSAPP_GROUP_ALLOWED_USERS", "120363001234567890@g.us")
monkeypatch.setenv("WHATSAPP_GROUP_POLICY", "allowlist")
adapter = _adapter_with_extra({})
assert adapter._group_policy == "allowlist"
assert adapter._group_allow_from == {"120363001234567890@g.us"}
assert adapter._is_group_allowed("120363001234567890@g.us") is True
assert adapter._is_group_allowed("99999999999@g.us") is False
def test_open_group_policy_unaffected(self, monkeypatch):
monkeypatch.setenv("WHATSAPP_GROUP_POLICY", "open")
monkeypatch.delenv("WHATSAPP_GROUP_ALLOWED_USERS", raising=False)
adapter = _adapter_with_extra({})
assert adapter._is_group_allowed("120363001234567890@g.us") is True
def test_config_group_allow_from_wins_over_env(self, monkeypatch):
"""An explicit config list (either spelling) stays authoritative — env must not broaden it."""
monkeypatch.setenv("WHATSAPP_GROUP_ALLOWED_USERS", "1111111111@g.us")
assert _adapter_with_extra({"group_allow_from": ["120363001234567890@g.us"]})._group_allow_from == {"120363001234567890@g.us"}
assert _adapter_with_extra({"groupAllowFrom": []})._group_allow_from == set()

View File

@@ -54,8 +54,8 @@ def _make_adapter(bridge_script: str = "/tmp/test-bridge.js",
adapter._bridge_process = None
adapter._reply_prefix = None
adapter._send_read_receipts = False
adapter._dm_policy = "pairing"
adapter._allow_from = set()
adapter._dm_policy = adapter._group_policy = "pairing"
adapter._allow_from = adapter._group_allow_from = set()
adapter._running = False
adapter._message_handler = None
adapter._fatal_error_code = None
@@ -86,11 +86,11 @@ def _setup_bridge_dir(tmp_path: Path) -> Path:
"""Create a real bridge dir with bridge.js + package.json + creds."""
bridge_dir = tmp_path / "whatsapp-bridge"
bridge_dir.mkdir()
(bridge_dir / "bridge.js").write_text("// current bridge code\n")
(bridge_dir / "package.json").write_text('{"name": "bridge"}\n')
(bridge_dir / "bridge.js").write_text("// current bridge code\n", encoding="utf-8")
(bridge_dir / "package.json").write_text('{"name": "bridge"}\n', encoding="utf-8")
session_path = tmp_path / "session"
session_path.mkdir()
(session_path / "creds.json").write_text("{}")
(session_path / "creds.json").write_text("{}", encoding="utf-8")
return bridge_dir
@@ -110,7 +110,7 @@ class TestFileContentHash:
from plugins.platforms.whatsapp.adapter import _file_content_hash
f = tmp_path / "x.js"
f.write_text("abc")
f.write_text("abc", encoding="utf-8")
h = _file_content_hash(f)
assert len(h) == 16
assert h == _file_content_hash(f) # deterministic