fix(gateway): require auth for interaction buttons by LaPhilosophie · Pull Request #30964 · NousResearch/hermes-agent

LaPhilosophie · 2026-05-23T13:36:34Z

What does this PR do?

Aligns Slack Block Kit callbacks and Discord component button callbacks with the gateway external-surface authorization model.

Interactive button callbacks bypass the normal message dispatch authorization path, so they now require one of the configured authorization mechanisms before resolving gateway state:

platform user/role allowlists
GATEWAY_ALLOWED_USERS
explicit allow-all flags

Slack also keeps compatibility with allow_from when no SLACK_ALLOWED_USERS env allowlist is configured.

Related Issue

Related: #29627, #30742

Type of Change

Changes Made

gateway/platforms/slack.py
- Add shared Slack interaction authorization for button callbacks.
- Apply it to approval and slash-confirm actions.
- Honor SLACK_ALLOWED_USERS, GATEWAY_ALLOWED_USERS, SLACK_ALLOW_ALL_USERS, GATEWAY_ALLOW_ALL_USERS, and config allow_from.
plugins/platforms/discord/adapter.py
- Make component button auth fail closed when no allowlist or explicit allow-all is configured.
- Honor GATEWAY_ALLOWED_USERS, wildcard user allowlists, and explicit allow-all flags.
- Preserve Discord role allowlist behavior.
tests/gateway/test_discord_component_auth.py
- Add/adjust regression coverage for fail-closed defaults, global allowlists, explicit allow-all, wildcard users, and all Discord component views.
tests/gateway/test_slack_approval_buttons.py
- Add Slack interaction auth coverage.
- Add regression coverage proving approval buttons do not resolve without an allowlist.

How to Test

/private/tmp/hermes-agent-pr-venv/bin/python -m pytest -o addopts='' \
  tests/gateway/test_discord_component_auth.py \
  tests/gateway/test_slack_approval_buttons.py \
  -q

Result:

59 passed, 4 warnings in 0.27s

The warnings are existing Slack AsyncMock runtime warnings from thread-context tests, not failures from this change.

/private/tmp/hermes-agent-pr-venv/bin/python -m ruff check \
  gateway/platforms/slack.py \
  plugins/platforms/discord/adapter.py \
  tests/gateway/test_discord_component_auth.py \
  tests/gateway/test_slack_approval_buttons.py

Result:

All checks passed!

git diff --check

No output.

Checklist

Code

I've read the Contributing Guide
My commit messages follow Conventional Commits
I searched for existing PRs to make sure this isn't a duplicate
My PR contains only changes related to this fix
I've run pytest tests/ -q and all tests pass
I've added tests for my changes
I've tested on my platform: macOS 26.5, Python 3.13.1

Documentation & Housekeeping

I've updated relevant documentation — N/A
I've updated cli-config.yaml.example — N/A
I've updated CONTRIBUTING.md or AGENTS.md — N/A
I've considered cross-platform impact — pure env parsing and callback authorization checks
I've updated tool descriptions/schemas — N/A

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Tightens authorization for interactive UI surfaces (Discord component views and Slack button clicks) so they fail closed unless explicitly allowed via per-platform allowlists, a global allowlist, or explicit allow-all flags.

Changes:

Added Slack interaction authorization helper and applied it to approval + slash-confirm button handlers.
Updated Discord component-view auth helper to incorporate global allowlist and explicit allow-all, and to deny by default when no policy is configured.
Expanded regression tests for Slack/Discord interaction authorization, including a new ClarifyChoiceView case.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
tests/gateway/test_slack_approval_buttons.py	Adds unit tests for Slack interaction authorization and updates existing approval-button tests to include user IDs + allowlist setup.
tests/gateway/test_discord_component_auth.py	Updates regression tests to reflect fail-closed defaults, explicit allow-all, global allowlist support, and ClarifyChoiceView coverage.
plugins/platforms/discord/adapter.py	Changes `_component_check_auth` semantics to require explicit authorization (allow-all / user allowlists / role allowlists) and support global allowlist.
gateway/platforms/slack.py	Introduces reusable env parsing + Slack interaction authorization helper and enforces it in Slack interactive handlers.

Comments suppressed due to low confidence (1)

tests/gateway/test_discord_component_auth.py:1

Several tests asserting rejection don’t clear DISCORD_ALLOW_ALL_USERS/GATEWAY_ALLOW_ALL_USERS/GATEWAY_ALLOWED_USERS. If any of those are set in the external test environment, _component_check_auth() may return True and these tests will fail intermittently. Recommend adding monkeypatch to these tests and delenv the relevant vars at the start (similar to the other fail-closed tests in this file).

