Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions gateway/authz_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,76 @@ def _adapter_group_has_sender_allowlist(
return any(str(item).strip() for item in sender_allow)
return False

def _whatsapp_group_chat_authorized(self, source: SessionSource) -> bool:
"""Authorize WhatsApp senders by an allowlisted group chat.

WhatsApp ``WHATSAPP_ALLOWED_USERS`` is the DM/user allowlist, while
``group_policy`` / ``group_allow_from`` are chat-scoped. A participant
in an explicitly allowlisted group should not also need DM access.
"""
if (
source.platform != Platform.WHATSAPP
or source.chat_type not in {"group", "forum"}
or not source.chat_id
):
return False
if not self._adapter_enforces_own_access_policy(source.platform):
return False

adapters = getattr(self, "adapters", None) or {}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Port this through current main's _authorization_adapter(source.platform, source.profile) rather than self.adapters. The current resolver is profile-aware, so direct lookup can read the default profile's WhatsApp policy for a secondary-profile message.

adapter = adapters.get(source.platform)

policy = getattr(adapter, "_group_policy", None) if adapter is not None else None
allowed = getattr(adapter, "_group_allow_from", None) if adapter is not None else None

config = getattr(self, "config", None)
platform_cfg = (
config.platforms.get(source.platform)
if config is not None and hasattr(config, "platforms")
else None
)
extra = getattr(platform_cfg, "extra", None) if platform_cfg else None
if isinstance(extra, dict):
if policy is None:
policy = extra.get("group_policy")
if allowed is None:
allowed = extra.get("group_allow_from") or extra.get("groupAllowFrom")

if policy is None:
policy = os.getenv("WHATSAPP_GROUP_POLICY", "")
if allowed is None:
allowed = os.getenv("WHATSAPP_GROUP_ALLOWED_USERS", "")

if str(policy or "").strip().lower() != "allowlist":
return False

is_group_allowed = getattr(adapter, "_is_group_allowed", None)
if callable(is_group_allowed):
try:
return bool(is_group_allowed(source.chat_id))
except Exception:
pass

if isinstance(allowed, (set, list, tuple)):
allowed_ids = {str(value).strip() for value in allowed if str(value).strip()}
else:
allowed_ids = {
part.strip()
for part in str(allowed or "").split(",")
if part.strip()
}

if "*" in allowed_ids:
return True
chat_id = str(source.chat_id).strip()
if chat_id in allowed_ids:
return True
if chat_id.endswith("@g.us") and chat_id[:-5] in allowed_ids:
return True
if not chat_id.endswith("@g.us") and f"{chat_id}@g.us" in allowed_ids:
return True
return False

def _is_user_authorized(self, source: SessionSource) -> bool:
"""
Check if a user is authorized to use the bot.
Expand Down Expand Up @@ -221,6 +291,9 @@ def _is_user_authorized(self, source: SessionSource) -> bool:
if "*" in allowed_group_ids or source.chat_id in allowed_group_ids:
return True

if self._whatsapp_group_chat_authorized(source):
return True

if not user_id:
return False

Expand Down
14 changes: 13 additions & 1 deletion gateway/platforms/whatsapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,11 @@ def __init__(self, config: PlatformConfig):
self._dm_policy = str(config.extra.get("dm_policy") or os.getenv("WHATSAPP_DM_POLICY", "open")).strip().lower()
self._allow_from = self._coerce_allow_list(config.extra.get("allow_from") or config.extra.get("allowFrom"))
self._group_policy = str(config.extra.get("group_policy") or os.getenv("WHATSAPP_GROUP_POLICY", "open")).strip().lower()
self._group_allow_from = self._coerce_allow_list(config.extra.get("group_allow_from") or config.extra.get("groupAllowFrom"))
self._group_allow_from = self._coerce_allow_list(
config.extra.get("group_allow_from")
or config.extra.get("groupAllowFrom")
or os.getenv("WHATSAPP_GROUP_ALLOWED_USERS")
)
self._mention_patterns = self._compile_mention_patterns()
self._message_queue: asyncio.Queue = asyncio.Queue()
self._bridge_log_fh = None
Expand Down Expand Up @@ -505,6 +509,14 @@ async def connect(self) -> bool:
bridge_env["HERMES_IMAGE_CACHE_DIR"] = str(_get_img_dir())
bridge_env["HERMES_AUDIO_CACHE_DIR"] = str(_get_audio_dir())
bridge_env["HERMES_DOCUMENT_CACHE_DIR"] = str(_get_doc_dir())
group_policy = getattr(self, "_group_policy", os.getenv("WHATSAPP_GROUP_POLICY", "open"))
group_allow_from = getattr(
self,
"_group_allow_from",
self._coerce_allow_list(os.getenv("WHATSAPP_GROUP_ALLOWED_USERS", "")),
)
bridge_env["WHATSAPP_GROUP_POLICY"] = group_policy
bridge_env["WHATSAPP_GROUP_ALLOWED_USERS"] = ",".join(sorted(group_allow_from))

self._bridge_process = subprocess.Popen(
[
Expand Down
16 changes: 15 additions & 1 deletion gateway/platforms/whatsapp_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,21 @@ def _is_group_allowed(self, chat_id: str) -> bool:
if self._group_policy == "disabled":
return False
if self._group_policy == "allowlist":
return chat_id in self._group_allow_from
allowed_groups = getattr(self, "_group_allow_from", set())
normalized = str(chat_id or "").strip()
if "*" in allowed_groups:
return True
if normalized in allowed_groups:
return True
if normalized.endswith("@g.us") and normalized[:-5] in allowed_groups:
return True
if (
normalized
and not normalized.endswith("@g.us")
and f"{normalized}@g.us" in allowed_groups
):
return True
return False
# "open" — all groups allowed
return True

Expand Down
13 changes: 13 additions & 0 deletions scripts/whatsapp-bridge/allowlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,19 @@ export function parseAllowedUsers(rawValue) {
);
}

export function matchesAllowedIdentifier(identifier, allowedIdentifiers) {
if (!allowedIdentifiers || allowedIdentifiers.size === 0) {
return false;
}

if (allowedIdentifiers.has('*')) {
return true;
}

const normalized = normalizeWhatsAppIdentifier(identifier);
return Boolean(normalized && allowedIdentifiers.has(normalized));
}

function readMappingFile(sessionDir, identifier, suffix = '') {
const filePath = path.join(sessionDir, `lid-mapping-${identifier}${suffix}.json`);
if (!existsSync(filePath)) {
Expand Down
16 changes: 16 additions & 0 deletions scripts/whatsapp-bridge/allowlist.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';

import {
expandWhatsAppIdentifiers,
matchesAllowedIdentifier,
matchesAllowedUser,
normalizeWhatsAppIdentifier,
parseAllowedUsers,
Expand Down Expand Up @@ -78,3 +79,18 @@ test('matchesAllowedUser rejects everyone when allowlist is empty (#8389)', () =
rmSync(sessionDir, { recursive: true, force: true });
}
});

test('matchesAllowedIdentifier matches explicit group identifiers only', () => {
const allowedGroups = parseAllowedUsers('120363001234567890@g.us');

assert.equal(matchesAllowedIdentifier('120363001234567890@g.us', allowedGroups), true);
assert.equal(matchesAllowedIdentifier('120363001234567890', allowedGroups), true);
assert.equal(matchesAllowedIdentifier('120363999999999999@g.us', allowedGroups), false);
assert.equal(matchesAllowedIdentifier('267383306489914@lid', allowedGroups), false);
});

test('matchesAllowedIdentifier preserves secure empty-list default and wildcard', () => {
assert.equal(matchesAllowedIdentifier('120363001234567890@g.us', parseAllowedUsers('')), false);
assert.equal(matchesAllowedIdentifier('120363001234567890@g.us', null), false);
assert.equal(matchesAllowedIdentifier('120363001234567890@g.us', parseAllowedUsers('*')), true);
});
30 changes: 28 additions & 2 deletions scripts/whatsapp-bridge/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { randomBytes, createHash } from 'crypto';
import { execSync } from 'child_process';
import { tmpdir } from 'os';
import qrcode from 'qrcode-terminal';
import { matchesAllowedUser, parseAllowedUsers } from './allowlist.js';
import { matchesAllowedIdentifier, matchesAllowedUser, parseAllowedUsers } from './allowlist.js';

// Parse CLI args
const args = process.argv.slice(2);
Expand Down Expand Up @@ -71,6 +71,8 @@ try {
const PAIR_ONLY = args.includes('--pair-only');
const WHATSAPP_MODE = getArg('mode', process.env.WHATSAPP_MODE || 'self-chat'); // "bot" or "self-chat"
const ALLOWED_USERS = parseAllowedUsers(process.env.WHATSAPP_ALLOWED_USERS || '');
const GROUP_POLICY = String(process.env.WHATSAPP_GROUP_POLICY || '').trim().toLowerCase();
const ALLOWED_GROUPS = parseAllowedUsers(process.env.WHATSAPP_GROUP_ALLOWED_USERS || '');
const DEFAULT_REPLY_PREFIX = '⚕ *Hermes Agent*\n────────────\n';
const REPLY_PREFIX = process.env.WHATSAPP_REPLY_PREFIX === undefined
? DEFAULT_REPLY_PREFIX
Expand Down Expand Up @@ -321,7 +323,31 @@ async function startSocket() {
} catch {}
continue;
}
if (!matchesAllowedUser(senderId, ALLOWED_USERS, SESSION_DIR)) {
if (isGroup && GROUP_POLICY === 'disabled') {
try {
console.log(JSON.stringify({
event: 'ignored',
reason: 'group_policy_disabled',
chatId,
senderId,
}));
} catch {}
continue;
}

if (isGroup && GROUP_POLICY === 'allowlist') {
if (!matchesAllowedIdentifier(chatId, ALLOWED_GROUPS)) {
try {
console.log(JSON.stringify({
event: 'ignored',
reason: 'group_allowlist_mismatch',
chatId,
senderId,
}));
} catch {}
continue;
}
} else if (!matchesAllowedUser(senderId, ALLOWED_USERS, SESSION_DIR)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A current-main port must retain the WHATSAPP_DM_POLICY !== 'pairing' guard before applying the sender allowlist here. scripts/whatsapp-bridge/bridge.js:637 deliberately forwards pairing DMs for the gateway pairing handshake; this unconditional fallback would drop them.

try {
console.log(JSON.stringify({
event: 'ignored',
Expand Down
53 changes: 53 additions & 0 deletions tests/gateway/test_config_driven_access_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ def _clear_auth_env(monkeypatch) -> None:
"QQ_ALLOWED_USERS",
"QQ_GROUP_ALLOWED_USERS",
"WHATSAPP_ALLOWED_USERS",
"WHATSAPP_GROUP_ALLOWED_USERS",
"WHATSAPP_GROUP_POLICY",
"TELEGRAM_ALLOWED_USERS",
"GATEWAY_ALLOWED_USERS",
"GATEWAY_ALLOW_ALL_USERS",
Expand Down Expand Up @@ -287,6 +289,57 @@ def test_env_allowlist_still_takes_precedence_for_own_policy_platform(monkeypatc
assert runner._is_user_authorized(stranger) is False


def test_whatsapp_group_allowlist_authorizes_group_sender_even_with_dm_allowlist(monkeypatch):
"""WhatsApp group allowlist is chat-scoped and must not grant DM access."""
_clear_auth_env(monkeypatch)
monkeypatch.setenv("WHATSAPP_ALLOWED_USERS", "owner-user")

family_chat = "120363001234567890@g.us"
config = GatewayConfig(
platforms={
Platform.WHATSAPP: PlatformConfig(
enabled=True,
extra={
"dm_policy": "allowlist",
"allow_from": ["owner-user"],
"group_policy": "allowlist",
"group_allow_from": [family_chat],
},
)
}
)
runner, adapter = _make_runner(Platform.WHATSAPP, config, enforces=True)
adapter._group_policy = "allowlist"
adapter._group_allow_from = {family_chat}
adapter._is_group_allowed = lambda chat_id: chat_id == family_chat

group_sender = SessionSource(
platform=Platform.WHATSAPP,
user_id="other-member@lid",
chat_id=family_chat,
user_name="member",
chat_type="group",
)
other_group_sender = SessionSource(
platform=Platform.WHATSAPP,
user_id="other-member@lid",
chat_id="120363999999999999@g.us",
user_name="member",
chat_type="group",
)
dm_sender = SessionSource(
platform=Platform.WHATSAPP,
user_id="other-member@lid",
chat_id="other-member@lid",
user_name="member",
chat_type="dm",
)

assert runner._is_user_authorized(group_sender) is True
assert runner._is_user_authorized(other_group_sender) is False
assert runner._is_user_authorized(dm_sender) is False


def test_unknown_adapter_does_not_crash_trust_check(monkeypatch):
"""No adapter registered for the platform → safe default-deny."""
_clear_auth_env(monkeypatch)
Expand Down
Loading