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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 47 additions & 24 deletions gateway/platforms/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ def __init__(self, config: PlatformConfig):
self._socket_watchdog_task: Optional[asyncio.Task] = None
self._socket_reconnect_lock = asyncio.Lock()
self._socket_watchdog_interval_s = 15.0
self.gateway_runner = None

def _start_socket_mode_handler(self) -> None:
"""Start the Slack Socket Mode background task."""
Expand Down Expand Up @@ -2809,17 +2810,13 @@ async def _handle_slash_confirm_action(self, ack, body, action) -> None:
user_name = body.get("user", {}).get("name", "unknown")
user_id = body.get("user", {}).get("id", "")

# Authorization — reuse the exec-approval allowlist.
allowed_csv = os.getenv("SLACK_ALLOWED_USERS", "").strip()
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:
logger.warning(
"[Slack] Unauthorized slash-confirm click by %s (%s) — ignoring",
user_name,
user_id,
)
return
if not self._is_action_user_authorized(user_id):
logger.warning(
"[Slack] Unauthorized slash-confirm click by %s (%s) — ignoring",
user_name,
user_id,
)
return

# Parse session_key|confirm_id back out
if "|" not in value:
Expand Down Expand Up @@ -2917,19 +2914,15 @@ async def _handle_approval_action(self, ack, body, action) -> None:
user_name = body.get("user", {}).get("name", "unknown")
user_id = body.get("user", {}).get("id", "")

# 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()
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:
logger.warning(
"[Slack] Unauthorized approval click by %s (%s) — ignoring",
user_name,
user_id,
)
return
# Button clicks bypass the normal message auth flow in gateway/run.py,
# so reuse both env allowlists and the pairing store here.
if not self._is_action_user_authorized(user_id):
logger.warning(
"[Slack] Unauthorized approval click by %s (%s) — ignoring",
user_name,
user_id,
)
return

