diff --git a/slack-app-manifest.yaml b/slack-app-manifest.yaml index 40ecb8dd..67427d8a 100644 --- a/slack-app-manifest.yaml +++ b/slack-app-manifest.yaml @@ -33,6 +33,10 @@ oauth_config: - users:read.email pkce_enabled: false settings: + event_subscriptions: + bot_events: + - message.channels + - message.groups interactivity: is_enabled: true org_deploy_enabled: false diff --git a/src/firetower/slack_app/bolt.py b/src/firetower/slack_app/bolt.py index b0c9c699..d9466876 100644 --- a/src/firetower/slack_app/bolt.py +++ b/src/firetower/slack_app/bolt.py @@ -47,6 +47,7 @@ handle_statuspage_submission, ) from firetower.slack_app.handlers.subject import handle_subject_command +from firetower.slack_app.handlers.topic_guard import handle_channel_topic_change from firetower.slack_app.handlers.update_incident import ( handle_update_command, handle_update_incident_submission, @@ -108,6 +109,7 @@ def get_bolt_app() -> App: _bolt_app.command("/inc")(handle_command) _bolt_app.command("/testinc")(handle_command) _register_views(_bolt_app) + _register_event_handlers(_bolt_app) return _bolt_app @@ -213,6 +215,33 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: return decorator +def _with_event_metrics(event_type: str) -> Callable[..., Callable[..., Any]]: + """Wrap a Bolt event handler to emit submitted/completed/failed metrics. + + Uses a ``slack_app.events`` namespace so event volume does not pollute the + ``slack_app.views`` dashboards. + """ + + def decorator(func: Callable[..., Any]) -> Callable[..., Any]: + @wraps(func) + def wrapper(*args: Any, **kwargs: Any) -> Any: + close_old_connections() + tags = [f"event_type:{event_type}"] + statsd.increment("slack_app.events.submitted", tags=tags) + try: + result = func(*args, **kwargs) + statsd.increment("slack_app.events.completed", tags=tags) + return result + except Exception: + logger.exception("Event handler failed: %s", event_type) + statsd.increment("slack_app.events.failed", tags=tags) + raise + + return wrapper + + return decorator + + def _register_views(app: App) -> None: """Register view handlers (modals, etc.) on the Bolt app.""" app.view("backfill_incident_modal")( @@ -256,3 +285,23 @@ def _register_views(app: App) -> None: "affected_region_tags", ): app.options(action_id)(handle_tag_options) + + +def _ignore_message_event(ack: Any) -> None: + """No-op catch-all for message events. + + Subscribing to message.channels/message.groups opts the bot into the full + message firehose. Without a catch-all listener Bolt logs an "Unhandled + request" warning for every non-topic message. This listener is registered + after the channel_topic handler, so for topic changes the specific handler + runs first and returns before this one is reached. + """ + ack() + + +def _register_event_handlers(app: App) -> None: + """Register Slack event subscriptions on the Bolt app.""" + app.event({"type": "message", "subtype": "channel_topic"})( + _with_event_metrics("channel_topic")(handle_channel_topic_change) + ) + app.event("message")(_ignore_message_event) diff --git a/src/firetower/slack_app/handlers/topic_guard.py b/src/firetower/slack_app/handlers/topic_guard.py new file mode 100644 index 00000000..b1533250 --- /dev/null +++ b/src/firetower/slack_app/handlers/topic_guard.py @@ -0,0 +1,114 @@ +import logging +import threading +from collections import OrderedDict +from typing import Any + +from firetower.incidents.hooks import build_channel_topic +from firetower.integrations.services.slack import SlackService, escape_slack_text +from firetower.slack_app.handlers.utils import get_incident_from_channel + +logger = logging.getLogger(__name__) + +_slack_service = SlackService() + +_bot_user_id: str | None = None + +# Bounded set of recently handled channel_topic event ids, used to drop Slack +# redeliveries. Socket Mode strips the X-Slack-Retry-Num header (it lives on the +# envelope, not the event payload Bolt forwards), so we dedup on the event's own +# id/timestamp instead. This in-memory cache is per-process, so it only dedups +# correctly while the firetower-slack-app Cloud Run service stays single-instance +# (max_instance_count = 1); scaling it out would split the cache across replicas. +_RECENT_EVENT_CACHE_SIZE = 256 +_recent_event_ids: "OrderedDict[str, None]" = OrderedDict() +# Bolt dispatches event listeners on a thread pool, so guard the cache against +# concurrent check-then-set/evict races. +_recent_event_ids_lock = threading.Lock() + + +def _get_bot_user_id(client: Any) -> str | None: + """Return the bot's own Slack user id, caching it after the first lookup.""" + global _bot_user_id # noqa: PLW0603 + if _bot_user_id is None: + try: + _bot_user_id = client.auth_test()["user_id"] + except Exception: + logger.exception("auth_test failed while resolving bot user id") + return None + return _bot_user_id + + +def _event_id(event: dict) -> str | None: + return event.get("event_ts") or event.get("ts") + + +def _seen_recently(event_id: str) -> bool: + """Return True if this event id was already handled, recording it otherwise.""" + with _recent_event_ids_lock: + if event_id in _recent_event_ids: + return True + _recent_event_ids[event_id] = None + while len(_recent_event_ids) > _RECENT_EVENT_CACHE_SIZE: + _recent_event_ids.popitem(last=False) + return False + + +def _build_reset_message(attempted: str) -> str: + lines = [ + "Channel topics here are managed by Firetower and reflect the incident's " + "current status, so I reset it.", + ] + if attempted: + quoted = "\n".join( + f"> {escape_slack_text(line)}" for line in attempted.splitlines() + ) + lines.append(f"You tried to set:\n{quoted}") + lines.append( + "To change incident details (which update the topic automatically), use " + "`/ft subject `, `/ft severity <P0-P4>`, or `/ft captain`." + ) + return "\n\n".join(lines) + + +def handle_channel_topic_change(event: dict, client: Any) -> None: + """Reset manual incident-channel topic edits and nudge the editor. + + Slack emits a ``channel_topic`` message event whenever a channel's topic + changes, including when Firetower sets it. The bot-author guard prevents an + infinite reset loop on our own edits, and the event-id dedup guard prevents a + second reset and ephemeral when Slack redelivers the event. + """ + if event.get("user") == _get_bot_user_id(client): + return + + channel_id = event.get("channel") + if not channel_id: + return + + event_id = _event_id(event) + if event_id is not None and _seen_recently(event_id): + logger.info("Skipping duplicate channel_topic event for channel %s", channel_id) + return + + incident = get_incident_from_channel(channel_id) + if incident is None: + return + + canonical = build_channel_topic(incident) + attempted = event.get("topic", "") + + logger.info( + "Resetting manual topic change in incident channel %s (incident %s)", + channel_id, + incident.incident_number, + ) + _slack_service.set_channel_topic(channel_id, canonical) + + try: + client.chat_postEphemeral( + channel=channel_id, + user=event.get("user"), + text=_build_reset_message(attempted), + ) + except Exception: + logger.exception("Failed to post topic-reset ephemeral to %s", channel_id) diff --git a/src/firetower/slack_app/tests/test_topic_guard.py b/src/firetower/slack_app/tests/test_topic_guard.py new file mode 100644 index 00000000..b752a2dc --- /dev/null +++ b/src/firetower/slack_app/tests/test_topic_guard.py @@ -0,0 +1,177 @@ +from unittest.mock import MagicMock, patch + +import pytest + +from firetower.slack_app.handlers import topic_guard +from firetower.slack_app.handlers.topic_guard import handle_channel_topic_change + +CHANNEL_ID = "C_TEST_CHANNEL" +BOT_USER_ID = "U0000" +CANONICAL = "[P2] <https://ft/INC-1|INC-1 Test>" + + +@pytest.fixture(autouse=True) +def reset_module_state(): + topic_guard._bot_user_id = None + topic_guard._recent_event_ids.clear() + yield + topic_guard._bot_user_id = None + topic_guard._recent_event_ids.clear() + + +@pytest.fixture +def client(): + c = MagicMock() + c.auth_test.return_value = {"user_id": BOT_USER_ID} + return c + + +_TS_COUNTER = [0] + + +def _event(user="U_OTHER", topic="Something custom", channel=CHANNEL_ID, ts=None): + if ts is None: + _TS_COUNTER[0] += 1 + ts = f"1700000000.{_TS_COUNTER[0]:06d}" + return { + "type": "message", + "subtype": "channel_topic", + "channel": channel, + "user": user, + "topic": topic, + "event_ts": ts, + } + + +@patch("firetower.slack_app.handlers.topic_guard._slack_service") +@patch("firetower.slack_app.handlers.topic_guard.build_channel_topic") +@patch("firetower.slack_app.handlers.topic_guard.get_incident_from_channel") +def test_manual_change_resets_and_notifies( + mock_get_incident, mock_build_topic, mock_slack_service, client +): + mock_get_incident.return_value = MagicMock() + mock_build_topic.return_value = CANONICAL + + handle_channel_topic_change(_event(topic="My custom topic"), client) + + mock_slack_service.set_channel_topic.assert_called_once_with(CHANNEL_ID, CANONICAL) + client.chat_postEphemeral.assert_called_once() + kwargs = client.chat_postEphemeral.call_args[1] + assert kwargs["channel"] == CHANNEL_ID + assert kwargs["user"] == "U_OTHER" + assert "My custom topic" in kwargs["text"] + + +@patch("firetower.slack_app.handlers.topic_guard._slack_service") +@patch("firetower.slack_app.handlers.topic_guard.get_incident_from_channel") +def test_bot_authored_event_is_ignored(mock_get_incident, mock_slack_service, client): + handle_channel_topic_change(_event(user=BOT_USER_ID), client) + + mock_get_incident.assert_not_called() + mock_slack_service.set_channel_topic.assert_not_called() + client.chat_postEphemeral.assert_not_called() + + +@patch("firetower.slack_app.handlers.topic_guard._slack_service") +@patch("firetower.slack_app.handlers.topic_guard.get_incident_from_channel") +def test_non_incident_channel_is_noop(mock_get_incident, mock_slack_service, client): + mock_get_incident.return_value = None + + handle_channel_topic_change(_event(), client) + + mock_slack_service.set_channel_topic.assert_not_called() + client.chat_postEphemeral.assert_not_called() + + +@patch("firetower.slack_app.handlers.topic_guard._slack_service") +@patch("firetower.slack_app.handlers.topic_guard.get_incident_from_channel") +def test_missing_channel_is_noop(mock_get_incident, mock_slack_service, client): + event = _event() + del event["channel"] + + handle_channel_topic_change(event, client) + + mock_get_incident.assert_not_called() + mock_slack_service.set_channel_topic.assert_not_called() + client.chat_postEphemeral.assert_not_called() + + +@patch("firetower.slack_app.handlers.topic_guard._slack_service") +@patch("firetower.slack_app.handlers.topic_guard.build_channel_topic") +@patch("firetower.slack_app.handlers.topic_guard.get_incident_from_channel") +def test_attempted_topic_is_escaped( + mock_get_incident, mock_build_topic, mock_slack_service, client +): + mock_get_incident.return_value = MagicMock() + mock_build_topic.return_value = CANONICAL + + handle_channel_topic_change( + _event(topic="<!channel> see <https://evil|here> & <@U999>"), client + ) + + text = client.chat_postEphemeral.call_args[1]["text"] + assert "<!channel>" not in text + assert "<@U999>" not in text + assert "<https://evil|here>" not in text + assert "<!channel>" in text + assert "&" in text + + +@patch("firetower.slack_app.handlers.topic_guard._slack_service") +@patch("firetower.slack_app.handlers.topic_guard.build_channel_topic") +@patch("firetower.slack_app.handlers.topic_guard.get_incident_from_channel") +def test_ephemeral_failure_still_resets( + mock_get_incident, mock_build_topic, mock_slack_service, client +): + mock_get_incident.return_value = MagicMock() + mock_build_topic.return_value = CANONICAL + client.chat_postEphemeral.side_effect = Exception("boom") + + handle_channel_topic_change(_event(), client) + + mock_slack_service.set_channel_topic.assert_called_once_with(CHANNEL_ID, CANONICAL) + client.chat_postEphemeral.assert_called_once() + + +@patch("firetower.slack_app.handlers.topic_guard._slack_service") +@patch("firetower.slack_app.handlers.topic_guard.build_channel_topic") +@patch("firetower.slack_app.handlers.topic_guard.get_incident_from_channel") +def test_bot_id_cache_is_reused( + mock_get_incident, mock_build_topic, mock_slack_service, client +): + mock_get_incident.return_value = MagicMock() + mock_build_topic.return_value = CANONICAL + + handle_channel_topic_change(_event(), client) + handle_channel_topic_change(_event(), client) + + assert client.auth_test.call_count == 1 + + +@patch("firetower.slack_app.handlers.topic_guard._slack_service") +@patch("firetower.slack_app.handlers.topic_guard.get_incident_from_channel") +def test_auth_test_failure_does_not_crash( + mock_get_incident, mock_slack_service, client +): + client.auth_test.side_effect = Exception("transient") + + handle_channel_topic_change(_event(), client) + + mock_get_incident.assert_called_once() + + +@patch("firetower.slack_app.handlers.topic_guard._slack_service") +@patch("firetower.slack_app.handlers.topic_guard.build_channel_topic") +@patch("firetower.slack_app.handlers.topic_guard.get_incident_from_channel") +def test_duplicate_event_id_is_skipped( + mock_get_incident, mock_build_topic, mock_slack_service, client +): + mock_get_incident.return_value = MagicMock() + mock_build_topic.return_value = CANONICAL + event = _event(ts="1700000000.999999") + + handle_channel_topic_change(event, client) + handle_channel_topic_change(event, client) + + mock_slack_service.set_channel_topic.assert_called_once() + client.chat_postEphemeral.assert_called_once()