Make check_event_allowed module API callback not fail open (accept events) when an exception is raised

changelog.d/11033.bugfix

@@ -0,0 +1 @@
+Do not accept events if a third-party rule module API callback raises an exception.

docs/modules/third_party_rules_callbacks.md

@@ -41,6 +41,14 @@ event with new data by returning the new event's data as a dictionary. In order
 that, it is recommended the module calls `event.get_dict()` to get the current event as a
 dictionary, and modify the returned dictionary accordingly.
 
+If `check_event_allowed` raises an exception, the module is assumed to have failed.
+The event will not be accepted but is not treated as explicitly rejected, either.
+An HTTP request causing the module check will likely result in a 500 Internal
+Server Error.
+
+When the boolean returned by the module is `False`, the event is rejected.
+(Module developers should not use exceptions for rejection.)
+
 Note that replacing the event only works for events sent by local users, not for events
 received over federation.
 

synapse/api/errors.py

@@ -619,3 +619,10 @@ class ShadowBanError(Exception):
 
     This should be caught and a proper "fake" success response sent to the user.
     """
+
+
+class ModuleFailedException(Exception):
+    """
+    Raised when a module API callback fails, for example because it raised an
+    exception.
+    """

synapse/events/third_party_rules.py

@@ -14,7 +14,7 @@
 import logging
 from typing import TYPE_CHECKING, Awaitable, Callable, List, Optional, Tuple
 
-from synapse.api.errors import SynapseError
+from synapse.api.errors import ModuleFailedException, SynapseError
 from synapse.events import EventBase
 from synapse.events.snapshot import EventContext
 from synapse.types import Requester, StateMap
@@ -218,8 +218,9 @@ async def check_event_allowed(
             try:
                 res, replacement_data = await callback(event, state_events)
             except Exception as e:
-                logger.warning("Failed to run module API callback %s: %s", callback, e)
-                continue
+                raise ModuleFailedException(
+                    "Failed to run `check_event_allowed` module API callback"
+                ) from e
 
             # Return if the event shouldn't be allowed or if the module came up with a
             # replacement dict for the event.