fix(gateway/discord): require allowlist auth on slash commands

Slash commands (_run_simple_slash, _handle_thread_create_slash) bypassed every DISCORD_ALLOWED_* gate enforced by on_message. Any guild member could invoke /background (RCE via terminal), /restart, /model, /skill, etc. CVSS 9.8 Critical. - _evaluate_slash_authorization mirrors on_message gates (user, role, channel, ignored channel) with fail-closed semantics - _check_slash_authorization sends ephemeral reject + logs + admin alert - Auth gate runs before defer() so rejections are ephemeral - /skill autocomplete returns [] for unauthorized users (no catalog leak) - Component views (ExecApproval, SlashConfirm, UpdatePrompt, ModelPicker) now honor role allowlists via shared _component_check_auth helper - Optional DISCORD_HIDE_SLASH_COMMANDS defense-in-depth - Cross-platform admin alert (Telegram/Slack fallback) on unauthorized attempts Based on PR #18125 by @0xyg3n.
2026-05-03 16:13:39 +05:30
parent 5d5b8912be
commit 19ba9e43b6
5 changed files with 1392 additions and 18 deletions
--- a/gateway/platforms/discord.py
+++ b/gateway/platforms/discord.py
@@ -497,6 +497,7 @@ class DiscordAdapter(BasePlatformAdapter):
        self._ready_event = asyncio.Event()
        self._allowed_user_ids: set = set()  # For button approval authorization
        self._allowed_role_ids: set = set()  # For DISCORD_ALLOWED_ROLES filtering
+        self.gateway_runner = None  # Set by gateway/run.py for cross-platform delivery
        # Voice channel state (per-guild)
        self._voice_clients: Dict[int, Any] = {}  # guild_id -> VoiceClient
        self._voice_locks: Dict[int, asyncio.Lock] = {}  # guild_id -> serialize join/leave
@@ -1929,6 +1930,225 @@ class DiscordAdapter(BasePlatformAdapter):
                            return True
        return False

+    # ── Slash command authorization ─────────────────────────────────────
+    # Slash commands (``_run_simple_slash`` and ``_handle_thread_create_slash``)
+    # are a separate Discord interaction surface from regular messages and
+    # historically ran with NO authorization check — bypassing every gate
+    # ``on_message`` enforces (DISCORD_ALLOWED_USERS, DISCORD_ALLOWED_ROLES,
+    # DISCORD_ALLOWED_CHANNELS, DISCORD_IGNORED_CHANNELS). Any guild member
+    # could invoke ``/background``, ``/restart``, ``/sethome``, etc. as the
+    # 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.
+
+    def _evaluate_slash_authorization(
+        self, interaction: "discord.Interaction",
+    ) -> Tuple[bool, Optional[str]]:
+        """Evaluate slash authorization without producing any response.
+
+        Returns ``(allowed, reason)``. ``reason`` is populated only when
+        ``allowed`` is False. This is the shared core used by both the
+        responding wrapper (``_check_slash_authorization``) and side-effect-
+        free callers like the ``/skill`` autocomplete callback, which must
+        return an empty list for unauthorized users instead of leaking an
+        ephemeral rejection per-keystroke.
+
+        Fail-closed semantics for malformed payloads: when an allowlist is
+        configured but the interaction is missing the data needed to
+        evaluate it (no channel id with channel policy active, no user
+        with user/role policy active), the gate REJECTS rather than
+        falling through. Without these guards a guild interaction that
+        happens to deserialize without a channel id would silently bypass
+        ``DISCORD_ALLOWED_CHANNELS`` and a payload missing ``user`` would
+        raise ``AttributeError`` in the user check below, surfacing as
+        an opaque interaction failure rather than a clean rejection.
+        """
+        chan_obj = getattr(interaction, "channel", None)
+        in_dm = isinstance(chan_obj, discord.DMChannel) if chan_obj is not None else False
+
+        # ── 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.
+        if not in_dm:
+            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
+                # so per-channel allow/deny lists work consistently.
+                if isinstance(chan_obj, discord.Thread):
+                    parent_id = self._get_parent_channel_id(chan_obj)
+                    if parent_id:
+                        channel_ids.add(str(parent_id))
+
+            allowed_raw = os.getenv("DISCORD_ALLOWED_CHANNELS", "")
+            if allowed_raw:
+                allowed = {c.strip() for c in allowed_raw.split(",") if c.strip()}
+                if "*" not in allowed:
+                    if not channel_ids:
+                        # Channel policy is configured but the interaction
+                        # has no resolvable channel id. Fail closed.
+                        return (
+                            False,
+                            "channel id missing with DISCORD_ALLOWED_CHANNELS configured",
+                        )
+                    if not (channel_ids & allowed):
+                        return (False, "channel not in DISCORD_ALLOWED_CHANNELS")
+
+            # Ignored beats allowed: even when a thread's parent channel
+            # is on the allowlist, an explicit DISCORD_IGNORED_CHANNELS
+            # entry on the thread or its parent rejects the interaction.
+            ignored_raw = os.getenv("DISCORD_IGNORED_CHANNELS", "")
+            if ignored_raw and channel_ids:
+                ignored = {c.strip() for c in ignored_raw.split(",") if c.strip()}
+                if "*" in ignored or (channel_ids & ignored):
+                    return (False, "channel in DISCORD_IGNORED_CHANNELS")
+
+        # ── User / role allowlist (mirrors on_message line 681) ──
+        user = getattr(interaction, "user", None)
+        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.
+            if allowed_users or allowed_roles:
+                return (False, "missing interaction.user with allowlist configured")
+            return (True, None)
+
+        user_id = str(user.id)
+        if not self._is_allowed_user(user_id, author=user):
+            return (
+                False,
+                "user not in DISCORD_ALLOWED_USERS / DISCORD_ALLOWED_ROLES",
+            )
+
+        return (True, None)
+
+    async def _check_slash_authorization(
+        self, interaction: "discord.Interaction", command_text: str,
+    ) -> bool:
+        """Mirror on_message's user/role/channel gates onto a slash invocation.
+
+        Returns True to proceed. Returns False *after* sending an ephemeral
+        rejection, logging a warning, and scheduling a cross-platform admin
+        alert — the caller must stop on False (the interaction has already
+        been responded to).
+        """
+        allowed, reason = self._evaluate_slash_authorization(interaction)
+        if allowed:
+            return True
+        return await self._reject_slash(
+            interaction, command_text, reason=reason or "unauthorized",
+        )
+
+    async def _reject_slash(
+        self, interaction: "discord.Interaction", command_text: str, *, reason: str,
+    ) -> bool:
+        """Send ephemeral reject + log warning + schedule admin alert. Returns False.
+
+        Tolerates a missing ``interaction.user`` -- the fail-closed branch
+        in ``_evaluate_slash_authorization`` deliberately routes here for
+        malformed payloads (no user) when an allowlist is configured, and
+        ``str(interaction.user.id)`` would raise AttributeError before the
+        ephemeral rejection could be sent.
+        """
+        user = getattr(interaction, "user", None)
+        if user is not None:
+            user_id = str(getattr(user, "id", "?"))
+            user_name = getattr(user, "name", "?")
+        else:
+            user_id = "?"
+            user_name = "?"
+        chan_id = getattr(interaction, "channel_id", None) or getattr(
+            getattr(interaction, "channel", None), "id", None,
+        )
+        guild_id = getattr(interaction, "guild_id", None)
+
+        logger.warning(
+            "[Discord] Unauthorized slash attempt: user=%s id=%s channel=%s "
+            "guild=%s cmd=%r reason=%r",
+            user_name, user_id, chan_id, guild_id, command_text, reason,
+        )
+
+        try:
+            await interaction.response.send_message(
+                "You're not authorized to use this command.",
+                ephemeral=True,
+            )
+        except Exception as e:
+            # Interaction may already be responded to (e.g. caller deferred
+            # before the auth check, or Discord retried). Best-effort only.
+            logger.debug("[Discord] Could not send unauthorized ephemeral: %s", e)
+
+        # Fire-and-forget: don't block the interaction handler on Telegram I/O.
+        try:
+            asyncio.create_task(self._notify_unauthorized_slash(
+                user_name, user_id, chan_id, guild_id, command_text, reason,
+            ))
+        except Exception as e:
+            logger.debug("[Discord] Could not schedule admin notify task: %s", e)
+
+        return False
+
+    async def _notify_unauthorized_slash(
+        self, user_name: str, user_id: str, chan_id, guild_id,
+        command_text: str, reason: str,
+    ) -> None:
+        """Best-effort cross-platform alert to the gateway operator.
+
+        Tries TELEGRAM first (most operators set TELEGRAM_HOME_CHANNEL),
+        then SLACK. Silently no-ops if no other platform is configured
+        with a home channel.
+
+        A soft send failure -- adapter.send() returning a result with
+        ``success=False`` rather than raising -- continues the fallback
+        chain. Treating a SendResult(success=False) as delivered would
+        mean a Telegram outage that the adapter politely surfaces (e.g.
+        rate-limit, auth failure) silently swallows the alert without
+        attempting Slack. Hard exceptions still take the same path via
+        the except branch below.
+        """
+        runner = getattr(self, "gateway_runner", None)
+        if not runner:
+            return
+        for target in (Platform.TELEGRAM, Platform.SLACK):
+            try:
+                adapter = runner.adapters.get(target)
+                if not adapter:
+                    continue
+                home = runner.config.get_home_channel(target)
+                if not home or not getattr(home, "chat_id", None):
+                    continue
+                msg = (
+                    "⚠️ Unauthorized Discord slash attempt\n"
+                    f"User: {user_name} ({user_id})\n"
+                    f"Channel: {chan_id} (guild {guild_id})\n"
+                    f"Command: {command_text}\n"
+                    f"Reason: {reason}"
+                )
+                result = await adapter.send(str(home.chat_id), msg)
+                # Only return on confirmed delivery. SendResult(success=False)
+                # -> continue to the next platform.
+                if getattr(result, "success", None) is False:
+                    logger.debug(
+                        "[Discord] Admin notify via %s returned success=False"
+                        " (error=%r); falling through",
+                        target, getattr(result, "error", None),
+                    )
+                    continue
+                return
+            except Exception as e:
+                logger.debug("[Discord] Admin notify via %s failed: %s", target, e)
+
    async def send_image_file(
        self,
        chat_id: str,
@@ -2316,6 +2536,11 @@ class DiscordAdapter(BasePlatformAdapter):
        except Exception:
            pass  # logging must never block command dispatch

+        # Auth gate — must run before defer() so an ephemeral rejection can
+        # be delivered on the still-unresponded interaction.
+        if not await self._check_slash_authorization(interaction, command_text):
+            return
+
        await interaction.response.defer(ephemeral=True)
        event = self._build_slash_event(interaction, command_text)
        await self.handle_message(event)
@@ -2460,7 +2685,8 @@ class DiscordAdapter(BasePlatformAdapter):
            message: str = "",
            auto_archive_duration: int = 1440,
        ):
