diff --git a/changelog/68105.fixed.md b/changelog/68105.fixed.md new file mode 100644 index 000000000000..5321617335bf --- /dev/null +++ b/changelog/68105.fixed.md @@ -0,0 +1 @@ +Fixed ``slack_bolt`` engine crashing with ``UnboundLocalError`` when a Slack workflow or other bot posts a message to a monitored channel. Bot messages (``subtype: bot_message``) carry ``bot_id`` and ``username`` instead of a ``user`` field, and these are now used as fallbacks so the engine continues running. diff --git a/salt/engines/slack_bolt_engine.py b/salt/engines/slack_bolt_engine.py index 7cd82ff877f9..1e164c53b989 100644 --- a/salt/engines/slack_bolt_engine.py +++ b/salt/engines/slack_bolt_engine.py @@ -584,6 +584,8 @@ def generate_triggered_messages( def just_data(m_data): """Always try to return the user and channel anyway""" + user_id = None + user_name = None if "user" not in m_data: if "message" in m_data and "user" in m_data["message"]: log.debug( @@ -595,6 +597,17 @@ def just_data(m_data): elif "comment" in m_data and "user" in m_data["comment"]: log.debug("Comment was added, so we look for user in the comment.") user_id = m_data["comment"]["user"] + elif m_data.get("subtype") == "bot_message": + # Workflows and other bot-posted messages do not carry a + # ``user`` field. Fall back to the bot identity so the + # message can still be processed instead of crashing the + # engine with UnboundLocalError. See issue #68105. + log.debug( + "Message was posted by a bot/workflow, " + "so we use the bot id and username." + ) + user_id = m_data.get("bot_id") + user_name = m_data.get("username") else: user_id = m_data.get("user") channel_id = m_data.get("channel") @@ -602,10 +615,12 @@ def just_data(m_data): channel_name = "private chat" else: channel_name = all_slack_channels.get(channel_id) + if user_name is None: + user_name = all_slack_users.get(user_id) data = { "message_data": m_data, "user_id": user_id, - "user_name": all_slack_users.get(user_id), + "user_name": user_name, "channel_name": channel_name, } if not data["user_name"]: diff --git a/tests/pytests/unit/engines/test_slack_bolt_engine.py b/tests/pytests/unit/engines/test_slack_bolt_engine.py index 1d2e30a045be..737c2894adb3 100644 --- a/tests/pytests/unit/engines/test_slack_bolt_engine.py +++ b/tests/pytests/unit/engines/test_slack_bolt_engine.py @@ -2,6 +2,8 @@ unit tests for the slack engine """ +from collections import deque + import pytest import salt.engines.slack_bolt_engine as slack_bolt_engine @@ -416,6 +418,80 @@ def test_run_commands_from_slack_async(slack_client): mock_event_send.asser_has_calls(event_send_calls) +def test_generate_triggered_messages_bot_message_68105(slack_client): + """ + Regression test for issue #68105. + + A Slack workflow that posts to a channel produces a message with + ``subtype: 'bot_message'`` and no ``user`` field (it carries ``bot_id`` + and ``username`` instead). Previously this raised + ``UnboundLocalError: local variable 'user_id' referenced before assignment`` + inside ``just_data`` and crashed the engine. The engine must now handle + these bot messages gracefully. + """ + + # A workflow-posted message: ``subtype: bot_message`` with no ``user`` + # field. The text does not start with the trigger string so the engine + # should simply yield it back without dispatching a command. + bot_msg = { + "subtype": "bot_message", + "text": "hello from a workflow", + "username": "Salt Run", + "type": "message", + "ts": "1750761076.971029", + "bot_id": "B092CEC63HV", + "app_id": "A092TF7N4R2", + "channel": "C02QY11UQ", + "event_ts": "1750761076.971029", + "channel_type": "group", + } + + slack_client.msg_queue = deque([bot_msg]) + + patch_run_until = patch.object( + slack_client, "_run_until", MagicMock(side_effect=[True, False]) + ) + patch_get_users = patch.object( + slack_client, + "get_slack_users", + MagicMock(return_value={"U02QY11UJ": "garethgreenaway"}), + ) + patch_get_channels = patch.object( + slack_client, + "get_slack_channels", + MagicMock(return_value={"C02QY11UQ": "general"}), + ) + patch_get_config_groups = patch.object( + slack_client, + "get_config_groups", + MagicMock(return_value={}), + ) + + with patch_run_until, patch_get_users, patch_get_channels, patch_get_config_groups: + # Consume the generator. The bug raised UnboundLocalError inside + # just_data, propagating out of the generator on the first ``next``. + results = list( + slack_client.generate_triggered_messages( + token="xoxb-test", + trigger_string="!", + groups={}, + groups_pillar_name="", + ) + ) + + # The generator must complete without raising. The bot message is + # yielded back as ``data`` (non-trigger path), then the final + # ``done`` sentinel. + assert any(item.get("done") for item in results) + yielded = [item for item in results if not item.get("done")] + assert len(yielded) == 1 + # The bot identity is carried through in place of ``user_id`` / + # ``user_name`` so downstream code has something to log. + assert yielded[0]["user_id"] == "B092CEC63HV" + assert yielded[0]["user_name"] == "Salt Run" + assert yielded[0]["message_data"] is bot_msg + + def test_run_command_async(slack_client): """ Test slack engine: test_run_command_async