Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions gateway/authz_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,23 @@ def _is_user_authorized(self, source: SessionSource) -> bool:
if "*" in allowed_group_ids or source.chat_id in allowed_group_ids:
return True

# Bots admitted by {PLATFORM}_ALLOW_BOTS bypass the human allowlist (#4466).
# Checked before the no-user-id guard below: some platforms deliver
# bot/automation traffic with no user_id at all -- e.g. Slack Workflow
# Builder posts arrive as subtype=bot_message with user=None -- so
# deferring past the guard would reject them outright (the same reason
# the chat-scoped allowlist above runs early).
platform_allow_bots_map = {
Platform.DISCORD: "DISCORD_ALLOW_BOTS",
Platform.FEISHU: "FEISHU_ALLOW_BOTS",
Platform.TELEGRAM: "TELEGRAM_ALLOW_BOTS",
Platform.SLACK: "SLACK_ALLOW_BOTS",
}
if getattr(source, "is_bot", False):
allow_bots_var = platform_allow_bots_map.get(source.platform)
if allow_bots_var and os.getenv(allow_bots_var, "none").lower().strip() in {"mentions", "all"}:
return True

if not user_id:
return False

Expand Down Expand Up @@ -325,12 +342,6 @@ def _is_user_authorized(self, source: SessionSource) -> bool:
Platform.QQBOT: "QQ_ALLOW_ALL_USERS",
Platform.YUANBAO: "YUANBAO_ALLOW_ALL_USERS",
}
# Bots admitted by {PLATFORM}_ALLOW_BOTS bypass the human allowlist (#4466).
platform_allow_bots_map = {
Platform.DISCORD: "DISCORD_ALLOW_BOTS",
Platform.FEISHU: "FEISHU_ALLOW_BOTS",
Platform.TELEGRAM: "TELEGRAM_ALLOW_BOTS",
}

# Plugin platforms: check the registry for auth env var names
if source.platform not in platform_env_map:
Expand Down Expand Up @@ -358,11 +369,6 @@ def _is_user_authorized(self, source: SessionSource) -> bool:
if getattr(source, "role_authorized", False) is True:
return True

if getattr(source, "is_bot", False):
allow_bots_var = platform_allow_bots_map.get(source.platform)
if allow_bots_var and os.getenv(allow_bots_var, "none").lower().strip() in {"mentions", "all"}:
return True

# Check pairing store (always checked, regardless of allowlists)
platform_name = source.platform.value if source.platform else ""
if self.pairing_store.is_approved(platform_name, user_id):
Expand Down
70 changes: 5 additions & 65 deletions plugins/platforms/slack/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,14 +427,6 @@ class SlackAdapter(BasePlatformAdapter):
# the prefix that works everywhere — instruction text must show it.
typed_command_prefix = "!"

# Slack has both halves the ``in_channel`` continuable-cron surface needs:
# a flat-reply outbound gate (``reply_in_thread: false`` → ``_resolve_thread_ts``
# returns None for top-level channel messages) AND a whole-channel inbound
# session bucket keyed ``(platform, channel_id, None)`` (the same
# ``reply_in_thread: false`` path in ``_handle_slack_message``). So a
# continuable cron delivered flat here continues in-context on a plain reply.
supports_inchannel_continuable = True

def __init__(self, config: PlatformConfig):
super().__init__(config, Platform.SLACK)
self._app: Optional[Any] = None
Expand Down Expand Up @@ -1081,7 +1073,6 @@ async def connect(self, *, is_reconnect: bool = False) -> bool:

self._warn_if_missing_group_dm_scopes(auth_response, team_name)
self._warn_if_not_bot_token(auth_response, team_name)
self._warn_if_inchannel_without_flat_reply(team_name)

# Register message event handler
@self._app.event("message")
Expand Down Expand Up @@ -1571,62 +1562,6 @@ def _dm_top_level_threads_as_sessions(self) -> bool:
return True # default: each DM thread is its own session
return str(raw).strip().lower() in {"1", "true", "yes", "on"}