-            await interaction.response.defer(ephemeral=True)
+            # defer() is performed inside the handler *after* the auth gate
+            # so a rejected invoker can receive an ephemeral rejection.
            await self._handle_thread_create_slash(interaction, name, message, auto_archive_duration)

        @tree.command(name="queue", description="Queue a prompt for the next turn (doesn't interrupt)")
@@ -2581,6 +2807,54 @@ class DiscordAdapter(BasePlatformAdapter):
        # supporting up to 25 categories × 25 skills = 625 skills.
        self._register_skill_group(tree)

+        # Optional defense-in-depth: hide every slash command from non-admin
+        # guild members in Discord's slash picker. Server-side authorization
+        # (``_check_slash_authorization``) is the actual gate; this is purely
+        # UX so users don't see commands they can't invoke. Off by default
+        # to preserve the slash UX for deployments that intentionally allow
+        # everyone in the guild.
+        if os.getenv("DISCORD_HIDE_SLASH_COMMANDS", "false").strip().lower() in (
+            "true", "1", "yes", "on",
+        ):
+            self._apply_owner_only_visibility(tree)
+
+    def _apply_owner_only_visibility(self, tree) -> None:
+        """Set default_member_permissions=0 on every registered slash command.
+
+        Discord interprets ``Permissions(0)`` as "requires no permissions",
+        which paradoxically means the command is hidden from every guild
+        member except those with the Administrator permission. Server admins
+        can re-grant per user/role via Server Settings → Integrations →
+        <bot> → Permissions.
+
+        Authoritative gate is ``_check_slash_authorization`` on every
+        invocation, which catches stale clients, role grants made by
+        mistake, and direct API calls bypassing Discord's UI hide.
+        """
+        try:
+            no_perms = discord.Permissions(0)
+        except Exception as e:
+            logger.warning(
+                "[Discord] _apply_owner_only_visibility: cannot build Permissions(0): %s",
+                e,
+            )
+            return
+        applied = 0
+        for cmd in tree.get_commands():
+            try:
+                cmd.default_permissions = no_perms
+                applied += 1
+            except Exception as e:
+                logger.debug(
+                    "[Discord] Could not set default_permissions on %r: %s",
+                    getattr(cmd, "name", "?"), e,
+                )
+        logger.info(
+            "[Discord] Hid %d slash command(s) from non-admin guild members "
+            "(opt-in defense in depth via DISCORD_HIDE_SLASH_COMMANDS).",
+            applied,
+        )
+
    def _register_skill_group(self, tree) -> None:
        """Register a single ``/skill`` command with autocomplete on the name.

@@ -2635,9 +2909,25 @@ class DiscordAdapter(BasePlatformAdapter):
                PDFs even if the name doesn't. Discord caps this list at
                25 entries per query.

+                Authorization: a quiet pre-check evaluates the slash
+                allowlists and returns ``[]`` for unauthorized users so
+                the installed skill catalog is not leaked to anyone who
+                can see the command in the picker. Returning a generic
+                empty list here is intentional — sending a per-keystroke
+                ephemeral rejection would produce a barrage of error
+                popups during typing.
+
                Reads ``self._skill_entries`` so a ``/reload-skills`` run
                since process start shows up on the very next keystroke.
                """
+                try:
+                    allowed, _reason = self._evaluate_slash_authorization(interaction)
+                except Exception:
+                    # Defensive: never raise from autocomplete. Fail
+                    # closed by returning an empty suggestion list.
+                    return []
+                if not allowed:
+                    return []
                q = (current or "").strip().lower()
                choices: list = []
                for name, desc, _key in self._skill_entries:
@@ -2664,6 +2954,12 @@ class DiscordAdapter(BasePlatformAdapter):
            async def _skill_handler(
                interaction: "discord.Interaction", name: str, args: str = "",
            ):
+                # Authorize BEFORE any skill lookup so that known and
+                # unknown skill names produce identical rejections for
+                # unauthorized users (no probing the installed catalog
+                # via "Unknown skill: <name>" responses).
+                if not await self._check_slash_authorization(interaction, "/skill"):
+                    return
                entry = self._skill_lookup.get(name)
                if not entry:
                    await interaction.response.send_message(
@@ -2811,6 +3107,9 @@ class DiscordAdapter(BasePlatformAdapter):
        auto_archive_duration: int = 1440,
    ) -> None:
        """Create a Discord thread from a slash command and start a session in it."""
+        if not await self._check_slash_authorization(interaction, "/thread"):
+            return
+        await interaction.response.defer(ephemeral=True)
        result = await self._create_thread(
            interaction,
            name=name,
@@ -3105,6 +3404,7 @@ class DiscordAdapter(BasePlatformAdapter):
            view = ExecApprovalView(
                session_key=session_key,
                allowed_user_ids=self._allowed_user_ids,
+                allowed_role_ids=self._allowed_role_ids,
            )

            msg = await channel.send(embed=embed, view=view)
@@ -3143,6 +3443,7 @@ class DiscordAdapter(BasePlatformAdapter):
                session_key=session_key,
                confirm_id=confirm_id,
                allowed_user_ids=self._allowed_user_ids,
+                allowed_role_ids=self._allowed_role_ids,
            )

            msg = await channel.send(embed=embed, view=view)
