forked from NousResearch/hermes-agent
-
Notifications
You must be signed in to change notification settings - Fork 0
fix(gateway): preserve media + reply payload when /queue defers a turn #65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 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_authorizedfrom authz_mixin.py) and deletes all threeadapter.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 7820The
BasePlatformAdapter._authorization_checkattribute defaults toNone(base.py:2317). The_is_sender_authorized()method (base.py:2764) returnsNonewhen no check is registered — the exact condition introduced by this removal.The Slack adapter at
plugins/platforms/slack/adapter.py:3675callsself._is_sender_authorized(msg_user, ...)during thread context fetching. At line 3678, the guardif is_authorized is False:uses identity comparison —None is not False, sotrust_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,
Callablewas removed from the typing imports (line 44) since_make_adapter_auth_checkwas its only consumer in run.py.The test suite (
tests/gateway/test_slack.py) still mocksset_authorization_check()directly on adapters, giving false confidence that the production path works.💡 Suggestion: Restore the
_make_adapter_auth_checkmethod and reinstate the threeadapter.set_authorization_check()calls. If the feature was intentionally removed, the cleanup is incomplete —set_authorization_check()and_is_sender_authorized()remain onBasePlatformAdapter(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
Callableto the typing import line (currentlyfrom typing import Dict, Optional, Any, List, Union— add backCallable). (2) After_create_adapter()ends at line 7987, re-insert the_make_adapter_auth_checkmethod (the deleted closure factory that wraps_is_user_authorized). (3) At each of the three adapter setup sites, addadapter.set_authorization_check(self._make_adapter_auth_check(adapter.platform))after theadapter.set_topic_recovery_fn(...)line: instart()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 viaSessionSourceand_is_user_authorized— use that same implementation. If this feature is being deliberately removed instead, also removeset_authorization_check()and_is_sender_authorized()fromgateway/platforms/base.py(lines 2750-2786), remove the[unverified]tagging logic fromplugins/platforms/slack/adapter.py(lines 3669-3697), and updatetests/gateway/test_slack.pyto remove directset_authorization_check()mock calls.