From 9d0c89bd04bc3ca0bd939feb397573ac958fc24e Mon Sep 17 00:00:00 2001 From: Jade Ellis Date: Tue, 23 Sep 2025 21:46:07 +0100 Subject: [PATCH] fix(v12): Create tombstone event on room upgrade --- src/api/client/room/upgrade.rs | 60 +++++++++++++++++++++++++--- src/service/rooms/timeline/create.rs | 2 - 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/api/client/room/upgrade.rs b/src/api/client/room/upgrade.rs index fc87bd96..87044cdd 100644 --- a/src/api/client/room/upgrade.rs +++ b/src/api/client/room/upgrade.rs @@ -68,6 +68,33 @@ pub(crate) async fn upgrade_room_route( return Err!(Request(UserSuspended("You cannot perform this action while suspended."))); } + // First, check if the user has permission to upgrade the room (send tombstone + // event) + let old_room_state_lock = services.rooms.state.mutex.lock(&body.room_id).await; + + // Check tombstone permission by attempting to create (but not send) the event + // Note that this does internally call the policy server with a fake room ID, + // which may not be good? + let tombstone_test_result = services + .rooms + .timeline + .create_hash_and_sign_event( + PduBuilder::state(StateKey::new(), &RoomTombstoneEventContent { + body: "This room has been replaced".to_owned(), + replacement_room: RoomId::new(services.globals.server_name()), + }), + sender_user, + Some(&body.room_id), + &old_room_state_lock, + ) + .await; + + if let Err(_e) = tombstone_test_result { + return Err!(Request(Forbidden("User does not have permission to upgrade this room."))); + } + + drop(old_room_state_lock); + // Create a replacement room let room_features = RoomVersion::new(&body.new_version)?; let replacement_room: Option<&RoomId> = if room_features.room_ids_as_hashes { @@ -86,20 +113,18 @@ pub(crate) async fn upgrade_room_route( .get_or_create_shortroomid(replacement_room_tmp) .await; - let tombstone_event_id = if room_features.room_ids_as_hashes { - None - } else { + // For pre-v12 rooms, send tombstone before creating replacement room + let tombstone_event_id = if !room_features.room_ids_as_hashes { let state_lock = services.rooms.state.mutex.lock(&body.room_id).await; // Send a m.room.tombstone event to the old room to indicate that it is not - // intended to be used any further Fail if the sender does not have the required - // permissions + // intended to be used any further let tombstone_event_id = services .rooms .timeline .build_and_append_pdu( PduBuilder::state(StateKey::new(), &RoomTombstoneEventContent { body: "This room has been replaced".to_owned(), - replacement_room: replacement_room.clone().unwrap().to_owned(), + replacement_room: replacement_room.unwrap().to_owned(), }), sender_user, Some(&body.room_id), @@ -109,6 +134,8 @@ pub(crate) async fn upgrade_room_route( // Change lock to replacement room drop(state_lock); Some(tombstone_event_id) + } else { + None }; let state_lock = services.rooms.state.mutex.lock(replacement_room_tmp).await; @@ -330,6 +357,27 @@ pub(crate) async fn upgrade_room_route( drop(state_lock); + // For v12 rooms, send tombstone AFTER creating replacement room + if room_features.room_ids_as_hashes { + let old_room_state_lock = services.rooms.state.mutex.lock(&body.room_id).await; + // For v12 rooms, no event reference in predecessor due to cyclic dependency - + // could best effort one maybe? + services + .rooms + .timeline + .build_and_append_pdu( + PduBuilder::state(StateKey::new(), &RoomTombstoneEventContent { + body: "This room has been replaced".to_owned(), + replacement_room: replacement_room.unwrap().to_owned(), + }), + sender_user, + Some(&body.room_id), + &old_room_state_lock, + ) + .await?; + drop(old_room_state_lock); + } + // Check if the old room has a space parent, and if so, whether we should update // it (m.space.parent, room_id) let parents = services diff --git a/src/service/rooms/timeline/create.rs b/src/service/rooms/timeline/create.rs index 1924bb9c..386e1f02 100644 --- a/src/service/rooms/timeline/create.rs +++ b/src/service/rooms/timeline/create.rs @@ -274,8 +274,6 @@ pub async fn create_hash_and_sign_event( pdu_json.insert("event_id".into(), CanonicalJsonValue::String(pdu.event_id.clone().into())); // Check with the policy server - // TODO(hydra): Skip this check for create events (why didnt we do this - // already?) if room_id.is_some() { trace!( "Checking event {} in room {} with policy server",