fix(spaces): wire up enforcement hooks, fix deadlocks, validate spaces

- Add spawn_enforcement methods (handle_state_event_change,
  handle_space_child_change, handle_space_member_join) that run
  enforcement as background tasks to avoid recursive Send issues
- Expand append_pdu hook to trigger enforcement on role events,
  space child changes, and space member joins
- Fix deadlock risk in get_user_power_level and user_qualifies_for_room
  by dropping read guards before acquiring new ones
- Batch room_to_space writes in populate_space with a single write lock
- Add space type validation to all admin commands
- Fix PL rejection check to reject any change (!=) not just lowering (<)
- Fix sync_power_levels to also lower PLs for users who lost their roles

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
ember33 2026-03-18 10:13:47 +01:00
parent f2532df652
commit 40646eb4ba
4 changed files with 355 additions and 31 deletions

View file

@ -83,6 +83,12 @@ pub enum SpaceRolesCommand {
#[admin_command]
async fn list(&self, space: OwnedRoomOrAliasId) -> Result {
let space_id = self.services.rooms.alias.resolve(&space).await?;
if !matches!(
self.services.rooms.state_accessor.get_room_type(&space_id).await,
Ok(ruma::room::RoomType::Space)
) {
return self.write_str("Error: The specified room is not a Space.").await;
}
let roles_event_type = StateEventType::from("m.space.roles".to_owned());
let content: SpaceRolesEventContent = self
@ -119,6 +125,12 @@ async fn add(
power_level: Option<i64>,
) -> Result {
let space_id = self.services.rooms.alias.resolve(&space).await?;
if !matches!(
self.services.rooms.state_accessor.get_room_type(&space_id).await,
Ok(ruma::room::RoomType::Space)
) {
return self.write_str("Error: The specified room is not a Space.").await;
}
let roles_event_type = StateEventType::from("m.space.roles".to_owned());
let mut content: SpaceRolesEventContent = self
@ -159,6 +171,12 @@ async fn add(
#[admin_command]
async fn remove(&self, space: OwnedRoomOrAliasId, role_name: String) -> Result {
let space_id = self.services.rooms.alias.resolve(&space).await?;
if !matches!(
self.services.rooms.state_accessor.get_room_type(&space_id).await,
Ok(ruma::room::RoomType::Space)
) {
return self.write_str("Error: The specified room is not a Space.").await;
}
let roles_event_type = StateEventType::from("m.space.roles".to_owned());
let mut content: SpaceRolesEventContent = self
@ -199,6 +217,12 @@ async fn assign(
role_name: String,
) -> Result {
let space_id = self.services.rooms.alias.resolve(&space).await?;
if !matches!(
self.services.rooms.state_accessor.get_room_type(&space_id).await,
Ok(ruma::room::RoomType::Space)
) {
return self.write_str("Error: The specified room is not a Space.").await;
}
let member_event_type = StateEventType::from("m.space.role.member".to_owned());
let mut content: SpaceRoleMemberEventContent = self
@ -243,6 +267,12 @@ async fn revoke(
role_name: String,
) -> Result {
let space_id = self.services.rooms.alias.resolve(&space).await?;
if !matches!(
self.services.rooms.state_accessor.get_room_type(&space_id).await,
Ok(ruma::room::RoomType::Space)
) {
return self.write_str("Error: The specified room is not a Space.").await;
}
let member_event_type = StateEventType::from("m.space.role.member".to_owned());
let mut content: SpaceRoleMemberEventContent = self
@ -288,6 +318,12 @@ async fn require(
role_name: String,
) -> Result {
let space_id = self.services.rooms.alias.resolve(&space).await?;
if !matches!(
self.services.rooms.state_accessor.get_room_type(&space_id).await,
Ok(ruma::room::RoomType::Space)
) {
return self.write_str("Error: The specified room is not a Space.").await;
}
let room_event_type = StateEventType::from("m.space.role.room".to_owned());
let mut content: SpaceRoleRoomEventContent = self
@ -332,6 +368,12 @@ async fn unrequire(
role_name: String,
) -> Result {
let space_id = self.services.rooms.alias.resolve(&space).await?;
if !matches!(
self.services.rooms.state_accessor.get_room_type(&space_id).await,
Ok(ruma::room::RoomType::Space)
) {
return self.write_str("Error: The specified room is not a Space.").await;
}
let room_event_type = StateEventType::from("m.space.role.room".to_owned());
let mut content: SpaceRoleRoomEventContent = self
@ -372,6 +414,12 @@ async fn unrequire(
#[admin_command]
async fn user(&self, space: OwnedRoomOrAliasId, user_id: OwnedUserId) -> Result {
let space_id = self.services.rooms.alias.resolve(&space).await?;
if !matches!(
self.services.rooms.state_accessor.get_room_type(&space_id).await,
Ok(ruma::room::RoomType::Space)
) {
return self.write_str("Error: The specified room is not a Space.").await;
}
let user_roles = self.services.rooms.roles.user_roles.read().await;
let roles = user_roles
@ -401,6 +449,12 @@ async fn user(&self, space: OwnedRoomOrAliasId, user_id: OwnedUserId) -> Result
#[admin_command]
async fn room(&self, space: OwnedRoomOrAliasId, room_id: OwnedRoomId) -> Result {
let space_id = self.services.rooms.alias.resolve(&space).await?;
if !matches!(
self.services.rooms.state_accessor.get_room_type(&space_id).await,
Ok(ruma::room::RoomType::Space)
) {
return self.write_str("Error: The specified room is not a Space.").await;
}
let room_reqs = self.services.rooms.roles.room_requirements.read().await;
let requirements = room_reqs

View file

@ -303,6 +303,8 @@ pub async fn populate_space(&self, space_id: &RoomId) {
.insert(space_id.to_owned(), room_reqs_map);
// 4. Read all m.space.child state events → build room_to_space reverse index
let mut child_rooms: Vec<OwnedRoomId> = Vec::new();
self.services
.state_accessor
.state_keys_with_ids(shortstatehash, &StateEventType::SpaceChild)
@ -316,7 +318,6 @@ pub async fn populate_space(&self, space_id: &RoomId) {
.await
})
.ready_filter_map(|(state_key, pdu)| {
// Only index children that have a valid via list
if let Ok(content) = pdu.get_content::<SpaceChildEventContent>() {
if content.via.is_empty() {
return None;
@ -328,15 +329,18 @@ pub async fn populate_space(&self, space_id: &RoomId) {
Some(child_room_id)
})
.for_each(|child_room_id| {
let space_owned = space_id.to_owned();
async move {
self.room_to_space
.write()
.await
.insert(child_room_id, space_owned);
}
child_rooms.push(child_room_id);
async {}
})
.await;
// Single write lock for all children
{
let mut room_to_space = self.room_to_space.write().await;
for child_room_id in child_rooms {
room_to_space.insert(child_room_id, space_id.to_owned());
}
}
}
}
@ -348,10 +352,14 @@ pub async fn get_user_power_level(
space_id: &RoomId,
user_id: &UserId,
) -> Option<i64> {
let roles_map = self.roles.read().await;
let user_roles_map = self.user_roles.read().await;
let role_defs = roles_map.get(space_id)?;
let user_assigned = user_roles_map.get(space_id)?.get(user_id)?;
let role_defs = {
let guard = self.roles.read().await;
guard.get(space_id).cloned()?
};
let user_assigned = {
let guard = self.user_roles.read().await;
guard.get(space_id)?.get(user_id).cloned()?
};
user_assigned
.iter()
.filter_map(|role_name| role_defs.get(role_name)?.power_level)
@ -366,22 +374,28 @@ pub async fn user_qualifies_for_room(
room_id: &RoomId,
user_id: &UserId,
) -> bool {
let reqs = self.room_requirements.read().await;
let Some(space_reqs) = reqs.get(space_id) else {
return true;
let required = {
let guard = self.room_requirements.read().await;
let Some(space_reqs) = guard.get(space_id) else {
return true;
};
let Some(required) = space_reqs.get(room_id) else {
return true;
};
if required.is_empty() {
return true;
}
required.clone()
};
let Some(required) = space_reqs.get(room_id) else {
return true;
};
if required.is_empty() {
return true;
}
let user_map = self.user_roles.read().await;
let Some(space_users) = user_map.get(space_id) else {
return false;
};
let Some(user_assigned) = space_users.get(user_id) else {
return false;
let user_assigned = {
let guard = self.user_roles.read().await;
let Some(space_users) = guard.get(space_id) else {
return false;
};
let Some(assigned) = space_users.get(user_id) else {
return false;
};
assigned.clone()
};
required.iter().all(|r| user_assigned.contains(r))
}
@ -435,6 +449,14 @@ pub async fn sync_power_levels(&self, space_id: &RoomId, room_id: &RoomId) -> Re
.insert(user_id.clone(), space_pl_int);
changed = true;
}
} else {
// User has no space role PL — ensure they're at default
if let Some(current) = power_levels_content.users.get(user_id) {
if *current != power_levels_content.users_default {
power_levels_content.users.remove(user_id);
changed = true;
}
}
}
}
@ -549,6 +571,206 @@ pub async fn auto_join_qualifying_rooms(
Ok(())
}
/// Handle a state event change that may require enforcement.
/// Spawns enforcement as a background task to avoid recursive Send issues
/// in the append_pdu path.
impl Service {
pub fn handle_state_event_change(
self: &Arc<Self>,
space_id: OwnedRoomId,
event_type: String,
state_key: String,
) {
if !self.is_enabled() {
return;
}
let this = Arc::clone(self);
self.server.runtime().spawn(async move {
// Always repopulate cache first
this.populate_space(&space_id).await;
match event_type.as_str() {
| "m.space.roles" => {
// Role definitions changed — sync PLs in all child rooms
let child_rooms: Vec<OwnedRoomId> = this
.room_to_space
.read()
.await
.iter()
.filter(|(_, parent)| **parent == *space_id)
.map(|(child, _)| child.clone())
.collect();
for child_room_id in &child_rooms {
if let Err(e) =
this.sync_power_levels(&space_id, child_room_id).await
{
debug_warn!("Failed to sync PLs in {child_room_id}: {e}");
}
}
},
| "m.space.role.member" => {
// User's roles changed — auto-join/kick + PL sync
if let Ok(user_id) = UserId::parse(state_key.as_str()) {
if let Err(e) =
this.auto_join_qualifying_rooms(&space_id, &user_id).await
{
debug_warn!(
"Space role auto-join failed for {user_id}: {e}"
);
}
if let Err(e) =
this.kick_unqualified_from_rooms(&space_id, &user_id).await
{
debug_warn!(
"Space role auto-kick failed for {user_id}: {e}"
);
}
// Sync power levels in all child rooms
let child_rooms: Vec<OwnedRoomId> = this
.room_to_space
.read()
.await
.iter()
.filter(|(_, parent)| **parent == *space_id)
.map(|(child, _)| child.clone())
.collect();
for child_room_id in &child_rooms {
if let Err(e) =
this.sync_power_levels(&space_id, child_room_id).await
{
debug_warn!(
"Failed to sync PLs in {child_room_id}: {e}"
);
}
}
}
},
| "m.space.role.room" => {
// Room requirements changed — kick unqualified members
if let Ok(target_room) = RoomId::parse(state_key.as_str()) {
let members: Vec<OwnedUserId> = this
.services
.state_cache
.room_members(&target_room)
.map(ToOwned::to_owned)
.collect()
.await;
for member in &members {
if !this
.user_qualifies_for_room(
&space_id,
&target_room,
member,
)
.await
{
if let Err(e) = this
.kick_unqualified_from_rooms(&space_id, member)
.await
{
debug_warn!(
"Space role requirement kick failed for {member}: {e}"
);
}
}
}
}
},
| _ => {},
}
});
}
/// Handle a new m.space.child event — update index and auto-join qualifying
/// members.
pub fn handle_space_child_change(
self: &Arc<Self>,
space_id: OwnedRoomId,
child_room_id: OwnedRoomId,
) {
if !self.is_enabled() {
return;
}
let this = Arc::clone(self);
self.server.runtime().spawn(async move {
// Update the reverse index
this.room_to_space
.write()
.await
.insert(child_room_id.clone(), space_id.clone());
// Auto-join all qualifying space members
let space_members: Vec<OwnedUserId> = this
.services
.state_cache
.room_members(&space_id)
.map(ToOwned::to_owned)
.collect()
.await;
for member in &space_members {
if this
.user_qualifies_for_room(&space_id, &child_room_id, member)
.await
{
if !this
.services
.state_cache
.is_joined(member, &child_room_id)
.await
{
if let Err(e) =
this.auto_join_qualifying_rooms(&space_id, member).await
{
debug_warn!(
"Auto-join failed for {member} on new child room: {e}"
);
}
}
}
}
});
}
/// Handle a user joining a Space — auto-join them to qualifying child rooms.
pub fn handle_space_member_join(
self: &Arc<Self>,
space_id: OwnedRoomId,
user_id: OwnedUserId,
) {
if !self.is_enabled() {
return;
}
let this = Arc::clone(self);
self.server.runtime().spawn(async move {
if let Err(e) = this.auto_join_qualifying_rooms(&space_id, &user_id).await {
debug_warn!("Auto-join on Space join failed for {user_id}: {e}");
}
// Also sync their power levels
let child_rooms: Vec<OwnedRoomId> = this
.room_to_space
.read()
.await
.iter()
.filter(|(_, parent)| **parent == *space_id)
.map(|(child, _)| child.clone())
.collect();
for child_room_id in &child_rooms {
if let Err(e) =
this.sync_power_levels(&space_id, &child_room_id).await
{
debug_warn!(
"Failed to sync PLs on join for {child_room_id}: {e}"
);
}
}
});
}
}
/// Remove a user from all child rooms they no longer qualify for.
///
/// Iterates over child rooms that have role requirements for the given

View file

@ -361,15 +361,63 @@ where
// Space permission cascading: react to role-related state events
if self.services.roles.is_enabled() {
if let Some(_state_key) = pdu.state_key() {
if let Some(state_key) = pdu.state_key() {
let event_type_str = pdu.event_type().to_string();
match event_type_str.as_str() {
| "m.space.roles" | "m.space.role.member" | "m.space.role.room" => {
self.services.roles.populate_space(room_id).await;
let roles: Arc<crate::rooms::roles::Service> =
Arc::clone(&*self.services.roles);
roles.handle_state_event_change(
room_id.to_owned(),
event_type_str,
state_key.to_string(),
);
},
| _ => {},
}
}
// Handle m.space.child changes
if *pdu.kind() == TimelineEventType::SpaceChild {
if let Some(state_key) = pdu.state_key() {
if let Ok(child_room_id) = ruma::RoomId::parse(state_key) {
let roles: Arc<crate::rooms::roles::Service> =
Arc::clone(&*self.services.roles);
roles.handle_space_child_change(
room_id.to_owned(),
child_room_id.to_owned(),
);
}
}
}
// Handle m.room.member join in a Space — auto-join child rooms
if *pdu.kind() == TimelineEventType::RoomMember {
if let Some(state_key) = pdu.state_key() {
if let Ok(content) =
pdu.get_content::<ruma::events::room::member::RoomMemberEventContent>()
{
if content.membership
== ruma::events::room::member::MembershipState::Join
{
if let Ok(user_id) = UserId::parse(state_key) {
if matches!(
self.services
.state_accessor
.get_room_type(room_id)
.await,
Ok(ruma::room::RoomType::Space)
) {
let roles: Arc<crate::rooms::roles::Service> =
Arc::clone(&*self.services.roles);
roles.handle_space_member_join(
room_id.to_owned(),
user_id.to_owned(),
);
}
}
}
}
}
}
}
// CONCERN: If we receive events with a relation out-of-order, we never write

View file

@ -110,9 +110,9 @@ pub async fn build_and_append_pdu(
if let Some(space_pl) =
self.services.roles.get_user_power_level(&parent_space, user_id).await
{
if i64::from(*proposed_pl) < space_pl {
if i64::from(*proposed_pl) != space_pl {
return Err!(Request(Forbidden(
"Cannot set power level below Space-granted level"
"Cannot change power level that is set by Space roles"
)));
}
}