From 784d8dd2c24ed00e43e4b1e18660d1fc30fd1216 Mon Sep 17 00:00:00 2001 From: EloquentBrush0x <283442588+EloquentBrush0x@users.noreply.github.com> Date: Fri, 22 May 2026 00:44:40 +0300 Subject: [PATCH] fix(matrix): fail-closed approval reaction auth when MATRIX_ALLOWED_USERS is empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The _on_reaction approval handler used: if self._allowed_user_ids and sender not in self._allowed_user_ids: When MATRIX_ALLOWED_USERS is not configured, _allowed_user_ids is an empty set. The short-circuit on the empty set caused the deny block to never execute, allowing any Matrix room member to approve or deny tool calls via ✅/❎ reactions — even users that run.py's _is_user_authorized would reject for regular messages. Fix mirrors the Telegram _is_callback_user_authorized fix (commit 89d32052e, PR #28494): deny by default when no allowlist is configured, unless GATEWAY_ALLOW_ALL_USERS=true is explicitly set. --- gateway/platforms/matrix.py | 3 +- ...st_matrix_approval_reaction_fail_closed.py | 130 ++++++++++++++++++ 2 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 tests/gateway/test_matrix_approval_reaction_fail_closed.py diff --git a/gateway/platforms/matrix.py b/gateway/platforms/matrix.py index f7837a1f7..5c1cb9a18 100644 --- a/gateway/platforms/matrix.py +++ b/gateway/platforms/matrix.py @@ -2236,7 +2236,8 @@ class MatrixAdapter(BasePlatformAdapter): if prompt and not prompt.resolved: if room_id != prompt.chat_id: return - if self._allowed_user_ids and sender not in self._allowed_user_ids: + _allow_all = os.getenv("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"} + if not _allow_all and not (self._allowed_user_ids and sender in self._allowed_user_ids): logger.info( "Matrix: ignoring approval reaction from unauthorized user %s on %s", sender, reacts_to, diff --git a/tests/gateway/test_matrix_approval_reaction_fail_closed.py b/tests/gateway/test_matrix_approval_reaction_fail_closed.py new file mode 100644 index 000000000..c9b5277ee --- /dev/null +++ b/tests/gateway/test_matrix_approval_reaction_fail_closed.py @@ -0,0 +1,130 @@ +"""Tests for Matrix adapter fail-closed approval reaction auth. + +When MATRIX_ALLOWED_USERS is not configured, _on_reaction must deny +approval reactions by default unless GATEWAY_ALLOW_ALL_USERS=true. +Mirrors the Telegram _is_callback_user_authorized fix (commit 89d32052e, +PR #28494). +""" + +import asyncio +import sys +import types +from collections import deque +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +import pytest + + +# --------------------------------------------------------------------------- +# Stub mautrix so gateway.platforms.matrix can be imported without the SDK. +# --------------------------------------------------------------------------- + +def _stub_mautrix(): + stub = types.ModuleType("mautrix") + for sub in ("mautrix.types", "mautrix.client", "mautrix.client.api", + "mautrix.errors", "mautrix.crypto", "mautrix.util", + "mautrix.util.config"): + sys.modules.setdefault(sub, types.ModuleType(sub)) + sys.modules.setdefault("mautrix", stub) + m = sys.modules["mautrix.types"] + for attr in ( + "ContentURI", "EventID", "EventType", "PaginationDirection", + "PresenceState", "RoomCreatePreset", "RoomID", "SyncToken", + "TrustState", "UserID", + ): + if not hasattr(m, attr): + setattr(m, attr, str) + + +_stub_mautrix() + +from gateway.platforms.matrix import MatrixAdapter, _MatrixApprovalPrompt # noqa: E402 + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_adapter(allowed_user_ids=None): + """Construct a MatrixAdapter with only the state needed by _on_reaction.""" + adapter = object.__new__(MatrixAdapter) + adapter._user_id = "@bot:matrix.org" + adapter._allowed_user_ids = set(allowed_user_ids) if allowed_user_ids else set() + adapter._approval_reaction_map = {"✅": "once", "❎": "deny"} + adapter._approval_prompts_by_event = {} + adapter._approval_prompt_by_session = {} + adapter._processed_events = deque(maxlen=512) + adapter._processed_events_set = set() + return adapter + + +def _make_event(sender, reacts_to, key="✅"): + """Minimal Matrix reaction event.""" + return SimpleNamespace( + sender=sender, + event_id=f"$reaction-{sender.split(':')[0]}", + room_id="!testroom:matrix.org", + content={"m.relates_to": {"event_id": reacts_to, "key": key}}, + ) + + +def _make_prompt(chat_id="!testroom:matrix.org"): + return _MatrixApprovalPrompt( + session_key="session-abc", + chat_id=chat_id, + message_id="$prompt-event-1", + ) + + +def _run(adapter, event): + """Run _on_reaction and return whether the prompt was resolved.""" + prompt_event_id = "$prompt-event-1" + prompt = _make_prompt() + adapter._approval_prompts_by_event[prompt_event_id] = prompt + adapter._redact_bot_approval_reactions = AsyncMock() + + fake_approval = types.ModuleType("tools.approval") + fake_approval.resolve_gateway_approval = lambda session_key, choice: 1 + with patch.dict(sys.modules, {"tools.approval": fake_approval}): + asyncio.run(adapter._on_reaction(event)) + + return prompt.resolved + + +# --------------------------------------------------------------------------- +# Test class +# --------------------------------------------------------------------------- + +class TestApprovalReactionFailClosed: + """_on_reaction approval auth must be fail-closed (parity with Telegram).""" + + def test_no_allowlist_no_allow_all_denies(self, monkeypatch): + """No MATRIX_ALLOWED_USERS + no GATEWAY_ALLOW_ALL_USERS → deny.""" + monkeypatch.delenv("MATRIX_ALLOWED_USERS", raising=False) + monkeypatch.delenv("GATEWAY_ALLOW_ALL_USERS", raising=False) + adapter = _make_adapter(allowed_user_ids=None) + event = _make_event("@stranger:matrix.org", "$prompt-event-1") + assert _run(adapter, event) is False + + def test_no_allowlist_allow_all_permits(self, monkeypatch): + """No MATRIX_ALLOWED_USERS + GATEWAY_ALLOW_ALL_USERS=true → allow.""" + monkeypatch.delenv("MATRIX_ALLOWED_USERS", raising=False) + monkeypatch.setenv("GATEWAY_ALLOW_ALL_USERS", "true") + adapter = _make_adapter(allowed_user_ids=None) + event = _make_event("@anyone:matrix.org", "$prompt-event-1") + assert _run(adapter, event) is True + + def test_listed_sender_permits(self, monkeypatch): + """Sender in MATRIX_ALLOWED_USERS → allow.""" + monkeypatch.delenv("GATEWAY_ALLOW_ALL_USERS", raising=False) + adapter = _make_adapter(allowed_user_ids=["@alice:matrix.org"]) + event = _make_event("@alice:matrix.org", "$prompt-event-1") + assert _run(adapter, event) is True + + def test_unlisted_sender_denies(self, monkeypatch): + """Sender not in MATRIX_ALLOWED_USERS → deny.""" + monkeypatch.delenv("GATEWAY_ALLOW_ALL_USERS", raising=False) + adapter = _make_adapter(allowed_user_ids=["@alice:matrix.org"]) + event = _make_event("@mallory:matrix.org", "$prompt-event-1") + assert _run(adapter, event) is False