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

style(spaces): convert to structured logging, fix import ordering, add lock docs

Browse source 
Convert all log statements in space roles code to use structured
key-value fields instead of string interpolation, matching the project
code style. Fix import ordering (serde_json moved after conduwuit_core),
move a misplaced `use futures::StreamExt` from function body to
file-level imports, add lock ordering comments to prevent deadlocks,
fix populate_space to acquire locks in the same order as
handle_space_child_change, add diagnostic debug_warn before PL
rejection errors, and document the nested cascade limitation on
get_parent_spaces.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
ember33 2026-03-18 12:17:14 +01:00
parent dd1e9f0979
commit d569ef2e40
4 changed files with 66 additions and 68 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
conduwuit-example.toml
View file

@ -476,6 +476,11 @@
#
#space_permission_cascading = false
# Maximum number of spaces to cache role data for. When exceeded the
# cache is cleared and repopulated on demand.
#
#space_roles_cache_capacity = 1000
# Enabling this setting opens registration to anyone without restrictions.
# This makes your server vulnerable to abuse
#

3
src/admin/space/roles.rs
View file

@ -11,6 +11,7 @@ use ruma::{OwnedRoomId, OwnedRoomOrAliasId, OwnedUserId, events::StateEventType}
use serde_json::value::to_raw_value;
use conduwuit::matrix::pdu::PduBuilder;
use futures::StreamExt;
use crate::{admin_command, admin_command_dispatch};
@ -235,8 +236,6 @@ async fn remove(&self, space: OwnedRoomOrAliasId, role_name: String) -> Result {
.get_room_shortstatehash(&space_id)
.await
{
use futures::StreamExt;
let user_entries: Vec<(_, ruma::OwnedEventId)> = self
.services
.rooms

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

@ -17,7 +17,6 @@ use conduwuit::{
matrix::pdu::PduBuilder,
warn,
};
use serde_json::value::to_raw_value;
use conduwuit_core::{
matrix::space_roles::{
RoleDefinition, SpaceRoleMemberEventContent, SpaceRoleRoomEventContent,
@ -41,6 +40,7 @@ use ruma::{
space::child::SpaceChildEventContent,
},
};
use serde_json::value::to_raw_value;
use tokio::sync::RwLock;
use crate::{Dep, globals, rooms};
@ -134,7 +134,7 @@ impl crate::Service for Service {
return Ok(());
}
info!("Rebuilding space roles cache from all known rooms...");
info!("Rebuilding space roles cache from all known rooms");
let mut space_count: usize = 0;
let room_ids: Vec<OwnedRoomId> = self
@ -148,7 +148,7 @@ impl crate::Service for Service {
for room_id in &room_ids {
match self.services.state_accessor.get_room_type(room_id).await {
| Ok(RoomType::Space) => {
debug!("Populating space roles cache for {room_id}");
debug!(room_id = %room_id, "Populating space roles cache");
self.populate_space(room_id).await;
space_count = space_count.saturating_add(1);
},
@ -156,7 +156,7 @@ impl crate::Service for Service {
}
}
info!("Space roles cache rebuilt for {space_count} spaces");
info!(space_count, "Space roles cache rebuilt");
Ok(())
}
@ -225,7 +225,7 @@ pub async fn ensure_default_roles(&self, space_id: &RoomId) -> Result {
.build_and_append_pdu(pdu, sender, Some(space_id), &state_lock)
.await?;
debug!("Sent default {SPACE_ROLES_EVENT_TYPE} event for {space_id}");
debug!(space_id = %space_id, event_type = SPACE_ROLES_EVENT_TYPE, "Sent default space roles event");
Ok(())
}
@ -274,7 +274,7 @@ pub async fn populate_space(&self, space_id: &RoomId) {
{
| Ok(hash) => hash,
| Err(e) => {
debug_warn!("Failed to get shortstatehash for {space_id}, cache may be stale: {e}");
debug_warn!(space_id = %space_id, error = ?e, "Failed to get shortstatehash, cache may be stale");
return;
},
};
@ -373,13 +373,8 @@ pub async fn populate_space(&self, space_id: &RoomId) {
})
.await;
// Update forward index
{
let mut space_to_rooms = self.space_to_rooms.write().await;
space_to_rooms.insert(space_id.to_owned(), child_rooms.iter().cloned().collect())
};
// Single write lock for all children — clear stale entries first
// Lock ordering: room_to_space before space_to_rooms.
// This order must be consistent to avoid deadlocks.
{
let mut room_to_space = self.room_to_space.write().await;
// Remove this space from all existing entries
@ -388,13 +383,19 @@ pub async fn populate_space(&self, space_id: &RoomId) {
!parents.is_empty()
});
// Insert fresh children
for child_room_id in child_rooms {
for child_room_id in &child_rooms {
room_to_space
.entry(child_room_id)
.entry(child_room_id.clone())
.or_default()
.insert(space_id.to_owned());
}
}
// Update forward index (after room_to_space to maintain lock ordering)
{
let mut space_to_rooms = self.space_to_rooms.write().await;
space_to_rooms.insert(space_id.to_owned(), child_rooms.into_iter().collect())
};
}
}
@ -467,6 +468,9 @@ pub async fn user_qualifies_for_room(
}
/// Get the parent Spaces of a child room, if any.
///
/// Only direct parent spaces are returned. Nested sub-space cascading
/// is not supported (see design doc requirement 6).
#[implement(Service)]
pub async fn get_parent_spaces(&self, room_id: &RoomId) -> Vec<OwnedRoomId> {
if !self.is_enabled() {
@ -508,9 +512,7 @@ pub async fn sync_power_levels(&self, space_id: &RoomId, room_id: &RoomId) -> Re
.is_joined(server_user, room_id)
.await
{
debug_warn!(
"Server user is not joined to {room_id}, skipping PL sync"
);
debug_warn!(room_id = %room_id, "Server user is not joined, skipping PL sync");
return Ok(());
}
@ -641,9 +643,7 @@ pub async fn auto_join_qualifying_rooms(
.is_joined(server_user, child_room_id)
.await
{
debug_warn!(
"Server user is not joined to {child_room_id}, skipping auto-join"
);
debug_warn!(room_id = %child_room_id, "Server user is not joined, skipping auto-join");
continue;
}
@ -664,9 +664,7 @@ pub async fn auto_join_qualifying_rooms(
)
.await
{
debug_warn!(
"Failed to invite {user_id} to {child_room_id} during auto-join: {e}"
);
debug_warn!(user_id = %user_id, room_id = %child_room_id, error = ?e, "Failed to invite user during auto-join");
continue;
}
@ -685,7 +683,7 @@ pub async fn auto_join_qualifying_rooms(
)
.await
{
warn!("Failed to auto-join {user_id} to {child_room_id}: {e}");
warn!(user_id = %user_id, room_id = %child_room_id, error = ?e, "Failed to auto-join user");
}
}
@ -734,7 +732,7 @@ impl Service {
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}");
debug_warn!(room_id = %child_room_id, error = ?e, "Failed to sync power levels");
}
}
// Revalidate all space members against all child rooms
@ -749,9 +747,7 @@ impl Service {
if let Err(e) =
Box::pin(this.kick_unqualified_from_rooms(&space_id, member)).await
{
debug_warn!(
"Role definition revalidation kick failed for {member}: {e}"
);
debug_warn!(user_id = %member, error = ?e, "Role definition revalidation kick failed");
}
}
},
@ -761,16 +757,12 @@ impl Service {
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}"
);
debug_warn!(user_id = %user_id, error = ?e, "Space role auto-join failed");
}
if let Err(e) =
Box::pin(this.kick_unqualified_from_rooms(&space_id, user_id)).await
{
debug_warn!(
"Space role auto-kick failed for {user_id}: {e}"
);
debug_warn!(user_id = %user_id, error = ?e, "Space role auto-kick failed");
}
// Sync power levels in all child rooms
let child_rooms = this.get_child_rooms(&space_id).await;
@ -778,9 +770,7 @@ impl Service {
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}"
);
debug_warn!(room_id = %child_room_id, error = ?e, "Failed to sync power levels");
}
}
}
@ -808,9 +798,7 @@ impl Service {
.kick_unqualified_from_rooms(&space_id, member))
.await
{
debug_warn!(
"Space role requirement kick failed for {member}: {e}"
);
debug_warn!(user_id = %member, error = ?e, "Space role requirement kick failed");
}
}
}
@ -860,7 +848,8 @@ impl Service {
};
if is_removal {
// Remove child from room_to_space reverse index
// Lock ordering: room_to_space before space_to_rooms.
// This order must be consistent to avoid deadlocks.
let mut room_to_space = this.room_to_space.write().await;
if let Some(parents) = room_to_space.get_mut(&child_room_id) {
parents.remove(&space_id);
@ -900,10 +889,7 @@ impl Service {
.is_joined(server_user, &child_room_id)
.await
{
debug_warn!(
"Server user is not joined to {child_room_id}, \
skipping auto-join enforcement for new child"
);
debug_warn!(room_id = %child_room_id, "Server user is not joined, skipping auto-join enforcement for new child");
return;
}
@ -947,9 +933,7 @@ impl Service {
)
.await
{
debug_warn!(
"Failed to invite {member} to {child_room_id}: {e}"
);
debug_warn!(user_id = %member, room_id = %child_room_id, error = ?e, "Failed to invite user");
continue;
}
@ -970,9 +954,7 @@ impl Service {
)
.await
{
warn!(
"Failed to auto-join {member} to {child_room_id}: {e}"
);
warn!(user_id = %member, room_id = %child_room_id, error = ?e, "Failed to auto-join user");
}
}
}
@ -1005,7 +987,7 @@ impl Service {
let _permit = this.enforcement_semaphore.acquire().await;
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}");
debug_warn!(user_id = %user_id, error = ?e, "Auto-join on Space join failed");
}
// Also sync their power levels
let child_rooms = this.get_child_rooms(&space_id).await;
@ -1013,9 +995,7 @@ impl Service {
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}"
);
debug_warn!(room_id = %child_room_id, error = ?e, "Failed to sync power levels on join");
}
}
});
@ -1059,9 +1039,7 @@ pub async fn kick_unqualified_from_rooms(
.is_joined(server_user, child_room_id)
.await
{
debug_warn!(
"Server user is not joined to {child_room_id}, skipping kick enforcement"
);
debug_warn!(room_id = %child_room_id, "Server user is not joined, skipping kick enforcement");
continue;
}
// Skip if not joined
@ -1089,9 +1067,7 @@ pub async fn kick_unqualified_from_rooms(
.get_member(child_room_id, user_id)
.await
else {
debug_warn!(
"Could not get member event for {user_id} in {child_room_id}, skipping kick"
);
debug_warn!(user_id = %user_id, room_id = %child_room_id, "Could not get member event, skipping kick");
continue;
};
@ -1119,9 +1095,7 @@ pub async fn kick_unqualified_from_rooms(
)
.await
{
warn!(
"Failed to kick {user_id} from {child_room_id} for missing roles: {e}"
);
warn!(user_id = %user_id, room_id = %child_room_id, error = ?e, "Failed to kick user for missing roles");
}
}

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

@ -5,7 +5,7 @@ use std::{
use conduwuit_core::matrix::space_roles::RoleDefinition;
use conduwuit::trace;
use conduwuit::{debug_warn, trace};
use conduwuit_core::{
Err, Result, implement,
matrix::{event::Event, pdu::PduBuilder},
@ -122,6 +122,13 @@ pub async fn build_and_append_pdu(
self.services.roles.get_user_power_level(parent_space, user_id).await
{
if i64::from(*proposed_pl) != space_pl {
debug_warn!(
user_id = %user_id,
room_id = %room_id,
proposed_pl = i64::from(*proposed_pl),
space_pl,
"Rejecting PL change conflicting with space role"
);
return Err!(Request(Forbidden(
"Cannot change power level that is set by Space roles"
)));
@ -160,11 +167,24 @@ pub async fn build_and_append_pdu(
if let Some(space_pl) = space_pl {
match proposed.users.get(user_id) {
| None => {
debug_warn!(
user_id = %user_id,
room_id = %room_id,
space_pl,
"Rejecting PL change: space-managed user omitted"
);
return Err!(Request(Forbidden(
"Cannot omit a user whose power level is managed by Space roles"
)));
},
| Some(pl) if i64::from(*pl) != space_pl => {
debug_warn!(
user_id = %user_id,
room_id = %room_id,
proposed_pl = i64::from(*pl),
space_pl,
"Rejecting PL change conflicting with space role"
);
return Err!(Request(Forbidden(
"Cannot change power level that is set by Space roles"
)));