def _cron_continuable_surface(self) -> str:
"""Resolve the continuable-cron delivery surface for this platform.

Values: ``"thread"`` (default — today's behaviour: a continuable cron
job opens a dedicated hidden thread and seeds it) or ``"in_channel"``
(deliver FLAT into the channel timeline; the shared-channel session
``(slack, channel_id, None)`` is the continuation surface). Set
``platforms.slack.extra.cron_continuable_surface: in_channel`` in
config.yaml. Pair with ``reply_in_thread: false`` so the user's reply
is answered flat in the channel and keyed to the same shared session —
see ``_warn_if_inchannel_without_flat_reply``. Any unrecognised value
coerces to ``"thread"`` (fail safe).
"""
raw = self.config.extra.get("cron_continuable_surface")
if raw is None:
return "thread"
val = str(raw).strip().lower()
return "in_channel" if val == "in_channel" else "thread"

def _warn_if_inchannel_without_flat_reply(self, team_name: str) -> None:
"""Warn when ``in_channel`` is set without the required ``reply_in_thread: false`` pairing.

The two knobs are orthogonal (D4/D5): ``cron_continuable_surface:
in_channel`` skips thread creation on delivery, and ``reply_in_thread:
false`` makes the bot answer inbound channel messages flat and key them
to the whole-channel session ``(slack, channel_id, None)``. For a
continuable in-channel cron to actually continue on a plain reply, BOTH
must hold: the seed lands in the shared-channel session, and the reply
must resolve to (and be answered in) that same flat session.

Enforcement is WARN, not hard-require (D5): the misconfiguration fails
SAFE — ``in_channel`` without ``reply_in_thread: false`` yields a
threaded continuation (≈ today's behaviour), never a dropped/orphaned
session — so a config-load rejection would be heavier than warranted
and would make the two knobs non-orthogonal. Mirrors the existing
connect-time warning pattern (``_warn_if_missing_group_dm_scopes``,
``_warn_if_not_bot_token``).
"""
try:
if self._cron_continuable_surface() != "in_channel":
return
# reply_in_thread defaults True (legacy: reply in a thread).
if self.config.extra.get("reply_in_thread", True):
logger.warning(
"[Slack] %s: cron_continuable_surface=in_channel is set "
"WITHOUT reply_in_thread=false. A continuable in-channel "
"cron job will deliver flat, but the bot will still reply "
"to your continuation in a thread — so it falls back to a "
"threaded continuation (\u2248 default behaviour), not the "
"flat channel session you asked for. Set "
"platforms.slack.extra.reply_in_thread: false to pair them.",
team_name,
)
except Exception:
pass

def _resolve_thread_ts(
self,
reply_to: Optional[str] = None,
Expand Down Expand Up @@ -3159,6 +3094,11 @@ async def _handle_slack_message(self, event: dict) -> None:
user_id=user_id,
user_name=user_name,
thread_id=thread_ts,
# Slack Workflow Builder / app posts arrive as
# subtype=bot_message with user=None; flag them so the
# gateway SLACK_ALLOW_BOTS bypass can authorize them
# (they carry no user_id to match against the allowlist).
is_bot=bool(event.get("bot_id")) or event.get("subtype") == "bot_message",
)

