diff --git a/gateway/authz_mixin.py b/gateway/authz_mixin.py index bff02153a3a..a32200ad547 100644 --- a/gateway/authz_mixin.py +++ b/gateway/authz_mixin.py @@ -223,11 +223,28 @@ class GatewayAuthorizationMixin: # connector observed, never gateway-asserted). There is no local # RELAY_ALLOWED_USERS env allowlist to consult, and default-denying for # its absence is the bug this branch fixes. This is delegation to a - # trusted upstream, NOT a fail-open: it fires only for an adapter that - # explicitly sets authorization_is_upstream=True; every direct - # network-exposed adapter leaves it False and the env-allowlist - # default-deny below still applies unchanged. - if self._adapter_authorization_is_upstream(source.platform): + # trusted upstream, NOT a fail-open: it fires only for an event that was + # actually delivered over the authenticated relay WS (the transport + # stamps ``delivered_via_upstream_relay``), or whose platform's adapter + # explicitly declares ``authorization_is_upstream=True``; every direct + # network-exposed adapter leaves the flag False and its events unmarked, + # so the env-allowlist default-deny below still applies unchanged. + # + # The delivery marker is the PRIMARY signal: a relay *message* inbound + # carries the UNDERLYING platform (``source.platform`` == discord/…), + # NOT ``Platform.RELAY``, because that's what session-keying and egress + # need — so keying authz off ``source.platform`` would miss (the relay + # adapter is registered under ``Platform.RELAY``) and default-deny the + # user ("Unauthorized user on discord"). The adapter-flag check is + # retained for events whose ``source.platform`` IS ``Platform.RELAY`` + # (e.g. the interaction-passthrough path). + # ``is True`` (not just truthiness): the marker is a real bool on a + # SessionSource, and an explicit identity check refuses to authorize a + # non-bool stand-in (e.g. a MagicMock attribute auto-vivifies truthy in + # tests) — defensive against accidental fail-open. + if source.delivered_via_upstream_relay is True or self._adapter_authorization_is_upstream( + source.platform + ): return True user_id = source.user_id diff --git a/gateway/relay/ws_transport.py b/gateway/relay/ws_transport.py index 3532aa81e91..5d66da9045f 100644 --- a/gateway/relay/ws_transport.py +++ b/gateway/relay/ws_transport.py @@ -120,6 +120,14 @@ def _event_from_wire(raw: Dict[str, Any]) -> MessageEvent: guild_id=src.get("guild_id"), parent_chat_id=src.get("parent_chat_id"), message_id=src.get("message_id"), + # Authentic upstream-trust signal: this event arrived over the + # per-instance-authenticated relay WS, so the connector already resolved + # it to this instance's owner-bound author. ``platform`` is the + # UNDERLYING platform (e.g. discord), not ``relay`` — authz keys the + # upstream-trust decision off THIS flag, not off ``platform`` (which + # would miss because the relay adapter is registered under + # ``Platform.RELAY``). Stamped here, never read off the wire. + delivered_via_upstream_relay=True, ) try: msg_type = MessageType(raw.get("message_type", "text")) diff --git a/gateway/session.py b/gateway/session.py index 8ec5060c645..8f59d54951e 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -119,7 +119,20 @@ class SessionSource: # None => the gateway's active/default profile. Drives both session-key # namespacing and the per-turn config/credential scope. profile: Optional[str] = None - + + # Internal, wire-INVISIBLE trust signal: True when this event was delivered + # to the gateway over the per-instance-authenticated relay WebSocket (the + # Team Gateway connector). The connector authenticates the gateway's socket + # with a per-instance secret and resolves owner-only author bindings BEFORE + # delivering, so a relay-delivered event is already authorized as this + # instance's bound user. ``platform`` carries the UNDERLYING platform + # (e.g. ``discord``) for session-keying/egress, NOT ``relay`` — so authz + # must key the upstream-trust decision off THIS flag, not off ``platform``. + # Set locally by the relay transport (``ws_transport._event_from_wire``); + # deliberately excluded from ``to_dict``/``from_dict`` so a peer can never + # forge it across the wire or have it restored from persistence. + delivered_via_upstream_relay: bool = False + @property def description(self) -> str: """Human-readable description of the source.""" diff --git a/tests/gateway/test_relay_upstream_authz.py b/tests/gateway/test_relay_upstream_authz.py index bee1b60a6df..1fca02b1745 100644 --- a/tests/gateway/test_relay_upstream_authz.py +++ b/tests/gateway/test_relay_upstream_authz.py @@ -144,3 +144,102 @@ def test_upstream_authz_helper_false_for_unknown_platform(monkeypatch): # A platform with no registered adapter must not be treated as upstream-authz. assert runner._adapter_authorization_is_upstream(Platform.DISCORD) is False assert runner._adapter_authorization_is_upstream(None) is False + + +# --------------------------------------------------------------------------- +# The underlying-platform regression: a relay *message* inbound carries the +# UNDERLYING platform (source.platform == Platform.DISCORD), not Platform.RELAY, +# because the connector's wire payload sets platform="discord" and +# ws_transport._event_from_wire maps it straight onto SessionSource. The relay +# adapter is registered ONLY under Platform.RELAY, so keying upstream-authz off +# source.platform misses and the user hits default-deny ("Unauthorized user +# () on discord"). The authentic trust signal is that the event was +# delivered over the per-instance-authenticated relay WS — carried by +# source.delivered_via_upstream_relay, set by the relay transport, NOT by +# source.platform. +# --------------------------------------------------------------------------- + + +def test_relay_message_with_underlying_discord_platform_authorized(monkeypatch): + """The live message path: source.platform=DISCORD + relay-delivered marker. + + Reproduces the staging bug exactly — a relay-delivered Discord message whose + source.platform is the underlying "discord" (not "relay"). The relay adapter + is registered only under Platform.RELAY, so the OLD source.platform-keyed + check missed and default-denied. Authorization must come from the + relay-delivery marker instead. + """ + _clear_auth_env(monkeypatch) + runner, _ = _make_runner(platform=Platform.RELAY, authorization_is_upstream=True) + src = SessionSource( + platform=Platform.DISCORD, # underlying platform off the wire + user_id="267171776755269633", + chat_id="1400724139874058314", + user_name="rewbs", + chat_type="dm", + delivered_via_upstream_relay=True, + ) + assert runner._is_user_authorized(src) is True + + +def test_direct_discord_event_not_authorized_by_relay_presence(monkeypatch): + """A DIRECT Discord event must NOT be authorized just because a relay adapter + is registered (multiplexing gateway: direct Discord adapter + relay adapter). + + Without the delivery marker, the relay's upstream-authz must not leak onto a + direct Discord inbound — that would be a fail-open. Only events the relay + transport actually delivered carry delivered_via_upstream_relay=True. + """ + _clear_auth_env(monkeypatch) + runner, _ = _make_runner(platform=Platform.RELAY, authorization_is_upstream=True) + src = SessionSource( + platform=Platform.DISCORD, + user_id="999", + chat_id="456", + user_name="direct_discord_user", + chat_type="dm", + # delivered_via_upstream_relay defaults to False (direct delivery) + ) + assert runner._is_user_authorized(src) is False + + +def test_relay_delivery_marker_is_wire_invisible(): + """delivered_via_upstream_relay is an INTERNAL trust signal, never serialized. + + It must not appear in to_dict() (the wire/persistence surface) — it is set + locally by the relay transport from the authenticated socket, never trusted + off the wire. + """ + src = SessionSource( + platform=Platform.DISCORD, + chat_id="1", + user_id="2", + delivered_via_upstream_relay=True, + ) + assert "delivered_via_upstream_relay" not in src.to_dict() + # And it does not survive a wire round-trip (a peer can't forge it). + assert SessionSource.from_dict(src.to_dict()).delivered_via_upstream_relay is False + + +def test_event_from_wire_sets_relay_delivery_marker(): + """The relay transport stamps the marker on every event it rebuilds. + + This is the authentic injection point: _event_from_wire only runs for frames + that arrived over the per-instance-authenticated relay WS. + """ + from gateway.relay.ws_transport import _event_from_wire + + event = _event_from_wire( + { + "text": "hello!", + "source": { + "platform": "discord", + "chat_id": "123", + "chat_type": "dm", + "user_id": "267171776755269633", + "user_name": "rewbs", + }, + } + ) + assert event.source.platform is Platform.DISCORD + assert event.source.delivered_via_upstream_relay is True