From 3ecbb039e0f8399cc6c89a2eb0a2a2b335c6d263 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Apr 2026 13:57:44 -0600 Subject: [PATCH 1/8] Switch list quarantined media API to use max to_id instead of current --- synapse/rest/admin/media.py | 2 +- synapse/storage/databases/main/room.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index 5539b01af01..16e15e89651 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -249,7 +249,7 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: from_id = parse_integer(request, "from", default=0) limit = 100 # arbitrary; not enough to cause problems (hopefully) - to_id = await self.store.get_current_quarantined_media_stream_id() + to_id = await self.store.get_max_quarantined_media_stream_id() if to_id < from_id: # The caller is trying to get future data, which isn't possible. diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 0a9e69e1bf5..4fdaaa07388 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1302,13 +1302,13 @@ def _get_media_ids_by_user_txn( return local_media_ids - async def get_current_quarantined_media_stream_id(self) -> int: - """Gets the position of the quarantined media changes stream. + async def get_max_quarantined_media_stream_id(self) -> int: + """Gets the maximum position of the quarantined media changes stream. Returns: - int - the current stream ID + int - the maximum stream ID """ - return self._quarantined_media_changes_id_gen.get_current_token() + return self._quarantined_media_changes_id_gen.get_max_allocated_token() async def wait_for_quarantined_media_stream_id(self, target_id: int) -> bool: """Waits until the quarantined media changes stream reaches the given stream ID. From dd3abf808f306c3cee0bf1aa11702860a6142008 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Apr 2026 13:59:48 -0600 Subject: [PATCH 2/8] overlapping changelog --- changelog.d/19677.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/19677.feature diff --git a/changelog.d/19677.feature b/changelog.d/19677.feature new file mode 100644 index 00000000000..f4c8f042bf0 --- /dev/null +++ b/changelog.d/19677.feature @@ -0,0 +1 @@ +Add a ["Listing quarantined media changes" Admin API](https://element-hq.github.io/synapse/latest/admin_api/media_admin_api.html#listing-quarantined-media-changes) for retrieving a paginated record of when media became (un)quarantined. \ No newline at end of file From 0ffbec729b54f82abe01392d7c822a7c751fec73 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Apr 2026 14:04:23 -0600 Subject: [PATCH 3/8] await --- synapse/storage/databases/main/room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 4fdaaa07388..4f35e61ca44 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1308,7 +1308,7 @@ async def get_max_quarantined_media_stream_id(self) -> int: Returns: int - the maximum stream ID """ - return self._quarantined_media_changes_id_gen.get_max_allocated_token() + return await self._quarantined_media_changes_id_gen.get_max_allocated_token() async def wait_for_quarantined_media_stream_id(self, target_id: int) -> bool: """Waits until the quarantined media changes stream reaches the given stream ID. From 675d36d369af515b079bd882b360bcba6ed3bc22 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Apr 2026 14:34:36 -0600 Subject: [PATCH 4/8] Update synapse/rest/admin/media.py Co-authored-by: Eric Eastwood --- synapse/rest/admin/media.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index 16e15e89651..fc10bfa622c 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -255,7 +255,11 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: # The caller is trying to get future data, which isn't possible. raise SynapseError( HTTPStatus.BAD_REQUEST, - "The `from` position is ahead of the currently persisted position.", + "The `from` token is considered invalid because it includes stream positions " + "greater than the furthest persisted position across all of the workers." + "This indicates either a Synapse programming error (as we should never hand out " + "invalid future tokens) or a fabricated `from` token. If you've modified the token, " + "you can try paginating from the beginning again.", errcode=Codes.INVALID_PARAM, ) From a45672918902990e9a6036113bcb2f7c7e461b45 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Apr 2026 14:35:50 -0600 Subject: [PATCH 5/8] Restore to_id as current; Introduce max_id as max --- synapse/rest/admin/media.py | 5 +++-- synapse/storage/databases/main/room.py | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index fc10bfa622c..de1cc7fbe07 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -249,9 +249,9 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: from_id = parse_integer(request, "from", default=0) limit = 100 # arbitrary; not enough to cause problems (hopefully) - to_id = await self.store.get_max_quarantined_media_stream_id() - if to_id < from_id: + max_id = await self.store.get_max_quarantined_media_stream_id() + if from_id > max_id: # The caller is trying to get future data, which isn't possible. raise SynapseError( HTTPStatus.BAD_REQUEST, @@ -275,6 +275,7 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: errcode=Codes.UNKNOWN, ) + to_id = await self.store.get_current_quarantined_media_stream_id() changes = await self.store.get_quarantined_media_changes( from_id=from_id, to_id=to_id, diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 4f35e61ca44..047eebace4d 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1302,6 +1302,14 @@ def _get_media_ids_by_user_txn( return local_media_ids + async def get_current_quarantined_media_stream_id(self) -> int: + """Gets the position of the quarantined media changes stream. + + Returns: + int - the current stream ID + """ + return self._quarantined_media_changes_id_gen.get_current_token() + async def get_max_quarantined_media_stream_id(self) -> int: """Gets the maximum position of the quarantined media changes stream. From 49ad7a65efb9ab4e0bf963e26c36449e838744cb Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Apr 2026 14:46:15 -0600 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Eric Eastwood Co-authored-by: Travis Ralston --- synapse/rest/admin/media.py | 8 ++++++-- synapse/storage/databases/main/room.py | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index de1cc7fbe07..b7baa3dd46a 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -250,9 +250,13 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: from_id = parse_integer(request, "from", default=0) limit = 100 # arbitrary; not enough to cause problems (hopefully) - max_id = await self.store.get_max_quarantined_media_stream_id() + max_id = await self.store.get_max_allocated_quarantined_media_stream_id() if from_id > max_id: - # The caller is trying to get future data, which isn't possible. + # The caller is trying to get future data, which we don't allow because + # we know it's an invalid state that should never happen. We could + # wait until we reach the token but we might as well now waste our + # resources on that which is why `wait_for_quarantined_media_stream_id(...)` + # has assertions around this. raise SynapseError( HTTPStatus.BAD_REQUEST, "The `from` token is considered invalid because it includes stream positions " diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 047eebace4d..8dbecc16e44 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1310,8 +1310,8 @@ async def get_current_quarantined_media_stream_id(self) -> int: """ return self._quarantined_media_changes_id_gen.get_current_token() - async def get_max_quarantined_media_stream_id(self) -> int: - """Gets the maximum position of the quarantined media changes stream. + async def get_max_allocated_quarantined_media_stream_id(self) -> int: + """Gets the maximum allocated position of the quarantined media changes stream. Returns: int - the maximum stream ID From a134c7fbcb2bc13d38abd706953b6a48974faa22 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Apr 2026 14:55:43 -0600 Subject: [PATCH 7/8] Update synapse/rest/admin/media.py Co-authored-by: Eric Eastwood --- synapse/rest/admin/media.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index b7baa3dd46a..593ecd57fc0 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -254,7 +254,7 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: if from_id > max_id: # The caller is trying to get future data, which we don't allow because # we know it's an invalid state that should never happen. We could - # wait until we reach the token but we might as well now waste our + # wait until we reach the token but we might as well not waste our # resources on that which is why `wait_for_quarantined_media_stream_id(...)` # has assertions around this. raise SynapseError( From c94df03b0244645e4e2b92ff5420af4a2d73692f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 9 Apr 2026 14:56:49 -0600 Subject: [PATCH 8/8] Update synapse/rest/admin/media.py Co-authored-by: Eric Eastwood --- synapse/rest/admin/media.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index 593ecd57fc0..1633cca884a 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -250,6 +250,7 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: from_id = parse_integer(request, "from", default=0) limit = 100 # arbitrary; not enough to cause problems (hopefully) + # Validate the `from` token max_id = await self.store.get_max_allocated_quarantined_media_stream_id() if from_id > max_id: # The caller is trying to get future data, which we don't allow because