# Per-channel ephemeral prompt
Expand Down
5 changes: 1 addition & 4 deletions scripts/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@
# Auto-extracted from noreply emails + manual overrides
AUTHOR_MAP = {
"259353979+testingbuddies24@users.noreply.github.com": "testingbuddies24", # PR #43192 salvage (strip orphan think-tag close tags in progressive gateway stream so a bare </think> whose open was dropped upstream can't leak to the user)
"shx_929@163.com": "Lazymonter", # PR #42914 salvage (retry launchd bootstrap after bootout on EIO for install/start instead of degrading to detached)
"96322396+WXBR@users.noreply.github.com": "WXBR", # PR #46183 salvage (persist recovered final_response at the finalize_turn chokepoint so recovery-path breaks don't drop the delivered assistant row)
"5848605+itenev@users.noreply.github.com": "itenev", # PR #22753 salvage (asyncify model-context resolution in gateway message path so blocking requests.get can't starve Discord heartbeats)
"arthur.zhang@ingenico.com": "arthurzhang", # PR #34718 salvage (redact Slack App-Level xapp- tokens in agent/redact.py + gateway/run.py)
"290873280+rrevenanttt@users.noreply.github.com": "rrevenanttt", # PR #40773 salvage (close hardline rm bypass via quoted paths and ${HOME} brace form)
Expand Down Expand Up @@ -179,7 +177,7 @@
"dkobi16@gmail.com": "Diyoncrz18",
"arnaud@nolimitdevelopment.com": "ali-nld",
"sswdarius@gmail.com": "necoweb3",
"3483421977@qq.com": "xy200303", # PR #40663 (approval shell-command-name deobfuscation)
"t.chen@aftership.com": "cypctlinux", # PR #52403 salvage (Slack bot/workflow auth before no-user-id guard)
"30854794+YLChen-007@users.noreply.github.com": "YLChen-007", # PR #26965 (approval remote command substitution)
"1078345+egilewski@users.noreply.github.com": "egilewski", # co-author, PR #40663
"peterhao@Peters-MacBook-Air.local": "pinguarmy",
Expand Down Expand Up @@ -222,7 +220,6 @@
"290859878+synapsesx@users.noreply.github.com": "synapsesx",
"157689911+itsflownium@users.noreply.github.com": "itsflownium",
"dirtyren@users.noreply.github.com": "dirtyren",
"tgmerritt@gmail.com": "tgmerritt", # PR #43553 salvage (parse vLLM's token-based output-cap error format so over-cap max_tokens 400s reduce the output cap instead of death-looping into compression)
"13277570+justin-cyhuang@users.noreply.github.com": "justin-cyhuang",
"agent@tranquil-flow.dev": "Tranquil-Flow",
"jason@hermes-jc": "jcjc81",
Expand Down
92 changes: 92 additions & 0 deletions tests/gateway/test_slack_bot_auth_bypass.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
"""Regression guard for Slack bot/workflow-sender authorization bypass.

Mirrors tests/gateway/test_feishu_bot_auth_bypass.py for Platform.SLACK.

Slack Workflow Builder posts (and other app/bot messages) arrive as
``subtype=bot_message`` with ``user=None``, so the SessionSource carries
``is_bot=True`` and ``user_id=None``. Without the #4466 bot bypass running
*before* the no-user-id guard, these senders are rejected at
``_is_user_authorized`` even when the operator enabled ``SLACK_ALLOW_BOTS`` --
the bug that makes @mentioning the bot from a Slack workflow do nothing.
"""

from __future__ import annotations

from types import SimpleNamespace

import pytest

from gateway.session import Platform, SessionSource


@pytest.fixture(autouse=True)
def _isolate_slack_env(monkeypatch):
for var in (
"SLACK_ALLOW_BOTS",
"SLACK_ALLOWED_USERS",
"SLACK_ALLOW_ALL_USERS",
"GATEWAY_ALLOW_ALL_USERS",
"GATEWAY_ALLOWED_USERS",
):
monkeypatch.delenv(var, raising=False)


def _make_bare_runner():
from gateway.run import GatewayRunner

runner = object.__new__(GatewayRunner)
runner.pairing_store = SimpleNamespace(is_approved=lambda *_a, **_kw: False)
return runner


def _make_slack_bot_source():
# Workflow Builder / app posts: subtype=bot_message, user=None.
return SessionSource(
platform=Platform.SLACK,
chat_id="C0123",
chat_type="group",
user_id=None,
user_name="",
is_bot=True,
)


def _make_slack_human_source(user_id="U_human"):
return SessionSource(
platform=Platform.SLACK,
chat_id="C0123",
chat_type="group",
user_id=user_id,
user_name="Human",
is_bot=False,
)


def test_slack_bot_authorized_when_allow_bots_all(monkeypatch):
runner = _make_bare_runner()
monkeypatch.setenv("SLACK_ALLOW_BOTS", "all")
assert runner._is_user_authorized(_make_slack_bot_source()) is True


def test_slack_bot_authorized_when_allow_bots_mentions(monkeypatch):
runner = _make_bare_runner()
monkeypatch.setenv("SLACK_ALLOW_BOTS", "mentions")
assert runner._is_user_authorized(_make_slack_bot_source()) is True


def test_slack_bot_denied_when_allow_bots_unset(monkeypatch):
# No SLACK_ALLOW_BOTS + no user_id => denied (no bypass, hits guard).
runner = _make_bare_runner()
assert runner._is_user_authorized(_make_slack_bot_source()) is False


def test_slack_bot_denied_when_allow_bots_none(monkeypatch):
runner = _make_bare_runner()
monkeypatch.setenv("SLACK_ALLOW_BOTS", "none")
assert runner._is_user_authorized(_make_slack_bot_source()) is False


def test_slack_human_unaffected_by_bot_bypass(monkeypatch):
runner = _make_bare_runner()
monkeypatch.setenv("SLACK_ALLOW_ALL_USERS", "true")
assert runner._is_user_authorized(_make_slack_human_source()) is True
Loading