feat: add ntfy platform adapter#4043
Conversation
| ), | ||
| "ntfy": ( | ||
| "You are communicating via ntfy push notifications. Use plain text only — " | ||
| "no markdown formatting. Keep responses concise; ntfy messages are push " |
There was a problem hiding this comment.
It would be nice to actually make the use of markdown syntax as a configurable option, since ntfy actually supports it.
Anyway, thanks for your contribution!
There was a problem hiding this comment.
thanks for the feedback! I've updated the PR to make markdown a configurable option. Set markdown: true under extra: in config.yaml and the adapter will send X-Markdown: true header on outbound messages. Defaults to false for backward compatibility.
Implement full ntfy messaging platform support. ntfy is a lightweight, open-source push notification service that works with self-hosted servers and ntfy.sh. Changes: - gateway/platforms/ntfy.py: new adapter with HTTP streaming subscription, exponential backoff reconnection, message deduplication, Bearer/Basic auth support - gateway/config.py: NTFY platform enum entry and env var loading - gateway/run.py: adapter factory and authorization maps - cron/scheduler.py: ntfy delivery in cron platform map - tools/send_message_tool.py: ntfy routing and _send_ntfy() - tools/cronjob_tools.py: ntfy in deliver parameter docs - toolsets.py: hermes-ntfy toolset and gateway composite - hermes_cli/status.py: ntfy status display - agent/prompt_builder.py: ntfy platform hint - gateway/channel_directory.py: session-based channel discovery - tests/gateway/test_ntfy.py: 81 unit tests
8e13390 to
4182e3d
Compare
|
|
||
| # ntfy has no native user ID — use the title field if present, | ||
| # otherwise fall back to the topic name as the "sender". | ||
| user_id = sender or topic |
There was a problem hiding this comment.
This worries me. title comes straight from whoever published the message, and ntfy topics are open to publish by default. Auth is keyed on NTFY_ALLOWED_USERS, so anyone who knows the topic can just set title to an allowed username and walk right past the allowlist. I don't think we should be deriving the user identity from title at all. Better to treat ntfy as a single trusted channel and call out in the docs that NTFY_ALLOWED_USERS isn't a real trust boundary unless the topic has a read token on it.
| async def _consume_stream(self, url: str, headers: Dict[str, str]) -> None: | ||
| """Open an HTTP streaming connection and dispatch events.""" | ||
| # poll=true keeps the connection alive with keepalive events | ||
| params = {"poll": "false"} |
There was a problem hiding this comment.
The comment right above says poll=true but we're sending "false". The value is right for streaming, just the comment that's stale.
| return | ||
| if response.status_code == 404: | ||
| logger.error("[%s] Topic not found (404): %s", self.name, self._topic) | ||
| return |
There was a problem hiding this comment.
We bail out quietly here on 401/404, but _run_stream will just turn around and reconnect, so on a bad token or wrong topic we end up hammering the server every 60s forever. Can we treat 401/404 as fatal and stop the loop?
| delay = RECONNECT_BACKOFF[min(backoff_idx, len(RECONNECT_BACKOFF) - 1)] | ||
| logger.info("[%s] Reconnecting in %ds...", self.name, delay) | ||
| await asyncio.sleep(delay) | ||
| backoff_idx += 1 |
There was a problem hiding this comment.
backoff_idx only ever goes up. If a connection stays up for hours and then drops, we reconnect at the 60s ceiling instead of starting back at 2s. Worth resetting it to 0 once a stream has been alive for a bit.
| if markdown_enabled: | ||
| headers["X-Markdown"] = "true" | ||
|
|
||
| body = content[:self.MAX_MESSAGE_LENGTH] |
There was a problem hiding this comment.
This drops anything past 4096 chars without telling anyone. The other adapters chunk long messages. Can we chunk here too, or at least log when we truncate?
| extra = {} | ||
| try: | ||
| from gateway.config import load_gateway_config, Platform as _P | ||
| cfg = load_gateway_config() |
There was a problem hiding this comment.
This runs the whole config load (which also writes to os.environ) just to check whether a topic is set, and it runs on every adapter pre-check. Reading NTFY_TOPIC / the extra dict directly would be a lot lighter.
There was a problem hiding this comment.
This one bakes in the spoofable-identity behavior from the adapter. If we change it so identity isn't pulled from title, can we add a test that an unknown publisher can't impersonate an allowed user?
There was a problem hiding this comment.
This one bakes in the spoofable-identity behavior from the adapter. If we change it so identity isn't pulled from title, can we add a test that an unknown publisher can't impersonate an allowed user?
Reopened as #30625 with all reviewer feedback addressed. Thanks for the thorough review @TheophileDiot @gerrydoro — all 6 issues fixed.
|
Reopened as #30625 with all reviewer feedback addressed: @TheophileDiot — identity spoofing (title field removed from user_id), 401/404 fatal stop, backoff reset, truncation warning, config load overhead fixed 81 tests passing. Thanks for the detailed review! |
What does this PR do?
Adds full support for ntfy as a messaging platform. ntfy is a lightweight, open-source push notification service that works with self-hosted servers and ntfy.sh.
Type of Change
Changes Made
gateway/platforms/ntfy.py— new adapter with HTTP streaming subscription, exponential backoff reconnection, message deduplication, Bearer/Basic auth supportgateway/config.py— NTFY platform enum entry and env var loadinggateway/run.py— adapter factory and authorization mapscron/scheduler.py— ntfy delivery in cron platform maptools/send_message_tool.py— ntfy routing and_send_ntfy()tools/cronjob_tools.py— ntfy in deliver parameter docstoolsets.py— hermes-ntfy toolset and gateway compositehermes_cli/status.py— ntfy status displayagent/prompt_builder.py— ntfy platform hintgateway/channel_directory.py— session-based channel discoverytests/gateway/test_ntfy.py— 81 unit testsHow to Test
Checklist