Skip to content

fix(discord): isolate multiplex access policy per profile #76166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
OneNoted wants to merge 1 commit into from
Closed

fix(discord): isolate multiplex access policy per profile #76166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
110 changes: 99 additions & 11 deletions gateway/authz_mixin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def _authorization_adapter(
if not platform:
return None
profile_name = (profile or "").strip() or None
if profile_name and profile_name != "default":
if profile_name:
active_profile = None
active_profile_fn = getattr(self, "_active_profile_name", None)
if callable(active_profile_fn):
Expand Down Expand Up @@ -120,9 +120,12 @@ def _adapter_for_source(self, source: Optional[SessionSource]):
return adapters.get(Platform.RELAY)
# ``getattr`` guards test fixtures that build a bare source via
# SimpleNamespace and omit ``profile`` (see AGENTS.md pitfall #17).
transport_profile = getattr(source, "transport_profile", None)
return self._authorization_adapter(
getattr(source, "platform", None),
getattr(source, "profile", None),
transport_profile
if isinstance(transport_profile, str) and transport_profile.strip()
else getattr(source, "profile", None),
)

def _registered_transport_adapter(self, source: SessionSource):
Expand Down Expand Up @@ -160,6 +163,9 @@ def _adapter_profile_for_source(self, source: SessionSource) -> Optional[str]:
).items():
if adapter is profile_adapters.get(platform):
return profile
transport_profile = getattr(source, "transport_profile", None)
if isinstance(transport_profile, str) and transport_profile.strip():
return transport_profile
return getattr(source, "profile", None)

