Skip to content

fix(slack): approval and confirm buttons ignore configured authorization - #38069

Closed
coygeek wants to merge 1 commit into
NousResearch:mainfrom
coygeek:fix/slack-button-click-authz-drops-gateway-authorization-context
Closed

fix(slack): approval and confirm buttons ignore configured authorization#38069
coygeek wants to merge 1 commit into
NousResearch:mainfrom
coygeek:fix/slack-button-click-authz-drops-gateway-authorization-context

Conversation

@coygeek

@coygeek coygeek commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Summary

An unauthorized Slack user who can see a pending approval or slash-confirm message in a shared channel or thread can approve, deny, or persist another caller's dangerous-command decision when the operator relies on GATEWAY_ALLOWED_USERS or pairing instead of SLACK_ALLOWED_USERS.

That changes protected approval state and can unblock execution or destructive slash-command handling across the Slack external surface.

  • Approval and slash-confirm buttons bypass the normal Slack message authorization path.
  • The callbacks only check SLACK_ALLOWED_USERS; when unset, there is no caller authorization check.
  • The callbacks then resolve session approval or destructive slash-confirm state.
  • A visible but unauthorized Slack user can decide another session's blocked command or slash-confirm prompt.

Route Slack interactive callbacks through the same authorization helper used for normal Slack messages, or require the callback user to satisfy an equivalent combined allowlist/pairing decision before resolving approvals.

Linked context

Closes #38068

Real behavior proof (required for external PRs)

Affected component (issue scope)

File: gateway/platforms/slack.py:2812-2822,2920-2932

        allowed_csv = os.getenv("SLACK_ALLOWED_USERS", "").strip()
        if allowed_csv:
            allowed_ids = {uid.strip() for uid in allowed_csv.split(",") if uid.strip()}
            if "*" not in allowed_ids and user_id not in allowed_ids:
                return
        result_text = await _slash_confirm_mod.resolve(session_key, confirm_id, choice)
        count = resolve_gateway_approval(session_key, choice)

Secondary paths: gateway/platforms/slack.py:2740-2799 posts slash-confirm buttons; gateway/platforms/slack.py:2666-2719 posts approval buttons; gateway/run.py:7073-7154 defines the broader gateway authorization set.

Files changed in this PR

  • gateway/run.py
  • gateway/platforms/slack.py
  • tests/gateway/test_slack_approval_buttons.py

Behavior reproduced or verified

  1. Against current main commit e2ea648a08265164ef103005d533ed28ec58ceb8, configure Slack with GATEWAY_ALLOWED_USERS=U_AUTHORIZED and leave SLACK_ALLOWED_USERS unset.
  2. Place U_AUTHORIZED and U_ATTACKER in the same Slack channel.
  3. Have U_AUTHORIZED trigger a dangerous command or destructive slash confirmation so Slack posts the callback buttons.
  4. Click Allow Once, Deny, Confirm Once, or Always Confirm as U_ATTACKER and observe that the callback reaches resolve_gateway_approval(...) or slash_confirm.resolve(...).
  5. Expected: the callback rejects users outside the gateway authorization set before changing approval or confirmation state.

Expected fixed behavior

Route Slack interactive callbacks through the same authorization helper used for normal Slack messages, or require the callback user to satisfy an equivalent combined allowlist/pairing decision before resolving approvals.

Tests and validation

  • bash scripts/run_tests.sh tests/gateway/test_slack_approval_buttons.py
  • Result: 1 files, 33 tests passed, 0 failed (100% complete) in 1.5s (20 workers) ===

Environment

Verified against latest upstream main via source receipt (source_ready, upstream-main, e2ea648a08265164ef103005d533ed28ec58ceb8). Full-public submit-fix route is declared by candidate metadata. Redaction checked: no secrets, tokens, credentials, private logs, or unrelated local paths are included.

Risk checklist

  • Security/auth/secrets impact: Yes; this is a full-public security route.
  • Approval gate: Current-turn full-public approval evidence was confirmed before live publication.
  • Public disclosure safety: Redaction and public-body validation run before GitHub mutation.
  • Regression risk: Scoped to the linked fix branch and covered by the tests above.
  • Issue-body contract: Unchanged; the linked issue carries the canonical security disclosure.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists platform/slack Slack app adapter comp/gateway Gateway runner, session dispatch, delivery area/auth Authentication, OAuth, credential pools duplicate This issue or pull request already exists labels Jun 3, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing fix for the same Slack interactive-callback auth gap as PRs #29627, #33280, #33844, and #36670. Maintainers should pick one canonical implementation.

@coygeek

coygeek commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Acknowledged. I checked the competing Slack interactive-callback auth PRs (#29627, #33280, #33844, and #36670) and this branch is another implementation of the same issue. There is no additional code change to push from this comment alone; maintainers should choose the canonical fix, and I am fine with this PR being superseded if one of the earlier branches is preferred.

@coygeek
coygeek force-pushed the fix/slack-button-click-authz-drops-gateway-authorization-context branch from d325a06 to ab964a5 Compare June 4, 2026 03:16
@coygeek coygeek changed the title fix(slack): enforce gateway auth on approval buttons fix(slack): approval and confirm buttons ignore configured authorization Jun 4, 2026
Slack Block Kit approval and slash-confirm callbacks previously checked only Slack-specific allowlist state before resolving protected approval decisions. That bypassed gateway-level allowlists and paired-user authorization when callbacks arrived outside normal message dispatch.

Bind gateway pairing state into platform adapters and gate Slack callback resolution through Slack/global allowlists, pairing, and allow-all fallback semantics before mutating messages or resolving approvals.
@coygeek
coygeek force-pushed the fix/slack-button-click-authz-drops-gateway-authorization-context branch from ab964a5 to 121ef40 Compare June 4, 2026 03:21
@coygeek

coygeek commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

This PR has been superseded by the fix that landed on main.

The same Slack interactive-callback authorization gap is now fixed by #41226 (fix(gateway): fail closed for approval-button auth on Slack, Feishu, Discord when no allowlist set), merged on 2026-06-07.

Relevant attribution:

I checked current main, and the landed implementation now gates both Slack approval buttons and slash-confirm buttons through _is_interactive_user_authorized() before calling resolve_gateway_approval(...) or slash_confirm.resolve(...). That addresses the behavior this PR was intended to fix.

I am fine with this PR being closed as superseded by #41226.

@coygeek

coygeek commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #41226, which landed the same Slack interactive-callback authorization fix on main.

@coygeek coygeek closed this Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/slack Slack app adapter type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(slack): approval and confirm buttons ignore configured authorization

2 participants