From ca9234a9eba4fba02d8d50e5d5eff079bfaf0ebd Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Tue, 8 Mar 2022 08:09:11 -0500
Subject: [PATCH] Do not return allowed_room_ids from /hierarchy response.
 (#12175)

This field is only to be used in the Server-Server API, and not the
Client-Server API, but was being leaked when a federation response
was used in the /hierarchy API.
---
 changelog.d/12175.bugfix            |  1 +
 synapse/handlers/room_summary.py    | 15 +++++++++++++--
 tests/handlers/test_room_summary.py |  3 +++
 3 files changed, 17 insertions(+), 2 deletions(-)
 create mode 100644 changelog.d/12175.bugfix

diff --git a/changelog.d/12175.bugfix b/changelog.d/12175.bugfix
new file mode 100644
index 000000000..881cb9b76
--- /dev/null
+++ b/changelog.d/12175.bugfix
@@ -0,0 +1 @@
+Fix a bug where non-standard information was returned from the `/hierarchy` API. Introduced in Synapse v1.41.0.
diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py
index 3979cbba7..486145f48 100644
--- a/synapse/handlers/room_summary.py
+++ b/synapse/handlers/room_summary.py
@@ -295,7 +295,7 @@ class RoomSummaryHandler:
             # inaccessible to the requesting user.
             if room_entry:
                 # Add the room (including the stripped m.space.child events).
-                rooms_result.append(room_entry.as_json())
+                rooms_result.append(room_entry.as_json(for_client=True))
 
                 # If this room is not at the max-depth, check if there are any
                 # children to process.
@@ -843,14 +843,25 @@ class _RoomEntry:
     # This may not include all children.
     children_state_events: Sequence[JsonDict] = ()
 
-    def as_json(self) -> JsonDict:
+    def as_json(self, for_client: bool = False) -> JsonDict:
         """
         Returns a JSON dictionary suitable for the room hierarchy endpoint.
 
         It returns the room summary including the stripped m.space.child events
         as a sub-key.
+
+        Args:
+            for_client: If true, any server-server only fields are stripped from
+                the result.
+
         """
         result = dict(self.room)
+
+        # Before returning to the client, remove the allowed_room_ids key, if it
+        # exists.
+        if for_client:
+            result.pop("allowed_room_ids", False)
+
         result["children_state"] = self.children_state_events
         return result
 
diff --git a/tests/handlers/test_room_summary.py b/tests/handlers/test_room_summary.py
index cff07a897..d37292ce1 100644
--- a/tests/handlers/test_room_summary.py
+++ b/tests/handlers/test_room_summary.py
@@ -172,6 +172,9 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
         result_room_ids = []
         result_children_ids = []
         for result_room in result["rooms"]:
+            # Ensure federation results are not leaking over the client-server API.
+            self.assertNotIn("allowed_room_ids", result_room)
+
             result_room_ids.append(result_room["room_id"])
             result_children_ids.append(
                 [