ember/continuwuity
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(spaces): address fourth review - stale cache, PL membership check, multi-space safety

Browse source 
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
ember33 2026-03-18 10:46:30 +01:00
parent 939f7e3d72
commit 4c0ec5f7a0
4 changed files with 101 additions and 74 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
docs/plans/2026-03-17-space-permission-cascading-design.md
View file

@ -117,7 +117,7 @@ For every user in a child room of a Space:
### 3. Role revocation
When an `com.continuwuity.space.role.member` event is updated and a role is removed:
When a `com.continuwuity.space.role.member` event is updated and a role is removed:
- Identify all child rooms that require the removed role.
- Auto-kick the user from rooms they no longer qualify for.
@ -125,14 +125,14 @@ When an `com.continuwuity.space.role.member` event is updated and a role is remo
### 4. Room requirement change
When an `com.continuwuity.space.role.room` event is updated with new requirements:
When a `com.continuwuity.space.role.room` event is updated with new requirements:
- Check all current members of the room.
- Auto-kick members who do not hold all newly required roles.
### 5. Auto-join on role grant
When an `com.continuwuity.space.role.member` event is updated and a role is added:
When a `com.continuwuity.space.role.member` event is updated and a role is added:
- Find all child rooms where the user now meets all required roles.
- Auto-join the user to qualifying rooms they are not already in.

97
src/service/rooms/roles/mod.rs
View file

