Repository navigation
fix(discord): a role-authorized member's speech, native slash commands and /thread starters reach the agent (#130302, #118958) #130303
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
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
6985ea6
fix(discord): a role-authorized member's speech in a voice channel re…
jonpol01 7976b4a
docs(voice): voice-channel access follows DISCORD_ALLOWED_ROLES too
jonpol01 f5f538e
fix(discord): carry the role verdict into slash command events
Christopher-Schulze e4165fc
docs(discord): state what the slash role flag means
Christopher-Schulze 357e77d
fix(discord): a native slash or /thread starter stamps the role grant…
jonpol01 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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| """A spoken turn from a member admitted by ``DISCORD_ALLOWED_ROLES`` reaches the agent. | ||
|
|
||
| The listen loop admits a speaker through the adapter's role check, but the voice source is rebuilt | ||
| from the ``/voice join`` copy, which never carries the per-event role grant, so the gateway's own | ||
| check refused every role-only speaker. The grant is recomputed for the current speaker against the | ||
| guild's current member, never inherited from whoever ran ``/voice join``. | ||
| """ | ||
|
|
||
| from types import SimpleNamespace | ||
| from unittest.mock import AsyncMock, MagicMock | ||
|
|
||
| import pytest | ||
|
|
||
| import gateway.run as gateway_run | ||
| from gateway.config import Platform, PlatformConfig | ||
| from gateway.session import SessionSource | ||
| from plugins.platforms.discord.adapter import DiscordAdapter | ||
|
|
||
| _ROLE, _GUILD, _TEXT, _JOINER = 1234, 1, 700, 42 | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| @pytest.mark.parametrize("speaker_roles,dispatched", [ | ||
| ([_ROLE], 1), # role member: admitted on both gates | ||
| ([], 0), # the joiner holds the role, this speaker does not | ||
| (None, 0), # speaker no longer in the guild | ||
| ]) | ||
| async def test_role_member_speech_passes_the_gateway_gate(monkeypatch, speaker_roles, dispatched): | ||
| for var in ("DISCORD_ALLOWED_USERS", "DISCORD_ALLOW_ALL_USERS", "GATEWAY_ALLOWED_USERS", | ||
| "GATEWAY_ALLOW_ALL_USERS"): | ||
| monkeypatch.delenv(var, raising=False) | ||
| monkeypatch.setenv("DISCORD_ALLOWED_ROLES", str(_ROLE)) | ||
| adapter = DiscordAdapter(PlatformConfig(enabled=True, token="fake")) | ||
| adapter._allowed_user_ids, adapter._allowed_role_ids = set(), {_ROLE} | ||
| speaker = 43 | ||
| members = {_JOINER: SimpleNamespace(id=_JOINER, roles=[SimpleNamespace(id=_ROLE)])} | ||
| if speaker_roles is not None: | ||
| members[speaker] = SimpleNamespace(id=speaker, roles=[SimpleNamespace(id=r) for r in speaker_roles]) | ||
| guild = SimpleNamespace(id=_GUILD, name="Hermes Server", get_member=members.get) | ||
| adapter._client = SimpleNamespace(get_guild=lambda _id: guild, get_channel=lambda _id: None) | ||
| adapter._voice_text_channels = {_GUILD: _TEXT} | ||
| joined = SessionSource(platform=Platform.DISCORD, chat_id=str(_TEXT), chat_type="group", | ||
| user_id=str(_JOINER), user_name="joiner", guild_id=str(_GUILD), | ||
| role_authorized=True) | ||
| adapter._voice_sources = {_GUILD: joined.to_dict()} | ||
| adapter.handle_message = AsyncMock() | ||
| runner = object.__new__(gateway_run.GatewayRunner) | ||
| runner.adapters = {Platform.DISCORD: adapter} | ||
| runner._voice_mode, runner._session_db, runner.session_store = {}, None, MagicMock() | ||
|
|
||
| await runner._handle_voice_channel_input(_GUILD, speaker, "what broke?", adapter=adapter) | ||
|
|
||
| assert adapter.handle_message.await_count == dispatched | ||
| for call in adapter.handle_message.await_args_list: | ||
| assert runner._is_user_authorized(call.args[0].source) |
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
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.
Fold this voice repair into #128788 and finish its native siblings. Rechecking the current speaker instead of inheriting the joiner's grant is the correct bounded behavior, and the new positive case passes through real gateway authorization. But the composed Discord set still fails a legitimate role-only native slash member: _build_slash_event and _dispatch_thread_session omit the analogous verified verdict. Please make one admission-to-event repair with positive gateway assertions for speech, slash and thread starter, and retain role-removal/missing-member controls. This comment is about the unresolved class boundary; I found no newly reproduced defect in this voice verdict itself.
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.
Native siblings fixed on this branch.
f5f538ef4bande4165fc2edare #125620, salvaged with its author kept.357e77d092makes_build_slash_eventand_dispatch_thread_sessionstamp the grant from_slash_role_grant(interaction), the slash gate re-evaluated for this actor. One parametrized test runs both builders through the real gateway_is_user_authorized: a role member is admitted, and an unrelated-role actor is refused. The voice role-removal and missing-member controls are unchanged. It stays a separate PR from #128788 for the reason in the top-level reply.