diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 1025964dc43b..8a261bfed776 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -546,12 +546,13 @@ async def _ssrf_redirect_guard(response): Must be async because httpx.AsyncClient awaits response event hooks. """ - from tools.url_safety import is_safe_url, redirect_target_from_response - redirect_url = redirect_target_from_response(response) - if redirect_url and not is_safe_url(redirect_url): - raise ValueError( - f"Blocked redirect to private/internal address: {safe_url_for_log(redirect_url)}" - ) + if response.is_redirect and response.next_request: + redirect_url = str(response.next_request.url) + from tools.url_safety import is_safe_url + if not is_safe_url(redirect_url): + raise ValueError( + f"Blocked redirect to private/internal address: {safe_url_for_log(redirect_url)}" + ) # --------------------------------------------------------------------------- @@ -1159,18 +1160,12 @@ def _media_delivery_denied_paths() -> List[Path]: # Bitwarden Secrets Manager plaintext disk cache. os.path.join("cache", "bws_cache.json"), ) - # Directory trees whose every child is credential material. - # - # mcp-tokens/ holds live MCP OAuth access tokens (.json) and - # dynamically-registered client credentials (.client.json); see - # tools/mcp_oauth.py. Same credential class as auth.json/credentials/. - # The write side already denies it (file_tools _check_sensitive_path); - # this pairs the media-delivery (exfil) side so a prompt-injection MEDIA - # tag can't deliver a live bearer token as a native attachment. - # (session/kanban SQLite stores are handled by #41071 — kept out here.) + # Directory trees whose every child is credential material. (MCP OAuth + # tokens under mcp-tokens/ are handled by the sibling targeted PR #37222; + # session/kanban SQLite stores by #41071 — kept out of this diff to avoid + # overlap.) _ROOT_CREDENTIAL_DIRS = ( "pairing", - "mcp-tokens", ) for hermes_root in (_HERMES_HOME, _HERMES_ROOT): for rel in _ROOT_CREDENTIAL_FILES: @@ -1781,14 +1776,15 @@ class MessageEvent: def is_command(self) -> bool: """Check if this is a command message (e.g., /new, /reset).""" - return self.text.startswith("/") + return (self.text or "").lstrip().startswith("/") def get_command(self) -> Optional[str]: """Extract command name if this is a command message.""" if not self.is_command(): return None # Split on space and get first word, strip the / - parts = self.text.split(maxsplit=1) + command_text = (self.text or "").lstrip() + parts = command_text.split(maxsplit=1) raw = parts[0][1:].lower() if parts else None if raw and "@" in raw: raw = raw.split("@", 1)[0] @@ -1801,7 +1797,8 @@ def get_command_args(self) -> str: """Get the arguments after a command.""" if not self.is_command(): return self.text - parts = self.text.split(maxsplit=1) + command_text = (self.text or "").lstrip() + parts = command_text.split(maxsplit=1) args = parts[1] if len(parts) > 1 else "" # iOS auto-corrects -- to — (em dash) and - to – (en dash) args = args.replace("\u2014\u2014", "--").replace("\u2014", "--").replace("\u2013", "-") @@ -1914,42 +1911,6 @@ class SendResult: } ) -# ``not_found`` substrings split by blast radius. A *chat-level* not_found means -# the chat/user/group itself is gone, so the whole target is dead. A -# *thread/topic/message-level* not_found (a deleted forum topic, an edited-away -# message) leaves the parent chat reachable — it must NOT mark the whole chat -# dead. ``classify_send_error`` collapses both into ``"not_found"``; -# ``is_chat_level_not_found`` recovers the distinction for the dead-target path. -# See gateway.dead_targets. -_CHAT_LEVEL_NOT_FOUND_SUBSTRINGS = ("chat not found",) -_SUBCHAT_NOT_FOUND_SUBSTRINGS = ( - "message to edit not found", - "message to reply not found", - "thread not found", - "topic_deleted", - "message_id_invalid", -) - - -def _error_blob(exc: Optional[BaseException] = None, error_text: str = "") -> str: - """Build the lowercased text blob both send-error classifiers match against. - - Single source of truth so ``classify_send_error`` and - ``is_chat_level_not_found`` can never drift (e.g. one including the - exception class name and the other not) and silently disagree on the same - failure. Includes ``str(exc)`` (when non-empty) and the exception's class - name, plus any explicit ``error_text``. - """ - parts = [] - if error_text: - parts.append(error_text) - if exc is not None: - exc_str = str(exc) - if exc_str: - parts.append(exc_str) - parts.append(exc.__class__.__name__) - return " ".join(parts).lower() - def classify_send_error(exc: Optional[BaseException], error_text: str = "") -> str: """Map a send exception / error string to a :data:`SEND_ERROR_KINDS` value. @@ -1959,7 +1920,13 @@ def classify_send_error(exc: Optional[BaseException], error_text: str = "") -> s use. Conservative — anything unrecognized returns ``"unknown"`` so callers never mistake an unclassified failure for a benign one. """ - blob = _error_blob(exc, error_text) + parts = [] + if error_text: + parts.append(error_text) + if exc is not None: + parts.append(str(exc)) + parts.append(exc.__class__.__name__) + blob = " ".join(parts).lower() if not blob.strip(): return "unknown" if "message_too_long" in blob or "too long" in blob or "message is too long" in blob: @@ -1983,8 +1950,13 @@ def classify_send_error(exc: Optional[BaseException], error_text: str = "") -> s or "not a member" in blob ): return "forbidden" - if any(s in blob for s in _CHAT_LEVEL_NOT_FOUND_SUBSTRINGS) or any( - s in blob for s in _SUBCHAT_NOT_FOUND_SUBSTRINGS + if ( + "chat not found" in blob + or "message to edit not found" in blob + or "message to reply not found" in blob + or "thread not found" in blob + or "topic_deleted" in blob + or "message_id_invalid" in blob ): return "not_found" if ( @@ -2002,26 +1974,6 @@ def classify_send_error(exc: Optional[BaseException], error_text: str = "") -> s return "unknown" -def is_chat_level_not_found(exc: Optional[BaseException] = None, error_text: str = "") -> bool: - """Whether a ``not_found`` failure means the *whole chat* is gone. - - :func:`classify_send_error` collapses chat-level and thread/topic/message-level - not_found into the single ``"not_found"`` kind. Only the chat-level case (the - chat/user/group no longer exists) should mark a delivery target dead; a deleted - forum topic or an edited-away message leaves the parent chat reachable. When - both a chat-level and a sub-chat marker are present, the sub-chat reading wins - (conservative: never kill a chat that may still be reachable). - - Argument order mirrors :func:`classify_send_error` (``exc`` first) and both - share :func:`_error_blob`, so the two classifiers cannot disagree on the same - failure. - """ - blob = _error_blob(exc, error_text) - if any(s in blob for s in _SUBCHAT_NOT_FOUND_SUBSTRINGS): - return False - return any(s in blob for s in _CHAT_LEVEL_NOT_FOUND_SUBSTRINGS) - - class EphemeralReply(str): """System-notice reply that auto-deletes after a TTL. @@ -2307,21 +2259,6 @@ class BasePlatformAdapter(ABC): # "typed_command_prefix", "/"); no per-platform branching at call sites. typed_command_prefix: str = "/" - # Whether this adapter supports the ``in_channel`` continuable-cron surface - # (``platforms.

