From 8f2eabc3b75393be1110b40726586b9eafe8e9a4 Mon Sep 17 00:00:00 2001 From: Tim Paine <3105306+timkpaine@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:18:37 -0400 Subject: [PATCH] Let an authenticated caller send messages over REST csp-bot already serves its channels over csp-gateway's REST API, but messages_out was read-only, so a job had no way to ask the bot to deliver a message. allow_send_messages, off by default, registers it as a send channel so POST /api/v1/send/messages_out works. Two things had to change for a posted message to arrive: The publish loop filtered the edge built locally from command responses, not the channel, so anything else feeding messages_out never reached a backend. It now reads the channel, which the gateway merges with the send adapter after every module has connected. This is not a cycle: subscribe() is a source and publish() a sink, so nothing flows back into what composes the channel. Verified by running the graph. _filter_messages_for_backend only looked at metadata["backend"], which is written by inbound tagging. A caller posting a message sets chatom's backend field instead, so the message was accepted and then silently dropped. The filter now falls back to that field, with metadata still taking precedence. This is the same class of bug as the one TestMetadataPropagation covers, fixed at the filter rather than at each producer. Off by default because it turns the bot into a relay for anything that can reach its API. The documentation pairs it with configuring an API key, since MountAPIKeyMiddleware only rejects requests once it has one. Verified against a Symphony pod: an authenticated POST carrying MessageML built by chatom arrived in the room with its headings, table, and bold intact. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com> --- csp_bot/bot.py | 25 +++++++++-- csp_bot/bot_config.py | 10 +++++ csp_bot/tests/test_bot_integration.py | 60 ++++++++++++++++++++++++++- docs/wiki/Advanced.md | 47 +++++++++++++++++++++ 4 files changed, 138 insertions(+), 4 deletions(-) diff --git a/csp_bot/bot.py b/csp_bot/bot.py index c17097d..4c7a39c 100644 --- a/csp_bot/bot.py +++ b/csp_bot/bot.py @@ -212,10 +212,22 @@ def connect(self, channels: GatewayChannels) -> None: secondary_commands = csp.unroll(response_outputs.commands) channels.set_channel(GatewayChannels.commands, secondary_commands) + if self.config.allow_send_messages: + # Lets a caller POST to /api/v1/send/messages_out. The gateway wires + # the send adapter into the channel after every module has + # connected, so the merged edge below picks those messages up too. + channels.add_send_channel(GatewayChannels.messages_out) + # Publish responses to adapters # chatom handles conversion to backend-specific formats + # + # Read the channel rather than the edge built above, so anything else + # feeding messages_out, including the send adapter, reaches a backend. + # This is not a cycle: subscribe() is a source and publish() a sink, so + # nothing here flows back into what composes the channel. + outbound = channels.get_channel(GatewayChannels.messages_out) for backend, adapter in self._adapters.items(): - backend_messages = self._filter_messages_for_backend(backend, messages_out) + backend_messages = self._filter_messages_for_backend(backend, outbound) adapter.publish(backend_messages) # Set up presence updates for Symphony @@ -253,10 +265,17 @@ def _tag_message_backend(self, backend: str, msg: ts[Message]) -> ts[Message]: @csp.node def _filter_messages_for_backend(self, backend: str, msg: ts[Message]) -> ts[Message]: - """Filter messages for a specific backend.""" + """Filter messages for a specific backend. + + Messages the bot routes internally are tagged in metadata, but + chatom's Message carries a `backend` field of its own, and that is what + a caller sending one over REST naturally sets. Either names the + destination, with metadata taking precedence since that is what the + inbound tagging writes. + """ if csp.ticked(msg): metadata = msg.metadata or {} - msg_backend = metadata.get("backend", "") + msg_backend = metadata.get("backend") or msg.backend or "" if msg_backend == backend: return msg diff --git a/csp_bot/bot_config.py b/csp_bot/bot_config.py index 943fa66..2a3badb 100644 --- a/csp_bot/bot_config.py +++ b/csp_bot/bot_config.py @@ -114,3 +114,13 @@ class BotConfig(BaseModel): default=1.0, description="Minimum seconds between message outputs.", ) + + allow_send_messages: bool = Field( + default=False, + description=( + "Expose messages_out as a send channel, so an authenticated caller can POST a message " + "to /api/v1/send/messages_out and have it delivered to the backend named on the message. " + "Off by default: it turns the bot into a relay for anything that can reach the API, so it " + "should be enabled deliberately and only with authentication configured." + ), + ) diff --git a/csp_bot/tests/test_bot_integration.py b/csp_bot/tests/test_bot_integration.py index 8ddf263..06f65bf 100644 --- a/csp_bot/tests/test_bot_integration.py +++ b/csp_bot/tests/test_bot_integration.py @@ -8,6 +8,7 @@ """ import asyncio +from datetime import UTC, datetime, timedelta from unittest.mock import MagicMock, patch import pytest @@ -100,7 +101,8 @@ class TestMetadataPropagation: Bug context: Commands returning Message objects directly (like trout/slap) were not being routed to backends because metadata["backend"] wasn't set. - The _filter_messages_for_backend node filters on metadata, not msg.backend. + _execute_command now sets it, and _filter_messages_for_backend also falls + back to msg.backend, so either names the destination. """ def test_execute_command_sets_metadata_on_message(self, bot_with_symphony, sample_bot_command): @@ -1076,3 +1078,59 @@ def test_discord_dm_message_uses_metadata_channel_id_without_channel(self, bot_w assert channel_id == "dm123" assert text == "!help" assert mentions == [] + + +class TestBackendFiltering: + """Which field names a message's destination.""" + + @staticmethod + def _routed_to(bot, backend, message): + """Run the filter node on one message and report whether it passed.""" + import csp + + @csp.graph + def g(): + edge = csp.unroll(csp.const([message])) + csp.add_graph_output("out", bot._filter_messages_for_backend(backend, edge)) + + result = csp.run(g, starttime=datetime.now(UTC), endtime=timedelta(seconds=1)) + return len(result.get("out", [])) == 1 + + def test_metadata_routes_a_message(self, bot_with_symphony): + message = Message(content="hi", metadata={"backend": "symphony"}) + + assert self._routed_to(bot_with_symphony, "symphony", message) + assert not self._routed_to(bot_with_symphony, "slack", message) + + def test_backend_field_routes_a_message(self, bot_with_symphony): + """A message sent over REST sets `backend`, not metadata. + + Before this fell back to the field, such a message was accepted by the + send endpoint and then silently dropped, reaching no backend at all. + """ + message = Message(content="hi", backend="symphony") + + assert self._routed_to(bot_with_symphony, "symphony", message) + assert not self._routed_to(bot_with_symphony, "slack", message) + + def test_metadata_wins_over_the_field(self, bot_with_symphony): + """Inbound tagging writes metadata, so it stays authoritative.""" + message = Message(content="hi", backend="slack", metadata={"backend": "symphony"}) + + assert self._routed_to(bot_with_symphony, "symphony", message) + assert not self._routed_to(bot_with_symphony, "slack", message) + + def test_a_message_naming_no_backend_goes_nowhere(self, bot_with_symphony): + message = Message(content="hi") + + assert not self._routed_to(bot_with_symphony, "symphony", message) + + +class TestSendChannelOptIn: + """messages_out is only sendable when the config asks for it.""" + + def test_send_is_off_by_default(self): + assert BotConfig().allow_send_messages is False + + def test_send_can_be_enabled(self): + assert BotConfig(allow_send_messages=True).allow_send_messages is True diff --git a/docs/wiki/Advanced.md b/docs/wiki/Advanced.md index d4e7f7d..c2895cf 100644 --- a/docs/wiki/Advanced.md +++ b/docs/wiki/Advanced.md @@ -9,3 +9,50 @@ More information coming soon! ## Running long-running commands in the background More information coming soon! + +## Sending messages over REST + +csp-bot runs on csp-gateway, which serves its channels over a REST API. With +`allow_send_messages` turned on, `messages_out` becomes a send channel, so a job +can post a message and have the bot deliver it: + +```yaml +# @package _global_ +modules: + bot: + config: + allow_send_messages: true +``` + +```bash +curl -X POST http://localhost:8000/api/v1/send/messages_out \ + -H 'Content-Type: application/json' \ + -H "token: $CSP_BOT_API_KEY" \ + -d '[{"content": "deploy finished", + "backend": "symphony", + "channel": {"id": "..."}}]' +``` + +The `backend` field names where the message goes; `metadata["backend"]` does the +same and takes precedence, since that is what inbound tagging writes. A message +naming no backend reaches none of them. + +Content is whatever the destination expects, so build it with chatom and render +it for the backend rather than writing markup by hand: + +```python +from chatom import Format, MessageBuilder + +body = MessageBuilder().heading("Deploy finished", level=3).build() +content = body.render(Format.SYMPHONY_MESSAGEML) +``` + +This is off by default. Turning it on makes the bot relay anything that reaches +its API, so configure authentication at the same time — `MountAPIKeyMiddleware` +rejects unauthenticated requests, but only once it has a key: + +```yaml +modules: + mount_api_key_middleware: + api_key: ${oc.env:CSP_BOT_API_KEY} +```