# Map action_id to approval choice
choice_map = {
Expand Down Expand Up @@ -3005,6 +2998,36 @@ async def _handle_approval_action(self, ack, body, action) -> None:

# (approval state already consumed by atomic pop above)

def _is_action_user_authorized(self, user_id: str) -> bool:
"""Return whether a Slack action button click may affect agent state."""
if not user_id:
return False

allowed_ids = set()
for env_name in ("SLACK_ALLOWED_USERS", "GATEWAY_ALLOWED_USERS"):
raw = os.getenv(env_name, "").strip()
if raw:
allowed_ids.update(uid.strip() for uid in raw.split(",") if uid.strip())

if "*" in allowed_ids or user_id in allowed_ids:
return True

runner = getattr(self, "gateway_runner", None)
pairing_store = getattr(runner, "pairing_store", None) if runner else None
if pairing_store:
try:
if pairing_store.is_approved("slack", user_id):
return True
except Exception:
logger.debug(
"[Slack] Pairing-store auth check failed for %s",
user_id,
exc_info=True,
)

# Preserve previous behavior for setups with no explicit allowlists.
return not allowed_ids

# ----- Thread context fetching -----

async def _fetch_thread_context(
Expand Down
28 changes: 25 additions & 3 deletions gateway/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,19 @@ def _platform_config_key(platform: "Platform") -> str:
return "cli" if platform == Platform.LOCAL else platform.value


def _platform_value(platform: Any) -> str:
"""Return the runtime platform value for enums and enum-like test doubles."""
return str(getattr(platform, "value", platform) or "")


def _is_slack_platform(platform: Any) -> bool:
return _platform_value(platform) == Platform.SLACK.value


def _want_gateway_status_messages(platform: Any) -> bool:
return not _is_slack_platform(platform)


def _teams_pipeline_plugin_enabled() -> bool:
"""Return True when the standalone Teams pipeline plugin is enabled."""
config = _load_gateway_config()
Expand Down Expand Up @@ -3415,6 +3428,12 @@ async def _handle_active_session_busy_message(self, event: MessageEvent, session
logger.debug("Busy ack suppressed for session %s", session_key)
return True # input still processed, just no ack sent

# Slack channels are shared operational surfaces. Keep interrupt/queue
# handling, but avoid posting lifecycle acks into the channel.
if _is_slack_platform(event.source.platform):
logger.debug("Busy ack suppressed for Slack session %s", session_key)
return True

# Debounce: only send an acknowledgment once every 30 seconds per session
# to avoid spamming the user when they send multiple messages quickly
_BUSY_ACK_COOLDOWN = 30
Expand Down Expand Up @@ -6692,7 +6711,9 @@ def _create_adapter(
if not check_slack_requirements():
logger.warning("Slack: slack-bolt not installed. Run: pip install 'hermes-agent[slack]'")
return None
return SlackAdapter(config)
adapter = SlackAdapter(config)
adapter.gateway_runner = self
return adapter

elif platform == Platform.SIGNAL:
from gateway.platforms.signal import SignalAdapter, check_signal_requirements
Expand Down Expand Up @@ -17251,6 +17272,7 @@ def _step_callback_sync(iteration: int, prev_tools: list) -> None:
# Bridge sync status_callback → async adapter.send for context pressure
_status_adapter = self.adapters.get(source.platform)
_status_chat_id = source.chat_id
_should_send_gateway_status = _want_gateway_status_messages(source.platform)
if source.platform == Platform.FEISHU and source.thread_id and event_message_id:
# Feishu topics only keep messages inside the topic when they are
# sent via the reply API with reply_in_thread=true. Status/interim,
Expand All @@ -17264,7 +17286,7 @@ def _step_callback_sync(iteration: int, prev_tools: list) -> None:
_status_thread_metadata = self._thread_metadata_for_source(source, event_message_id) if _progress_thread_id else None

def _status_callback_sync(event_type: str, message: str) -> None:
if not _status_adapter or not _run_still_current():
if not _should_send_gateway_status or not _status_adapter or not _run_still_current():
return
prepared_message = _prepare_gateway_status_message(
source.platform,
Expand Down Expand Up @@ -17539,7 +17561,7 @@ def _interim_assistant_cb(text: str, *, already_streamed: bool = False) -> None:
agent.step_callback = _step_callback_sync if _hooks_ref.loaded_hooks else None
agent.stream_delta_callback = _stream_delta_cb
agent.interim_assistant_callback = _interim_assistant_cb if _want_interim_messages else None
agent.status_callback = _status_callback_sync
agent.status_callback = _status_callback_sync if _should_send_gateway_status else None
agent.reasoning_config = reasoning_config
agent.service_tier = self._service_tier
agent.request_overrides = turn_route.get("request_overrides") or {}
Expand Down
31 changes: 31 additions & 0 deletions tests/gateway/test_busy_session_ack.py
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,37 @@ async def test_telegram_omits_status_detail_by_default(self):
assert "terminal" not in content
assert "10 min" not in content

@pytest.mark.asyncio
async def test_slack_suppresses_busy_ack_but_still_interrupts(self):
"""Slack channels should not receive lifecycle ack noise."""
runner, sentinel = _make_runner()
runner._busy_input_mode = "interrupt"
adapter = _make_adapter(platform_val="slack")

event = _make_event(text="new instruction", chat_id="C1", platform_val="slack")
sk = build_session_key(event.source)

agent = MagicMock()
runner._running_agents[sk] = agent
runner._running_agents_ts[sk] = time.time() - 600
runner.adapters[event.source.platform] = adapter

result = await runner._handle_active_session_busy_message(event, sk)

assert result is True
agent.interrupt.assert_called_once_with("new instruction")
adapter._send_with_retry.assert_not_called()
assert sk not in runner._busy_ack_ts

def test_slack_disables_gateway_status_messages(self):
"""Agent lifecycle/status callbacks should stay out of Slack channels."""
from gateway.config import Platform
from gateway.run import _want_gateway_status_messages

assert _want_gateway_status_messages(Platform.SLACK) is False
assert _want_gateway_status_messages(MagicMock(value="slack")) is False
assert _want_gateway_status_messages(Platform.TELEGRAM) is True

@pytest.mark.asyncio
async def test_draining_still_works(self):
"""Draining case should still produce the drain-specific message."""
Expand Down
95 changes: 92 additions & 3 deletions tests/gateway/test_slack_approval_buttons.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import sys
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock, patch

import pytest
Expand Down Expand Up @@ -165,7 +166,7 @@ async def test_resolves_approval(self):
],
},
"channel": {"id": "C1"},
"user": {"name": "norbert"},
"user": {"name": "norbert", "id": "U_NORBERT"},
}
action = {
"action_id": "hermes_approve_once",
Expand Down Expand Up @@ -195,7 +196,7 @@ async def test_prevents_double_click(self):
body = {
"message": {"ts": "1234.5678", "blocks": []},
"channel": {"id": "C1"},
"user": {"name": "norbert"},
"user": {"name": "norbert", "id": "U_NORBERT"},
}
action = {
"action_id": "hermes_approve_once",
Expand All @@ -220,7 +221,7 @@ async def test_deny_action(self):
{"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"}

Expand All @@ -235,6 +236,94 @@ async def test_deny_action(self):
assert "Denied by alice" in update_kwargs["text"]


class TestSlackActionAuthorization:
"""Button clicks must match normal Slack message authorization."""

def _approval_body(self, *, user_id="U_AUTH", user_name="authorized", ts="1.2"):
return {
"message": {
"ts": ts,
"blocks": [
{"type": "section", "text": {"type": "mrkdwn", "text": "cmd"}},
],
},
"channel": {"id": "C1"},
"user": {"name": user_name, "id": user_id},
}

@pytest.mark.asyncio
async def test_gateway_allowlist_can_authorize_button_clicks(self, monkeypatch):
adapter = _make_adapter()
adapter._approval_resolved["1.2"] = False
adapter._team_clients["T1"].chat_update = AsyncMock()
monkeypatch.setenv("GATEWAY_ALLOWED_USERS", "U_AUTH")

with patch("tools.approval.resolve_gateway_approval", return_value=1) as mock_resolve:
await adapter._handle_approval_action(
AsyncMock(),
self._approval_body(user_id="U_AUTH"),
{"action_id": "hermes_approve_once", "value": "session-key"},
)

mock_resolve.assert_called_once_with("session-key", "once")

@pytest.mark.asyncio
async def test_pairing_store_can_authorize_button_clicks(self, monkeypatch):
adapter = _make_adapter()
adapter._approval_resolved["1.2"] = False
adapter._team_clients["T1"].chat_update = AsyncMock()
pairing_store = SimpleNamespace(is_approved=MagicMock(return_value=True))
adapter.gateway_runner = SimpleNamespace(pairing_store=pairing_store)
monkeypatch.setenv("GATEWAY_ALLOWED_USERS", "U_OTHER")

with patch("tools.approval.resolve_gateway_approval", return_value=1) as mock_resolve:
await adapter._handle_approval_action(
AsyncMock(),
self._approval_body(user_id="U_PAIRED"),
{"action_id": "hermes_deny", "value": "session-key"},
)

pairing_store.is_approved.assert_called_once_with("slack", "U_PAIRED")
mock_resolve.assert_called_once_with("session-key", "deny")

@pytest.mark.asyncio
async def test_unauthorized_button_click_is_ignored(self, monkeypatch):
adapter = _make_adapter()
adapter._approval_resolved["1.2"] = False
adapter._team_clients["T1"].chat_update = AsyncMock()
monkeypatch.setenv("SLACK_ALLOWED_USERS", "U_OWNER")

with patch("tools.approval.resolve_gateway_approval") as mock_resolve:
await adapter._handle_approval_action(
AsyncMock(),
self._approval_body(user_id="U_INTRUDER"),
{"action_id": "hermes_approve_once", "value": "session-key"},
)

mock_resolve.assert_not_called()
adapter._team_clients["T1"].chat_update.assert_not_called()


def test_gateway_runner_attaches_pairing_store_to_slack_adapter():
from gateway.config import Platform
from gateway.run import GatewayRunner

runner = object.__new__(GatewayRunner)
runner.config = SimpleNamespace(
group_sessions_per_user=False,
thread_sessions_per_user=False,
)

adapter = GatewayRunner._create_adapter(
runner,
Platform.SLACK,
PlatformConfig(enabled=True, token="xoxb-test-token"),
)

assert adapter is not None
assert adapter.gateway_runner is runner


# ===========================================================================
# _fetch_thread_context
# ===========================================================================
Expand Down