.extra.cron_continuable_surface: in_channel``): a - # continuable cron job delivered FLAT into a channel (no dedicated thread), - # with the user's plain channel reply continuing the job in-context via the - # shared-channel session. Only coherent on a platform that has BOTH a - # flat-reply outbound gate AND a whole-channel inbound session bucket keyed - # ``(platform, chat_id, None)`` — today that is Slack (``reply_in_thread: - # false``). Default False: an unsupported platform fails SAFE, treating - # ``in_channel`` as ``thread`` (a threaded continuation ≈ today's - # behaviour), never a dropped continuation. Read generically by the cron - # scheduler via ``getattr(adapter, "supports_inchannel_continuable", - # False)`` — no per-platform branching at the call site (the key stays a - # generic seam; Slack is merely the first consumer). - supports_inchannel_continuable: bool = False - def __init__(self, config: PlatformConfig, platform: Platform): self.config = config self.platform = platform @@ -3961,22 +3898,15 @@ def register_post_delivery_callback( _prev = existing_cb _new = callback - async def _chained() -> None: - # Both _prev and _new may be sync or async. The chained - # wrapper itself must be async because the outer invoker - # (``_handle_message`` etc.) awaits awaitable callbacks; a - # sync wrapper here would call ``_prev()`` / ``_new()`` and - # silently drop any returned coroutine, breaking chained - # async post-delivery hooks (e.g. ``/goal`` continuations). - for _cb in (_prev, _new): - try: - _result = _cb() - if inspect.isawaitable(_result): - await _result - except Exception: - logger.debug( - "Post-delivery callback failed", exc_info=True - ) + def _chained() -> None: + try: + _prev() + except Exception: + logger.debug("Post-delivery callback failed", exc_info=True) + try: + _new() + except Exception: + logger.debug("Post-delivery callback failed", exc_info=True) callback = _chained diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index b535017b40e1..00ade2e9268e 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -2097,10 +2097,10 @@ async def send_image( async def _ssrf_redirect_guard(response): """Re-check redirect targets so public URLs cannot bounce into private IPs.""" - from tools.url_safety import redirect_target_from_response - redirect_url = redirect_target_from_response(response) - if redirect_url and not is_safe_url(redirect_url): - raise ValueError("Blocked redirect to private/internal address") + if response.is_redirect and response.next_request: + redirect_url = str(response.next_request.url) + if not is_safe_url(redirect_url): + raise ValueError("Blocked redirect to private/internal address") # Download the image first async with httpx.AsyncClient( @@ -2563,11 +2563,12 @@ async def _handle_slack_message(self, event: dict) -> None: # gateway dispatcher) handles it like a normal slash command. Only # rewrite when the first token resolves to a known gateway command # so casual messages like "!nice work" pass through unchanged. - if original_text.startswith("!"): + command_probe_text = original_text.lstrip() + if command_probe_text.startswith("!"): try: from hermes_cli.commands import is_gateway_known_command - first_token = original_text[1:].split(maxsplit=1)[0] + first_token = command_probe_text[1:].split(maxsplit=1)[0] # Strip "@suffix" the same way get_command() does, so # forms like ``!stop@hermes`` still resolve. cmd_name = first_token.split("@", 1)[0].lower() @@ -2576,10 +2577,12 @@ async def _handle_slack_message(self, event: dict) -> None: and "/" not in cmd_name and is_gateway_known_command(cmd_name) ): - original_text = "/" + original_text[1:] + original_text = "/" + command_probe_text[1:] + command_probe_text = original_text except Exception: # pragma: no cover - defensive pass + is_command_text = command_probe_text.startswith("/") text = original_text # Extract quoted/forwarded content from Slack blocks. @@ -2811,7 +2814,7 @@ async def _handle_slack_message(self, event: dict) -> None: # When entering a thread for the first time (no existing session), # fetch thread context so the agent understands the conversation. - if is_thread_reply and not self._has_active_session_for_thread( + if is_thread_reply and not is_command_text and not self._has_active_session_for_thread( channel_id=channel_id, thread_ts=event_thread_ts, user_id=user_id, @@ -2827,9 +2830,15 @@ async def _handle_slack_message(self, event: dict) -> None: # Determine message type msg_type = MessageType.TEXT - if (original_text or "").startswith("/"): + if is_command_text: msg_type = MessageType.COMMAND + # Commands typed as Slack text messages often intentionally carry a + # leading space (`` /stop``) so Slack itself does not intercept the + # slash. Once classified as a command, pass only the command text into + # the gateway dispatcher; do not prepend fetched thread context or + # block/attachment rendering before the leading slash. + # Handle file attachments media_urls = [] media_types = [] @@ -3094,11 +3103,6 @@ async def _handle_slack_message(self, event: dict) -> None: user_id=user_id, user_name=user_name, thread_id=thread_ts, - # Slack Workflow Builder / app posts arrive as - # subtype=bot_message with user=None; flag them so the - # gateway SLACK_ALLOW_BOTS bypass can authorize them - # (they carry no user_id to match against the allowlist). - is_bot=bool(event.get("bot_id")) or event.get("subtype") == "bot_message", ) # Per-channel ephemeral prompt @@ -3138,7 +3142,7 @@ async def _handle_slack_message(self, event: dict) -> None: reply_to_text = None msg_event = MessageEvent( - text=text, + text=(command_probe_text if is_command_text else text), message_type=msg_type, source=source, raw_message=event, diff --git a/tests/e2e/test_platform_commands.py b/tests/e2e/test_platform_commands.py index 4924eed6a9e2..ffd9db601fcf 100644 --- a/tests/e2e/test_platform_commands.py +++ b/tests/e2e/test_platform_commands.py @@ -56,6 +56,16 @@ async def test_stop_when_no_agent_running(self, adapter, platform): response_lower = response_text.lower() assert "no" in response_lower or "stop" in response_lower or "not running" in response_lower + @pytest.mark.asyncio + async def test_leading_space_stop_is_still_a_command(self, adapter, platform): + """Slack users type `` /stop`` to avoid native Slack slash interception.""" + send = await send_and_capture(adapter, " /stop", platform) + + send.assert_called_once() + response_text = send.call_args[1].get("content") or send.call_args[0][1] + response_lower = response_text.lower() + assert "no" in response_lower or "stop" in response_lower or "not running" in response_lower + @pytest.mark.asyncio async def test_commands_shows_listing(self, adapter, platform): send = await send_and_capture(adapter, "/commands", platform)