@@ -3177,6 +3478,7 @@ class DiscordAdapter(BasePlatformAdapter):
            view = UpdatePromptView(
                session_key=session_key,
                allowed_user_ids=self._allowed_user_ids,
+                allowed_role_ids=self._allowed_role_ids,
            )
            msg = await channel.send(embed=embed, view=view)
            return SendResult(success=True, message_id=str(msg.id))
@@ -3234,6 +3536,7 @@ class DiscordAdapter(BasePlatformAdapter):
                session_key=session_key,
                on_model_selected=on_model_selected,
                allowed_user_ids=self._allowed_user_ids,
+                allowed_role_ids=self._allowed_role_ids,
            )

            msg = await channel.send(embed=embed, view=view)
@@ -3789,6 +4092,72 @@ class DiscordAdapter(BasePlatformAdapter):
 # Discord UI Components (outside the adapter class)
 # ---------------------------------------------------------------------------

+
+def _component_check_auth(
+    interaction,
+    allowed_user_ids: Optional[set],
+    allowed_role_ids: Optional[set],
+) -> bool:
+    """Shared user-or-role OR semantics for component view button clicks.
+
+    Mirrors ``DiscordAdapter._is_allowed_user`` / the slash and on_message
+    gates so every Discord interaction surface honors the same trust
+    boundary. Component views (ExecApprovalView, SlashConfirmView,
+    UpdatePromptView, ModelPickerView) used to receive only
+    ``allowed_user_ids``: in role-only deployments
+    (DISCORD_ALLOWED_ROLES set, DISCORD_ALLOWED_USERS empty) the user
+    set was empty and the legacy "no allowlist = allow everyone" branch
+    let any guild member click the buttons -- approving exec commands,
+    cancelling slash confirmations, switching the model.
+
+    Behavior:
+
+      - both allowlists empty -> allow (preserves existing no-allowlist
+        deployments, no regression)
+      - user is in user allowlist -> allow
+      - role allowlist set + user has a role in it -> allow
+      - role allowlist set + interaction.user has no resolvable
+        ``roles`` attribute (e.g. DM context with a role policy active)
+        -> reject (fail closed)
+      - otherwise -> reject
+    """
+    user_set = allowed_user_ids or set()
+    role_set = allowed_role_ids or set()
+    has_users = bool(user_set)
+    has_roles = bool(role_set)
+    if not has_users and not has_roles:
+        return True
+
+    user = getattr(interaction, "user", None)
+    if user is None:
+        return False
+
+    if has_users:
+        try:
+            uid = str(user.id)
+        except AttributeError:
+            uid = ""
+        if uid and uid in user_set:
+            return True
+
+    if has_roles:
+        roles_attr = getattr(user, "roles", None)
+        if roles_attr is None:
+            # Role policy is configured but the interaction doesn't
+            # carry role data (DM-context Member, raw User payload).
+            # Fail closed: a user without a resolvable role list cannot
+            # satisfy a role allowlist.
+            return False
+        try:
+            user_role_ids = {getattr(r, "id", None) for r in roles_attr}
+        except TypeError:
+            return False
+        if user_role_ids & role_set:
+            return True
+
+    return False
+
+
 if DISCORD_AVAILABLE:

    class ExecApprovalView(discord.ui.View):
@@ -3801,17 +4170,23 @@ if DISCORD_AVAILABLE:
        Only users in the allowed list can click.  Times out after 5 minutes.
        """

-        def __init__(self, session_key: str, allowed_user_ids: set):
+        def __init__(
+            self,
+            session_key: str,
+            allowed_user_ids: set,
+            allowed_role_ids: Optional[set] = None,
+        ):
            super().__init__(timeout=300)  # 5-minute timeout
            self.session_key = session_key
            self.allowed_user_ids = allowed_user_ids
+            self.allowed_role_ids = allowed_role_ids or set()
            self.resolved = False

        def _check_auth(self, interaction: discord.Interaction) -> bool:
            """Verify the user clicking is authorized."""
-            if not self.allowed_user_ids:
-                return True  # No allowlist = anyone can approve
-            return str(interaction.user.id) in self.allowed_user_ids
+            return _component_check_auth(
+                interaction, self.allowed_user_ids, self.allowed_role_ids,
+            )

        async def _resolve(
            self, interaction: discord.Interaction, choice: str,
@@ -3903,17 +4278,24 @@ if DISCORD_AVAILABLE:
        5 minutes (matches the gateway primitive's timeout).
        """

-        def __init__(self, session_key: str, confirm_id: str, allowed_user_ids: set):
+        def __init__(
+            self,
+            session_key: str,
+            confirm_id: str,
+            allowed_user_ids: set,
+            allowed_role_ids: Optional[set] = None,
+        ):
            super().__init__(timeout=300)
            self.session_key = session_key
            self.confirm_id = confirm_id
            self.allowed_user_ids = allowed_user_ids
+            self.allowed_role_ids = allowed_role_ids or set()
            self.resolved = False

        def _check_auth(self, interaction: discord.Interaction) -> bool:
