fix(slack): re-check gateway auth on approval and slash-confirm buttons

gateway/platforms/slack.py

@@ -2797,6 +2797,55 @@ class SlackAdapter(BasePlatformAdapter):
             logger.error("[Slack] send_slash_confirm failed: %s", e, exc_info=True)
             return SendResult(success=False, error=str(e))
 
+    def _is_interactive_user_authorized(
+        self,
+        user_id: str,
+        *,
+        channel_id: str = "",
+        user_name: Optional[str] = None,
+    ) -> bool:
+        """Return whether a Slack interactive caller may perform gated actions."""
+        normalized_user_id = str(user_id or "").strip()
+        if not normalized_user_id:
+            return False
+
+        runner = getattr(getattr(self, "_message_handler", None), "__self__", None)
+        auth_fn = getattr(runner, "_is_user_authorized", None)
+        if callable(auth_fn):
+            try:
+                from gateway.session import SessionSource
+
+                source = SessionSource(
+                    platform=Platform.SLACK,
+                    chat_id=str(channel_id or normalized_user_id),
+                    chat_type="dm" if str(channel_id or "").startswith("D") else "group",
+                    user_id=normalized_user_id,
+                    user_name=str(user_name).strip() if user_name else None,
+                )
+                return bool(auth_fn(source))
+            except Exception:
+                logger.debug(
+                    "[Slack] Falling back to env-only interactive auth for user %s",
+                    normalized_user_id,
+                    exc_info=True,
+                )
+
+        if os.getenv("SLACK_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}:
+            return True
+
+        allowed_ids = set()
+        platform_allowlist = os.getenv("SLACK_ALLOWED_USERS", "").strip()
+        if platform_allowlist:
+            allowed_ids.update(uid.strip() for uid in platform_allowlist.split(",") if uid.strip())
+        global_allowlist = os.getenv("GATEWAY_ALLOWED_USERS", "").strip()
+        if global_allowlist:
+            allowed_ids.update(uid.strip() for uid in global_allowlist.split(",") if uid.strip())
+
+        if allowed_ids:
+            return "*" in allowed_ids or normalized_user_id in allowed_ids
+
+        return os.getenv("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}
+
     async def _handle_slash_confirm_action(self, ack, body, action) -> None:
         """Handle a slash-confirm button click from Block Kit."""
         await ack()
@@ -2808,9 +2857,19 @@ class SlackAdapter(BasePlatformAdapter):
         channel_id = body.get("channel", {}).get("id", "")
         user_name = body.get("user", {}).get("name", "unknown")
         user_id = body.get("user", {}).get("id", "")
+        if not self._is_interactive_user_authorized(
+            user_id,
+            channel_id=channel_id,
+            user_name=user_name,
+        ):
+            logger.warning(
+                "[Slack] Unauthorized slash-confirm click by %s (%s) - ignoring",
+                user_name, user_id,
+            )
+            return
 
         # Authorization — reuse the exec-approval allowlist.
-        allowed_csv = os.getenv("SLACK_ALLOWED_USERS", "").strip()
+        allowed_csv = ""  # Interactive auth already ran above.
         if allowed_csv:
             allowed_ids = {uid.strip() for uid in allowed_csv.split(",") if uid.strip()}
             if "*" not in allowed_ids and user_id not in allowed_ids:
@@ -2917,10 +2976,21 @@ class SlackAdapter(BasePlatformAdapter):
         user_name = body.get("user", {}).get("name", "unknown")
         user_id = body.get("user", {}).get("id", "")
 
+        if not self._is_interactive_user_authorized(
+            user_id,
+            channel_id=channel_id,
+            user_name=user_name,
+        ):
+            logger.warning(
+                "[Slack] Unauthorized approval click by %s (%s) - ignoring",
+                user_name, user_id,
+            )
+            return
+
         # Only authorized users may click approval buttons.  Button clicks
         # bypass the normal message auth flow in gateway/run.py, so we must
         # check here as well.
-        allowed_csv = os.getenv("SLACK_ALLOWED_USERS", "").strip()
+        allowed_csv = ""  # Interactive auth already ran above.
         if allowed_csv:
             allowed_ids = {uid.strip() for uid in allowed_csv.split(",") if uid.strip()}
             if "*" not in allowed_ids and user_id not in allowed_ids:

tests/gateway/test_slack_approval_buttons.py

@@ -1,5 +1,6 @@
 """Tests for Slack Block Kit approval buttons and thread context fetching."""
 
+import asyncio
 import sys
 from pathlib import Path
 from unittest.mock import AsyncMock, MagicMock, patch
@@ -42,7 +43,7 @@ def _ensure_slack_mock():
 _ensure_slack_mock()
 
 from gateway.platforms.slack import SlackAdapter
-from gateway.config import PlatformConfig
+from gateway.config import PlatformConfig, Platform
 
 
 def _make_adapter():
@@ -57,6 +58,25 @@ def _make_adapter():
     return adapter
 
 
+class _AuthRunner:
+    def __init__(self, auth_fn=None):
+        self._auth_fn = auth_fn or (lambda _source: True)
+        self.seen_sources = []
+
+    async def handle(self, event):
+        return None
+
+    def _is_user_authorized(self, source):
+        self.seen_sources.append(source)
+        return self._auth_fn(source)
+
+
+def _attach_auth_runner(adapter, auth_fn=None):
+    runner = _AuthRunner(auth_fn=auth_fn)
+    adapter.set_message_handler(runner.handle)
+    return runner
+
+
 # ===========================================================================
 # send_exec_approval — Block Kit buttons
 # ===========================================================================
@@ -153,6 +173,7 @@ class TestSlackApprovalAction:
     @pytest.mark.asyncio
     async def test_resolves_approval(self):
         adapter = _make_adapter()
+        _attach_auth_runner(adapter)
         adapter._approval_resolved["1234.5678"] = False
 
         ack = AsyncMock()
@@ -165,7 +186,7 @@ class TestSlackApprovalAction:
                 ],
             },
             "channel": {"id": "C1"},
