diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index e4f6687f4207..2be9668576ff 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -67,6 +67,15 @@ except ImportError: # pragma: no cover - plugin loaded outside package context logger = logging.getLogger(__name__) +# User-Agent prefix for outbound Slack API calls so platform partners can +# identify HermesAgent traffic — matching other Hermes outbound surfaces +# that already set ``HermesAgent/`` for platform-partner attribution. +try: + from hermes_cli import __version__ as _HERMES_VERSION +except Exception: + _HERMES_VERSION = "unknown" +_HERMES_SLACK_USER_AGENT_PREFIX = f"HermesAgent/{_HERMES_VERSION}" + _SLACK_ERROR_BODY_LIMIT_BYTES = 8 * 1024 @@ -1879,12 +1888,19 @@ class SlackAdapter(BasePlatformAdapter): # First token is the primary — used for AsyncApp / Socket Mode primary_token = bot_tokens[0] - self._app = AsyncApp(token=primary_token) + primary_client = AsyncWebClient( + token=primary_token, + user_agent_prefix=_HERMES_SLACK_USER_AGENT_PREFIX, + ) + self._app = AsyncApp(token=primary_token, client=primary_client) _apply_slack_proxy(self._app.client, proxy_url) # Register each bot token and map team_id → client for token in bot_tokens: - client = AsyncWebClient(token=token) + client = AsyncWebClient( + token=token, + user_agent_prefix=_HERMES_SLACK_USER_AGENT_PREFIX, + ) _apply_slack_proxy(client, proxy_url) auth_response = await client.auth_test() team_id = auth_response.get("team_id", "") diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index 773893c3d2c3..7ea92de0858d 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -1298,7 +1298,8 @@ class TestSlackProxyBehavior: created_clients = [] class FakeWebClient: - def __init__(self, token): + # **_kwargs absorbs adapter kwargs we don't model here (e.g. user_agent_prefix). + def __init__(self, token, **_kwargs): self.token = token self.proxy = "constructor-default" suffix = token.split("-")[-1] @@ -1313,9 +1314,13 @@ class TestSlackProxyBehavior: created_clients.append(self) class FakeApp: - def __init__(self, token): + # **_kwargs absorbs adapter kwargs we don't model here. + def __init__(self, token, client=None, **_kwargs): self.token = token - self.client = FakeWebClient(token) + # Honor the ``client=`` kwarg the production adapter passes + # (so the User-Agent prefix sticks on ``self._app.client``). + # Fall back to building our own fake client when not provided. + self.client = client if client is not None else FakeWebClient(token) self.registered_events = [] self.registered_commands = [] self.registered_actions = [] @@ -1407,7 +1412,8 @@ class TestSlackProxyBehavior: created_clients = [] class FakeWebClient: - def __init__(self, token): + # **_kwargs absorbs adapter kwargs we don't model here (e.g. user_agent_prefix). + def __init__(self, token, **_kwargs): self.token = token self.proxy = "constructor-default" suffix = token.split("-")[-1] @@ -1422,9 +1428,13 @@ class TestSlackProxyBehavior: created_clients.append(self) class FakeApp: - def __init__(self, token): + # **_kwargs absorbs adapter kwargs we don't model here. + def __init__(self, token, client=None, **_kwargs): self.token = token - self.client = FakeWebClient(token) + # Honor the ``client=`` kwarg the production adapter passes + # (so the User-Agent prefix sticks on ``self._app.client``). + # Fall back to building our own fake client when not provided. + self.client = client if client is not None else FakeWebClient(token) self.registered_events = [] self.registered_commands = [] self.registered_actions = [] @@ -8735,3 +8745,95 @@ class TestFormatMessageTableIntegration: ln for ln in out.split("\n") if ln.startswith("```") ) assert first_fence_line == "```" + +# TestSlackUserAgent +# --------------------------------------------------------------------------- + + +class TestSlackUserAgent: + """Pin the User-Agent attribution wired in connect(). + + Slack platform partners (analytics, abuse-detection, etc.) attribute + outbound API traffic by ``User-Agent``. The Slack adapter sets + ``user_agent_prefix=_HERMES_SLACK_USER_AGENT_PREFIX`` on every + ``AsyncWebClient`` it builds and threads the primary client into + ``AsyncApp(client=...)`` so the prefix sticks on the app-owned client too. + Pin both behaviors at the actual call sites — a future refactor that + drops either kwarg would silently break attribution otherwise. + """ + + def test_hermes_slack_user_agent_prefix_format(self): + """Module constant matches the HermesAgent/ convention used + elsewhere in the codebase for platform-partner attribution.""" + assert _slack_mod._HERMES_SLACK_USER_AGENT_PREFIX.startswith("HermesAgent/") + + @pytest.mark.asyncio + async def test_async_web_client_constructed_with_hermes_user_agent_prefix(self): + """Every AsyncWebClient built by ``connect()`` carries the prefix, and + ``AsyncApp`` receives a pre-built ``client=`` so the prefix sticks.""" + # Multi-token config exercises both construction sites: + # the primary AsyncApp client AND the per-token loop. + config = PlatformConfig( + enabled=True, token="xoxb-fake-1,xoxb-fake-2" + ) + adapter = SlackAdapter(config) + + mock_app = MagicMock() + mock_app.event = lambda *a, **kw: (lambda fn: fn) + mock_app.command = lambda *a, **kw: (lambda fn: fn) + mock_app.client = AsyncMock() + + mock_web_client = MagicMock() + mock_web_client.auth_test = AsyncMock( + return_value={ + "user_id": "U_BOT", + "user": "testbot", + "team_id": "T_FAKE", + "team": "FakeTeam", + } + ) + + socket_mode_handler = MagicMock() + socket_mode_handler.start_async = AsyncMock(return_value=None) + + with ( + patch.object(_slack_mod, "AsyncApp", return_value=mock_app) as async_app_mock, + patch.object( + _slack_mod, "AsyncWebClient", return_value=mock_web_client + ) as web_client_mock, + patch.object( + _slack_mod, + "AsyncSocketModeHandler", + return_value=socket_mode_handler, + ), + patch.dict(os.environ, {"SLACK_APP_TOKEN": "xapp-fake"}), + patch( + "gateway.status.acquire_scoped_lock", return_value=(True, None) + ), + patch("asyncio.create_task", side_effect=_fake_create_task), + ): + await adapter.connect() + + expected_prefix = _slack_mod._HERMES_SLACK_USER_AGENT_PREFIX + + # AsyncWebClient must be constructed at least once (primary) and + # every construction must pass user_agent_prefix. + assert web_client_mock.call_count >= 1, ( + "AsyncWebClient was never constructed during connect()" + ) + for idx, call_args in enumerate(web_client_mock.call_args_list): + assert call_args.kwargs.get("user_agent_prefix") == expected_prefix, ( + f"AsyncWebClient call #{idx} missing " + f"user_agent_prefix={expected_prefix!r}: {call_args}" + ) + + # AsyncApp must be wired with the pre-built primary client. Without + # the ``client=`` kwarg, the bolt SDK would build its own client and + # the User-Agent prefix would not stick on ``self._app.client``, + # which the rest of the adapter uses for app-scoped API calls. + async_app_kwargs = async_app_mock.call_args.kwargs + assert "client" in async_app_kwargs, ( + "AsyncApp must receive a pre-built client= so the " + "user_agent_prefix sticks on the app-owned client; got " + f"kwargs={async_app_kwargs}" + )