def _adapter_authorization_is_upstream(
Expand Down Expand Up @@ -340,18 +346,20 @@ def _adapter_group_has_sender_allowlist(
return False

def _pairing_store_for(self, source: "SessionSource"):
"""Pick the per-profile PairingStore for a source, falling back to global.
"""Pick the PairingStore owned by ``source`` without crossing profiles.

In a multiplexing gateway, each profile owns its own pairing whitelist
so isolation is preserved. When the source has no profile (single-
profile gateway, or a path that hasn't stamped profile yet) or the
profile isn't registered, fall back to ``self.pairing_store`` (the
global default) so existing behavior is preserved.
A multiplex source with an explicit profile must resolve to that
profile's store. Missing registration fails closed. Unstamped and
single-profile sources retain the legacy global fallback.
"""
per_profile = getattr(self, "pairing_stores", None) or {}
profile = getattr(source, "profile", None)
if profile and profile in per_profile:
return per_profile[profile]
if profile:
if profile in per_profile:
return per_profile[profile]
config = getattr(self, "config", None)
if bool(getattr(config, "multiplex_profiles", False)):
return None
return getattr(self, "pairing_store", None)

def _is_user_authorized(self, source: SessionSource) -> bool:
Expand Down Expand Up @@ -469,12 +477,92 @@ def _is_user_authorized(self, source: SessionSource) -> bool:
}
if getattr(source, "is_bot", False):
allow_bots_var = platform_allow_bots_map.get(source.platform)
if allow_bots_var and os.getenv(allow_bots_var, "none").lower().strip() in {"mentions", "all"}:
# Discord owns this decision when a concrete adapter is available;
# bare-runner/direct tests retain the historical env fallback.
adapter_owns_bot_policy = False
if source.platform == Platform.DISCORD:
bot_adapter = self._authorization_adapter(
source.platform, adapter_profile
)
adapter_owns_bot_policy = callable(
getattr(type(bot_adapter), "_authorization_policy_allows", None)
)
if (
not adapter_owns_bot_policy
and allow_bots_var
and os.getenv(allow_bots_var, "none").lower().strip()
in {"mentions", "all"}
):
return True

if not user_id:
return False

# Discord adapters capture every user/role/channel/global fallback and
# pairing callback under their owning profile scope. Treat that snapshot
# as the complete authorization decision at this second gateway gate;
# reopening process globals or applying pairing again here could bypass
# an adapter-level hard channel denial.
adapter = self._authorization_adapter(source.platform, adapter_profile)
policy_authorizer = getattr(
type(adapter), "_authorization_policy_allows", None
)
if callable(policy_authorizer):
channel_keys = {
str(value).strip()
for value in getattr(source, "authorization_channel_keys", [])
if str(value).strip()
}
for value in (source.chat_id, getattr(source, "parent_chat_id", None)):
if value:
channel_keys.add(str(value).strip())
chat_name = str(getattr(source, "chat_name", "") or "").strip()
if chat_name:
channel_keys.add(chat_name)
leaf_name = chat_name.rsplit(" / ", 1)[-1].strip()
if leaf_name:
channel_keys.add(leaf_name)
bare_name = leaf_name.removeprefix("#")
if bare_name:
channel_keys.update({bare_name, f"#{bare_name}"})
role_authorized = getattr(source, "role_authorized", False) is True
if not role_authorized:
role_checker = getattr(type(adapter), "_source_has_allowed_role", None)
if callable(role_checker):
try:
role_authorized = bool(
role_checker(
adapter,
user_id,
guild_id=(
getattr(source, "scope_id", None)
or getattr(source, "guild_id", None)
),
is_dm=getattr(source, "chat_type", "dm") == "dm",
)
)
except Exception:
role_authorized = False
try:
return bool(
policy_authorizer(
adapter,
user_id,
chat_id=source.chat_id,
channel_keys=channel_keys,
is_dm=getattr(source, "chat_type", "dm") == "dm",
role_authorized=role_authorized,
is_bot=getattr(source, "is_bot", False) is True,
)
)
except Exception:
logger.warning(
"Adapter-local authorization check failed for %s",
source.platform.value,
exc_info=True,
)
return False

platform_env_map = {
Platform.TELEGRAM: "TELEGRAM_ALLOWED_USERS",
Platform.DISCORD: "DISCORD_ALLOWED_USERS",
Expand Down
18 changes: 14 additions & 4 deletions gateway/platforms/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ def is_host_excluded_by_no_proxy(hostname: str, no_proxy_value: str | None = Non
from dataclasses import dataclass, field
from datetime import datetime
from pathlib import Path
from typing import Dict, List, Optional, Any, Callable, Awaitable, Tuple, Union
from typing import Dict, List, Optional, Any, Callable, Awaitable, Tuple, Union, Iterable
from enum import Enum

from pathlib import Path as _Path
Expand Down Expand Up @@ -2747,6 +2747,9 @@ def set_status_text(self, chat_id: str, text: Optional[str]) -> None:
def __init__(self, config: PlatformConfig, platform: Platform):
self.config = config
self.platform = platform
# Transport-owning profile, stamped by GatewayRunner before connect.
# Persisted through SessionSource separately from runtime routing.
self._gateway_profile_name: Optional[str] = None
self._message_handler: Optional[MessageHandler] = None
# Optional gateway-supplied fan-out for platform-native emoji
# reaction events (see ``set_reaction_handler``).
Expand Down Expand Up @@ -6567,6 +6570,7 @@ def build_source(
scope_id: Optional[str] = None,
guild_id: Optional[str] = None,
parent_chat_id: Optional[str] = None,
authorization_channel_keys: Optional[Iterable[str]] = None,
message_id: Optional[str] = None,
role_authorized: bool = False,
auto_thread_created: bool = False,
Expand Down Expand Up @@ -6605,6 +6609,9 @@ def build_source(
scope_id=str(scope_id) if scope_id else None,
guild_id=str(guild_id) if guild_id else None,
parent_chat_id=str(parent_chat_id) if parent_chat_id else None,
authorization_channel_keys=[
str(value) for value in (authorization_channel_keys or [])
],
message_id=str(message_id) if message_id else None,
)
)
Expand All @@ -6629,15 +6636,18 @@ def build_source(
scope_id=str(scope_id) if scope_id else None,
guild_id=str(guild_id) if guild_id else None,
parent_chat_id=str(parent_chat_id) if parent_chat_id else None,
authorization_channel_keys=[
str(value) for value in (authorization_channel_keys or [])
],
message_id=str(message_id) if message_id else None,
profile=profile,
transport_profile=getattr(self, "_gateway_profile_name", None),
role_authorized=role_authorized,
auto_thread_created=auto_thread_created,
auto_thread_initial_name=auto_thread_initial_name,
)
# In-process transport provenance is deliberately not serialized by
# SessionSource.to_dict(). The live receiving adapter is authoritative
# for this turn even when profile_routes selects a different runtime.
# Retain the exact live adapter for this process. ``transport_profile``
# above is the serialized fallback used after restart.
source._transport_adapter_ref = weakref.ref(self)
return source

Expand Down
Loading