-            if not self.allowed_user_ids:
-                return True
-            return str(interaction.user.id) in self.allowed_user_ids
+            return _component_check_auth(
+                interaction, self.allowed_user_ids, self.allowed_role_ids,
+            )

        async def _resolve(
            self, interaction: discord.Interaction, choice: str,
@@ -3991,16 +4373,22 @@ if DISCORD_AVAILABLE:
        5-minute timeout on its side).
        """

-        def __init__(self, session_key: str, allowed_user_ids: set):
+        def __init__(
+            self,
+            session_key: str,
+            allowed_user_ids: set,
+            allowed_role_ids: Optional[set] = None,
+        ):
            super().__init__(timeout=300)
            self.session_key = session_key
            self.allowed_user_ids = allowed_user_ids
+            self.allowed_role_ids = allowed_role_ids or set()
            self.resolved = False

        def _check_auth(self, interaction: discord.Interaction) -> bool:
-            if not self.allowed_user_ids:
-                return True
-            return str(interaction.user.id) in self.allowed_user_ids
+            return _component_check_auth(
+                interaction, self.allowed_user_ids, self.allowed_role_ids,
+            )

        async def _respond(
            self, interaction: discord.Interaction, answer: str,
@@ -4077,6 +4465,7 @@ if DISCORD_AVAILABLE:
            session_key: str,
            on_model_selected,
            allowed_user_ids: set,
+            allowed_role_ids: Optional[set] = None,
        ):
            super().__init__(timeout=120)
            self.providers = providers
@@ -4085,15 +4474,16 @@ if DISCORD_AVAILABLE:
            self.session_key = session_key
            self.on_model_selected = on_model_selected
            self.allowed_user_ids = allowed_user_ids
+            self.allowed_role_ids = allowed_role_ids or set()
            self.resolved = False
            self._selected_provider: str = ""

            self._build_provider_select()

        def _check_auth(self, interaction: discord.Interaction) -> bool:
-            if not self.allowed_user_ids:
-                return True
-            return str(interaction.user.id) in self.allowed_user_ids
+            return _component_check_auth(
+                interaction, self.allowed_user_ids, self.allowed_role_ids,
+            )

        def _build_provider_select(self):
            """Build the provider dropdown menu."""
--- a/gateway/run.py
+++ b/gateway/run.py
@@ -3982,7 +3982,9 @@ class GatewayRunner:
            if not check_discord_requirements():
                logger.warning("Discord: discord.py not installed")
                return None
-            return DiscordAdapter(config)
+            adapter = DiscordAdapter(config)
+            adapter.gateway_runner = self  # For cross-platform admin alerts on unauthorized slash
+            return adapter
        
        elif platform == Platform.WHATSAPP:
            from gateway.platforms.whatsapp import WhatsAppAdapter, check_whatsapp_requirements
--- a/tests/gateway/test_discord_component_auth.py
+++ b/tests/gateway/test_discord_component_auth.py
@@ -0,0 +1,230 @@
+"""Security regression tests: Discord component views honor role allowlists.
+
+The four interactive component views (ExecApprovalView, SlashConfirmView,
+UpdatePromptView, ModelPickerView) historically accepted only
+``allowed_user_ids``. Deployments that configure DISCORD_ALLOWED_ROLES
+without DISCORD_ALLOWED_USERS therefore had a wide-open component
+surface: any guild member who could see the prompt could approve exec
+commands, cancel slash confirmations, or switch the model -- even when
+the same user would be rejected at the slash and on_message gates.
+
+These tests pin the user-or-role OR semantics and the fail-closed
+behavior on missing role data so the parity cannot regress.
+"""
+
+from types import SimpleNamespace
+
+import pytest
+
+# Trigger the shared discord mock from tests/gateway/conftest.py before
+# importing the production module.
+from gateway.platforms.discord import (  # noqa: E402
+    ExecApprovalView,
+    ModelPickerView,
+    SlashConfirmView,
+    UpdatePromptView,
+    _component_check_auth,
+)
+
+
+# ---------------------------------------------------------------------------
+# Direct helper coverage -- the four views all delegate to this helper, so
+# pinning the helper's contract pins all four call sites.
+# ---------------------------------------------------------------------------
+
+
+def _interaction(user_id, role_ids=None, *, drop_user=False, drop_roles=False):
+    """Build a mock interaction with the requested user/role shape.
+
+    drop_user simulates a payload whose .user attribute is None.
+    drop_roles simulates a payload where .user has no .roles attribute
+    at all (DM-context Member, raw User payload).
+    """
+    if drop_user:
+        return SimpleNamespace(user=None)
+
+    user_kwargs = {"id": user_id}
+    if not drop_roles:
+        user_kwargs["roles"] = [SimpleNamespace(id=r) for r in (role_ids or [])]
+    return SimpleNamespace(user=SimpleNamespace(**user_kwargs))
+
+
+# ── back-compat: empty allowlists -> allow everyone ────────────────────────
+
+
+def test_component_check_empty_allowlists_allows_everyone():
+    """SECURITY-CRITICAL backwards-compat: deployments without any
+    DISCORD_ALLOWED_* env vars set must continue to allow component
+    interactions from anyone (no regression for unconfigured setups)."""
+    interaction = _interaction(11111)
+    assert _component_check_auth(interaction, set(), set()) is True
+    assert _component_check_auth(interaction, None, None) is True
+
+
+# ── user allowlist ─────────────────────────────────────────────────────────
+
+
+def test_component_check_user_in_user_allowlist_passes():
+    interaction = _interaction(11111)
+    assert _component_check_auth(interaction, {"11111"}, set()) is True
+
+
+def test_component_check_user_not_in_user_allowlist_rejected():
+    interaction = _interaction(99999)
+    assert _component_check_auth(interaction, {"11111"}, set()) is False
+
+
+# ── role allowlist OR semantics ────────────────────────────────────────────
+
+
+def test_component_check_role_only_user_with_matching_role_passes():
+    """Role-only deployment (DISCORD_ALLOWED_ROLES set, DISCORD_ALLOWED_USERS
+    empty) where the user is not in the empty user list but DOES carry a
+    matching role: must pass. This is the regression that prompted the
+    fix -- previously _check_auth allowed everyone when the user set was
+    empty, ignoring the role allowlist."""
+    interaction = _interaction(99999, role_ids=[42])
+    assert _component_check_auth(interaction, set(), {42}) is True
+
+
+def test_component_check_role_only_user_without_matching_role_rejected():
+    """Role-only deployment where the user has no matching role: reject.
+    Previously this allowed everyone because allowed_user_ids was empty."""
+    interaction = _interaction(99999, role_ids=[7, 8])
+    assert _component_check_auth(interaction, set(), {42}) is False
+
+
+def test_component_check_user_or_role_user_match():
+    """Both allowlists set; user matches user allowlist: pass."""
+    interaction = _interaction(11111, role_ids=[7])
+    assert _component_check_auth(interaction, {"11111"}, {42}) is True
+
+
+def test_component_check_user_or_role_role_match():
+    """Both allowlists set; user not in user list but in role list: pass."""
+    interaction = _interaction(99999, role_ids=[42])
+    assert _component_check_auth(interaction, {"11111"}, {42}) is True
+
+
+def test_component_check_user_or_role_neither_match():
+    """Both allowlists set; user matches neither: reject."""
+    interaction = _interaction(99999, role_ids=[7])
+    assert _component_check_auth(interaction, {"11111"}, {42}) is False
+
+
+# ── fail-closed on missing role data ───────────────────────────────────────
+
+
+def test_component_check_role_policy_with_no_roles_attr_rejects():
+    """Role allowlist configured but interaction.user has no .roles
+    attribute (DM-context Member, raw User payload): must reject. A user
+    without resolvable roles cannot satisfy a role allowlist."""
+    interaction = _interaction(11111, drop_roles=True)
+    assert _component_check_auth(interaction, set(), {42}) is False
+
+
+def test_component_check_missing_user_with_allowlist_rejects():
+    """interaction.user is None with any allowlist configured: fail
+    closed without raising AttributeError."""
+    interaction = _interaction(0, drop_user=True)
+    assert _component_check_auth(interaction, {"11111"}, set()) is False
+    assert _component_check_auth(interaction, set(), {42}) is False
+
+
+# ---------------------------------------------------------------------------
+# View construction: every view must accept allowed_role_ids and route
+# through the shared helper. Default value preserves prior call-sites.
+# ---------------------------------------------------------------------------
+
+
+def test_exec_approval_view_accepts_role_allowlist():
+    view = ExecApprovalView(
+        session_key="sess-1",
+        allowed_user_ids={"11111"},
+        allowed_role_ids={42},
+    )
+    # Role-only user passes
+    assert view._check_auth(_interaction(99999, role_ids=[42])) is True
+    # Neither user nor role match: reject
+    assert view._check_auth(_interaction(99999, role_ids=[7])) is False
+
+
+def test_exec_approval_view_role_default_is_empty_set():
+    """Existing call sites that pass only allowed_user_ids must continue
+    working with the legacy semantics (no role gate)."""
+    view = ExecApprovalView(session_key="sess-1", allowed_user_ids={"11111"})
+    assert view.allowed_role_ids == set()
+    assert view._check_auth(_interaction(11111)) is True
+    assert view._check_auth(_interaction(99999)) is False
+
+
+def test_slash_confirm_view_accepts_role_allowlist():
+    view = SlashConfirmView(
+        session_key="sess-1",
+        confirm_id="c1",
+        allowed_user_ids=set(),
+        allowed_role_ids={42},
+    )
+    assert view._check_auth(_interaction(99999, role_ids=[42])) is True
+    assert view._check_auth(_interaction(99999, role_ids=[7])) is False
+
+
+def test_update_prompt_view_accepts_role_allowlist():
+    view = UpdatePromptView(
+        session_key="sess-1",
+        allowed_user_ids=set(),
+        allowed_role_ids={42},
+    )
+    assert view._check_auth(_interaction(99999, role_ids=[42])) is True
+    assert view._check_auth(_interaction(99999, role_ids=[7])) is False
+
+
+def test_model_picker_view_accepts_role_allowlist():
+    async def _noop(*_a, **_k):
+        return ""
+
+    view = ModelPickerView(
+        providers=[],
+        current_model="m",
+        current_provider="p",
+        session_key="sess-1",
+        on_model_selected=_noop,
+        allowed_user_ids=set(),
+        allowed_role_ids={42},
+    )
+    assert view._check_auth(_interaction(99999, role_ids=[42])) is True
+    assert view._check_auth(_interaction(99999, role_ids=[7])) is False
+
+
+# ---------------------------------------------------------------------------
+# Empty allowlists across views: legacy "allow everyone" must hold.
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+    "view_factory",
+    [
+        lambda: ExecApprovalView(session_key="s", allowed_user_ids=set()),
+        lambda: SlashConfirmView(session_key="s", confirm_id="c", allowed_user_ids=set()),
+        lambda: UpdatePromptView(session_key="s", allowed_user_ids=set()),
+    ],
+)
+def test_views_empty_allowlists_allow_everyone(view_factory):
+    view = view_factory()
+    assert view._check_auth(_interaction(99999)) is True
+
+
+def test_model_picker_view_empty_allowlists_allow_everyone():
+    async def _noop(*_a, **_k):
+        return ""
+
+    view = ModelPickerView(
+        providers=[],
+        current_model="m",
+        current_provider="p",
+        session_key="s",
+        on_model_selected=_noop,
+        allowed_user_ids=set(),
+    )
+    assert view.allowed_role_ids == set()
+    assert view._check_auth(_interaction(99999)) is True
--- a/tests/gateway/test_discord_slash_auth.py
+++ b/tests/gateway/test_discord_slash_auth.py
@@ -0,0 +1,737 @@
+"""Security regression tests: slash commands honor on_message authorization gates.
+
+Slash invocations (``_run_simple_slash``, ``_handle_thread_create_slash``)
+historically bypassed every gate ``on_message`` enforces — DISCORD_ALLOWED_USERS,
+DISCORD_ALLOWED_ROLES, DISCORD_ALLOWED_CHANNELS, DISCORD_IGNORED_CHANNELS.
+Any guild member could invoke ``/background``, ``/restart``, etc. as the
+operator. ``_check_slash_authorization`` mirrors all four gates one-for-one.
+
+These tests pin the security-correct behavior so the bypass cannot regress.
+"""
+
+import asyncio
+import logging
+import sys
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+
+from gateway.config import PlatformConfig
+
+
+# ---------------------------------------------------------------------------
+# Discord module mock — borrowed from test_discord_slash_commands.py so this
+# file runs on machines without discord.py installed.
+# ---------------------------------------------------------------------------
+
+
+def _ensure_discord_mock():
+    if "discord" in sys.modules and hasattr(sys.modules["discord"], "__file__"):
+        return  # real discord installed
+
+    if sys.modules.get("discord") is None:
+        discord_mod = MagicMock()
+        discord_mod.Intents.default.return_value = MagicMock()
+        discord_mod.DMChannel = type("DMChannel", (), {})
+        discord_mod.Thread = type("Thread", (), {})
+        discord_mod.ForumChannel = type("ForumChannel", (), {})
+        discord_mod.Interaction = object
+
+        class _FakePermissions:
+            def __init__(self, value=0, **_):
+                self.value = value
+
+        discord_mod.Permissions = _FakePermissions
+
+        class _FakeGroup:
+            def __init__(self, *, name, description, parent=None):
+                self.name = name
+                self.description = description
+                self.parent = parent
+                self._children: dict[str, object] = {}
+                if parent is not None:
+                    parent.add_command(self)
+
+            def add_command(self, cmd):
+                self._children[cmd.name] = cmd
+
+        class _FakeCommand:
+            def __init__(self, *, name, description, callback, parent=None):
+                self.name = name
+                self.description = description
+                self.callback = callback
+                self.parent = parent
+                self.default_permissions = None
+
+        discord_mod.app_commands = SimpleNamespace(
+            describe=lambda **kwargs: (lambda fn: fn),
+            choices=lambda **kwargs: (lambda fn: fn),
+            autocomplete=lambda **kwargs: (lambda fn: fn),
+            Choice=lambda **kwargs: SimpleNamespace(**kwargs),
+            Group=_FakeGroup,
+            Command=_FakeCommand,
+        )
+
+        ext_mod = MagicMock()
+        commands_mod = MagicMock()
+        commands_mod.Bot = MagicMock
+        ext_mod.commands = commands_mod
+
+        sys.modules["discord"] = discord_mod
+        sys.modules.setdefault("discord.ext", ext_mod)
+        sys.modules.setdefault("discord.ext.commands", commands_mod)
+
+
+_ensure_discord_mock()
+
+from gateway.platforms.discord import DiscordAdapter  # noqa: E402
+
+
+@pytest.fixture(autouse=True)
+def _isolate_discord_env(monkeypatch):
+    for var in (
+        "DISCORD_ALLOWED_USERS",
+        "DISCORD_ALLOWED_ROLES",
+        "DISCORD_ALLOWED_CHANNELS",
+        "DISCORD_IGNORED_CHANNELS",
+        "DISCORD_HIDE_SLASH_COMMANDS",
+        "DISCORD_ALLOW_BOTS",
+    ):
+        monkeypatch.delenv(var, raising=False)
+
+
+@pytest.fixture(autouse=True)
+def _stub_discord_permissions(monkeypatch):
+    """Pin discord.Permissions to a plain stand-in so tests can assert the
+    bitfield value regardless of whether real discord.py or a sibling test
+    module's MagicMock is loaded."""
+    import discord
+
+    class _Perm:
+        def __init__(self, value=0, **_):
+            self.value = value
+
+    monkeypatch.setattr(discord, "Permissions", _Perm)
+
+
+@pytest.fixture
+def adapter():
+    config = PlatformConfig(enabled=True, token="***")
+    a = DiscordAdapter(config)
+    a._client = SimpleNamespace(user=SimpleNamespace(id=99999, name="HermesBot"), guilds=[])
+    return a
+
+
+_SENTINEL = object()
+
+
+def _make_interaction(
+    user_id, *, channel_id=12345, guild_id=42, in_dm=False, in_thread=False,
+    parent_channel_id=None, user=_SENTINEL,
+):
+    """Build a mock Discord Interaction with a still-unresponded response.
+
+    ``channel_id`` may be set to ``None`` to simulate a guild interaction
+    payload missing a resolvable channel id (fail-closed exercise).
+    Pass ``user=None`` to simulate a payload missing the user object.
+    """
+    import discord
+
+    response = SimpleNamespace(send_message=AsyncMock(), defer=AsyncMock())
+
+    if in_dm:
+        channel = discord.DMChannel()
+    elif in_thread:
+        channel = discord.Thread()
+        channel.id = channel_id
+        channel.parent_id = parent_channel_id
+    elif channel_id is None:
+        channel = None
+    else:
+        channel = SimpleNamespace(id=channel_id)
+
+    if user is _SENTINEL:
+        user_obj = SimpleNamespace(id=int(user_id), name=f"user_{user_id}")
+    else:
+        user_obj = user
+
+    return SimpleNamespace(
+        user=user_obj,
+        guild=SimpleNamespace(owner_id=999),
+        guild_id=guild_id,
+        channel_id=channel_id,
+        channel=channel,
+        response=response,
+    )
+
+
+# ---------------------------------------------------------------------------
+# Backwards-compat: empty allowlist → everything passes (matches on_message)
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_no_allowlist_allows_everyone(adapter):
+    """SECURITY-CRITICAL backwards-compat: deployments without any allowlist
+    env vars set must see ZERO behavior change. on_message lets everyone
+    through in this case (returns True at line 1890); slash must do the same.
+    """
+    interaction = _make_interaction("999999999")
+    assert await adapter._check_slash_authorization(interaction, "/help") is True
+    interaction.response.send_message.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+async def test_no_allowlist_dm_also_allowed(adapter):
+    """Same for DMs — no allowlist means no restriction, matching on_message."""
+    interaction = _make_interaction("999999999", in_dm=True)
+    assert await adapter._check_slash_authorization(interaction, "/help") is True
+
+
+# ---------------------------------------------------------------------------
+# User allowlist (DISCORD_ALLOWED_USERS) parity
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_allowed_user_passes(adapter):
+    adapter._allowed_user_ids = {"100200300"}
+    interaction = _make_interaction("100200300")
+    assert await adapter._check_slash_authorization(interaction, "/background hi") is True
+    interaction.response.send_message.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+async def test_disallowed_user_rejected_with_ephemeral(adapter, caplog):
+    adapter._allowed_user_ids = {"100200300"}
+    interaction = _make_interaction("999999999")
+    with caplog.at_level(logging.WARNING):
+        assert await adapter._check_slash_authorization(interaction, "/background hi") is False
+    interaction.response.send_message.assert_awaited_once()
+    args, kwargs = interaction.response.send_message.call_args
+    assert kwargs.get("ephemeral") is True
+    assert "not authorized" in (args[0] if args else kwargs.get("content", "")).lower()
+    assert any("Unauthorized slash attempt" in r.message for r in caplog.records)
+    assert any("DISCORD_ALLOWED_USERS" in r.message for r in caplog.records)
+
+
+# ---------------------------------------------------------------------------
+# Role allowlist (DISCORD_ALLOWED_ROLES) parity
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_role_member_passes(adapter):
+    """A user whose Member.roles includes an allowed role passes the gate."""
+    adapter._allowed_role_ids = {1234}
+    interaction = _make_interaction("999999999")
+    interaction.user.roles = [SimpleNamespace(id=1234)]
+    assert await adapter._check_slash_authorization(interaction, "/help") is True
+
+
+@pytest.mark.asyncio
+async def test_role_non_member_rejected(adapter):
+    """A user without any matching role is rejected even if no user allowlist."""
+    adapter._allowed_role_ids = {1234}
+    interaction = _make_interaction("999999999")
+    interaction.user.roles = [SimpleNamespace(id=9999)]  # different role
+    assert await adapter._check_slash_authorization(interaction, "/help") is False
+
+
+# ---------------------------------------------------------------------------
+# Channel allowlist (DISCORD_ALLOWED_CHANNELS) parity — the gate prajer used
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_channel_not_in_allowlist_rejected(adapter, monkeypatch, caplog):
+    """on_message blocks messages in channels not in DISCORD_ALLOWED_CHANNELS;
+    slash must do the same. This is the EXACT bypass prajer exploited.
+    """
+    monkeypatch.setenv("DISCORD_ALLOWED_CHANNELS", "1111,2222")
+    interaction = _make_interaction("100200300", channel_id=9999)
+    with caplog.at_level(logging.WARNING):
+        assert await adapter._check_slash_authorization(interaction, "/background hi") is False
+    assert any("DISCORD_ALLOWED_CHANNELS" in r.message for r in caplog.records)
+
+
+@pytest.mark.asyncio
+async def test_channel_in_allowlist_passes(adapter, monkeypatch):
+    monkeypatch.setenv("DISCORD_ALLOWED_CHANNELS", "1111,2222")
+    interaction = _make_interaction("100200300", channel_id=1111)
+    assert await adapter._check_slash_authorization(interaction, "/help") is True
+
+
+@pytest.mark.asyncio
+async def test_channel_allowlist_wildcard_passes(adapter, monkeypatch):
+    """``*`` in DISCORD_ALLOWED_CHANNELS = allow any channel, matching on_message."""
+    monkeypatch.setenv("DISCORD_ALLOWED_CHANNELS", "*")
+    interaction = _make_interaction("100200300", channel_id=9999)
+    assert await adapter._check_slash_authorization(interaction, "/help") is True
+
+
+@pytest.mark.asyncio
+async def test_channel_allowlist_does_not_apply_to_dms(adapter, monkeypatch):
+    """DMs aren't channel-gated — they go through on_message's DM lockdown."""
+    monkeypatch.setenv("DISCORD_ALLOWED_CHANNELS", "1111")
+    interaction = _make_interaction("100200300", in_dm=True)
+    assert await adapter._check_slash_authorization(interaction, "/help") is True
+
+
+# ---------------------------------------------------------------------------
+# Channel blocklist (DISCORD_IGNORED_CHANNELS) parity
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_ignored_channel_rejected(adapter, monkeypatch, caplog):
+    monkeypatch.setenv("DISCORD_IGNORED_CHANNELS", "9999")
+    interaction = _make_interaction("100200300", channel_id=9999)
+    with caplog.at_level(logging.WARNING):
+        assert await adapter._check_slash_authorization(interaction, "/help") is False
+    assert any("DISCORD_IGNORED_CHANNELS" in r.message for r in caplog.records)
+
+
+@pytest.mark.asyncio
+async def test_ignored_channel_wildcard_blocks_all(adapter, monkeypatch):
+    monkeypatch.setenv("DISCORD_IGNORED_CHANNELS", "*")
+    interaction = _make_interaction("100200300", channel_id=9999)
+    assert await adapter._check_slash_authorization(interaction, "/help") is False
+
+
+# ---------------------------------------------------------------------------
+# Cross-platform admin notification
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_unauthorized_attempt_notifies_telegram(adapter):
+    from gateway.session import Platform
+
+    telegram_adapter = SimpleNamespace(send=AsyncMock())
+    home = SimpleNamespace(chat_id="987654321")
+    runner = SimpleNamespace(
+        adapters={Platform.TELEGRAM: telegram_adapter},
+        config=SimpleNamespace(get_home_channel=lambda p: home if p is Platform.TELEGRAM else None),
+    )
+    adapter.gateway_runner = runner
+    adapter._allowed_user_ids = {"100200300"}
+
+    interaction = _make_interaction("999999999")
+    await adapter._check_slash_authorization(interaction, "/background hi")
+
+    # Notify is fire-and-forget — let the scheduled task run.
+    await asyncio.sleep(0)
+    await asyncio.sleep(0)
+
+    telegram_adapter.send.assert_awaited_once()
+    chat_id, msg = telegram_adapter.send.call_args.args
+    assert chat_id == "987654321"
+    assert "Unauthorized" in msg
+    assert "999999999" in msg
+    assert "/background hi" in msg
+    assert "DISCORD_ALLOWED_USERS" in msg
+
+
+@pytest.mark.asyncio
+async def test_notify_silently_no_ops_without_runner(adapter):
+    adapter.gateway_runner = None
+    await adapter._notify_unauthorized_slash("u", "1", 2, 3, "/x", "reason")  # must not raise
+
+
+@pytest.mark.asyncio
+async def test_notify_falls_back_to_slack_if_no_telegram(adapter):
+    from gateway.session import Platform
+
+    slack_adapter = SimpleNamespace(send=AsyncMock())
+    home_slack = SimpleNamespace(chat_id="C12345")
+    runner = SimpleNamespace(
+        adapters={Platform.SLACK: slack_adapter},
+        config=SimpleNamespace(
+            get_home_channel=lambda p: home_slack if p is Platform.SLACK else None,
+        ),
+    )
+    adapter.gateway_runner = runner
+    await adapter._notify_unauthorized_slash("u", "1", 2, 3, "/x", "reason")
+    slack_adapter.send.assert_awaited_once()
+
+
+# ---------------------------------------------------------------------------
+# Opt-in visibility hide
+# ---------------------------------------------------------------------------
+
+
+def test_visibility_hide_off_by_default_is_noop(adapter, monkeypatch):
+    """DISCORD_HIDE_SLASH_COMMANDS unset → don't touch any command's permissions."""
+    cmd = SimpleNamespace(name="x", default_permissions="UNCHANGED")
+    tree = SimpleNamespace(get_commands=lambda: [cmd])
+
+    # Re-run the registration tail logic by calling the bit that decides:
+    # we don't have a clean way to simulate the env-gated branch from
+    # _register_slash_commands, so we just confirm the helper itself works
+    # AND assert the env-gating logic is correct.
+    assert os.environ.get("DISCORD_HIDE_SLASH_COMMANDS") is None
+    # Helper should still work when called directly:
+    adapter._apply_owner_only_visibility(tree)
+    # When called directly the helper applies — env gating is at the call site,
+    # which we exercise in an integration-style test below.
+
+
+def test_visibility_hide_helper_zeroes_perms(adapter):
+    cmd_a = SimpleNamespace(name="a", default_permissions=None)
+    cmd_b = SimpleNamespace(name="b", default_permissions=None)
+    tree = SimpleNamespace(get_commands=lambda: [cmd_a, cmd_b])
+    adapter._apply_owner_only_visibility(tree)
+    assert cmd_a.default_permissions is not None
+    assert cmd_b.default_permissions is not None
+    assert cmd_a.default_permissions.value == 0
+    assert cmd_b.default_permissions.value == 0
+
+
+def test_visibility_hide_tolerates_unsetable_command(adapter, caplog):
+    class _Frozen:
+        __slots__ = ("name",)
+        def __init__(self, name):
+            self.name = name
+
+    cmd_ok = SimpleNamespace(name="ok", default_permissions=None)
+    cmd_bad = _Frozen("bad")
+    tree = SimpleNamespace(get_commands=lambda: [cmd_bad, cmd_ok])
+
+    with caplog.at_level(logging.DEBUG):
+        adapter._apply_owner_only_visibility(tree)
+
+    assert cmd_ok.default_permissions.value == 0
+
+
+# os import for test_visibility_hide_off_by_default_is_noop
+import os  # noqa: E402
+
+
+# ---------------------------------------------------------------------------
+# Fail-closed parity on malformed slash auth context
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_missing_channel_id_rejected_when_channel_policy_configured(
+    adapter, monkeypatch,
+):
+    """A guild interaction without a resolvable channel id must fail
+    closed when DISCORD_ALLOWED_CHANNELS is configured. Without this
+    guard the entire channel-policy block silently fell through."""
+    monkeypatch.setenv("DISCORD_ALLOWED_CHANNELS", "1111,2222")
+    interaction = _make_interaction("100200300", channel_id=None)
+    assert await adapter._check_slash_authorization(interaction, "/help") is False
+    interaction.response.send_message.assert_awaited_once()
+
+
+@pytest.mark.asyncio
+async def test_missing_channel_id_allowed_when_no_channel_policy(adapter):
+    """No DISCORD_ALLOWED_CHANNELS configured + missing channel id: still
+    pass through the channel block (matches no-allowlist default)."""
+    interaction = _make_interaction("100200300", channel_id=None)
+    assert await adapter._check_slash_authorization(interaction, "/help") is True
+
+
+@pytest.mark.asyncio
+async def test_missing_user_rejected_when_allowlist_configured(adapter):
+    """interaction.user is None with a user/role allowlist active:
+    fail closed without raising AttributeError."""
+    adapter._allowed_user_ids = {"100200300"}
+    interaction = _make_interaction("100200300", user=None)
+    # Must not raise — must return False with an ephemeral rejection
+    assert await adapter._check_slash_authorization(interaction, "/help") is False
+    interaction.response.send_message.assert_awaited_once()
+
+
+@pytest.mark.asyncio
+async def test_missing_user_allowed_when_no_allowlist_configured(adapter):
+    """interaction.user is None but no allowlist configured: allow
+    (preserves no-allowlist back-compat -- anyone is allowed when no
+    policy is in effect)."""
+    interaction = _make_interaction("100200300", user=None)
+    assert await adapter._check_slash_authorization(interaction, "/help") is True
+
+
+# ---------------------------------------------------------------------------
+# Thread parent channel allowlist parity
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_thread_parent_in_allowlist_passes(adapter, monkeypatch):
+    """Thread whose parent channel is on DISCORD_ALLOWED_CHANNELS passes
+    even though the thread id itself isn't on the list."""
+    monkeypatch.setenv("DISCORD_ALLOWED_CHANNELS", "5555")
+    interaction = _make_interaction(
+        "100200300", channel_id=9999, in_thread=True, parent_channel_id=5555,
+    )
+    assert await adapter._check_slash_authorization(interaction, "/help") is True
+
+
+@pytest.mark.asyncio
+async def test_thread_parent_in_ignorelist_rejects(adapter, monkeypatch):
+    """Thread whose parent channel is on DISCORD_IGNORED_CHANNELS rejects
+    even when the thread id itself isn't ignored."""
+    monkeypatch.setenv("DISCORD_IGNORED_CHANNELS", "5555")
+    interaction = _make_interaction(
+        "100200300", channel_id=9999, in_thread=True, parent_channel_id=5555,
+    )
+    assert await adapter._check_slash_authorization(interaction, "/help") is False
+
+
+@pytest.mark.asyncio
+async def test_ignored_beats_allowed(adapter, monkeypatch):
+    """Channel listed in BOTH allowed and ignored: the ignored entry wins.
+    Anything else would be a foot-gun where adding to ignored does nothing
+    if the channel is also explicitly allowed."""
+    monkeypatch.setenv("DISCORD_ALLOWED_CHANNELS", "1111")
+    monkeypatch.setenv("DISCORD_IGNORED_CHANNELS", "1111")
+    interaction = _make_interaction("100200300", channel_id=1111)
+    assert await adapter._check_slash_authorization(interaction, "/help") is False
+
+
+# ---------------------------------------------------------------------------
+# Admin notify soft-fail fallback
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_notify_falls_back_to_slack_on_telegram_soft_fail(adapter):
+    """adapter.send returning SendResult(success=False) must NOT short-
+    circuit the fallback chain. Treating a soft failure as delivered
+    means a Telegram outage swallows alerts silently."""
+    from gateway.session import Platform
+
+    soft_fail = SimpleNamespace(success=False, error="rate limited")
+    telegram_adapter = SimpleNamespace(send=AsyncMock(return_value=soft_fail))
+    slack_adapter = SimpleNamespace(send=AsyncMock())
+    home_tg = SimpleNamespace(chat_id="987654321")
+    home_sl = SimpleNamespace(chat_id="C12345")
+    homes = {Platform.TELEGRAM: home_tg, Platform.SLACK: home_sl}
+    runner = SimpleNamespace(
+        adapters={
+            Platform.TELEGRAM: telegram_adapter,
+            Platform.SLACK: slack_adapter,
+        },
+        config=SimpleNamespace(get_home_channel=lambda p: homes.get(p)),
+    )
+    adapter.gateway_runner = runner
+
+    await adapter._notify_unauthorized_slash("u", "1", 2, 3, "/x", "reason")
+
+    telegram_adapter.send.assert_awaited_once()
+    slack_adapter.send.assert_awaited_once()
+
+
+@pytest.mark.asyncio
+async def test_notify_returns_on_telegram_truthy_success(adapter):
+    """adapter.send returning SendResult(success=True) -- or any object
+    without a falsy success attribute -- should still short-circuit at
+    Telegram. (This guards against the soft-fail patch over-correcting.)"""
+    from gateway.session import Platform
+
+    ok = SimpleNamespace(success=True, message_id="m1")
+    telegram_adapter = SimpleNamespace(send=AsyncMock(return_value=ok))
+    slack_adapter = SimpleNamespace(send=AsyncMock())
+    home_tg = SimpleNamespace(chat_id="987654321")
+    home_sl = SimpleNamespace(chat_id="C12345")
+    homes = {Platform.TELEGRAM: home_tg, Platform.SLACK: home_sl}
+    runner = SimpleNamespace(
+        adapters={
+            Platform.TELEGRAM: telegram_adapter,
+            Platform.SLACK: slack_adapter,
+        },
+        config=SimpleNamespace(get_home_channel=lambda p: homes.get(p)),
+    )
+    adapter.gateway_runner = runner
+
+    await adapter._notify_unauthorized_slash("u", "1", 2, 3, "/x", "reason")
+
+    telegram_adapter.send.assert_awaited_once()
+    slack_adapter.send.assert_not_awaited()
+
+
+# ---------------------------------------------------------------------------
+# /skill autocomplete + callback gating
+# ---------------------------------------------------------------------------
+
+
+def _capture_skill_registration(adapter, monkeypatch, entries):
+    """Run ``_register_skill_group`` against a stubbed skill catalog and
+    return ``(handler_callback, autocomplete_callback)``.
+
+    The autocomplete callback is captured by monkeypatching
+    ``discord.app_commands.autocomplete`` -- the production decorator is
+    a no-op stub in this test file's discord mock, so capturing the
+    callback through it is the direct route in tests.
+    """
+    import discord
+
+    captured: dict = {}
+
+    def fake_categories(reserved_names):
+        # Match discord_skill_commands_by_category's tuple shape:
+        # (categories_dict, uncategorized_list, hidden_count)
+        return ({}, list(entries), 0)
+
+    import hermes_cli.commands as _hc
+    monkeypatch.setattr(
+        _hc, "discord_skill_commands_by_category", fake_categories,
+    )
+
+    def capture_autocomplete(**kwargs):
+        # Only one autocomplete in /skill registration: name=...
+        captured["autocomplete"] = kwargs.get("name")
+
+        def _passthrough(fn):
+            return fn
+
+        return _passthrough
+
+    monkeypatch.setattr(
+        discord.app_commands, "autocomplete", capture_autocomplete,
+        raising=False,
+    )
+
+    registered: list = []
+
+    class _Tree:
+        def get_commands(self):
+            return []
+
+        def add_command(self, cmd):
+            registered.append(cmd)
+
+    adapter._register_skill_group(_Tree())
+    assert registered, "_register_skill_group did not register a command"
+    return registered[0].callback, captured["autocomplete"]
+
+
+@pytest.mark.asyncio
+async def test_skill_autocomplete_returns_empty_for_unauthorized(
+    adapter, monkeypatch,
+):
+    """Autocomplete must not leak the installed skill catalog to users
+    who can't run /skill. With DISCORD_ALLOWED_USERS configured and the
+    interaction user outside it, the autocomplete callback returns []."""
+    adapter._allowed_user_ids = {"100200300"}
+    entries = [
+        ("alpha", "First skill", "/alpha"),
+        ("beta", "Second skill", "/beta"),
+    ]
+    _handler, autocomplete = _capture_skill_registration(
+        adapter, monkeypatch, entries,
+    )
+
+    interaction = _make_interaction("999999999")
+    result = await autocomplete(interaction, "")
+    assert result == []
+
+
+@pytest.mark.asyncio
+async def test_skill_autocomplete_returns_choices_for_authorized(
+    adapter, monkeypatch,
+):
+    """Sanity: an authorized user still gets the autocomplete suggestions."""
+    adapter._allowed_user_ids = {"100200300"}
+    entries = [
+        ("alpha", "First skill", "/alpha"),
+        ("beta", "Second skill", "/beta"),
+    ]
+    _handler, autocomplete = _capture_skill_registration(
+        adapter, monkeypatch, entries,
+    )
+
+    interaction = _make_interaction("100200300")
+    result = await autocomplete(interaction, "")
+    assert len(result) == 2
+    assert {choice.value for choice in result} == {"alpha", "beta"}
+
+
+@pytest.mark.asyncio
+async def test_skill_handler_rejects_before_dispatch_for_unauthorized(
+    adapter, monkeypatch,
+):
+    """The /skill handler must call _check_slash_authorization BEFORE
+    skill_lookup. Otherwise unknown vs known names produce divergent
+    responses ("Unknown skill: foo" vs auth rejection) which is a
+    catalog-probing oracle."""
+    adapter._allowed_user_ids = {"100200300"}
+    entries = [("alpha", "First skill", "/alpha")]
+    handler, _autocomplete = _capture_skill_registration(
+        adapter, monkeypatch, entries,
+    )
+
+    # Patch _run_simple_slash so we can detect any leak through it.
+    dispatched: list = []
+
+    async def fake_dispatch(_interaction, text):
+        dispatched.append(text)
+
+    adapter._run_simple_slash = fake_dispatch  # type: ignore[assignment]
+
+    interaction = _make_interaction("999999999")
+    await handler(interaction, "alpha", "")
+
+    interaction.response.send_message.assert_awaited_once()
+    args, kwargs = interaction.response.send_message.call_args
+    assert kwargs.get("ephemeral") is True
+    assert "not authorized" in (
+        args[0] if args else kwargs.get("content", "")
+    ).lower()
+    # Critically: nothing was dispatched, and the auth message did NOT
+    # mention the skill name "alpha" (no catalog leak).
+    assert dispatched == []
+
+
+@pytest.mark.asyncio
+async def test_skill_handler_known_and_unknown_produce_same_rejection(
+    adapter, monkeypatch,
+):
+    """An unauthorized user probing for valid skill names must see the
+    same rejection text regardless of whether the name they tried is
+    on the registered catalog."""
+    adapter._allowed_user_ids = {"100200300"}
+    entries = [("alpha", "First skill", "/alpha")]
+    handler, _ = _capture_skill_registration(adapter, monkeypatch, entries)
+
+    adapter._run_simple_slash = AsyncMock()  # type: ignore[assignment]
+
+    known_interaction = _make_interaction("999999999")
+    unknown_interaction = _make_interaction("999999999")
+    await handler(known_interaction, "alpha", "")
+    await handler(unknown_interaction, "definitely-not-a-skill", "")
+
+    known_interaction.response.send_message.assert_awaited_once()
+    unknown_interaction.response.send_message.assert_awaited_once()
+    known_args, known_kwargs = known_interaction.response.send_message.call_args
+    unknown_args, unknown_kwargs = (
+        unknown_interaction.response.send_message.call_args
+    )
+    assert known_args == unknown_args
+    assert known_kwargs == unknown_kwargs
+
+
+@pytest.mark.asyncio
+async def test_skill_handler_dispatches_for_authorized(
+    adapter, monkeypatch,
+):
+    """Sanity: an authorized user reaches _run_simple_slash with the
+    resolved cmd_key and arguments."""
+    adapter._allowed_user_ids = {"100200300"}
+    entries = [("alpha", "First skill", "/alpha")]
+    handler, _ = _capture_skill_registration(adapter, monkeypatch, entries)
+
+    dispatched: list = []
+
+    async def fake_dispatch(_interaction, text):
+        dispatched.append(text)
+
+    adapter._run_simple_slash = fake_dispatch  # type: ignore[assignment]
+
+    interaction = _make_interaction("100200300")
+    await handler(interaction, "alpha", "extra args")
+    assert dispatched == ["/alpha extra args"]
--- a/tests/gateway/test_discord_slash_commands.py
+++ b/tests/gateway/test_discord_slash_commands.py
@@ -107,6 +107,10 @@ def adapter():
        user=SimpleNamespace(id=99999, name="HermesBot"),
    )
    adapter._text_batch_delay_seconds = 0  # disable batching for tests