"""Security regression tests: Discord component views honor allowlists.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    if os.getenv("DISCORD_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}:
+        return True
+    if os.getenv("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}:


+        for uid in os.getenv("GATEWAY_ALLOWED_USERS", "").split(",")
+        if uid.strip()
+    }
+    user_set.update(global_allowed)
    role_set = allowed_role_ids or set()


+    def test_platform_allowlist_permits_user(self, monkeypatch):
+        monkeypatch.setenv("SLACK_ALLOWED_USERS", "U123,U456")
+
+        assert _slack_interaction_user_authorized("U123") is True
+        assert _slack_interaction_user_authorized("U999") is False
+
+    def test_global_allowlist_permits_user(self, monkeypatch):
+        monkeypatch.setenv("GATEWAY_ALLOWED_USERS", "U123")
+
+        assert _slack_interaction_user_authorized("U123") is True
+        assert _slack_interaction_user_authorized("U999") is False


+        # Authorization — button clicks bypass normal message auth flow.
+        if not _slack_interaction_user_authorized(user_id, self.config.extra):
+            logger.warning(
+                "[Slack] Unauthorized slash-confirm click by %s (%s) — ignoring",
+                user_name, user_id or "missing-user-id",
+            )
+            return


daimon-nous · 2026-05-23T13:54:34Z

Related: #29627 (Slack-only fix for allow_from on Block Kit callbacks). This PR is a cross-platform consolidation that also hardens Discord component auth to fail-closed. Supersedes #29627.

Also related: #30742 (referenced in PR body), #18016 (Telegram callback auth).

teknium1 · 2026-06-07T12:10:45Z

The Discord half of this is consolidated into #41226 (open, pending review) — thank you @LaPhilosophie.

Your _component_check_auth fail-closed change is cherry-picked with authorship preserved. The Slack half of this PR is superseded by #33844's _is_interactive_user_authorized helper (it delegates to the gateway runner's central auth, which matches the Telegram precedent), so #41226 carries the Discord change from here and the Slack change from #33844. Verified live: empty allowlists now deny; DISCORD_ALLOWED_USERS / DISCORD_ALLOWED_ROLES / GATEWAY_ALLOWED_USERS membership and the DISCORD_ALLOW_ALL_USERS / GATEWAY_ALLOW_ALL_USERS opt-in all still work.

@LaPhilosophie

… set Salvage of the Discord half of PR #30964 by @LaPhilosophie. Discord component button callbacks (ExecApprovalView, SlashConfirmView, UpdatePromptView, ModelPickerView) bypass the normal message dispatch authorization path. _component_check_auth previously returned True when both the user and role allowlists were empty, so any guild member who could see an approval prompt could click Approve on a dangerous command. Fail closed instead: require DISCORD_ALLOWED_USERS / DISCORD_ALLOWED_ROLES / GATEWAY_ALLOWED_USERS membership, or an explicit DISCORD_ALLOW_ALL_USERS / GATEWAY_ALLOW_ALL_USERS opt-in for deliberately-open deployments. Mirrors the Telegram (#24457) and Matrix fail-closed precedent. The Slack half of #30964 is superseded by PR #33844's helper. Reported via GHSA-mc26-p6fw-7pp6 (@whyiug). Co-authored-by: LaPhilosophie <804436395@qq.com>

… salvage (#33844, #33866, #30964)

* upstream/main: (430 commits) fix(yuanbao): bound ws.close() so an idle server can't stall shutdown ~5s (NousResearch#40607) docs: add Urdu translation of README (NousResearch#40578) fix(hindsight): send only new-turn delta on append retains instead of whole session (NousResearch#40605) feat(gateway): render terminal tool calls as native bash code blocks on markdown platforms (NousResearch#41215) feat(desktop): stop the chat viewport from following streaming output (NousResearch#41414) chore(release): map AlchemistChaos co-author email for NousResearch#40135 salvage fix(desktop): recover chat after sleep/wake by revalidating a stale remote backend fix(web): make _has_env config-aware so SEARXNG_URL auto-detect honors Hermes config fix(web): honor Hermes config-aware SEARXNG_URL lookup install.sh: hint at root-owned npm cache when desktop npm install fails (NousResearch#39688) fix(tools): percent-encode non-ascii URL components fix(skills): browse shows full catalog, not first 5000 (NousResearch#41413) feat(desktop+gateway): remote media relay — attach images/PDFs and display gateway images over the network feat(desktop): full tool-backend config (pickers + per-backend settings) in Settings (NousResearch#41232) hardening(api-server): scan cron prompts on REST create/update for parity with the agent tool fix: skip MCP preflight content-type probe on reconnect when already ready (NousResearch#40604) fix(kanban): sweep deferred scratch parent on non-scratch child completion + tests fix: defer scratch workspace cleanup when task has active children (NousResearch#33774) feat(onboarding): opt-in structured profile-build path on first contact (NousResearch#41114) feat(compression): temporal anchoring in compaction summaries (NousResearch#41102) test(discord): align clarify/model-picker tests with fail-closed component auth (NousResearch#41338) chore(release): map Dusk1e and LaPhilosophie for approval fail-closed salvage (NousResearch#33844, NousResearch#33866, NousResearch#30964) fix(discord): fail closed for component button auth when no allowlist set fix(feishu): fail closed for update prompt card actions fix(slack): re-check gateway auth on approval and slash-confirm buttons fix: guard int(os.getenv()) casts against malformed env vars (NousResearch#40598) fix: respect Honcho env var fallback in doctor and honcho status chore(release): add synapsesx to AUTHOR_MAP for NousResearch#40495 salvage fix(research): keep tool_call/tool_response pairs intact when compressing trajectories fix(simplex): accept display name in SIMPLEX_ALLOWED_USERS fix(desktop): make the running-turn timer per-session (NousResearch#41182) test(approval): regression for shell-escape denylist bypass (NousResearch#36846, NousResearch#36847) fix(security): strip shell escapes in denylist normalizer; fail-closed on missing approval module fix(stream+output-cap): guard empty streams and parse OpenRouter output-cap errors (NousResearch#40589) fix(desktop): bootstrap falls back to installed agent install.sh on GitHub 404 feat(dashboard): change UI font from the theme picker, independent of theme (NousResearch#41145) fix(cli): return bool (not None) when a destructive-slash confirmation is cancelled (NousResearch#40583) fix(desktop): preserve configured base_url on same-provider model switch (NousResearch#41121) fix(desktop): stop bare-URL autolinker swallowing trailing emphasis asterisks (NousResearch#41093) fix(cron): bound the desktop run-history query to one job (NousResearch#41088) fix(desktop): scope in-session /model switch per-session, stop process-env leak (NousResearch#41120) chore: map bmoore210 author email for PR NousResearch#40550 salvage fix(desktop): scope session list to active profile + longer timeout fix: harden gateway startup and turn persistence fix(computer_use): honor custom vision routing fix(aux): honor model.default_headers on auxiliary client too (NousResearch#40033) fix(agent): honor model.default_headers for custom OpenAI-compatible providers (NousResearch#40033) docs(i18n): port deep-audit corrections to zh-Hans mirror (NousResearch#41104) fix(compression): don't overwrite the -1 post-compression sentinel in preflight seed (NousResearch#36718) chore(release): map singhsanidhya741@gmail.com to sanidhyasin (NousResearch#41094) ...

@LaPhilosophie

… set Salvage of the Discord half of PR NousResearch#30964 by @LaPhilosophie. Discord component button callbacks (ExecApprovalView, SlashConfirmView, UpdatePromptView, ModelPickerView) bypass the normal message dispatch authorization path. _component_check_auth previously returned True when both the user and role allowlists were empty, so any guild member who could see an approval prompt could click Approve on a dangerous command. Fail closed instead: require DISCORD_ALLOWED_USERS / DISCORD_ALLOWED_ROLES / GATEWAY_ALLOWED_USERS membership, or an explicit DISCORD_ALLOW_ALL_USERS / GATEWAY_ALLOW_ALL_USERS opt-in for deliberately-open deployments. Mirrors the Telegram (NousResearch#24457) and Matrix fail-closed precedent. The Slack half of NousResearch#30964 is superseded by PR NousResearch#33844's helper. Reported via GHSA-mc26-p6fw-7pp6 (@whyiug). Co-authored-by: LaPhilosophie <804436395@qq.com>

… salvage (NousResearch#33844, NousResearch#33866, NousResearch#30964)

Copilot AI review requested due to automatic review settings May 23, 2026 13:36

Copilot AI reviewed May 23, 2026

View reviewed changes

fix(gateway): require auth for interaction buttons

1b2248b

LaPhilosophie force-pushed the fix-interaction-button-auth branch from 3f63562 to 1b2248b Compare May 23, 2026 13:44

memosr mentioned this pull request May 24, 2026

fix(security): apply user allowlist and escape delimiters in Discord channel backfill to prevent prompt injection #29230

Open

8 tasks

This was referenced May 27, 2026

fix(slack): fail closed for interactive approval callbacks #33280

Closed

fix(discord): re-check gateway authorization for interactive component clicks #33896

Open

teknium1 mentioned this pull request Jun 7, 2026

fix(gateway): fail closed for approval-button auth on Slack, Feishu, Discord when no allowlist set #41226

Merged

teknium1 added a commit that referenced this pull request Jun 7, 2026

chore(release): map Dusk1e and LaPhilosophie for approval fail-closed…

a317e54

… salvage (#33844, #33866, #30964)

teknium1 closed this in #41226 Jun 7, 2026

changman pushed a commit to changman/hermes-agent that referenced this pull request Jun 10, 2026

chore(release): map Dusk1e and LaPhilosophie for approval fail-closed…

bc0c588

… salvage (NousResearch#33844, NousResearch#33866, NousResearch#30964)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(gateway): require auth for interaction buttons#30964

fix(gateway): require auth for interaction buttons#30964
LaPhilosophie wants to merge 1 commit into
NousResearch:mainfrom
LaPhilosophie:fix-interaction-button-auth

LaPhilosophie commented May 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

daimon-nous Bot commented May 23, 2026

Uh oh!

teknium1 commented Jun 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

LaPhilosophie commented May 23, 2026

What does this PR do?

Related Issue

Type of Change

Changes Made

How to Test

Checklist

Code

Documentation & Housekeeping

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

daimon-nous Bot commented May 23, 2026

Uh oh!

teknium1 commented Jun 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants