fix(gateway): preserve media + reply payload when /queue defers a turn - #65
Conversation
|
Review Complete Files Reviewed: 2 By Severity:
Removes authorization check wiring from gateway adapter lifecycle, disabling Slack indirect prompt injection mitigation. Also removes tenki config bridging from gateway while leaving it in CLI, creating config asymmetry. Files Reviewed (2 files) |
There was a problem hiding this comment.
Risk: 🟠 High (78/100) — 1 high finding · 51 LOC across 2 files
Security: Authorization Check Chain Severed (HIGH)
The PR removes _make_adapter_auth_check() and all three adapter.set_authorization_check() calls from gateway/run.py (start(), _platform_reconnect_watcher(), _start_one_profile_adapters()). The BasePlatformAdapter._authorization_check attribute now stays None in production, causing _is_sender_authorized() to return None. The Slack adapter's thread-context code checks is_authorized is False — None is not False, so the [unverified] tagging for third-party messages in shared channels is silently disabled. The test suite still mocks set_authorization_check() directly, giving false confidence.
Config: Tenki Env-Var Bridge Asymmetry (MEDIUM)
11 tenki terminal config entries are removed from _terminal_env_map in gateway/run.py but remain in cli.py and hermes_cli/config.py. Gateway-spawned terminal subprocesses silently lose user-configured tenki settings, reverting to hardcoded defaults. CLI users are unaffected. Existing tests assert the maps agree and will fail.
| adapter.set_session_store(self.session_store) | ||
| adapter.set_busy_session_handler(self._handle_active_session_busy_message) | ||
| adapter.set_topic_recovery_fn(self._recover_telegram_topic_thread_id) | ||
| adapter.set_authorization_check(self._make_adapter_auth_check(adapter.platform)) | ||
| adapter._busy_text_mode = self._busy_text_mode |
There was a problem hiding this comment.
🟠 Authorization check callback chain removed, disabling Slack thread prompt injection mitigation (security)
The PR removes _make_adapter_auth_check() (a 30-line closure factory that wrapped _is_user_authorized from authz_mixin.py) and deletes all three adapter.set_authorization_check(self._make_adapter_auth_check(adapter.platform)) calls from the gateway's adapter lifecycle paths:
start()at line 6356_platform_reconnect_watcher()at line 7164_start_one_profile_adapters()at line 7820
The BasePlatformAdapter._authorization_check attribute defaults to None (base.py:2317). The _is_sender_authorized() method (base.py:2764) returns None when no check is registered — the exact condition introduced by this removal.
The Slack adapter at plugins/platforms/slack/adapter.py:3675 calls self._is_sender_authorized(msg_user, ...) during thread context fetching. At line 3678, the guard if is_authorized is False: uses identity comparison — None is not False, so trust_tag = "[unverified] " is never assigned. All thread-context messages from third-party senders in shared channels are presented to the LLM as authoritative input without the mitigation header (adapter.py:3688-3697) that instructs the model to treat unverified content as background reference.
Additionally, Callable was removed from the typing imports (line 44) since _make_adapter_auth_check was its only consumer in run.py.
The test suite (tests/gateway/test_slack.py) still mocks set_authorization_check() directly on adapters, giving false confidence that the production path works.
💡 Suggestion: Restore the _make_adapter_auth_check method and reinstate the three adapter.set_authorization_check() calls. If the feature was intentionally removed, the cleanup is incomplete — set_authorization_check() and _is_sender_authorized() remain on BasePlatformAdapter (base.py:2750-2786), the Slack adapter still calls _is_sender_authorized() (adapter.py:3675), and the test suite still exercises the feature via direct mock calls. Either restore the production wiring or complete the removal by also cleaning up the base class interface, the Slack adapter consumer code, and the tests.
📋 Prompt for AI Agents
In gateway/run.py: (1) Restore Callable to the typing import line (currently from typing import Dict, Optional, Any, List, Union — add back Callable). (2) After _create_adapter() ends at line 7987, re-insert the _make_adapter_auth_check method (the deleted closure factory that wraps _is_user_authorized). (3) At each of the three adapter setup sites, add adapter.set_authorization_check(self._make_adapter_auth_check(adapter.platform)) after the adapter.set_topic_recovery_fn(...) line: in start() after line 6356, in _platform_reconnect_watcher() after line 7164, in _start_one_profile_adapters() after line 7820. The code to add at each site is: adapter.set_authorization_check(self._make_adapter_auth_check(adapter.platform)). The deleted method (from the diff context) built a callback via SessionSource and _is_user_authorized — use that same implementation. If this feature is being deliberately removed instead, also remove set_authorization_check() and _is_sender_authorized() from gateway/platforms/base.py (lines 2750-2786), remove the [unverified] tagging logic from plugins/platforms/slack/adapter.py (lines 3669-3697), and update tests/gateway/test_slack.py to remove direct set_authorization_check() mock calls.
Summary
/queueno longer drops the media and reply context attached to the command — the deferred turn now runs with the full payload intact.Root cause: the running-agent
/queuehandler rebuilt the queuedMessageEventwith onlytext/message_type/source/message_id/channel_prompt, silently discardingmedia_urls,media_types,raw_message, and allreply_to_*fields. When the queued turn drained, the attachment was already gone.Changes
gateway/run.py: carrymedia_urls,media_types,raw_message,reply_to_*,auto_skill,internal,timestampthrough to the queued event; setmessage_typefrom the source event when it has media; accept a media-only/queue(no prompt text required when an attachment is present, e.g./queueas an image caption).tests/gateway/test_queue_command.py: new — 5 tests driving the real_handle_messagerunning-agent path (text-only, photo media, media-without-text, reply context, empty-usage guard).scripts/release.py: AUTHOR_MAP entry for the co-author.Validation
/queue look at this+ photomedia_urls/media_typespreserved/queue+ document, no textUsage: /queue <prompt>/queue and thisas a replyreply_to_*losttests/gateway/test_queue_command.pytests/gateway/test_queue_consumption.pySalvaged from NousResearch#13913 by @ypwcharles. The gateway busy-session/queue subsystem was rewritten since that April PR — Telegram moved to
plugins/platforms/,/queuenow uses the FIFO chain — so the media fix is reimplemented against the current handler. The PR's command-batching and_busy_session_bypasschanges targeted code paths that no longer exist and were dropped.Infographic
Mirror-of: NousResearch#55960
NousResearch#55960