@ -19,7 +19,6 @@ use conduwuit::{
};
use serde_json::value::to_raw_value;
use conduwuit_core::{
Err,
matrix::space_roles::{
RoleDefinition, SpaceRoleMemberEventContent, SpaceRoleRoomEventContent,
SpaceRolesEventContent,
@ -250,11 +249,18 @@ pub async fn populate_space(&self, space_id: &RoomId) {
// 2. Read all com.continuwuity.space.role.member state events (state key: user ID)
let member_event_type = StateEventType::from("com.continuwuity.space.role.member".to_owned());
if let Ok(shortstatehash) = self
let shortstatehash = match self
.services
.state
.get_room_shortstatehash(space_id)
.await
{
| Ok(hash) => hash,
| Err(e) => {
debug_warn!("Failed to get shortstatehash for {space_id}, cache may be stale: {e}");
return;
},
};
{
let mut user_roles_map: HashMap<OwnedUserId, HashSet<String>> = HashMap::new();
@ -350,9 +356,15 @@ pub async fn populate_space(&self, space_id: &RoomId) {
})
.await;
// Single write lock for all children
// Single write lock for all children — clear stale entries first
{
let mut room_to_space = self.room_to_space.write().await;
// Remove this space from all existing entries
room_to_space.retain(|_, parents| {
parents.remove(space_id);
!parents.is_empty()
});
// Insert fresh children
for child_room_id in child_rooms {
room_to_space
.entry(child_room_id)
@ -434,6 +446,18 @@ pub async fn get_parent_spaces(&self, room_id: &RoomId) -> Vec<OwnedRoomId> {
.unwrap_or_default()
}
/// Get all child rooms of a Space from the reverse index.
#[implement(Service)]
pub async fn get_child_rooms(&self, space_id: &RoomId) -> Vec<OwnedRoomId> {
self.room_to_space
.read()
.await
.iter()
.filter(|(_, parents)| parents.contains(space_id))
.map(|(child, _)| child.clone())
.collect()
}
/// Synchronize power levels in a child room based on Space roles.
/// This overrides per-room power levels with Space-granted levels.
#[implement(Service)]
@ -476,6 +500,9 @@ pub async fn sync_power_levels(&self, space_id: &RoomId, room_id: &RoomId) -> Re
// 3. For each member, check their space role power level
let mut changed = false;
for user_id in &members {
if user_id == server_user {
continue;
}
if let Some(space_pl) = self.get_user_power_level(space_id, user_id).await {
let space_pl_int = Int::new_saturating(space_pl);
let current_pl = power_levels_content
@ -492,11 +519,21 @@ pub async fn sync_power_levels(&self, space_id: &RoomId, room_id: &RoomId) -> Re
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;
// Check if any other parent space manages this user's PL
let parents = self.get_parent_spaces(room_id).await;
let mut managed_by_other = false;
for parent in &parents {
if self.get_user_power_level(parent, user_id).await.is_some() {
managed_by_other = true;
break;
}
}
if !managed_by_other {
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;
}
}
}
}
@ -536,15 +573,7 @@ pub async fn auto_join_qualifying_rooms(
}
// Get all child rooms from the room_to_space reverse index
// Filter to children of this space
let child_rooms: Vec<OwnedRoomId> = self
.room_to_space
.read()
.await
.iter()
.filter(|(_, parents)| parents.contains(space_id))
.map(|(child, _)| child.clone())
.collect();
let child_rooms = self.get_child_rooms(space_id).await;
let server_user = self.services.globals.server_user.as_ref();
@ -649,14 +678,7 @@ impl Service {
match event_type.as_str() {
| "com.continuwuity.space.roles" => {
// Role definitions changed — sync PLs in all child rooms
let child_rooms: Vec<OwnedRoomId> = this
.room_to_space
.read()
.await
.iter()
.filter(|(_, parents)| parents.contains(&space_id))
.map(|(child, _)| child.clone())
.collect();
let child_rooms = this.get_child_rooms(&space_id).await;
for child_room_id in &child_rooms {
if let Err(e) =
this.sync_power_levels(&space_id, child_room_id).await
@ -683,14 +705,7 @@ impl Service {
);
}
// Sync power levels in all child rooms
let child_rooms: Vec<OwnedRoomId> = this
.room_to_space
.read()
.await
.iter()
.filter(|(_, parents)| parents.contains(&space_id))
.map(|(child, _)| child.clone())
.collect();
let child_rooms = this.get_child_rooms(&space_id).await;
for child_room_id in &child_rooms {
if let Err(e) =
this.sync_power_levels(&space_id, child_room_id).await
@ -895,14 +910,7 @@ impl Service {
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(|(_, parents)| parents.contains(&space_id))
.map(|(child, _)| child.clone())
.collect();
let child_rooms = this.get_child_rooms(&space_id).await;
for child_room_id in &child_rooms {
if let Err(e) =
this.sync_power_levels(&space_id, &child_room_id).await
@ -931,6 +939,11 @@ pub async fn kick_unqualified_from_rooms(
return Ok(());
}
let server_user = self.services.globals.server_user.as_ref();
if user_id == server_user {
return Ok(());
}
// Get child rooms that have requirements
let child_rooms: Vec<OwnedRoomId> = self
.room_requirements
@ -940,8 +953,6 @@ pub async fn kick_unqualified_from_rooms(
.map(|reqs| reqs.keys().cloned().collect())
.unwrap_or_default();
let server_user = self.services.globals.server_user.as_ref();
for child_room_id in &child_rooms {
// Check if server user is joined to the child room
if !self

6
src/service/rooms/timeline/append.rs
View file

@ -3,7 +3,7 @@ use std::{
sync::Arc,
};
use conduwuit::{debug_warn, trace};
use conduwuit::trace;
use conduwuit_core::{
Result, err, error, implement,
matrix::{
@ -14,7 +14,7 @@ use conduwuit_core::{
};
use futures::StreamExt;
use ruma::{
CanonicalJsonObject, CanonicalJsonValue, EventId, OwnedUserId, RoomId, RoomVersionId, UserId,
CanonicalJsonObject, CanonicalJsonValue, EventId, RoomVersionId, UserId,
events::{
GlobalAccountDataEventType, StateEventType, TimelineEventType,
push_rules::PushRulesEvent,
@ -327,7 +327,7 @@ where
}
},
| TimelineEventType::SpaceChild =>
if let Some(state_key) = pdu.state_key() {
if pdu.state_key().is_some() {
self.services
.spaces
.roomid_spacehierarchy_cache

66
src/service/rooms/timeline/build.rs
View file

@ -121,32 +121,48 @@ pub async fn build_and_append_pdu(
}
}
// Also check that space-managed users aren't omitted
}
// Also check that space-managed users aren't omitted
// Clone data out of guards to avoid holding locks across await
let space_data: Vec<(ruma::OwnedRoomId, Vec<(OwnedUserId, HashSet<String>)>, std::collections::BTreeMap<String, conduwuit_core::matrix::space_roles::RoleDefinition>)> = {
let user_roles_guard = self.services.roles.user_roles.read().await;
if let Some(space_users) = user_roles_guard.get(parent_space) {
let roles_guard = self.services.roles.roles.read().await;
if let Some(role_defs) = roles_guard.get(parent_space) {
for (user_id, assigned_roles) in space_users {
let space_pl = assigned_roles
.iter()
.filter_map(|r| role_defs.get(r)?.power_level)
.max();
if let Some(space_pl) = space_pl {
// This user should be in the proposed event
match proposed.users.get(user_id) {
| None => {
return Err!(Request(Forbidden(
"Cannot omit a user whose power level is managed by Space roles"
)));
},
| Some(pl) if i64::from(*pl) != space_pl => {
return Err!(Request(Forbidden(
"Cannot change power level that is set by Space roles"
)));
},
| _ => {},
}
}
let roles_guard = self.services.roles.roles.read().await;
parent_spaces.iter().filter_map(|ps| {
let space_users = user_roles_guard.get(ps)?;
let role_defs = roles_guard.get(ps)?;
Some((
ps.clone(),
space_users.iter().map(|(u, r)| (u.clone(), r.clone())).collect(),
role_defs.clone(),
))
}).collect()
};
// Guards dropped here
for (_parent_space, space_users, role_defs) in &space_data {
for (user_id, assigned_roles) in space_users {
// Only enforce for users who are actually members of this room
if !self.services.state_cache.is_joined(user_id, &room_id).await {
continue;
}
let space_pl = assigned_roles
.iter()
.filter_map(|r| role_defs.get(r)?.power_level)
.max();
if let Some(space_pl) = space_pl {
match proposed.users.get(user_id) {
| None => {
return Err!(Request(Forbidden(
"Cannot omit a user whose power level is managed by Space roles"
)));
},
| Some(pl) if i64::from(*pl) != space_pl => {
return Err!(Request(Forbidden(
"Cannot change power level that is set by Space roles"
)));
},
| _ => {},
}
}
}