mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(gateway): fail-closed external-surface defaults + profile-aware multiplex authz
Aligns runtime behaviour with SECURITY.md 2.6: externally reachable messaging adapters must fail closed unless access is explicitly configured. Closes the confirmed multiplex authorization bypass a secondary profile's open dm/group policy no longer inherits the default profile's allowlist trust. - Own-policy adapters (WhatsApp, WeCom, Weixin, QQBot, Yuanbao) default dm_policy/group_policy to pairing/allowlist instead of open; open now requires an explicit GATEWAY_ALLOW_ALL_USERS or per-platform allow-all. - Startup guard (_own_policy_open_startup_violation) refuses to boot when an enabled adapter is open without the allow-all opt-in; the guard now runs for every secondary profile in multiplex mode too. - Profile-aware own-policy authorization: _authorization_adapter / _adapter_for_source resolve the live adapter via SessionSource.profile, so _is_user_authorized and the ingress/pairing/busy/queue paths read the originating profile's adapter policy, not the default profile's. - Fail-closed intake for Email, Feishu P2P, and Discord (blank-principal denial, empty-allowlist deny, missing-interaction.user deny). Salvaged from #44073 (external-surface hardening), split into a focused gateway-authz PR per maintainer request. Follow-up fix by Hermes Agent: the Discord slash-auth channel bypass now matches DISCORD_ALLOWED_CHANNELS by the same name-inclusive keys (id + name + #name + parent) the on_message scope gate uses, so a name-form channel allowlist authorizes slash interactions consistently (was id-only, breaking #name matching). Co-authored-by: Hermes Agent <agent@nousresearch.com>
This commit is contained in:
parent
8e94e8f882
commit
bb304b4914
24 changed files with 1386 additions and 136 deletions
|
|
@ -1075,11 +1075,18 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
# _is_allowed_user docstring).
|
||||
_msg_guild = getattr(message, "guild", None)
|
||||
_is_dm = isinstance(message.channel, discord.DMChannel) or _msg_guild is None
|
||||
_msg_channel_ids = None
|
||||
if not _is_dm:
|
||||
_msg_channel_ids = {str(message.channel.id)}
|
||||
_parent_id = adapter_self._get_parent_channel_id(message.channel)
|
||||
if _parent_id:
|
||||
_msg_channel_ids.add(_parent_id)
|
||||
if not self._is_allowed_user(
|
||||
str(message.author.id),
|
||||
message.author,
|
||||
guild=_msg_guild,
|
||||
is_dm=_is_dm,
|
||||
channel_ids=_msg_channel_ids,
|
||||
):
|
||||
return
|
||||
_role_authorized = bool(getattr(self, "_allowed_role_ids", set()))
|
||||
|
|
@ -3085,6 +3092,18 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
except OSError:
|
||||
pass
|
||||
|
||||
def _discord_channel_ids_allowed(self, channel_ids: set[str]) -> bool:
|
||||
"""True when *channel_ids* intersect ``DISCORD_ALLOWED_CHANNELS``."""
|
||||
if not channel_ids:
|
||||
return False
|
||||
allowed_raw = os.getenv("DISCORD_ALLOWED_CHANNELS", "").strip()
|
||||
if not allowed_raw:
|
||||
return False
|
||||
allowed = {c.strip() for c in allowed_raw.split(",") if c.strip()}
|
||||
if "*" in allowed:
|
||||
return True
|
||||
return bool(channel_ids & allowed)
|
||||
|
||||
def _is_allowed_user(
|
||||
self,
|
||||
user_id: str,
|
||||
|
|
@ -3092,11 +3111,15 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
*,
|
||||
guild=None,
|
||||
is_dm: bool = False,
|
||||
channel_ids: Optional[set[str]] = None,
|
||||
) -> bool:
|
||||
"""Check if user is allowed via DISCORD_ALLOWED_USERS or DISCORD_ALLOWED_ROLES.
|
||||
|
||||
Uses OR semantics: if the user matches EITHER allowlist, they're allowed.
|
||||
If both allowlists are empty, everyone is allowed (backwards compatible).
|
||||
With no user/role allowlists configured, guild traffic may still pass when
|
||||
``channel_ids`` matches ``DISCORD_ALLOWED_CHANNELS`` — but only when the
|
||||
caller supplies the validated channel context (on_message, slash). Calls
|
||||
without channel context (e.g. voice utterances) do not get this bypass.
|
||||
|
||||
Role checks are **scoped to the guild the message originated from**.
|
||||
For DMs (no guild context), role-based auth is disabled by default and
|
||||
|
|
@ -3111,6 +3134,8 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
author: Optional Member/User object for in-guild role lookup.
|
||||
guild: The guild the message arrived in (None for DMs).
|
||||
is_dm: True if the message came from a DM channel.
|
||||
channel_ids: Resolved text-channel ids for guild traffic when an
|
||||
upstream gate has already scoped the message to a channel.
|
||||
"""
|
||||
# ``getattr`` fallbacks here guard against test fixtures that build
|
||||
# an adapter via ``object.__new__(DiscordAdapter)`` and skip __init__
|
||||
|
|
@ -3120,7 +3145,20 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
has_users = bool(allowed_users)
|
||||
has_roles = bool(allowed_roles)
|
||||
if not has_users and not has_roles:
|
||||
return True
|
||||
if os.getenv("DISCORD_ALLOW_ALL_USERS", "").strip().lower() in {"true", "1", "yes"}:
|
||||
return True
|
||||
if os.getenv("GATEWAY_ALLOW_ALL_USERS", "").strip().lower() in {"true", "1", "yes"}:
|
||||
return True
|
||||
# Channel-scoped guild access requires validated channel context.
|
||||
# Do not treat DISCORD_ALLOWED_CHANNELS alone as a user-wide bypass
|
||||
# (voice loops and other guild-scoped callers may lack channel ids).
|
||||
if (
|
||||
not is_dm
|
||||
and channel_ids is not None
|
||||
and self._discord_channel_ids_allowed(channel_ids)
|
||||
):
|
||||
return True
|
||||
return False
|
||||
# Check user ID allowlist (works for both DMs and guild messages).
|
||||
# ``"*"`` is honored as an open-mode wildcard, mirroring
|
||||
# ``SIGNAL_ALLOWED_USERS`` and the existing ``DISCORD_ALLOWED_CHANNELS`` /
|
||||
|
|
@ -3184,11 +3222,11 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
# operator. ``_check_slash_authorization`` mirrors the on_message gates
|
||||
# one-for-one so the slash surface honors the same trust boundary.
|
||||
#
|
||||
# By design, this is a no-op for deployments with no allowlist env vars
|
||||
# set — ``_is_allowed_user`` returns True and the channel checks early-out
|
||||
# — preserving the existing "single-tenant, all guild members trusted"
|
||||
# default. Deployments that DO set any DISCORD_ALLOWED_* var get slash
|
||||
# parity with on_message.
|
||||
# Deployments with no allowlist env vars fail closed unless an explicit
|
||||
# allow-all opt-in is set. When only ``DISCORD_ALLOWED_CHANNELS`` is
|
||||
# configured, guild traffic is authorized per validated channel context
|
||||
# (not as a user-wide bypass). Slash and on_message both pass the
|
||||
# resolved channel ids into ``_is_allowed_user`` after the channel gate.
|
||||
|
||||
def _evaluate_slash_authorization(
|
||||
self, interaction: "discord.Interaction",
|
||||
|
|
@ -3215,6 +3253,8 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
chan_obj = getattr(interaction, "channel", None)
|
||||
in_dm = isinstance(chan_obj, discord.DMChannel) if chan_obj is not None else False
|
||||
|
||||
channel_ids: set = set()
|
||||
channel_keys: set = set()
|
||||
# ── Channel scope (mirrors on_message lines 3374-3388) ──
|
||||
# DMs aren't channel-gated — DMs follow on_message's DM lockdown
|
||||
# path which has its own user-allowlist enforcement.
|
||||
|
|
@ -3222,7 +3262,6 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
chan_id_raw = getattr(interaction, "channel_id", None) or getattr(
|
||||
chan_obj, "id", None,
|
||||
)
|
||||
channel_ids: set = set()
|
||||
if chan_id_raw is not None:
|
||||
channel_ids.add(str(chan_id_raw))
|
||||
# Mirror on_message: also test the parent channel for threads
|
||||
|
|
@ -3270,13 +3309,12 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
allowed_users = getattr(self, "_allowed_user_ids", set()) or set()
|
||||
allowed_roles = getattr(self, "_allowed_role_ids", set()) or set()
|
||||
if user is None or getattr(user, "id", None) is None:
|
||||
# No identifiable user. With any user/role allowlist
|
||||
# configured, fail closed rather than raise AttributeError
|
||||
# on ``interaction.user.id`` below. With no allowlist this
|
||||
# is the existing "no allowlist = everyone" backwards-compat.
|
||||
# No identifiable user — fail closed even with allow-all opt-in.
|
||||
# Downstream slash handlers (_build_slash_event, etc.) require
|
||||
# interaction.user.id and do not synthesize a safe identity.
|
||||
if allowed_users or allowed_roles:
|
||||
return (False, "missing interaction.user with allowlist configured")
|
||||
return (True, None)
|
||||
return (False, "missing interaction.user")
|
||||
|
||||
user_id = str(user.id)
|
||||
# Pass guild + is_dm so role check is scoped to the originating
|
||||
|
|
@ -3288,6 +3326,7 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
author=user,
|
||||
guild=interaction_guild,
|
||||
is_dm=in_dm,
|
||||
channel_ids=channel_keys if not in_dm else None,
|
||||
):
|
||||
return (
|
||||
False,
|
||||
|
|
@ -6255,6 +6294,10 @@ def _component_check_auth(
|
|||
- user is approved in the pairing store -> allow
|
||||
- otherwise -> reject
|
||||
"""
|
||||
user = getattr(interaction, "user", None)
|
||||
if user is None or getattr(user, "id", None) is None:
|
||||
return False
|
||||
|
||||
if os.getenv("DISCORD_ALLOW_ALL_USERS", "").strip().lower() in {"true", "1", "yes"}:
|
||||
return True
|
||||
if os.getenv("GATEWAY_ALLOW_ALL_USERS", "").strip().lower() in {"true", "1", "yes"}:
|
||||
|
|
@ -6270,9 +6313,6 @@ def _component_check_auth(
|
|||
role_set = set(allowed_role_ids or set())
|
||||
has_users = bool(user_set)
|
||||
has_roles = bool(role_set)
|
||||
user = getattr(interaction, "user", None)
|
||||
if user is None:
|
||||
return False
|
||||
|
||||
# Resolve user ID once for both allowlist and pairing checks.
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -776,7 +776,17 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
# a race between dispatch and authorization can result in the adapter
|
||||
# sending a reply even though the handler returned None.
|
||||
allowed_raw = os.getenv("EMAIL_ALLOWED_USERS", "").strip()
|
||||
if allowed_raw:
|
||||
if not allowed_raw:
|
||||
if os.getenv("EMAIL_ALLOW_ALL_USERS", "").strip().lower() not in {"true", "1", "yes"} and (
|
||||
os.getenv("GATEWAY_ALLOW_ALL_USERS", "").strip().lower() not in {"true", "1", "yes"}
|
||||
):
|
||||
logger.debug(
|
||||
"[Email] Dropping sender at dispatch — EMAIL_ALLOWED_USERS is unset "
|
||||
"and open access is not opted in: %s",
|
||||
sender_addr,
|
||||
)
|
||||
return
|
||||
else:
|
||||
allowed = {addr.strip().lower() for addr in allowed_raw.split(",") if addr.strip()}
|
||||
if sender_addr.lower() not in allowed:
|
||||
logger.debug("[Email] Dropping non-allowlisted sender at dispatch: %s", sender_addr)
|
||||
|
|
|
|||
|
|
@ -4200,6 +4200,17 @@ class FeishuAdapter(BasePlatformAdapter):
|
|||
return "bot_not_mentioned"
|
||||
|
||||
if not is_group:
|
||||
if os.getenv("FEISHU_ALLOW_ALL_USERS", "").strip().lower() in {"true", "1", "yes"}:
|
||||
return None
|
||||
if os.getenv("GATEWAY_ALLOW_ALL_USERS", "").strip().lower() in {"true", "1", "yes"}:
|
||||
return None
|
||||
# Empty FEISHU_ALLOWED_USERS is the pairing-mode default from setup:
|
||||
# forward DMs to gateway intake so the pairing handshake can run.
|
||||
# Gateway auth fail-closes agent access until approval.
|
||||
if not self._allowed_group_users:
|
||||
return None
|
||||
if not (sender_ids and (sender_ids & self._allowed_group_users)):
|
||||
return "dm_policy_rejected"
|
||||
return None
|
||||
|
||||
if not self._allow_group_message(
|
||||
|
|
|
|||
|
|
@ -18,9 +18,9 @@ Configuration in config.yaml:
|
|||
bot_id: "your-bot-id" # or WECOM_BOT_ID env var
|
||||
secret: "your-secret" # or WECOM_SECRET env var
|
||||
websocket_url: "wss://openws.work.weixin.qq.com"
|
||||
dm_policy: "open" # open | allowlist | disabled | pairing
|
||||
dm_policy: "pairing" # open | allowlist | disabled | pairing
|
||||
allow_from: ["user_id_1"]
|
||||
group_policy: "open" # open | allowlist | disabled
|
||||
group_policy: "pairing" # open | allowlist | disabled | pairing
|
||||
group_allow_from: ["group_id_1"]
|
||||
groups:
|
||||
group_id_1:
|
||||
|
|
@ -161,7 +161,7 @@ class WeComAdapter(BasePlatformAdapter):
|
|||
or os.getenv("WECOM_WEBSOCKET_URL", DEFAULT_WS_URL)
|
||||
).strip() or DEFAULT_WS_URL
|
||||
|
||||
self._dm_policy = str(extra.get("dm_policy") or os.getenv("WECOM_DM_POLICY", "open")).strip().lower()
|
||||
self._dm_policy = str(extra.get("dm_policy") or os.getenv("WECOM_DM_POLICY", "pairing")).strip().lower()
|
||||
# dm_policy already honors WECOM_DM_POLICY, so the allowlist must honor
|
||||
# WECOM_ALLOWED_USERS too. Without the env fallback an env-only setup
|
||||
# (dm_policy=allowlist via env, no config extra) runs with an empty
|
||||
|
|
@ -172,7 +172,7 @@ class WeComAdapter(BasePlatformAdapter):
|
|||
or os.getenv("WECOM_ALLOWED_USERS", "")
|
||||
)
|
||||
|
||||
self._group_policy = str(extra.get("group_policy") or os.getenv("WECOM_GROUP_POLICY", "open")).strip().lower()
|
||||
self._group_policy = str(extra.get("group_policy") or os.getenv("WECOM_GROUP_POLICY", "pairing")).strip().lower()
|
||||
self._group_allow_from = _coerce_list(extra.get("group_allow_from") or extra.get("groupAllowFrom"))
|
||||
self._groups = extra.get("groups") if isinstance(extra.get("groups"), dict) else {}
|
||||
|
||||
|
|
@ -514,7 +514,7 @@ class WeComAdapter(BasePlatformAdapter):
|
|||
if not self._is_group_allowed(chat_id, sender_id):
|
||||
logger.debug("[%s] Group %s / sender %s blocked by policy", self.name, chat_id, sender_id)
|
||||
return
|
||||
elif not self._is_dm_allowed(sender_id):
|
||||
elif not self._is_dm_intake_allowed(sender_id):
|
||||
logger.debug("[%s] DM sender %s blocked by policy", self.name, sender_id)
|
||||
return
|
||||
|
||||
|
|
@ -861,16 +861,39 @@ class WeComAdapter(BasePlatformAdapter):
|
|||
"""WeCom gates DM/group access at intake via dm_policy/group_policy."""
|
||||
return True
|
||||
|
||||
def _open_dm_opted_in(self) -> bool:
|
||||
if os.getenv("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}:
|
||||
return True
|
||||
return os.getenv("WECOM_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}
|
||||
|
||||
def _is_dm_allowed(self, sender_id: str) -> bool:
|
||||
if self._dm_policy == "disabled":
|
||||
return False
|
||||
if self._dm_policy == "allowlist":
|
||||
return _entry_matches(self._allow_from, sender_id)
|
||||
return True
|
||||
if self._dm_policy == "open":
|
||||
return self._open_dm_opted_in()
|
||||
return False
|
||||
|
||||
def _is_dm_intake_allowed(self, sender_id: str) -> bool:
|
||||
principal = str(sender_id or "").strip()
|
||||
if not principal:
|
||||
return False
|
||||
if self._dm_policy == "disabled":
|
||||
return False
|
||||
if self._dm_policy == "allowlist":
|
||||
return _entry_matches(self._allow_from, principal)
|
||||
if self._dm_policy == "pairing":
|
||||
return True
|
||||
if self._dm_policy == "open":
|
||||
return self._open_dm_opted_in()
|
||||
return False
|
||||
|
||||
def _is_group_allowed(self, chat_id: str, sender_id: str) -> bool:
|
||||
if self._group_policy == "disabled":
|
||||
return False
|
||||
if self._group_policy == "pairing":
|
||||
return False
|
||||
if self._group_policy == "allowlist" and not _entry_matches(self._group_allow_from, chat_id):
|
||||
return False
|
||||
|
||||
|
|
|
|||
|
|
@ -374,9 +374,9 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
|
|||
- bridge_script: Path to the Node.js bridge script
|
||||
- bridge_port: Port for HTTP communication (default: 3000)
|
||||
- session_path: Path to store WhatsApp session data
|
||||
- dm_policy: "open" | "allowlist" | "disabled" — how DMs are handled (default: "open")
|
||||
- dm_policy: "open" | "allowlist" | "disabled" | "pairing" — how DMs are handled (default: "pairing")
|
||||
- allow_from: List of sender IDs allowed in DMs (when dm_policy="allowlist")
|
||||
- group_policy: "open" | "allowlist" | "disabled" — which groups are processed (default: "open")
|
||||
- group_policy: "open" | "allowlist" | "disabled" | "pairing" — which groups are processed (default: "pairing")
|
||||
- group_allow_from: List of group JIDs allowed (when group_policy="allowlist")
|
||||
|
||||
Behavior (gating, mention parsing, markdown conversion, chunking) is
|
||||
|
|
@ -405,9 +405,9 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
|
|||
get_hermes_dir("platforms/whatsapp/session", "whatsapp/session")
|
||||
))
|
||||
self._reply_prefix: Optional[str] = config.extra.get("reply_prefix")
|
||||
self._dm_policy = str(config.extra.get("dm_policy") or os.getenv("WHATSAPP_DM_POLICY", "open")).strip().lower()
|
||||
self._dm_policy = str(config.extra.get("dm_policy") or os.getenv("WHATSAPP_DM_POLICY", "pairing")).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_policy = str(config.extra.get("group_policy") or os.getenv("WHATSAPP_GROUP_POLICY", "pairing")).strip().lower()
|
||||
self._group_allow_from = self._coerce_allow_list(config.extra.get("group_allow_from") or config.extra.get("groupAllowFrom"))
|
||||
self._mention_patterns = self._compile_mention_patterns()
|
||||
self._message_queue: asyncio.Queue = asyncio.Queue()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue