From 5b8b45a16d72a748d9a401b816c1c422c7007347 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 29 Jul 2025 12:57:33 -0600 Subject: [PATCH] Allow admins to see policy server-flagged events (#18585) --- changelog.d/18585.feature | 1 + .../admin_api/client_server_api_extensions.md | 42 ++++ rust/src/events/internal_metadata.rs | 24 ++ synapse/events/utils.py | 7 +- synapse/federation/federation_base.py | 1 + synapse/handlers/message.py | 3 + synapse/storage/admin_client_config.py | 4 + synapse/synapse_rust/events.pyi | 3 + synapse/visibility.py | 26 ++- tests/events/test_utils.py | 26 +++ tests/rest/client/test_relations.py | 10 +- tests/test_utils/event_injection.py | 7 + tests/test_visibility.py | 215 +++++++++++++++++- 13 files changed, 356 insertions(+), 13 deletions(-) create mode 100644 changelog.d/18585.feature diff --git a/changelog.d/18585.feature b/changelog.d/18585.feature new file mode 100644 index 0000000000..18be189ab0 --- /dev/null +++ b/changelog.d/18585.feature @@ -0,0 +1 @@ +When admins enable themselves to see soft-failed events, they will also see if the cause is due to the policy server flagging them as spam via `unsigned`. \ No newline at end of file diff --git a/docs/admin_api/client_server_api_extensions.md b/docs/admin_api/client_server_api_extensions.md index 9cf74b23eb..08fac6289b 100644 --- a/docs/admin_api/client_server_api_extensions.md +++ b/docs/admin_api/client_server_api_extensions.md @@ -22,4 +22,46 @@ To receive soft failed events in APIs like `/sync` and `/messages`, set `return_ to `true` in the admin client config. When `false`, the normal behaviour of these endpoints is to exclude soft failed events. +**Note**: If the policy server flagged the event as spam and that caused soft failure, that will be indicated +in the event's `unsigned` content like so: + +```json +{ + "type": "m.room.message", + "other": "event_fields_go_here", + "unsigned": { + "io.element.synapse.soft_failed": true, + "io.element.synapse.policy_server_spammy": true + } +} +``` + Default: `false` + +## See events marked spammy by policy servers + +Learn more about policy servers from [MSC4284](https://github.com/matrix-org/matrix-spec-proposals/pull/4284). + +Similar to `return_soft_failed_events`, clients logged in with admin accounts can see events which were +flagged by the policy server as spammy (and thus soft failed) by setting `return_policy_server_spammy_events` +to `true`. + +`return_policy_server_spammy_events` may be `true` while `return_soft_failed_events` is `false` to only see +policy server-flagged events. When `return_soft_failed_events` is `true` however, `return_policy_server_spammy_events` +is always `true`. + +Events which were flagged by the policy will be flagged as `io.element.synapse.policy_server_spammy` in the +event's `unsigned` content, like so: + +```json +{ + "type": "m.room.message", + "other": "event_fields_go_here", + "unsigned": { + "io.element.synapse.soft_failed": true, + "io.element.synapse.policy_server_spammy": true + } +} +``` + +Default: `true` if `return_soft_failed_events` is `true`, otherwise `false` diff --git a/rust/src/events/internal_metadata.rs b/rust/src/events/internal_metadata.rs index eeb6074c10..4711fc540f 100644 --- a/rust/src/events/internal_metadata.rs +++ b/rust/src/events/internal_metadata.rs @@ -54,6 +54,7 @@ enum EventInternalMetadataData { RecheckRedaction(bool), SoftFailed(bool), ProactivelySend(bool), + PolicyServerSpammy(bool), Redacted(bool), TxnId(Box), TokenId(i64), @@ -96,6 +97,13 @@ impl EventInternalMetadataData { .to_owned() .into_any(), ), + EventInternalMetadataData::PolicyServerSpammy(o) => ( + pyo3::intern!(py, "policy_server_spammy"), + o.into_pyobject(py) + .unwrap_infallible() + .to_owned() + .into_any(), + ), EventInternalMetadataData::Redacted(o) => ( pyo3::intern!(py, "redacted"), o.into_pyobject(py) @@ -155,6 +163,11 @@ impl EventInternalMetadataData { .extract() .with_context(|| format!("'{key_str}' has invalid type"))?, ), + "policy_server_spammy" => EventInternalMetadataData::PolicyServerSpammy( + value + .extract() + .with_context(|| format!("'{key_str}' has invalid type"))?, + ), "redacted" => EventInternalMetadataData::Redacted( value .extract() @@ -427,6 +440,17 @@ impl EventInternalMetadata { set_property!(self, ProactivelySend, obj); } + #[getter] + fn get_policy_server_spammy(&self) -> PyResult { + Ok(get_property_opt!(self, PolicyServerSpammy) + .copied() + .unwrap_or(false)) + } + #[setter] + fn set_policy_server_spammy(&mut self, obj: bool) { + set_property!(self, PolicyServerSpammy, obj); + } + #[getter] fn get_redacted(&self) -> PyResult { let bool = get_property!(self, Redacted)?; diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 02ffe2c95e..cd7d3e6687 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -538,8 +538,11 @@ def serialize_event( d["content"] = dict(d["content"]) d["content"]["redacts"] = e.redacts - if config.include_admin_metadata and e.internal_metadata.is_soft_failed(): - d["unsigned"]["io.element.synapse.soft_failed"] = True + if config.include_admin_metadata: + if e.internal_metadata.is_soft_failed(): + d["unsigned"]["io.element.synapse.soft_failed"] = True + if e.internal_metadata.policy_server_spammy: + d["unsigned"]["io.element.synapse.policy_server_spammy"] = True only_event_fields = config.only_event_fields if only_event_fields: diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py index 8d1e156dab..c6be60ac78 100644 --- a/synapse/federation/federation_base.py +++ b/synapse/federation/federation_base.py @@ -174,6 +174,7 @@ class FederationBase: "Event not allowed by policy server, soft-failing %s", pdu.event_id ) pdu.internal_metadata.soft_failed = True + pdu.internal_metadata.policy_server_spammy = True # Note: we don't redact the event so admins can inspect the event after the # fact. Other processes may redact the event, but that won't be applied to # the database copy of the event until the server's config requires it. diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index c46b734af5..36376e2679 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1101,6 +1101,9 @@ class EventCreationHandler: policy_allowed = await self._policy_handler.is_event_allowed(event) if not policy_allowed: + # We shouldn't need to set the metadata because the raise should + # cause the request to be denied, but just in case: + event.internal_metadata.policy_server_spammy = True logger.warning( "Event not allowed by policy server, rejecting %s", event.event_id, diff --git a/synapse/storage/admin_client_config.py b/synapse/storage/admin_client_config.py index 4359721a6d..07acddc660 100644 --- a/synapse/storage/admin_client_config.py +++ b/synapse/storage/admin_client_config.py @@ -15,8 +15,12 @@ class AdminClientConfig: # `unsigned` portion of the event to inform clients that the event # is soft-failed. self.return_soft_failed_events: bool = False + self.return_policy_server_spammy_events: bool = False if account_data: self.return_soft_failed_events = account_data.get( "return_soft_failed_events", False ) + self.return_policy_server_spammy_events = account_data.get( + "return_policy_server_spammy_events", self.return_soft_failed_events + ) diff --git a/synapse/synapse_rust/events.pyi b/synapse/synapse_rust/events.pyi index 7d3422572d..a82211283b 100644 --- a/synapse/synapse_rust/events.pyi +++ b/synapse/synapse_rust/events.pyi @@ -33,6 +33,9 @@ class EventInternalMetadata: proactively_send: bool redacted: bool + policy_server_spammy: bool + """whether the policy server indicated that this event is spammy""" + txn_id: str """The transaction ID, if it was set when the event was created.""" token_id: int diff --git a/synapse/visibility.py b/synapse/visibility.py index 280bcaa0ff..44659a4963 100644 --- a/synapse/visibility.py +++ b/synapse/visibility.py @@ -117,13 +117,29 @@ async def filter_events_for_client( # We copy the events list to guarantee any modifications we make will only # happen within the function. events_before_filtering = events.copy() + # Default case is to *exclude* soft-failed events + events = [e for e in events if not e.internal_metadata.is_soft_failed()] client_config = await storage.main.get_admin_client_config_for_user(user_id) - if not ( - filter_send_to_client - and client_config.return_soft_failed_events - and await storage.main.is_server_admin(UserID.from_string(user_id)) + if filter_send_to_client and await storage.main.is_server_admin( + UserID.from_string(user_id) ): - events = [e for e in events if not e.internal_metadata.is_soft_failed()] + if client_config.return_soft_failed_events: + # The user has requested that all events be included, so do that. + # We copy the list for mutation safety. + events = events_before_filtering.copy() + elif client_config.return_policy_server_spammy_events: + # Include events that were soft failed by a policy server (marked spammy), + # but exclude all other soft failed events. We also want to include all + # not-soft-failed events, per usual operation. + events = [ + e + for e in events_before_filtering + if not e.internal_metadata.is_soft_failed() + or e.internal_metadata.policy_server_spammy + ] + # else - no change in behaviour; use default case + # else - no change in behaviour; use default case + if len(events_before_filtering) != len(events): if filtered_event_logger.isEnabledFor(logging.DEBUG): filtered_event_logger.debug( diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index 1dc6004b35..c6ebefbf38 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -822,6 +822,32 @@ class SerializeEventTestCase(stdlib_unittest.TestCase): "unsigned": {"io.element.synapse.soft_failed": True}, }, ) + self.assertEqual( + self.serialize( + MockEvent( + type="foo", + event_id="test", + room_id="!foo:bar", + content={"foo": "bar"}, + internal_metadata={ + "soft_failed": True, + "policy_server_spammy": True, + }, + ), + [], + True, + ), + { + "type": "foo", + "event_id": "test", + "room_id": "!foo:bar", + "content": {"foo": "bar"}, + "unsigned": { + "io.element.synapse.soft_failed": True, + "io.element.synapse.policy_server_spammy": True, + }, + }, + ) def test_make_serialize_config_for_admin_retains_other_fields(self) -> None: non_default_config = SerializeEventConfig( diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py index 548aa15443..561da0553b 100644 --- a/tests/rest/client/test_relations.py +++ b/tests/rest/client/test_relations.py @@ -1181,7 +1181,7 @@ class BundledAggregationsTestCase(BaseRelationsTestCase): bundled_aggregations, ) - self._test_bundled_aggregations(RelationTypes.REFERENCE, assert_annotations, 8) + self._test_bundled_aggregations(RelationTypes.REFERENCE, assert_annotations, 9) def test_thread(self) -> None: """ @@ -1226,21 +1226,21 @@ class BundledAggregationsTestCase(BaseRelationsTestCase): # The "user" sent the root event and is making queries for the bundled # aggregations: they have participated. - self._test_bundled_aggregations(RelationTypes.THREAD, _gen_assert(True), 9) + self._test_bundled_aggregations(RelationTypes.THREAD, _gen_assert(True), 10) # The "user2" sent replies in the thread and is making queries for the # bundled aggregations: they have participated. # # Note that this re-uses some cached values, so the total number of # queries is much smaller. self._test_bundled_aggregations( - RelationTypes.THREAD, _gen_assert(True), 6, access_token=self.user2_token + RelationTypes.THREAD, _gen_assert(True), 7, access_token=self.user2_token ) # A user with no interactions with the thread: they have not participated. user3_id, user3_token = self._create_user("charlie") self.helper.join(self.room, user=user3_id, tok=user3_token) self._test_bundled_aggregations( - RelationTypes.THREAD, _gen_assert(False), 6, access_token=user3_token + RelationTypes.THREAD, _gen_assert(False), 7, access_token=user3_token ) def test_thread_with_bundled_aggregations_for_latest(self) -> None: @@ -1287,7 +1287,7 @@ class BundledAggregationsTestCase(BaseRelationsTestCase): bundled_aggregations["latest_event"].get("unsigned"), ) - self._test_bundled_aggregations(RelationTypes.THREAD, assert_thread, 9) + self._test_bundled_aggregations(RelationTypes.THREAD, assert_thread, 10) def test_nested_thread(self) -> None: """ diff --git a/tests/test_utils/event_injection.py b/tests/test_utils/event_injection.py index 35b3245708..c1eaf9a575 100644 --- a/tests/test_utils/event_injection.py +++ b/tests/test_utils/event_injection.py @@ -105,6 +105,13 @@ async def create_event( builder, prev_event_ids=prev_event_ids ) + # Copy over writable internal_metadata, if set + # Dev note: This isn't everything that's writable. `for k,v` doesn't work here :( + if kwargs.get("internal_metadata", {}).get("soft_failed", False): + event.internal_metadata.soft_failed = True + if kwargs.get("internal_metadata", {}).get("policy_server_spammy", False): + event.internal_metadata.policy_server_spammy = True + context = await unpersisted_context.persist(event) return event, context diff --git a/tests/test_visibility.py b/tests/test_visibility.py index 89cbe4e54b..285e28e0f9 100644 --- a/tests/test_visibility.py +++ b/tests/test_visibility.py @@ -21,7 +21,9 @@ import logging from typing import Optional from unittest.mock import patch -from synapse.api.constants import EventUnsignedContentFields +from twisted.test.proto_helpers import MemoryReactor + +from synapse.api.constants import AccountDataTypes, EventUnsignedContentFields from synapse.api.room_versions import RoomVersions from synapse.events import EventBase, make_event_from_dict from synapse.events.snapshot import EventContext @@ -29,6 +31,7 @@ from synapse.rest import admin from synapse.rest.client import login, room from synapse.server import HomeServer from synapse.types import create_requester +from synapse.util import Clock from synapse.visibility import filter_events_for_client, filter_events_for_server from tests import unittest @@ -272,6 +275,210 @@ class FilterEventsForServerTestCase(unittest.HomeserverTestCase): return event +class FilterEventsForServerAdminsTestCase(HomeserverTestCase): + servlets = [ + admin.register_servlets, + login.register_servlets, + room.register_servlets, + ] + + def prepare( + self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer + ) -> None: + self.register_user("admin", "password", admin=True) + self.tok = self.login("admin", "password") + self.room_id = self.helper.create_room_as("admin", tok=self.tok) + self.get_success( + inject_visibility_event(self.hs, self.room_id, "@admin:test", "joined") + ) + self.regular_event = self.get_success( + inject_message_event(self.hs, self.room_id, "@admin:test", body="regular") + ) + self.soft_failed_event = self.get_success( + inject_message_event( + self.hs, + self.room_id, + "@admin:test", + body="soft failed", + soft_failed=True, + ) + ) + self.spammy_event = self.get_success( + inject_message_event( + self.hs, + self.room_id, + "@admin:test", + body="spammy", + soft_failed=True, + policy_server_spammy=True, + ) + ) + + def test_normal_operation_as_admin(self) -> None: + # `filter_events_for_client` shouldn't include soft failed events by default + # for admins. + + # Reload events from DB + events_to_filter = [ + self.get_success( + self.hs.get_storage_controllers().main.get_event( + e.event_id, + get_prev_content=True, + ) + ) + for e in [self.regular_event, self.soft_failed_event] + ] + + # Do filter & assert + filtered_events = self.get_success( + filter_events_for_client( + self.hs.get_storage_controllers(), + "@admin:test", + events_to_filter, + ) + ) + self.assertEqual( + [e.event_id for e in [self.regular_event]], + [e.event_id for e in filtered_events], + ) + + def test_see_soft_failed_events(self) -> None: + # `filter_events_for_client` should include soft failed events when configured + + # Reload events from DB + events_to_filter = [ + self.get_success( + self.hs.get_storage_controllers().main.get_event( + e.event_id, + get_prev_content=True, + ) + ) + for e in [self.regular_event, self.soft_failed_event] + ] + + # Inject client config + self.get_success( + self.hs.get_account_data_handler().add_account_data_for_user( + "@admin:test", + AccountDataTypes.SYNAPSE_ADMIN_CLIENT_CONFIG, + {"return_soft_failed_events": True}, + ) + ) + + # Sanity check + self.assertEqual(True, events_to_filter[1].internal_metadata.soft_failed) + + # Do filter & assert + filtered_events = self.get_success( + filter_events_for_client( + self.hs.get_storage_controllers(), + "@admin:test", + events_to_filter, + ) + ) + self.assertEqual( + [e.event_id for e in [self.regular_event, self.soft_failed_event]], + [e.event_id for e in filtered_events], + ) + + def test_see_policy_server_spammy_events(self) -> None: + # `filter_events_for_client` should include policy server-flagged events, but + # not other soft-failed events, when asked. + + # Reload events from DB + events_to_filter = [ + self.get_success( + self.hs.get_storage_controllers().main.get_event( + e.event_id, + get_prev_content=True, + ) + ) + for e in [self.regular_event, self.soft_failed_event, self.spammy_event] + ] + + # Inject client config + self.get_success( + self.hs.get_account_data_handler().add_account_data_for_user( + "@admin:test", + AccountDataTypes.SYNAPSE_ADMIN_CLIENT_CONFIG, + { + "return_soft_failed_events": False, + "return_policy_server_spammy_events": True, + }, + ) + ) + + # Sanity checks + self.assertEqual(True, events_to_filter[1].internal_metadata.soft_failed) + self.assertEqual(True, events_to_filter[2].internal_metadata.soft_failed) + self.assertEqual( + True, events_to_filter[2].internal_metadata.policy_server_spammy + ) + + # Do filter & assert + filtered_events = self.get_success( + filter_events_for_client( + self.hs.get_storage_controllers(), + "@admin:test", + events_to_filter, + ) + ) + self.assertEqual( + [e.event_id for e in [self.regular_event, self.spammy_event]], + [e.event_id for e in filtered_events], + ) + + def test_see_soft_failed_and_policy_server_spammy_events(self) -> None: + # `filter_events_for_client` should include both types of soft failed events + # when configured. + + # Reload events from DB + events_to_filter = [ + self.get_success( + self.hs.get_storage_controllers().main.get_event( + e.event_id, + get_prev_content=True, + ) + ) + for e in [self.regular_event, self.soft_failed_event, self.spammy_event] + ] + + # Inject client config + self.get_success( + self.hs.get_account_data_handler().add_account_data_for_user( + "@admin:test", + AccountDataTypes.SYNAPSE_ADMIN_CLIENT_CONFIG, + { + "return_soft_failed_events": True, + "return_policy_server_spammy_events": True, + }, + ) + ) + + # Sanity checks + self.assertEqual(True, events_to_filter[1].internal_metadata.soft_failed) + self.assertEqual(True, events_to_filter[2].internal_metadata.soft_failed) + self.assertEqual( + True, events_to_filter[2].internal_metadata.policy_server_spammy + ) + + # Do filter & assert + filtered_events = self.get_success( + filter_events_for_client( + self.hs.get_storage_controllers(), + "@admin:test", + events_to_filter, + ) + ) + self.assertEqual( + [ + e.event_id + for e in [self.regular_event, self.soft_failed_event, self.spammy_event] + ], + [e.event_id for e in filtered_events], + ) + + class FilterEventsForClientTestCase(HomeserverTestCase): servlets = [ admin.register_servlets, @@ -487,6 +694,8 @@ async def inject_message_event( room_id: str, sender: str, body: Optional[str] = "testytest", + soft_failed: Optional[bool] = False, + policy_server_spammy: Optional[bool] = False, ) -> EventBase: return await inject_event( hs, @@ -494,4 +703,8 @@ async def inject_message_event( sender=sender, room_id=room_id, content={"body": body, "msgtype": "m.text"}, + internal_metadata={ + "soft_failed": soft_failed, + "policy_server_spammy": policy_server_spammy, + }, )