-            "user": {"name": "norbert"},
+            "user": {"name": "norbert", "id": "U_NORBERT"},
         }
         action = {
             "action_id": "hermes_approve_once",
@@ -189,13 +210,14 @@ class TestSlackApprovalAction:
     @pytest.mark.asyncio
     async def test_prevents_double_click(self):
         adapter = _make_adapter()
+        _attach_auth_runner(adapter)
         adapter._approval_resolved["1234.5678"] = True  # Already resolved
 
         ack = AsyncMock()
         body = {
             "message": {"ts": "1234.5678", "blocks": []},
             "channel": {"id": "C1"},
-            "user": {"name": "norbert"},
+            "user": {"name": "norbert", "id": "U_NORBERT"},
         }
         action = {
             "action_id": "hermes_approve_once",
@@ -212,6 +234,7 @@ class TestSlackApprovalAction:
     @pytest.mark.asyncio
     async def test_deny_action(self):
         adapter = _make_adapter()
+        _attach_auth_runner(adapter)
         adapter._approval_resolved["1.2"] = False
 
         ack = AsyncMock()
@@ -220,7 +243,7 @@ class TestSlackApprovalAction:
                 {"type": "section", "text": {"type": "mrkdwn", "text": "cmd"}},
             ]},
             "channel": {"id": "C1"},
-            "user": {"name": "alice"},
+            "user": {"name": "alice", "id": "U_ALICE"},
         }
         action = {"action_id": "hermes_deny", "value": "session-key"}
 
@@ -234,6 +257,95 @@ class TestSlackApprovalAction:
         update_kwargs = mock_client.chat_update.call_args[1]
         assert "Denied by alice" in update_kwargs["text"]
 
+    @pytest.mark.asyncio
+    async def test_global_allowlist_blocks_unauthorized_click(self, monkeypatch):
+        adapter = _make_adapter()
+        adapter._approval_resolved["1234.5678"] = False
+        monkeypatch.delenv("SLACK_ALLOWED_USERS", raising=False)
+        monkeypatch.delenv("SLACK_ALLOW_ALL_USERS", raising=False)
+        monkeypatch.delenv("GATEWAY_ALLOW_ALL_USERS", raising=False)
+        monkeypatch.setenv("GATEWAY_ALLOWED_USERS", "U_OWNER")
+
+        ack = AsyncMock()
+        body = {
+            "message": {"ts": "1234.5678", "blocks": []},
+            "channel": {"id": "C1"},
+            "user": {"name": "mallory", "id": "U_ATTACKER"},
+        }
+        action = {
+            "action_id": "hermes_approve_once",
+            "value": "agent:main:slack:group:C1:1111",
+        }
+
+        with patch("tools.approval.resolve_gateway_approval") as mock_resolve:
+            await adapter._handle_approval_action(ack, body, action)
+
+        ack.assert_called_once()
+        mock_resolve.assert_not_called()
+
+
+class TestSlackInteractiveAuth:
+    def test_delegates_to_gateway_runner_auth(self):
+        adapter = _make_adapter()
+        runner = _attach_auth_runner(adapter, auth_fn=lambda source: source.user_id == "U_OK")
+
+        assert adapter._is_interactive_user_authorized(
+            "U_OK",
+            channel_id="C1",
+            user_name="operator",
+        ) is True
+        assert adapter._is_interactive_user_authorized(
+            "U_BAD",
+            channel_id="C1",
+            user_name="intruder",
+        ) is False
+
+        assert len(runner.seen_sources) == 2
+        assert runner.seen_sources[0].platform == Platform.SLACK
+        assert runner.seen_sources[0].chat_id == "C1"
+        assert runner.seen_sources[0].chat_type == "group"
+
+
+class TestSlackSlashConfirmAction:
+    @pytest.mark.asyncio
+    async def test_global_allowlist_allows_authorized_click(self, monkeypatch):
+        adapter = _make_adapter()
+        mock_client = adapter._team_clients["T1"]
+        mock_client.chat_update = AsyncMock()
+        mock_client.chat_postMessage = AsyncMock()
+        monkeypatch.delenv("SLACK_ALLOWED_USERS", raising=False)
+        monkeypatch.delenv("SLACK_ALLOW_ALL_USERS", raising=False)
+        monkeypatch.delenv("GATEWAY_ALLOW_ALL_USERS", raising=False)
+        monkeypatch.setenv("GATEWAY_ALLOWED_USERS", "U_OWNER")
+
+        ack = AsyncMock()
+        body = {
+            "message": {
+                "ts": "2222.3333",
+                "blocks": [
+                    {"type": "section", "text": {"type": "mrkdwn", "text": "Original prompt"}},
+                ],
+            },
+            "channel": {"id": "C1"},
+            "user": {"name": "owner", "id": "U_OWNER"},
+        }
+        action = {
+            "action_id": "hermes_confirm_once",
+            "value": "agent:main:slack:group:C1:1111|confirm-1",
+        }
+
+        with patch("tools.slash_confirm.resolve", new=AsyncMock(return_value="follow-up")) as mock_resolve:
+            await adapter._handle_slash_confirm_action(ack, body, action)
+
+        ack.assert_called_once()
+        mock_resolve.assert_awaited_once_with(
+            "agent:main:slack:group:C1:1111",
+            "confirm-1",
+            "once",
+        )
+        mock_client.chat_update.assert_called_once()
+        mock_client.chat_postMessage.assert_called_once()
+
 
 # ===========================================================================
 # _fetch_thread_context