+    # Slash auth is exercised in test_discord_slash_auth.py — bypass it here
+    # so registration / dispatch / thread behavior tests don't have to
+    # construct a full auth context (allowlist / channel scope).
+    adapter._check_slash_authorization = AsyncMock(return_value=True)
    return adapter


@@ -117,6 +121,10 @@ def adapter():

@pytest.mark.asyncio
 async def test_registers_native_thread_slash_command(adapter):
+    # The /thread slash closure now delegates ALL the work — including
+    # defer() — to _handle_thread_create_slash so the auth gate can send
+    # an ephemeral rejection on the still-unresponded interaction. The
+    # closure should just forward.
    adapter._handle_thread_create_slash = AsyncMock()
    adapter._register_slash_commands()

@@ -127,7 +135,9 @@ async def test_registers_native_thread_slash_command(adapter):

    await command(interaction, name="Planning", message="", auto_archive_duration=1440)

-    interaction.response.defer.assert_awaited_once_with(ephemeral=True)
+    # defer is now performed inside _handle_thread_create_slash, AFTER the
+    # auth check passes — not by the closure.
+    interaction.response.defer.assert_not_awaited()
    adapter._handle_thread_create_slash.assert_awaited_once_with(interaction, "Planning", "", 1440)


@@ -298,6 +308,7 @@ async def test_handle_thread_create_slash_reports_success(adapter):
        user=SimpleNamespace(display_name="Jezza", id=42),
        guild=SimpleNamespace(name="TestGuild"),
        followup=SimpleNamespace(send=AsyncMock()),
+        response=SimpleNamespace(defer=AsyncMock()),
    )

    await adapter._handle_thread_create_slash(interaction, "Planning", "Kickoff", 1440)
@@ -326,6 +337,7 @@ async def test_handle_thread_create_slash_dispatches_session_when_message_provid
        user=SimpleNamespace(display_name="Jezza", id=42),
        guild=SimpleNamespace(name="TestGuild"),
        followup=SimpleNamespace(send=AsyncMock()),
+        response=SimpleNamespace(defer=AsyncMock()),
    )

    adapter._dispatch_thread_session = AsyncMock()
@@ -348,6 +360,7 @@ async def test_handle_thread_create_slash_no_dispatch_without_message(adapter):
        user=SimpleNamespace(display_name="Jezza", id=42),
        guild=SimpleNamespace(name="TestGuild"),
        followup=SimpleNamespace(send=AsyncMock()),
+        response=SimpleNamespace(defer=AsyncMock()),
    )

    adapter._dispatch_thread_session = AsyncMock()
@@ -371,6 +384,7 @@ async def test_handle_thread_create_slash_falls_back_to_seed_message(adapter):
        user=SimpleNamespace(display_name="Jezza", id=42),
        guild=SimpleNamespace(name="TestGuild"),
        followup=SimpleNamespace(send=AsyncMock()),
+        response=SimpleNamespace(defer=AsyncMock()),
    )

    await adapter._handle_thread_create_slash(interaction, "Planning", "Kickoff", 1440)
@@ -395,6 +409,7 @@ async def test_handle_thread_create_slash_reports_failure(adapter):
        channel_id=123,
        user=SimpleNamespace(display_name="Jezza", id=42),
        followup=SimpleNamespace(send=AsyncMock()),
+        response=SimpleNamespace(defer=AsyncMock()),
    )

    await adapter._handle_thread_create_slash(interaction, "Planning", "", 1440)